[object Object]

← back to Cli Printing Press

fix(cli): archive merged spec for multi-spec generate runs (#1432)

44f1ff7956099c218c7531f562d2fbddfa259659 · 2026-05-15 00:29:02 -0700 · Trevin Chow

* fix(cli): archive merged spec for multi-spec generate runs

`printing-press generate --spec A --spec B ...` was archiving only the
first input spec's raw bytes to <output>/spec.json, so every downstream
tool that re-reads that snapshot (mcp-sync, dogfood, validate-narrative)
saw one input instead of the merged surface. CLI naming, MCP tool
counts, and dogfood path validity all silently drifted on multi-spec
CLIs until polish manually rewrote spec.json.

Extract `archiveSpecBytes` to pick the archive bytes and filename:
single-spec runs preserve the user's original input verbatim
(post-redaction) so audit/replay stays byte-identical; multi-spec runs
serialize the merged in-memory APISpec as canonical JSON so the
snapshot reflects what the generator actually emitted.

The merged APISpec already survives a round-trip through `spec.ParseBytes`
because `expandOperations`, `enrichPathParams`, and
`promoteParamsToBodyForWriteEndpoints` are idempotent.

Closes #1195

* fix(cli): guard archiveSpecBytes multi-spec branch against nil apiSpec

json.MarshalIndent on a nil pointer succeeds with the literal "null"
bytes, which would write a syntactically-valid but useless snapshot.
The current call site never passes nil into the multi-spec branch, but
an explicit guard keeps the precondition obvious for future refactors
and tests.

Addresses Greptile P1 review feedback on #1432.

Files touched

Diff

commit 44f1ff7956099c218c7531f562d2fbddfa259659
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Fri May 15 00:29:02 2026 -0700

    fix(cli): archive merged spec for multi-spec generate runs (#1432)
    
    * fix(cli): archive merged spec for multi-spec generate runs
    
    `printing-press generate --spec A --spec B ...` was archiving only the
    first input spec's raw bytes to <output>/spec.json, so every downstream
    tool that re-reads that snapshot (mcp-sync, dogfood, validate-narrative)
    saw one input instead of the merged surface. CLI naming, MCP tool
    counts, and dogfood path validity all silently drifted on multi-spec
    CLIs until polish manually rewrote spec.json.
    
    Extract `archiveSpecBytes` to pick the archive bytes and filename:
    single-spec runs preserve the user's original input verbatim
    (post-redaction) so audit/replay stays byte-identical; multi-spec runs
    serialize the merged in-memory APISpec as canonical JSON so the
    snapshot reflects what the generator actually emitted.
    
    The merged APISpec already survives a round-trip through `spec.ParseBytes`
    because `expandOperations`, `enrichPathParams`, and
    `promoteParamsToBodyForWriteEndpoints` are idempotent.
    
    Closes #1195
    
    * fix(cli): guard archiveSpecBytes multi-spec branch against nil apiSpec
    
    json.MarshalIndent on a nil pointer succeeds with the literal "null"
    bytes, which would write a syntactically-valid but useless snapshot.
    The current call site never passes nil into the multi-spec branch, but
    an explicit guard keeps the precondition obvious for future refactors
    and tests.
    
    Addresses Greptile P1 review feedback on #1432.
---
 internal/cli/generate_test.go | 175 ++++++++++++++++++++++++++++++++++++++++++
 internal/cli/root.go          |  50 +++++++++---
 2 files changed, 216 insertions(+), 9 deletions(-)

diff --git a/internal/cli/generate_test.go b/internal/cli/generate_test.go
index b72f5e27..a9a819ef 100644
--- a/internal/cli/generate_test.go
+++ b/internal/cli/generate_test.go
@@ -1017,6 +1017,181 @@ resources:
 
 }
 
+// TestGenerateArchivesMergedSpecForMultiSpec asserts that a generate run
+// with multiple --spec inputs archives the merged in-memory APISpec — with
+// the merged title and the union of resources — rather than just the first
+// input's raw bytes.
+func TestGenerateArchivesMergedSpecForMultiSpec(t *testing.T) {
+	t.Parallel()
+
+	dir := t.TempDir()
+	coreSpecPath := filepath.Join(dir, "core.yaml")
+	wikiSpecPath := filepath.Join(dir, "wiki.yaml")
+	outputDir := filepath.Join(dir, "combo")
+	require.NoError(t, os.WriteFile(coreSpecPath, []byte(`name: core
+description: Core API
+version: 0.1.0
+base_url: https://tenant.example.com
+auth:
+  type: none
+resources:
+  projects:
+    description: Projects
+    endpoints:
+      list:
+        method: GET
+        path: /projects
+        description: List projects
+`), 0o644))
+	require.NoError(t, os.WriteFile(wikiSpecPath, []byte(`name: wiki
+description: Wiki API
+version: 0.1.0
+base_url: https://tenant.example.com/wiki/api/v2
+auth:
+  type: none
+resources:
+  pages:
+    description: Pages
+    endpoints:
+      list:
+        method: GET
+        path: /pages
+        description: List pages
+`), 0o644))
+
+	cmd := newGenerateCmd()
+	cmd.SetArgs([]string{
+		"--spec", coreSpecPath,
+		"--spec", wikiSpecPath,
+		"--name", "combo",
+		"--output", outputDir,
+		"--validate=false",
+		"--force",
+	})
+	require.NoError(t, cmd.Execute())
+
+	// Multi-spec must archive as spec.json (canonical JSON of merged APISpec),
+	// not spec.yaml (which would be the first input verbatim).
+	assert.NoFileExists(t, filepath.Join(outputDir, "spec.yaml"))
+	archived, err := os.ReadFile(filepath.Join(outputDir, "spec.json"))
+	require.NoError(t, err)
+
+	parsed, err := spec.ParseBytes(archived)
+	require.NoError(t, err, "archived merged spec must round-trip through spec.ParseBytes")
+
+	// The merged title is the --name, not "core" (the first input).
+	assert.Equal(t, "combo", parsed.Name)
+
+	// The merged BaseURL pins the archive to the merged struct rather than to
+	// either input alone — both inputs share https://tenant.example.com as the
+	// host, and that is what mergeSpecs resolves to.
+	assert.Equal(t, "https://tenant.example.com", parsed.BaseURL, "archived merged spec carries the merged BaseURL")
+
+	// Both inputs' resources are present in the archived snapshot.
+	assert.Contains(t, parsed.Resources, "projects", "first-input resource preserved")
+	assert.Contains(t, parsed.Resources, "pages", "second-input resource present in merged archive")
+}
+
+// TestArchiveSpecBytesBranches covers each branch of archiveSpecBytes
+// directly, including the json-input single-spec arm that the integration
+// tests above don't exercise (their fixtures are YAML).
+func TestArchiveSpecBytesBranches(t *testing.T) {
+	t.Parallel()
+
+	t.Run("multi-spec marshals merged APISpec as JSON", func(t *testing.T) {
+		merged := &spec.APISpec{
+			Name:      "combo",
+			Version:   "0.1.0",
+			BaseURL:   "https://example.com",
+			Resources: map[string]spec.Resource{"things": {Description: "Things"}},
+		}
+		specs := []*spec.APISpec{
+			{Name: "a", Resources: map[string]spec.Resource{}},
+			{Name: "b", Resources: map[string]spec.Resource{}},
+		}
+		data, name, ok := archiveSpecBytes(merged, specs, [][]byte{[]byte("a"), []byte("b")})
+		require.True(t, ok)
+		assert.Equal(t, "spec.json", name)
+		assert.True(t, json.Valid(data), "multi-spec archive must be valid JSON")
+		assert.Contains(t, string(data), `"name": "combo"`)
+	})
+
+	t.Run("single-spec JSON input returns raw bytes as spec.json", func(t *testing.T) {
+		jsonInput := []byte(`{"name":"solo","resources":{}}`)
+		data, name, ok := archiveSpecBytes(nil, []*spec.APISpec{{Name: "solo"}}, [][]byte{jsonInput})
+		require.True(t, ok)
+		assert.Equal(t, "spec.json", name)
+		assert.Equal(t, jsonInput, data, "single-spec JSON archive is byte-identical to input")
+	})
+
+	t.Run("single-spec YAML input returns raw bytes as spec.yaml", func(t *testing.T) {
+		yamlInput := []byte("name: solo\nresources: {}\n")
+		data, name, ok := archiveSpecBytes(nil, []*spec.APISpec{{Name: "solo"}}, [][]byte{yamlInput})
+		require.True(t, ok)
+		assert.Equal(t, "spec.yaml", name)
+		assert.Equal(t, yamlInput, data, "single-spec YAML archive is byte-identical to input")
+	})
+
+	t.Run("no inputs returns ok=false", func(t *testing.T) {
+		_, _, ok := archiveSpecBytes(nil, nil, nil)
+		assert.False(t, ok)
+	})
+
+	t.Run("multi-spec with nil apiSpec returns ok=false", func(t *testing.T) {
+		specs := []*spec.APISpec{
+			{Name: "a"},
+			{Name: "b"},
+		}
+		_, _, ok := archiveSpecBytes(nil, specs, [][]byte{[]byte("a"), []byte("b")})
+		assert.False(t, ok, "nil apiSpec must not silently archive the literal 'null'")
+	})
+}
+
+// TestGenerateArchivesRawSpecForSingleSpec asserts that single-spec runs
+// archive the user's original input bytes verbatim (post-redaction), the
+// negative case complementing the multi-spec merged-archive behavior.
+func TestGenerateArchivesRawSpecForSingleSpec(t *testing.T) {
+	t.Parallel()
+
+	dir := t.TempDir()
+	specPath := filepath.Join(dir, "spec.yaml")
+	outputDir := filepath.Join(dir, "solo")
+	input := []byte(`name: solo
+description: Solo API
+version: 0.1.0
+base_url: https://api.example.com
+auth:
+  type: none
+resources:
+  things:
+    description: Things
+    endpoints:
+      list:
+        method: GET
+        path: /things
+        description: List things
+`)
+	require.NoError(t, os.WriteFile(specPath, input, 0o644))
+
+	cmd := newGenerateCmd()
+	cmd.SetArgs([]string{
+		"--spec", specPath,
+		"--output", outputDir,
+		"--validate=false",
+		"--force",
+	})
+	require.NoError(t, cmd.Execute())
+
+	// YAML input archives as spec.yaml (not spec.json).
+	assert.NoFileExists(t, filepath.Join(outputDir, "spec.json"))
+	archived, err := os.ReadFile(filepath.Join(outputDir, "spec.yaml"))
+	require.NoError(t, err)
+
+	// Single-spec archive is byte-identical to the input (this spec contains
+	// no secrets, so redaction is a no-op).
+	assert.Equal(t, input, archived, "single-spec archive must preserve original bytes")
+}
+
 func TestMergeSpecsPrefersReplayableBrowserTransportOverUnshippablePageContext(t *testing.T) {
 	t.Parallel()
 
diff --git a/internal/cli/root.go b/internal/cli/root.go
index 39043fd6..1945a8e4 100644
--- a/internal/cli/root.go
+++ b/internal/cli/root.go
@@ -383,15 +383,10 @@ func newGenerateCmd() *cobra.Command {
 				fmt.Fprintf(os.Stderr, "warning: could not write manifest: %v\n", err)
 			}
 
-			// Archive the input spec alongside the CLI for reproducibility.
-			// The spec_url may change or disappear; this local copy is the
-			// only guaranteed way to regenerate from the exact same input.
-			if len(specRawBytes) > 0 {
-				archiveName := "spec.yaml"
-				if json.Valid(specRawBytes[0]) {
-					archiveName = "spec.json"
-				}
-				data := artifacts.RedactArchivedSpecSecrets(specRawBytes[0])
+			// Archive a snapshot of the spec alongside the CLI; multi-spec
+			// runs use the merged form (see archiveSpecBytes for why).
+			if archiveBytes, archiveName, ok := archiveSpecBytes(apiSpec, specs, specRawBytes); ok {
+				data := artifacts.RedactArchivedSpecSecrets(archiveBytes)
 				if err := os.WriteFile(filepath.Join(absOut, archiveName), data, 0o644); err != nil {
 					fmt.Fprintf(os.Stderr, "warning: could not archive spec: %v\n", err)
 				}
@@ -710,6 +705,43 @@ func parseOpenAPISpec(specFile string, data []byte, lenient bool) (*spec.APISpec
 	return openapi.ParseWithPath(data, specFile)
 }
 
+// archiveSpecBytes picks the bytes and filename for the spec snapshot that
+// generate writes alongside the CLI. Single-spec runs preserve the user's
+// original input (post-redaction at the call site) so audit/replay round-trip
+// against the same bytes the parser saw. Multi-spec runs serialize the merged
+// APISpec — its union of paths, merged title, and merged x-mcp config — so
+// downstream consumers that re-read this snapshot operate on the surface the
+// generator actually emitted rather than on whichever input happened to be
+// passed first.
+//
+// Returns ok=false when there is nothing to archive (no inputs) or when
+// marshalling the merged spec failed; the call site logs and continues so a
+// transient archive failure does not abort generation.
+func archiveSpecBytes(apiSpec *spec.APISpec, specs []*spec.APISpec, specRawBytes [][]byte) ([]byte, string, bool) {
+	if len(specs) > 1 {
+		// json.MarshalIndent on a nil pointer succeeds with the literal
+		// "null" bytes, which would write a syntactically-valid but
+		// useless snapshot. Surface the precondition explicitly.
+		if apiSpec == nil {
+			return nil, "", false
+		}
+		data, err := json.MarshalIndent(apiSpec, "", "  ")
+		if err != nil {
+			fmt.Fprintf(os.Stderr, "warning: could not marshal merged spec for archive: %v\n", err)
+			return nil, "", false
+		}
+		return data, "spec.json", true
+	}
+	if len(specRawBytes) == 0 {
+		return nil, "", false
+	}
+	raw := specRawBytes[0]
+	if json.Valid(raw) {
+		return raw, "spec.json", true
+	}
+	return raw, "spec.yaml", true
+}
+
 func mergeSpecs(specs []*spec.APISpec, name string) *spec.APISpec {
 	if len(specs) == 1 {
 		return specs[0]

← acbb3dfe fix(cli): bypass response cache when polling async jobs (#14  ·  back to Cli Printing Press  ·  fix(cli): extend verify-skill recipe scan to README.md (#143 73fe5673 →