[object Object]

← back to Cli Printing Press

fix(cli): preserve x-mcp config when merging combo specs (#1187)

660829ed3cce879fc15209d50fe39df96594ff8e · 2026-05-12 02:48:48 -0700 · Trevin Chow

mergeSpecs (internal/cli/root.go:706) iterates input specs once and copies
fields like Auth.AuthorizationURL via a first-wins precedence pattern but
never copied MCP. Combo runs with x-mcp declared on any input spec lost
the config silently, so the downstream code-orchestration gate
(IsCodeOrchestration()) saw the zero value and combo CLIs fell back to
the default endpoint-mirror MCP surface regardless of what the input
specs requested.

Add the missing precedence loop using the already-defined mcpConfigured
helper (root.go:1549, already used by applyCatalogEntry at line 1532).

Closes #1044

Files touched

Diff

commit 660829ed3cce879fc15209d50fe39df96594ff8e
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Tue May 12 02:48:48 2026 -0700

    fix(cli): preserve x-mcp config when merging combo specs (#1187)
    
    mergeSpecs (internal/cli/root.go:706) iterates input specs once and copies
    fields like Auth.AuthorizationURL via a first-wins precedence pattern but
    never copied MCP. Combo runs with x-mcp declared on any input spec lost
    the config silently, so the downstream code-orchestration gate
    (IsCodeOrchestration()) saw the zero value and combo CLIs fell back to
    the default endpoint-mirror MCP surface regardless of what the input
    specs requested.
    
    Add the missing precedence loop using the already-defined mcpConfigured
    helper (root.go:1549, already used by applyCatalogEntry at line 1532).
    
    Closes #1044
---
 internal/cli/generate_test.go | 130 ++++++++++++++++++++++++++++++++++++++++++
 internal/cli/root.go          |   4 ++
 2 files changed, 134 insertions(+)

diff --git a/internal/cli/generate_test.go b/internal/cli/generate_test.go
index 9a4d4d94..80b95418 100644
--- a/internal/cli/generate_test.go
+++ b/internal/cli/generate_test.go
@@ -1211,6 +1211,136 @@ func TestMergeSpecsLeavesPathsUnchangedWhenHostsDiffer(t *testing.T) {
 	assert.Equal(t, "/bar", merged.Resources["bar"].Endpoints["list"].Path)
 }
 
+// TestMergeSpecsCarriesMCPConfigFromFirstDeclaringSpec verifies that a combo
+// run preserves x-mcp configuration from the first input spec that declares
+// it. Without this precedence loop, mergeSpecs silently drops MCP config and
+// the downstream code-orchestration gate falls back to the endpoint-mirror
+// surface no matter what the input specs requested.
+func TestMergeSpecsCarriesMCPConfigFromFirstDeclaringSpec(t *testing.T) {
+	t.Parallel()
+
+	specA := &spec.APISpec{
+		Name:      "a",
+		Version:   "0.1.0",
+		BaseURL:   "https://api.example.com",
+		Resources: map[string]spec.Resource{},
+		Types:     map[string]spec.TypeDef{},
+	}
+	specB := &spec.APISpec{
+		Name:      "b",
+		Version:   "0.1.0",
+		BaseURL:   "https://api.example.com",
+		Resources: map[string]spec.Resource{},
+		Types:     map[string]spec.TypeDef{},
+		MCP: spec.MCPConfig{
+			Orchestration: "code",
+		},
+	}
+
+	merged := mergeSpecs([]*spec.APISpec{specA, specB}, "combo")
+
+	assert.True(t, merged.MCP.IsCodeOrchestration(), "merged MCP should retain x-mcp.orchestration=code from specB")
+}
+
+// TestMergeSpecsLeavesMCPZeroWhenNoSpecDeclares verifies that combos without
+// any x-mcp declaration produce a zero-valued merged MCP, so the existing
+// endpoint-mirror default and "consider code-orchestration" advisory keep
+// working unchanged.
+func TestMergeSpecsLeavesMCPZeroWhenNoSpecDeclares(t *testing.T) {
+	t.Parallel()
+
+	specA := &spec.APISpec{
+		Name:      "a",
+		Version:   "0.1.0",
+		BaseURL:   "https://api.example.com",
+		Resources: map[string]spec.Resource{},
+		Types:     map[string]spec.TypeDef{},
+	}
+	specB := &spec.APISpec{
+		Name:      "b",
+		Version:   "0.1.0",
+		BaseURL:   "https://api.example.com",
+		Resources: map[string]spec.Resource{},
+		Types:     map[string]spec.TypeDef{},
+	}
+
+	merged := mergeSpecs([]*spec.APISpec{specA, specB}, "combo")
+
+	assert.False(t, merged.MCP.IsCodeOrchestration())
+	assert.Empty(t, merged.MCP.Transport)
+	assert.Empty(t, merged.MCP.Intents)
+}
+
+// TestMergeSpecsCarriesMCPFullFieldsFromDeclaringSpec exercises the merge
+// loop with a non-Orchestration trigger field (Transport) and asserts that
+// Addr and Intents propagate alongside it. Without this case, a regression to
+// a partial field-by-field copy or an mcpConfigured branch that ignored
+// non-Orchestration fields would slip past the other tests.
+func TestMergeSpecsCarriesMCPFullFieldsFromDeclaringSpec(t *testing.T) {
+	t.Parallel()
+
+	specA := &spec.APISpec{
+		Name:      "a",
+		Version:   "0.1.0",
+		BaseURL:   "https://api.example.com",
+		Resources: map[string]spec.Resource{},
+		Types:     map[string]spec.TypeDef{},
+		MCP: spec.MCPConfig{
+			Transport: []string{"stdio", "http"},
+			Addr:      ":7777",
+			Intents: []spec.Intent{
+				{Name: "search_things"},
+			},
+		},
+	}
+	specB := &spec.APISpec{
+		Name:      "b",
+		Version:   "0.1.0",
+		BaseURL:   "https://api.example.com",
+		Resources: map[string]spec.Resource{},
+		Types:     map[string]spec.TypeDef{},
+	}
+
+	merged := mergeSpecs([]*spec.APISpec{specA, specB}, "combo")
+
+	assert.Equal(t, []string{"stdio", "http"}, merged.MCP.Transport)
+	assert.Equal(t, ":7777", merged.MCP.Addr)
+	assert.Len(t, merged.MCP.Intents, 1)
+	assert.Equal(t, "search_things", merged.MCP.Intents[0].Name)
+}
+
+// TestMergeSpecsFirstDeclaringMCPWins verifies that when more than one input
+// spec declares x-mcp, the first declaring spec wins, mirroring the existing
+// first-wins precedence used for Auth.AuthorizationURL.
+func TestMergeSpecsFirstDeclaringMCPWins(t *testing.T) {
+	t.Parallel()
+
+	specA := &spec.APISpec{
+		Name:      "a",
+		Version:   "0.1.0",
+		BaseURL:   "https://api.example.com",
+		Resources: map[string]spec.Resource{},
+		Types:     map[string]spec.TypeDef{},
+		MCP: spec.MCPConfig{
+			Orchestration: "code",
+		},
+	}
+	specB := &spec.APISpec{
+		Name:      "b",
+		Version:   "0.1.0",
+		BaseURL:   "https://api.example.com",
+		Resources: map[string]spec.Resource{},
+		Types:     map[string]spec.TypeDef{},
+		MCP: spec.MCPConfig{
+			Orchestration: "intent",
+		},
+	}
+
+	merged := mergeSpecs([]*spec.APISpec{specA, specB}, "combo")
+
+	assert.Equal(t, "code", merged.MCP.Orchestration)
+}
+
 func TestNormalizeHTTPTransportAllowsBrowserChromeH3(t *testing.T) {
 	t.Parallel()
 
diff --git a/internal/cli/root.go b/internal/cli/root.go
index 102cdd07..4e26b6ef 100644
--- a/internal/cli/root.go
+++ b/internal/cli/root.go
@@ -771,6 +771,10 @@ func mergeSpecs(specs []*spec.APISpec, name string) *spec.APISpec {
 		if s.Auth.AuthorizationURL != "" && merged.Auth.AuthorizationURL == "" {
 			merged.Auth = s.Auth
 		}
+
+		if mcpConfigured(s.MCP) && !mcpConfigured(merged.MCP) {
+			merged.MCP = s.MCP
+		}
 	}
 
 	return merged

← d0c4caca docs(skills): tighten Phase 3 Completion Gate to per-row Cob  ·  back to Cli Printing Press  ·  fix(cli): dedupe nested body-field flatten collisions with s c5a5441d →