← back to Cli Printing Press

fix(cli): populate manifest.Category from spec when catalog misses (#735)

a00e97b198eec5fc5f53b8877439b2462602e39d · 2026-05-08 13:23:46 -0700 · Joe Heitzeberg

* fix(cli): populate manifest.Category from spec when catalog misses

Synthetic CLIs (not in the embedded catalog) silently lost their
spec.Category at manifest-write time, breaking verify-skill's
canonical-sections check downstream.

## The chain

The README/SKILL templates correctly resolve `.Category` from the
generator's pipeline state and render `library/<category>/...` URLs.
But `internal/pipeline/climanifest.go` only sources `m.Category` from
the embedded catalog:

    if entry, err := catalogpkg.LookupFS(catalog.FS, p.APIName); err == nil {
        m.Category = entry.Category
        ...
    }

For a CLI with `category: travel` in spec.yaml but no catalog entry,
the catalog lookup fails silently and `m.Category` stays empty.
Downstream:

- `internal/cli/verify_skill.go:163` reads `manifest.Category`,
  passes it to `generator.CanonicalSkillInstallSection`.
- `internal/generator/install_section.go:60` falls back to "other"
  when category is empty.
- The verifier compares the SKILL (which says `library/travel/`,
  rendered from spec via the template's `.Category`) against the
  expected canonical (`library/other/`) and reports an install-
  section drift.

Each component was doing the right thing in isolation; only the
manifest writer dropped category on the floor.

## Fix

Two sites — both follow the same pattern: catalog wins, spec is the
fallback for synthetic CLIs.

- `WriteManifestForGenerate` (climanifest.go): fall back to
  `p.Spec.Category` after the catalog lookup.
- `PublishWorkingCLI` (publish.go): same fallback, sourced from the
  parsed spec (already loaded later in the function for MCP metadata).

## Tests

Three new tests in `climanifest_test.go`:

- `TestWriteManifestForGeneratePopulatesCategoryFromSpec` — pins the
  fix: synthetic CLI with `spec.Category=travel` produces
  `manifest.Category=travel`.
- `TestWriteManifestForGenerateCatalogCategoryWinsOverSpec` — pins
  precedence: catalog category wins over spec when both are present.
- `TestWriteManifestForGenerateNoCategoryAnywhere` — pins the empty-
  case behavior: manifest.Category stays empty when no source
  provides one (so install_section.go's "other" fallback still
  fires for un-categorized CLIs).

## Verification

- `go test ./...` — clean
- `scripts/golden.sh verify` — 13/13 PASS (no fixture drift, because
  every existing golden CLI either is in the catalog or has an empty
  spec.Category)
- `go vet ./...` — clean

## How this was found

This bug surfaced while an AI assistant (Claude Code) was running the
`/printing-press` skill for a user — `wanderlust-goat`, a synthetic
multi-source CLI with `category: travel` in its spec. `publish
validate` reported a `verify-skill` canonical-sections failure that
required hand-editing the SKILL's install URL from `library/other/`
to `library/travel/` before publish would proceed (see
mvanhorn/printing-press-library#291). Tracing the chain backward
isolated this issue to the manifest writer.

Note from the AI: I'm filing this on the user's behalf. The user
trusts the diagnosis and approved the PR; I added test coverage,
ran the full test suite + golden harness, and confirmed the fix
doesn't shift any existing fixture output. The PR body above is my
own write-up — please review the actual diff carefully rather than
relying on the prose.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* test(cli): cover publish manifest category fallback

---------

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

Files touched

• M internal/pipeline/climanifest.go
• M internal/pipeline/climanifest_test.go
• M internal/pipeline/publish.go
• M internal/pipeline/publish_test.go

Diff

commit a00e97b198eec5fc5f53b8877439b2462602e39d
Author: Joe Heitzeberg <jheitzeb@gmail.com>
Date:   Fri May 8 13:23:46 2026 -0700

    fix(cli): populate manifest.Category from spec when catalog misses (#735)
    
    * fix(cli): populate manifest.Category from spec when catalog misses
    
    Synthetic CLIs (not in the embedded catalog) silently lost their
    spec.Category at manifest-write time, breaking verify-skill's
    canonical-sections check downstream.
    
    ## The chain
    
    The README/SKILL templates correctly resolve `.Category` from the
    generator's pipeline state and render `library/<category>/...` URLs.
    But `internal/pipeline/climanifest.go` only sources `m.Category` from
    the embedded catalog:
    
        if entry, err := catalogpkg.LookupFS(catalog.FS, p.APIName); err == nil {
            m.Category = entry.Category
            ...
        }
    
    For a CLI with `category: travel` in spec.yaml but no catalog entry,
    the catalog lookup fails silently and `m.Category` stays empty.
    Downstream:
    
    - `internal/cli/verify_skill.go:163` reads `manifest.Category`,
      passes it to `generator.CanonicalSkillInstallSection`.
    - `internal/generator/install_section.go:60` falls back to "other"
      when category is empty.
    - The verifier compares the SKILL (which says `library/travel/`,
      rendered from spec via the template's `.Category`) against the
      expected canonical (`library/other/`) and reports an install-
      section drift.
    
    Each component was doing the right thing in isolation; only the
    manifest writer dropped category on the floor.
    
    ## Fix
    
    Two sites — both follow the same pattern: catalog wins, spec is the
    fallback for synthetic CLIs.
    
    - `WriteManifestForGenerate` (climanifest.go): fall back to
      `p.Spec.Category` after the catalog lookup.
    - `PublishWorkingCLI` (publish.go): same fallback, sourced from the
      parsed spec (already loaded later in the function for MCP metadata).
    
    ## Tests
    
    Three new tests in `climanifest_test.go`:
    
    - `TestWriteManifestForGeneratePopulatesCategoryFromSpec` — pins the
      fix: synthetic CLI with `spec.Category=travel` produces
      `manifest.Category=travel`.
    - `TestWriteManifestForGenerateCatalogCategoryWinsOverSpec` — pins
      precedence: catalog category wins over spec when both are present.
    - `TestWriteManifestForGenerateNoCategoryAnywhere` — pins the empty-
      case behavior: manifest.Category stays empty when no source
      provides one (so install_section.go's "other" fallback still
      fires for un-categorized CLIs).
    
    ## Verification
    
    - `go test ./...` — clean
    - `scripts/golden.sh verify` — 13/13 PASS (no fixture drift, because
      every existing golden CLI either is in the catalog or has an empty
      spec.Category)
    - `go vet ./...` — clean
    
    ## How this was found
    
    This bug surfaced while an AI assistant (Claude Code) was running the
    `/printing-press` skill for a user — `wanderlust-goat`, a synthetic
    multi-source CLI with `category: travel` in its spec. `publish
    validate` reported a `verify-skill` canonical-sections failure that
    required hand-editing the SKILL's install URL from `library/other/`
    to `library/travel/` before publish would proceed (see
    mvanhorn/printing-press-library#291). Tracing the chain backward
    isolated this issue to the manifest writer.
    
    Note from the AI: I'm filing this on the user's behalf. The user
    trusts the diagnosis and approved the PR; I added test coverage,
    ran the full test suite + golden harness, and confirmed the fix
    doesn't shift any existing fixture output. The PR body above is my
    own write-up — please review the actual diff carefully rather than
    relying on the prose.
    
    🤖 Generated with [Claude Code](https://claude.com/claude-code)
    
    Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
    
    * test(cli): cover publish manifest category fallback
    
    ---------
    
    Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
    Co-authored-by: Trevin Chow <trevin@trevinchow.com>
---
 internal/pipeline/climanifest.go      | 11 ++++++
 internal/pipeline/climanifest_test.go | 73 +++++++++++++++++++++++++++++++++++
 internal/pipeline/publish.go          | 10 +++++
 internal/pipeline/publish_test.go     | 31 +++++++++++++++
 4 files changed, 125 insertions(+)

diff --git a/internal/pipeline/climanifest.go b/internal/pipeline/climanifest.go
index 8741bff0..f69b6ea7 100644
--- a/internal/pipeline/climanifest.go
+++ b/internal/pipeline/climanifest.go
@@ -469,6 +469,17 @@ func WriteManifestForGenerate(p GenerateManifestParams) error {
 			m.DisplayName = entry.DisplayName
 		}
 	}
+	// Fall back to spec.Category for synthetic CLIs that aren't in the
+	// embedded catalog. Without this, manifest.Category stays empty even
+	// when the spec sets `category: travel`, and verify-skill's canonical-
+	// sections check then expects the install URL to use "other" — putting
+	// the rendered SKILL (which read category from the spec via the
+	// template's .Category) and the manifest-derived expected SKILL out of
+	// sync. The README/SKILL templates already resolve category through the
+	// spec; the manifest writer was the lone holdout.
+	if m.Category == "" && p.Spec != nil && p.Spec.Category != "" {
+		m.Category = p.Spec.Category
+	}
 
 	// Record the API version from the spec for provenance (not the CLI version).
 	if p.Spec != nil && p.Spec.Version != "" {
diff --git a/internal/pipeline/climanifest_test.go b/internal/pipeline/climanifest_test.go
index dabe29a4..b15b1613 100644
--- a/internal/pipeline/climanifest_test.go
+++ b/internal/pipeline/climanifest_test.go
@@ -1350,3 +1350,76 @@ func TestPopulateMCPMetadataCLIDescription(t *testing.T) {
 		assert.Equal(t, "Catalog description.", m.Description)
 	})
 }
+
+// TestWriteManifestForGeneratePopulatesCategoryFromSpec pins the fallback
+// that lets synthetic CLIs (not in the embedded catalog) carry their
+// spec.Category through to .printing-press.json. Without this fallback,
+// verify-skill's canonical-sections check expects the install URL to use
+// "other" while the rendered SKILL (which reads category from the spec
+// via the template's .Category) uses the real category — a structural
+// drift that breaks publish for any synthetic-CLI category.
+func TestWriteManifestForGeneratePopulatesCategoryFromSpec(t *testing.T) {
+	dir := t.TempDir()
+
+	err := WriteManifestForGenerate(GenerateManifestParams{
+		// "synthetic-travel-cli" is not in the embedded catalog; the
+		// catalog lookup will fail and the spec.Category fallback fires.
+		APIName:   "synthetic-travel-cli",
+		OutputDir: dir,
+		Spec: &spec.APISpec{
+			Name:     "synthetic-travel-cli",
+			Category: "travel",
+			Auth:     spec.AuthConfig{Type: "none"},
+		},
+	})
+	require.NoError(t, err)
+
+	got := readPublishedManifest(t, dir)
+	assert.Equal(t, "travel", got.Category, "spec.Category should populate manifest.Category for synthetic CLIs")
+}
+
+// TestWriteManifestForGenerateCatalogCategoryWinsOverSpec pins precedence:
+// when an API IS in the embedded catalog, the catalog's category wins.
+// The spec.Category fallback only fires when the catalog lookup misses.
+// Important because catalog-listed APIs may have richer category metadata
+// (e.g., curated overrides) the spec doesn't reflect.
+func TestWriteManifestForGenerateCatalogCategoryWinsOverSpec(t *testing.T) {
+	dir := t.TempDir()
+
+	// asana is in the embedded catalog with category=project-management.
+	// The spec carries a different category to confirm the catalog wins.
+	err := WriteManifestForGenerate(GenerateManifestParams{
+		APIName:   "asana",
+		OutputDir: dir,
+		Spec: &spec.APISpec{
+			Name:     "asana",
+			Category: "developer-tools", // would-be override
+			Auth:     spec.AuthConfig{Type: "none"},
+		},
+	})
+	require.NoError(t, err)
+
+	got := readPublishedManifest(t, dir)
+	assert.Equal(t, "project-management", got.Category, "catalog category must win over spec category")
+}
+
+// TestWriteManifestForGenerateNoCategoryAnywhere pins that the manifest's
+// category stays empty when neither the catalog nor the spec carries one.
+// (verify-skill / install_section.go then default to "other" downstream;
+// that fallback is the intended behavior for un-categorized CLIs.)
+func TestWriteManifestForGenerateNoCategoryAnywhere(t *testing.T) {
+	dir := t.TempDir()
+
+	err := WriteManifestForGenerate(GenerateManifestParams{
+		APIName:   "synthetic-uncategorized",
+		OutputDir: dir,
+		Spec: &spec.APISpec{
+			Name: "synthetic-uncategorized",
+			Auth: spec.AuthConfig{Type: "none"},
+		},
+	})
+	require.NoError(t, err)
+
+	got := readPublishedManifest(t, dir)
+	assert.Empty(t, got.Category, "manifest.Category should stay empty when no source provides one")
+}
diff --git a/internal/pipeline/publish.go b/internal/pipeline/publish.go
index 0d0a98b5..98bf4a1a 100644
--- a/internal/pipeline/publish.go
+++ b/internal/pipeline/publish.go
@@ -290,6 +290,16 @@ func writeCLIManifestForPublish(state *PipelineState, dir string) error {
 			populateMCPMetadata(&m, parsed)
 		}
 
+		// Fall back to spec.Category for synthetic CLIs not in the embedded
+		// catalog (mirrors the same fallback in WriteManifestForGenerate).
+		// The catalog lookup earlier in this function only fires for
+		// catalog-listed APIs; synthetic CLIs would otherwise lose the
+		// spec's category at publish time and break verify-skill's
+		// canonical-sections check.
+		if m.Category == "" && parsed != nil && parsed.Category != "" {
+			m.Category = parsed.Category
+		}
+
 		// Generate tools-manifest.json for diagnostic commands
 		// (auth-doctor, mcp-audit). Non-blocking: log warning on error
 		// but don't fail the publish.
diff --git a/internal/pipeline/publish_test.go b/internal/pipeline/publish_test.go
index 0df2b584..5fe4bf8d 100644
--- a/internal/pipeline/publish_test.go
+++ b/internal/pipeline/publish_test.go
@@ -197,6 +197,37 @@ paths: {}
 	assert.Equal(t, "Product Hunt", m.DisplayName)
 }
 
+func TestWriteCLIManifestForPublishPopulatesCategoryFromSpec(t *testing.T) {
+	tmp := t.TempDir()
+	t.Setenv("PRINTING_PRESS_HOME", tmp)
+	t.Setenv("PRINTING_PRESS_SCOPE", "test-scope")
+	t.Setenv("PRINTING_PRESS_REPO_ROOT", tmp)
+
+	state := NewStateWithRun("synthetic-travel-cli", filepath.Join(tmp, "working", "synthetic-travel-cli-pp-cli"), "20260508-category", "test-scope")
+	require.NoError(t, os.MkdirAll(state.WorkingDir, 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(state.WorkingDir, "spec.yaml"), []byte(`
+name: synthetic-travel-cli
+description: Synthetic travel CLI
+version: "1.0"
+base_url: https://api.example.com
+category: travel
+auth:
+  type: none
+resources:
+  trips:
+    description: Trips
+    endpoints:
+      list:
+        method: GET
+        path: /trips
+`), 0o644))
+
+	require.NoError(t, writeCLIManifestForPublish(state, state.WorkingDir))
+
+	m := readPublishedManifest(t, state.WorkingDir)
+	assert.Equal(t, "travel", m.Category)
+}
+
 // TestWriteCLIManifestForPublish_NovelFeaturesFromPrintFlowResearch covers the
 // printing-press print flow: research.json lives at <RunRoot>/pipeline/research.json
 // alongside phase artifacts. The fallback path keeps print-flow CLIs working.

← 6044e7ff Parallelize Main CI full suite jobs (#743)  ·  back to Cli Printing Press  ·  Add contributor PR template and guide (#744) 4ca4b7cf →