← back to Cli Printing Press
fix(cli): make .printing-press.json authoritative for parsed.Name in mcp-sync (#1252)
bb9b9cb3a71a3fea95df2075c515ae62bec44553 · 2026-05-12 15:35:03 -0700 · Trevin Chow
* fix(cli): make .printing-press.json authoritative for parsed.Name in mcp-sync
Closes #1050.
Pre-fix, mcp-sync's loadArchivedSpec returned parsed.Name derived from
the OpenAPI spec's info.title (e.g. "Telegram Bot API" → "telegram-bot").
Downstream emitters (WriteToolsManifest, populateMCPMetadata, the
generator's MCP-surface render) then regenerated mcp_binary, api_name,
manifest.json:name/entry_point, and cmd/<slug>-pp-{cli,mcp}/
directories under that title-derived slug — silently flipping CLIs
generated with --name to the spec-title slug on every sync.
This change preempts the regeneration: when .printing-press.json's
api_name diverges from parsed.Name, applyManifestNameOverride
replaces parsed.Name and (where the path matches the spec-derived
shape) parsed.Config.Path before downstream code runs. The manifest is
the user-chosen identity recorded at generate time; the spec slug is
only the fallback when the manifest is missing or api_name is empty.
This mirrors the preserve-on-regen pattern already established for
Owner and Printer in resolveOwnerForExisting.
Scope is intentionally narrow per the issue body's "Suggested
direction." The comment-expanded scope (binary-slug + env-var-prefix
derivation at generate time, stop-word stripping from cleanSpecName)
is deferred.
Notes for follow-up (out of scope here):
- reconcileSpecNameWithDir still uses the directory basename as the
truth source; for internal YAML specs where basename diverges from
manifest.APIName, reconcile would rewrite spec.yaml.name back to
the basename and re-set parsed.Name. The canonical #1050 case
(basename == manifest.APIName) is unaffected; the divergence case
is unusual and merits its own targeted change.
- pipeline.ReadCLIManifest does not distinguish fs.ErrNotExist from
JSON parse errors. A corrupted manifest currently falls through
silently; an operator-facing warning would be a small improvement.
* docs(cli): document manifest-as-authority pattern for identity fields
Captures the convention that recurring regen paths (mcp-sync,
regen-merge, future per-CLI rewriters) must consult the existing
.printing-press.json before re-deriving identity-bearing fields from
the spec or operator environment. Generalizes the preserve-on-regen
shape already established for owner and printer onto api_name and
explicitly calls out display_name and env-var prefix as the next two
candidates that should follow the same shape.
Cross-references the authorship-retrofit lesson and the dual-key
identity-field design-pattern doc so the three identity fields
(Owner, Printer, Name) hang off a single discoverable convention.
* fix(cli): distinguish missing manifest from parse error in applyManifestNameOverride
Greptile flagged that the prior single-arm error guard collapsed two
distinct failure modes into one silent fallthrough: fs.ErrNotExist
(expected for legacy CLIs without .printing-press.json) and JSON
parse failures (corrupted/partially-written manifest). The override
is the sole guard against the #1050 regression, so a corrupted
manifest silently reverting to spec-derived behavior with no signal
to the operator is exactly the failure mode the PR is meant to
prevent.
Now emits a stderr warning when the manifest exists but cannot be
parsed; legacy-CLI fall-through stays silent. Adds a test fixture
with a truncated JSON manifest to lock the behavior.
Files touched
A docs/solutions/conventions/manifest-wins-over-re-derivation-for-identity-fields-in-regen-paths-2026-05-12.mdM internal/pipeline/mcpsync/sync.goM internal/pipeline/mcpsync/sync_test.go
Diff
commit bb9b9cb3a71a3fea95df2075c515ae62bec44553
Author: Trevin Chow <trevin@trevinchow.com>
Date: Tue May 12 15:35:03 2026 -0700
fix(cli): make .printing-press.json authoritative for parsed.Name in mcp-sync (#1252)
* fix(cli): make .printing-press.json authoritative for parsed.Name in mcp-sync
Closes #1050.
Pre-fix, mcp-sync's loadArchivedSpec returned parsed.Name derived from
the OpenAPI spec's info.title (e.g. "Telegram Bot API" → "telegram-bot").
Downstream emitters (WriteToolsManifest, populateMCPMetadata, the
generator's MCP-surface render) then regenerated mcp_binary, api_name,
manifest.json:name/entry_point, and cmd/<slug>-pp-{cli,mcp}/
directories under that title-derived slug — silently flipping CLIs
generated with --name to the spec-title slug on every sync.
This change preempts the regeneration: when .printing-press.json's
api_name diverges from parsed.Name, applyManifestNameOverride
replaces parsed.Name and (where the path matches the spec-derived
shape) parsed.Config.Path before downstream code runs. The manifest is
the user-chosen identity recorded at generate time; the spec slug is
only the fallback when the manifest is missing or api_name is empty.
This mirrors the preserve-on-regen pattern already established for
Owner and Printer in resolveOwnerForExisting.
Scope is intentionally narrow per the issue body's "Suggested
direction." The comment-expanded scope (binary-slug + env-var-prefix
derivation at generate time, stop-word stripping from cleanSpecName)
is deferred.
Notes for follow-up (out of scope here):
- reconcileSpecNameWithDir still uses the directory basename as the
truth source; for internal YAML specs where basename diverges from
manifest.APIName, reconcile would rewrite spec.yaml.name back to
the basename and re-set parsed.Name. The canonical #1050 case
(basename == manifest.APIName) is unaffected; the divergence case
is unusual and merits its own targeted change.
- pipeline.ReadCLIManifest does not distinguish fs.ErrNotExist from
JSON parse errors. A corrupted manifest currently falls through
silently; an operator-facing warning would be a small improvement.
* docs(cli): document manifest-as-authority pattern for identity fields
Captures the convention that recurring regen paths (mcp-sync,
regen-merge, future per-CLI rewriters) must consult the existing
.printing-press.json before re-deriving identity-bearing fields from
the spec or operator environment. Generalizes the preserve-on-regen
shape already established for owner and printer onto api_name and
explicitly calls out display_name and env-var prefix as the next two
candidates that should follow the same shape.
Cross-references the authorship-retrofit lesson and the dual-key
identity-field design-pattern doc so the three identity fields
(Owner, Printer, Name) hang off a single discoverable convention.
* fix(cli): distinguish missing manifest from parse error in applyManifestNameOverride
Greptile flagged that the prior single-arm error guard collapsed two
distinct failure modes into one silent fallthrough: fs.ErrNotExist
(expected for legacy CLIs without .printing-press.json) and JSON
parse failures (corrupted/partially-written manifest). The override
is the sole guard against the #1050 regression, so a corrupted
manifest silently reverting to spec-derived behavior with no signal
to the operator is exactly the failure mode the PR is meant to
prevent.
Now emits a stderr warning when the manifest exists but cannot be
parsed; legacy-CLI fall-through stays silent. Adds a test fixture
with a truncated JSON manifest to lock the behavior.
---
...or-identity-fields-in-regen-paths-2026-05-12.md | 187 ++++++++++++++++
internal/pipeline/mcpsync/sync.go | 47 ++++
internal/pipeline/mcpsync/sync_test.go | 237 +++++++++++++++++++++
3 files changed, 471 insertions(+)
diff --git a/docs/solutions/conventions/manifest-wins-over-re-derivation-for-identity-fields-in-regen-paths-2026-05-12.md b/docs/solutions/conventions/manifest-wins-over-re-derivation-for-identity-fields-in-regen-paths-2026-05-12.md
new file mode 100644
index 00000000..28ebae04
--- /dev/null
+++ b/docs/solutions/conventions/manifest-wins-over-re-derivation-for-identity-fields-in-regen-paths-2026-05-12.md
@@ -0,0 +1,187 @@
+---
+title: "Existing manifest wins over re-derivation for identity fields in regen paths"
+date: 2026-05-12
+category: conventions
+module: cli-printing-press-generator
+problem_type: convention
+component: generator
+severity: high
+applies_when:
+ - "Writing a recurring regen / sync command (mcp-sync, regen-merge, future per-CLI rewriters) that reloads the spec and re-emits identity-bearing surfaces"
+ - "An identity field (api_name, owner, printer, display_name, env-var prefix) was set by the user at generate time and persisted to .printing-press.json"
+ - "The spec parser would otherwise re-derive that field from the spec — info.title slug, copyright header parse, git config — every time the regen runs"
+ - "The persisted manifest value and the re-derived value can disagree, and downstream emitters care which one wins"
+tags:
+ - preserve-on-regen
+ - manifest-authority
+ - identity-resolution
+ - mcp-sync
+ - regen-merge
+ - naming
+related_components:
+ - generator
+ - mcp-sync
+ - regenmerge
+---
+
+# Existing manifest wins over re-derivation for identity fields in regen paths
+
+## Context
+
+The Printing Press's per-CLI surfaces include several identity fields that the user chooses at generate time (or that the generator chooses on the user's behalf from the operator's environment) and that persist forever in `.printing-press.json`:
+
+| Field | Where it's first set | What pins it |
+|---|---|---|
+| `api_name` | `generate --name foo` | Manifest `api_name`, CLI binary directory name (`cmd/foo-pp-cli/`), README config path |
+| `owner` | `generate` reads copyright header / git config | Manifest `owner`, copyright headers across every emitted .go file |
+| `printer` | `generate` reads `git config github.user` | Manifest `printer`, README byline link |
+| `display_name` | spec `x-display-name`, catalog entry, or info.title fallback | Manifest `display_name`, MCPB display, MCP server identity |
+
+Every subsequent regen — `mcp-sync`, `regen-merge`, the per-CLI sweep tools — also reloads the spec, and the spec parser is happy to re-derive each of those fields from primary sources: `info.title` for the slug, the operator's environment for owner/printer. When those re-derivations disagree with the persisted manifest, downstream emitters (MCP surface generation, README rendering, tools-manifest emission) cascade the wrong value across many files.
+
+Two prior incidents made this concrete:
+
+- The **authorship retrofit** (see related doc below) flipped every `author:` field to the sweep operator on a multi-author library because the sweep trusted `git config user.name` over the existing copyright headers.
+- The **telegram CLI rename** ([#1050](https://github.com/mvanhorn/cli-printing-press/issues/1050)) flipped `mcp_binary`, `tools-manifest.json:api_name`, `manifest.json:name`/`entry_point`, the README `Config file:` path, and produced stray `cmd/telegram-bot-pp-{cli,mcp}/` directories on every `mcp-sync` run because the spec's `info.title` "Telegram Bot API" slugified to `telegram-bot`, overwriting the operator's chosen `--name telegram`.
+
+Same shape, different field. The fix shape is the same too.
+
+## Guidance
+
+Any regen path that reloads a spec and writes identity-bearing surfaces must consult the existing `.printing-press.json` first and prefer its value over re-derivation. The persisted manifest is authoritative; the spec derivation is the **fallback for entries the manifest doesn't cover** (older CLIs predating the field, fresh generates, library entries with no manifest yet).
+
+Mirror the resolution order already established for `owner` and `printer`:
+
+1. **Existing manifest field** — if `.printing-press.json` carries a non-empty value, use it verbatim
+2. **Spec / environment derivation** — only when the manifest is missing or the field is empty
+3. **Soft-fallback warning** — when the derivation produces a sentinel ("USER", empty slug), emit stderr and let publish-side validation fail closed
+
+```go
+// internal/pipeline/mcpsync/sync.go — the parsed.Name fix for #1050
+
+func applyManifestNameOverride(cliDir string, parsed *spec.APISpec) string {
+ if parsed == nil {
+ return ""
+ }
+ m, err := pipeline.ReadCLIManifest(cliDir)
+ if err != nil {
+ return ""
+ }
+ if m.APIName == "" || m.APIName == parsed.Name {
+ return ""
+ }
+ prior := parsed.Name
+ if parsed.Config.Path == fmt.Sprintf(defaultConfigPathFormat, naming.CLI(prior)) {
+ parsed.Config.Path = fmt.Sprintf(defaultConfigPathFormat, naming.CLI(m.APIName))
+ }
+ parsed.Name = m.APIName
+ return prior
+}
+```
+
+The function does three things by design and one by structure:
+
+1. **Read the manifest first.** No spec touching until the manifest answer is known.
+2. **Override `parsed.Name`** when the manifest disagrees. Downstream emitters all derive from `parsed.Name`, so a single point of override fixes every surface at once.
+3. **Cascade the override into spec-shaped Config.Path.** The README "Config file:" line is rendered from `parsed.Config.Path`; if the spec parser emitted the default `~/.config/<slug>-pp-cli/config.toml` shape, migrate the slug along with the name. Hand-customized paths (XDG-style overrides, per-environment roots) are left alone.
+4. **Fall through silently** when the manifest is missing, the field is empty, or values already agree. Older CLIs predating the manifest still work via the legacy `reconcileSpecNameWithDir` path that uses the directory basename.
+
+The `owner` and `printer` parallels use the same shape — see `resolveOwnerForExisting` (and its `printer` counterpart) in [`internal/pipeline/climanifest.go`](../../../internal/pipeline/climanifest.go) and the AGENTS.md "Owner vs OwnerName" / "Printer vs PrinterName" sections. The dual-key (slug / display) structure described there isn't directly relevant to `api_name` (which is single-key), but the **preserve-on-regen** tier ordering is the same.
+
+## Why This Matters
+
+Three failure modes the manifest-first approach prevents:
+
+1. **Silent identity flips on every sync.** Without the preempt, `mcp-sync` regenerates name-bearing surfaces against the spec slug every run. A CLI generated with `--name telegram` against the "Telegram Bot API" spec silently becomes `telegram-bot` in the MCPB manifest, the tools manifest, and the README config path — but the actual built binary is still `telegram-pp-mcp`, so the install instructions point at a binary that doesn't exist. Polish detects most of the drift; the manifest field can re-flip silently between polish and publish.
+2. **Cascading wrong values across many files.** The slug isn't just one field. `parsed.Name` flows into `cmd/<slug>-pp-cli/main.go` (the entry-point package dir), `mcp_binary` in the manifest, `api_name` in tools-manifest.json, `Server.EntryPoint: "bin/<slug>-pp-mcp"` in manifest.json, and the README `Config file:` line. Fixing each emitter individually is fragile — a future emitter added without coordination silently reintroduces the bug.
+3. **Multi-author trust damage.** When the re-derivation default flips an attribution field (owner, printer), the consequence is no longer "wrong path string" but "this tool just claimed someone else's work for whoever ran the sweep." The authorship retrofit incident bit hard enough that the curated per-CLI map (see related doc) was needed to clean up. The lesson generalized: the same default class of bug exists for any identity field, and the prevention is the same shape.
+
+The "use derivation as the default" instinct is not malicious — it's the path of least resistance when you write the spec parser, because the spec is the obvious source. The lesson is to recognize that, after the user has chosen a value once and the system has persisted it, the persisted value outranks every re-derivation.
+
+## When to Apply
+
+- Adding a new identity-bearing field to `.printing-press.json` that downstream emitters consume
+- Writing a new regen / sync / sweep tool that reloads a spec and re-emits per-CLI surfaces
+- Refactoring an existing emitter that derives an identity field from the spec — confirm the regen path consults the manifest first
+- Reviewing a change that touches `naming.CLI`, `naming.MCP`, `cleanSpecName`, or any helper that converts a spec value into a slug used in file paths or manifest fields
+
+Don't apply when:
+
+- The field genuinely derives from the spec at every read with no user-chosen-value semantics (e.g., `BaseURL`, `Version`, endpoint counts)
+- The field is intentionally derived (a computed display string built from multiple spec fields, where preserving an older value would be wrong)
+- The regen is a one-shot rewrite where the operator explicitly wants to overwrite the persisted value (publish-side `--force-rewrite` or similar; document the deviation at the call site)
+
+## Examples
+
+### Anti-pattern — re-derive every time
+
+```go
+// mcp-sync before the fix: parsed.Name comes from spec parsing every run.
+// Whatever info.title's slug is, that's what propagates forward.
+parsed, err := loadArchivedSpec(cliDir) // parsed.Name = "telegram-bot"
+if err != nil {
+ return Result{}, err
+}
+gen := generator.New(parsed, cliDir)
+gen.GenerateMCPSurface() // emits cmd/telegram-bot-pp-mcp/main.go
+pipeline.WriteToolsManifest(cliDir, parsed) // api_name: "telegram-bot"
+pipeline.WriteMCPBManifest(cliDir) // manifest name: "telegram-bot-pp-mcp"
+```
+
+Result: every `mcp-sync` run regenerates against the title-derived slug regardless of what the user chose at generate time. Polish detects some drift but can't reliably fix the manifest field. Publish ships a `manifest.json` whose `entry_point` points at a binary name that doesn't exist on disk.
+
+### Pattern — preempt with the manifest before downstream emitters run
+
+```go
+parsed, err := loadArchivedSpec(cliDir) // parsed.Name = "telegram-bot"
+if err != nil {
+ return Result{}, err
+}
+// Preempt: the manifest's api_name outranks the spec-derived slug.
+if prior := applyManifestNameOverride(cliDir, parsed); prior != "" {
+ fmt.Fprintf(os.Stderr,
+ "mcp-sync: using manifest api_name %q over spec-derived slug %q\n",
+ parsed.Name, prior)
+}
+// parsed.Name is now "telegram" — downstream emitters all derive correctly.
+gen := generator.New(parsed, cliDir)
+gen.GenerateMCPSurface() // emits cmd/telegram-pp-mcp/main.go
+pipeline.WriteToolsManifest(cliDir, parsed) // api_name: "telegram"
+pipeline.WriteMCPBManifest(cliDir) // manifest name: "telegram-pp-mcp"
+```
+
+Result: the user-chosen slug wins across every emitted surface. The override falls through silently for CLIs without a manifest, so the legacy path stays intact.
+
+### Pattern — test the cascade end-to-end, not just the override helper
+
+```go
+func TestSyncManifestAPINameWinsOverSpecDerivedSlug(t *testing.T) {
+ // Generate with --name telegram, then rewrite spec.yaml so the
+ // archived slug becomes "telegram-bot" — the exact #1050 shape.
+ // ...
+
+ result, err := Sync(cliDir, Options{Force: true})
+ require.NoError(t, err)
+
+ // Every name-bearing surface must keep tracking the manifest's api_name.
+ var prov pipeline.CLIManifest
+ // ...
+ assert.Equal(t, "telegram", prov.APIName)
+ assert.Equal(t, "telegram-pp-mcp", prov.MCPBinary)
+ assert.Equal(t, "bin/telegram-pp-mcp", manifest.Server.EntryPoint)
+
+ // And no stray cmd/<bad-slug>-pp-{cli,mcp}/ directories.
+ _, err = os.Stat(filepath.Join(cliDir, "cmd", "telegram-bot-pp-cli"))
+ assert.True(t, os.IsNotExist(err))
+}
+```
+
+Unit tests of the override helper alone aren't enough — the surfaces are where the bug bites, so the test must assert through to the on-disk manifests and the absence of stray directories.
+
+## Related
+
+- [`docs/solutions/conventions/preserve-original-authorship-in-multi-author-retrofits-2026-05-06.md`](preserve-original-authorship-in-multi-author-retrofits-2026-05-06.md) — the authorship-retrofit lesson; same shape, applied to `Owner`/`OwnerName` retrofits via a curated map. Together with this doc, three identity fields now use the manifest-as-authority pattern.
+- [`docs/solutions/design-patterns/snapshot-merge-with-ast-classifier-for-force-regen-2026-05-10.md`](../design-patterns/snapshot-merge-with-ast-classifier-for-force-regen-2026-05-10.md) — the broader principle that regen must preserve user-chosen identity over re-derivation from environment/spec.
+- [`docs/solutions/design-patterns/dual-key-identity-fields-2026-05-06.md`](../design-patterns/dual-key-identity-fields-2026-05-06.md) — the slug-vs-display dual-key model used for Owner/OwnerName and Printer/PrinterName. Not directly relevant to single-key `api_name`, but the preserve-on-regen tier ordering applies.
+- AGENTS.md "Owner vs OwnerName" and "Printer vs PrinterName" sections — the canonical reference for resolution-order rules across identity fields.
+- [#1050](https://github.com/mvanhorn/cli-printing-press/issues/1050) — the originating issue.
diff --git a/internal/pipeline/mcpsync/sync.go b/internal/pipeline/mcpsync/sync.go
index 4f9cf449..0b440f5f 100644
--- a/internal/pipeline/mcpsync/sync.go
+++ b/internal/pipeline/mcpsync/sync.go
@@ -70,6 +70,16 @@ func Sync(cliDir string, opts Options) (Result, error) {
if err != nil {
return Result{}, err
}
+ // The manifest's api_name is the user's chosen identity (set by
+ // generate --name) and outranks the spec's info.title slug. Without
+ // this preempt, downstream emitters (WriteToolsManifest,
+ // populateMCPMetadata, GenerateMCPSurface) regenerate mcp_binary,
+ // api_name, manifest.json's name/entry_point, and
+ // cmd/<slug>-pp-{cli,mcp}/ directories under the title-derived slug
+ // — silently flipping a "telegram" CLI to "telegram-bot" mid-sync.
+ if prior := applyManifestNameOverride(cliDir, parsed); prior != "" {
+ fmt.Fprintf(os.Stderr, "mcp-sync: using manifest api_name %q over spec-derived slug %q\n", parsed.Name, prior)
+ }
// Validate that spec.yaml.name matches the directory's basename.
// Older library CLIs sometimes have drift (weather-goat's
// spec.yaml.name = "weather"; open-meteo's name diverges similarly)
@@ -468,6 +478,43 @@ func newRootCmd(flags *rootFlags) *cobra.Command {
return writeFileAtomic(path, []byte(src))
}
+// defaultConfigPathFormat is the spec-derived shape the OpenAPI/internal
+// parsers emit for Config.Path. The override only migrates paths matching
+// this shape; hand-customized paths (XDG-style overrides, per-environment
+// roots) are left alone.
+const defaultConfigPathFormat = "~/.config/%s/config.toml"
+
+// applyManifestNameOverride replaces parsed.Name with the existing
+// CLI manifest's api_name when the two diverge. Returns the prior
+// parsed.Name when an override happened, "" otherwise (manifest
+// missing, api_name empty, or values already agreed).
+func applyManifestNameOverride(cliDir string, parsed *spec.APISpec) string {
+ if parsed == nil {
+ return ""
+ }
+ m, err := pipeline.ReadCLIManifest(cliDir)
+ if err != nil {
+ // fs.ErrNotExist is the expected legacy-CLI case — fall through
+ // silently. A JSON parse failure (corrupted/partially-written
+ // manifest) is not expected: it would silently revert to the
+ // pre-fix spec-derived slug with no operator signal, so surface
+ // it on stderr.
+ if !errors.Is(err, fs.ErrNotExist) {
+ fmt.Fprintf(os.Stderr, "mcp-sync: could not read .printing-press.json (%v); falling back to spec-derived slug\n", err)
+ }
+ return ""
+ }
+ if m.APIName == "" || m.APIName == parsed.Name {
+ return ""
+ }
+ prior := parsed.Name
+ if parsed.Config.Path == fmt.Sprintf(defaultConfigPathFormat, naming.CLI(prior)) {
+ parsed.Config.Path = fmt.Sprintf(defaultConfigPathFormat, naming.CLI(m.APIName))
+ }
+ parsed.Name = m.APIName
+ return prior
+}
+
// 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
diff --git a/internal/pipeline/mcpsync/sync_test.go b/internal/pipeline/mcpsync/sync_test.go
index 89af5da9..bc7625f7 100644
--- a/internal/pipeline/mcpsync/sync_test.go
+++ b/internal/pipeline/mcpsync/sync_test.go
@@ -1230,3 +1230,240 @@ func TestSyncFallsThroughToRegistryDisplayName(t *testing.T) {
require.NoError(t, json.Unmarshal(provData, &prov))
assert.Equal(t, "TestBrand!", prov.DisplayName)
}
+
+// TestApplyManifestNameOverrideReplacesParsedName locks the contract
+// that the manifest's api_name outranks the spec-derived slug and the
+// spec-shaped Config.Path migrates alongside parsed.Name so downstream
+// emitters (mcp_binary, tools-manifest api_name, cmd/<slug>-pp-{cli,mcp}/,
+// README config path) all follow the manifest.
+func TestApplyManifestNameOverrideReplacesParsedName(t *testing.T) {
+ t.Parallel()
+ cliDir := filepath.Join(t.TempDir(), "telegram-pp-cli")
+ require.NoError(t, os.MkdirAll(cliDir, 0o755))
+ require.NoError(t, pipeline.WriteCLIManifest(cliDir, pipeline.CLIManifest{
+ APIName: "telegram",
+ CLIName: "telegram-pp-cli",
+ }))
+
+ parsed := &spec.APISpec{
+ Name: "telegram-bot",
+ Config: spec.ConfigSpec{
+ Format: "toml",
+ Path: "~/.config/telegram-bot-pp-cli/config.toml",
+ },
+ }
+ prior := applyManifestNameOverride(cliDir, parsed)
+ assert.Equal(t, "telegram-bot", prior, "should report the prior spec-derived slug")
+ assert.Equal(t, "telegram", parsed.Name, "parsed.Name should adopt the manifest's api_name")
+ assert.Equal(t, "~/.config/telegram-pp-cli/config.toml", parsed.Config.Path,
+ "spec-derived Config.Path must follow the override so README's Config file: line tracks the binary")
+}
+
+// TestApplyManifestNameOverrideNoOpWhenManifestAgrees — most CLIs are
+// already aligned (manifest api_name matches spec-derived slug). The
+// override must be a no-op so it doesn't churn unrelated state.
+func TestApplyManifestNameOverrideNoOpWhenManifestAgrees(t *testing.T) {
+ t.Parallel()
+ cliDir := filepath.Join(t.TempDir(), "producthunt-pp-cli")
+ require.NoError(t, os.MkdirAll(cliDir, 0o755))
+ require.NoError(t, pipeline.WriteCLIManifest(cliDir, pipeline.CLIManifest{
+ APIName: "producthunt",
+ CLIName: "producthunt-pp-cli",
+ }))
+
+ parsed := &spec.APISpec{
+ Name: "producthunt",
+ Config: spec.ConfigSpec{
+ Format: "toml",
+ Path: "~/.config/producthunt-pp-cli/config.toml",
+ },
+ }
+ prior := applyManifestNameOverride(cliDir, parsed)
+ assert.Equal(t, "", prior, "agreement should report no override")
+ assert.Equal(t, "producthunt", parsed.Name)
+ assert.Equal(t, "~/.config/producthunt-pp-cli/config.toml", parsed.Config.Path)
+}
+
+// TestApplyManifestNameOverrideMissingManifest — older library CLIs
+// predate .printing-press.json. The override must fall through silently
+// so the legacy reconcileSpecNameWithDir path stays in charge.
+func TestApplyManifestNameOverrideMissingManifest(t *testing.T) {
+ t.Parallel()
+ cliDir := filepath.Join(t.TempDir(), "legacy-pp-cli")
+ require.NoError(t, os.MkdirAll(cliDir, 0o755))
+
+ parsed := &spec.APISpec{
+ Name: "legacy-api",
+ Config: spec.ConfigSpec{
+ Format: "toml",
+ Path: "~/.config/legacy-api-pp-cli/config.toml",
+ },
+ }
+ prior := applyManifestNameOverride(cliDir, parsed)
+ assert.Equal(t, "", prior, "missing manifest should report no override")
+ assert.Equal(t, "legacy-api", parsed.Name, "parsed.Name must be untouched when manifest is absent")
+ assert.Equal(t, "~/.config/legacy-api-pp-cli/config.toml", parsed.Config.Path)
+}
+
+// TestApplyManifestNameOverrideMalformedJSON — a corrupted
+// .printing-press.json (truncated, partial write, hand-edited and
+// broken) must fall through without overriding, and must NOT be
+// indistinguishable from the missing-file case. The stderr warning
+// is the operator's only signal that the guard was bypassed.
+func TestApplyManifestNameOverrideMalformedJSON(t *testing.T) {
+ t.Parallel()
+ cliDir := filepath.Join(t.TempDir(), "corrupt-pp-cli")
+ require.NoError(t, os.MkdirAll(cliDir, 0o755))
+ // Truncated JSON — would unmarshal to an error, not "{}".
+ require.NoError(t, os.WriteFile(
+ filepath.Join(cliDir, pipeline.CLIManifestFilename),
+ []byte(`{"api_name": "corrupt`),
+ 0o644,
+ ))
+
+ parsed := &spec.APISpec{Name: "spec-derived"}
+ prior := applyManifestNameOverride(cliDir, parsed)
+ assert.Equal(t, "", prior, "malformed manifest must not produce an override")
+ assert.Equal(t, "spec-derived", parsed.Name, "parsed.Name must be preserved when the manifest cannot be parsed")
+}
+
+// TestApplyManifestNameOverrideEmptyAPIName — a malformed manifest with
+// no api_name (corrupted, partial write) must not clobber parsed.Name
+// with the empty string.
+func TestApplyManifestNameOverrideEmptyAPIName(t *testing.T) {
+ t.Parallel()
+ cliDir := filepath.Join(t.TempDir(), "partial-pp-cli")
+ require.NoError(t, os.MkdirAll(cliDir, 0o755))
+ require.NoError(t, pipeline.WriteCLIManifest(cliDir, pipeline.CLIManifest{
+ // APIName intentionally empty — simulates a partially-written manifest.
+ CLIName: "partial-pp-cli",
+ }))
+
+ parsed := &spec.APISpec{Name: "partial-api"}
+ prior := applyManifestNameOverride(cliDir, parsed)
+ assert.Equal(t, "", prior)
+ assert.Equal(t, "partial-api", parsed.Name, "empty api_name must not override")
+}
+
+// TestApplyManifestNameOverridePreservesCustomConfigPath — when an
+// operator hand-customized Config.Path (e.g., per-environment paths,
+// XDG_CONFIG_HOME-style overrides), the override must not rewrite it.
+// Only the spec-derived shape `~/.config/<old>-pp-cli/config.toml` is
+// migrated; everything else is preserved.
+func TestApplyManifestNameOverridePreservesCustomConfigPath(t *testing.T) {
+ t.Parallel()
+ cliDir := filepath.Join(t.TempDir(), "telegram-pp-cli")
+ require.NoError(t, os.MkdirAll(cliDir, 0o755))
+ require.NoError(t, pipeline.WriteCLIManifest(cliDir, pipeline.CLIManifest{
+ APIName: "telegram",
+ CLIName: "telegram-pp-cli",
+ }))
+
+ parsed := &spec.APISpec{
+ Name: "telegram-bot",
+ Config: spec.ConfigSpec{
+ Format: "toml",
+ Path: "$XDG_CONFIG_HOME/telegram/config.toml",
+ },
+ }
+ prior := applyManifestNameOverride(cliDir, parsed)
+ assert.Equal(t, "telegram-bot", prior)
+ assert.Equal(t, "telegram", parsed.Name)
+ assert.Equal(t, "$XDG_CONFIG_HOME/telegram/config.toml", parsed.Config.Path,
+ "hand-customized Config.Path must not be rewritten by the override")
+}
+
+// TestSyncManifestAPINameWinsOverSpecDerivedSlug is the end-to-end
+// integration: generate a CLI under a "telegram-pp-cli/" directory,
+// then rewrite spec.yaml so the parsed slug diverges. Asserts that
+// every name-bearing surface (.printing-press.json, tools-manifest.json,
+// manifest.json, cmd/ directories) keeps tracking the manifest's
+// api_name instead of regenerating against the spec-derived slug.
+func TestSyncManifestAPINameWinsOverSpecDerivedSlug(t *testing.T) {
+ t.Parallel()
+
+ apiSpec := &spec.APISpec{
+ Name: "telegram",
+ Version: "0.1.0",
+ BaseURL: "https://api.telegram.org",
+ Auth: spec.AuthConfig{
+ Type: "api_key",
+ Header: "Authorization",
+ Format: "Bearer {token}",
+ EnvVars: []string{"TELEGRAM_TOKEN"},
+ },
+ Config: spec.ConfigSpec{
+ Format: "toml",
+ Path: "~/.config/telegram-pp-cli/config.toml",
+ },
+ Resources: map[string]spec.Resource{
+ "messages": {
+ Description: "Send messages",
+ Endpoints: map[string]spec.Endpoint{
+ "send": {Method: "POST", Path: "/messages", Description: "Send a message"},
+ },
+ },
+ },
+ }
+ cliDir := filepath.Join(t.TempDir(), "telegram-pp-cli")
+ gen := generator.New(apiSpec, cliDir)
+ require.NoError(t, gen.Generate())
+ require.NoError(t, pipeline.WriteManifestForGenerate(pipeline.GenerateManifestParams{
+ APIName: apiSpec.Name,
+ OutputDir: cliDir,
+ Spec: apiSpec,
+ }))
+
+ // Rewrite spec.yaml so the archived spec slug diverges from the
+ // manifest api_name — the shape loadArchivedSpec produces when
+ // info.title's slug differs from the chosen --name.
+ driftedSpec := *apiSpec
+ driftedSpec.Name = "telegram-bot"
+ driftedSpec.Config.Path = "~/.config/telegram-bot-pp-cli/config.toml"
+ specData, err := yaml.Marshal(&driftedSpec)
+ require.NoError(t, err)
+ require.NoError(t, os.WriteFile(filepath.Join(cliDir, "spec.yaml"), specData, 0o644))
+
+ result, err := Sync(cliDir, Options{Force: true})
+ require.NoError(t, err)
+ assert.True(t, result.Changed)
+
+ // .printing-press.json's mcp_binary must keep tracking the manifest
+ // api_name, not the spec-derived "telegram-bot" slug.
+ provData, err := os.ReadFile(filepath.Join(cliDir, pipeline.CLIManifestFilename))
+ require.NoError(t, err)
+ var prov pipeline.CLIManifest
+ require.NoError(t, json.Unmarshal(provData, &prov))
+ assert.Equal(t, "telegram", prov.APIName, "api_name must hold the manifest-authoritative slug")
+ assert.Equal(t, "telegram-pp-mcp", prov.MCPBinary, "mcp_binary must derive from manifest api_name, not spec title")
+
+ // tools-manifest.json's api_name must follow the same source of truth.
+ toolsData, err := os.ReadFile(filepath.Join(cliDir, pipeline.ToolsManifestFilename))
+ require.NoError(t, err)
+ var tools struct {
+ APIName string `json:"api_name"`
+ }
+ require.NoError(t, json.Unmarshal(toolsData, &tools))
+ assert.Equal(t, "telegram", tools.APIName, "tools-manifest api_name must hold the manifest-authoritative slug")
+
+ // The MCPB manifest's name/entry_point must point at the actual
+ // built binary "telegram-pp-mcp", not "telegram-bot-pp-mcp".
+ manifestData, err := os.ReadFile(filepath.Join(cliDir, pipeline.MCPBManifestFilename))
+ require.NoError(t, err)
+ var manifest struct {
+ Name string `json:"name"`
+ Server struct {
+ EntryPoint string `json:"entry_point"`
+ } `json:"server"`
+ }
+ require.NoError(t, json.Unmarshal(manifestData, &manifest))
+ assert.Equal(t, "telegram-pp-mcp", manifest.Name, "MCPB manifest name must derive from manifest api_name")
+ assert.Equal(t, "bin/telegram-pp-mcp", manifest.Server.EntryPoint, "entry_point must derive from manifest api_name, not spec title")
+
+ // No stray cmd/telegram-bot-pp-{cli,mcp}/ directories should have
+ // been created by GenerateMCPSurface.
+ _, err = os.Stat(filepath.Join(cliDir, "cmd", "telegram-bot-pp-cli"))
+ assert.True(t, os.IsNotExist(err), "no stray cmd/telegram-bot-pp-cli/ directory should be created")
+ _, err = os.Stat(filepath.Join(cliDir, "cmd", "telegram-bot-pp-mcp"))
+ assert.True(t, os.IsNotExist(err), "no stray cmd/telegram-bot-pp-mcp/ directory should be created")
+}
← 28e2fa1b fix(cli): guard nil Phases in LoadState to prevent lock prom
·
back to Cli Printing Press
·
fix(cli): populate parent FK in generated UpsertBatch typed- 9e0fe96c →