[object Object]

← back to Cli Printing Press

fix(cli): fold per-spec base URL path prefixes on multi-spec merge (#995)

6e086c00773dd0e17c088f222d1fda785ad5644e · 2026-05-10 17:47:32 -0700 · Trevin Chow

* fix(cli): fold per-spec base URL path prefixes into endpoint paths on multi-spec merge

When `printing-press generate` collapsed multiple specs to a single CLI, it
kept only `specs[0].BaseURL` and dropped any path component from later specs'
server URLs. Same-host different-path combos (Confluence at /wiki/api/v2 next
to Jira at the bare host) silently 404'd every later-spec command because
`client.Get("/pages", ...)` resolved against the host without the missing
/wiki/api/v2 prefix.

Now `mergeSpecs` detects when input specs share a scheme+host but diverge on
path: it collapses `merged.BaseURL` to the bare host and prepends each spec's
own path component onto that spec's endpoint paths (recursively, including
sub-resources). Endpoints that already declare an absolute `base_url`
override are left alone so the existing per-endpoint override path keeps
working.

Idempotent — specs that share the full BaseURL produce no rewrites. Cross-
host merges (already broken for unrelated reasons) keep the prior fallback so
this change doesn't expand scope.

Refs #831

* refactor(cli): drop unreachable defensive checks in multi-spec base URL helpers

Self-review cleanup before re-requesting Greptile:

- `planMultiSpecBaseURL` is only invoked from `mergeSpecs` after the
  `len(specs) == 1` early return, so it never sees an empty slice. Drop
  the unreachable empty-slice guard.
- The verifier short-circuit env-var check guarded a stderr info line in
  generator code that the verifier never executes (the verifier runs
  printed-CLI subprocesses, not `printing-press generate`). Drop the
  check and always log when the prefix-fold path fires.
- `prefixResourceEndpointPaths` was guarded against `prefix == ""`, but
  the only caller already filters on `prefix != ""` and the recursive
  call inherits the same non-empty prefix. Drop the inner guard.

No behavior change in any reachable code path; existing tests cover the
remaining surface.

Files touched

Diff

commit 6e086c00773dd0e17c088f222d1fda785ad5644e
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Sun May 10 17:47:32 2026 -0700

    fix(cli): fold per-spec base URL path prefixes on multi-spec merge (#995)
    
    * fix(cli): fold per-spec base URL path prefixes into endpoint paths on multi-spec merge
    
    When `printing-press generate` collapsed multiple specs to a single CLI, it
    kept only `specs[0].BaseURL` and dropped any path component from later specs'
    server URLs. Same-host different-path combos (Confluence at /wiki/api/v2 next
    to Jira at the bare host) silently 404'd every later-spec command because
    `client.Get("/pages", ...)` resolved against the host without the missing
    /wiki/api/v2 prefix.
    
    Now `mergeSpecs` detects when input specs share a scheme+host but diverge on
    path: it collapses `merged.BaseURL` to the bare host and prepends each spec's
    own path component onto that spec's endpoint paths (recursively, including
    sub-resources). Endpoints that already declare an absolute `base_url`
    override are left alone so the existing per-endpoint override path keeps
    working.
    
    Idempotent — specs that share the full BaseURL produce no rewrites. Cross-
    host merges (already broken for unrelated reasons) keep the prior fallback so
    this change doesn't expand scope.
    
    Refs #831
    
    * refactor(cli): drop unreachable defensive checks in multi-spec base URL helpers
    
    Self-review cleanup before re-requesting Greptile:
    
    - `planMultiSpecBaseURL` is only invoked from `mergeSpecs` after the
      `len(specs) == 1` early return, so it never sees an empty slice. Drop
      the unreachable empty-slice guard.
    - The verifier short-circuit env-var check guarded a stderr info line in
      generator code that the verifier never executes (the verifier runs
      printed-CLI subprocesses, not `printing-press generate`). Drop the
      check and always log when the prefix-fold path fires.
    - `prefixResourceEndpointPaths` was guarded against `prefix == ""`, but
      the only caller already filters on `prefix != ""` and the recursive
      call inherits the same non-empty prefix. Drop the inner guard.
    
    No behavior change in any reachable code path; existing tests cover the
    remaining surface.
---
 internal/cli/generate_test.go | 144 ++++++++++++++++++++++++++++++++++++++++++
 internal/cli/root.go          | 103 +++++++++++++++++++++++++++++-
 2 files changed, 245 insertions(+), 2 deletions(-)

diff --git a/internal/cli/generate_test.go b/internal/cli/generate_test.go
index ac96fd33..44136568 100644
--- a/internal/cli/generate_test.go
+++ b/internal/cli/generate_test.go
@@ -948,6 +948,150 @@ func TestMergeSpecsPrefersReplayableBrowserTransportOverUnshippablePageContext(t
 	assert.Equal(t, spec.HTTPTransportBrowserChrome, mergedChrome.HTTPTransport)
 }
 
+// TestMergeSpecsRewritesEndpointPathsForDivergentBaseURLPathPrefixes covers the
+// Atlassian-style case from issue #831: spec A's base URL is the bare host and
+// spec B's base URL adds a path prefix on the same host. Without rewriting,
+// every spec-B command 404s because its path prefix is silently dropped by the
+// merge.
+func TestMergeSpecsRewritesEndpointPathsForDivergentBaseURLPathPrefixes(t *testing.T) {
+	t.Parallel()
+
+	specA := &spec.APISpec{
+		Name:    "a",
+		Version: "0.1.0",
+		BaseURL: "https://example.com",
+		Resources: map[string]spec.Resource{
+			"foo": {
+				Endpoints: map[string]spec.Endpoint{
+					"list": {Method: "GET", Path: "/foo"},
+				},
+			},
+		},
+		Types: map[string]spec.TypeDef{},
+	}
+	specB := &spec.APISpec{
+		Name:    "b",
+		Version: "0.1.0",
+		BaseURL: "https://example.com/api/v2",
+		Resources: map[string]spec.Resource{
+			"bar": {
+				Endpoints: map[string]spec.Endpoint{
+					"list": {Method: "GET", Path: "/bar"},
+					"get":  {Method: "GET", Path: "/bar/{id}"},
+				},
+				SubResources: map[string]spec.Resource{
+					"comments": {
+						Endpoints: map[string]spec.Endpoint{
+							"list": {Method: "GET", Path: "/bar/{id}/comments"},
+						},
+					},
+				},
+			},
+		},
+		Types: map[string]spec.TypeDef{},
+	}
+
+	merged := mergeSpecs([]*spec.APISpec{specA, specB}, "combo")
+
+	// merged BaseURL collapses to the bare host shared by both specs.
+	assert.Equal(t, "https://example.com", merged.BaseURL)
+
+	// spec A's endpoints (no path prefix on its base URL) are unchanged.
+	assert.Equal(t, "/foo", merged.Resources["foo"].Endpoints["list"].Path)
+
+	// spec B's endpoints absorb the dropped "/api/v2" path component.
+	assert.Equal(t, "/api/v2/bar", merged.Resources["bar"].Endpoints["list"].Path)
+	assert.Equal(t, "/api/v2/bar/{id}", merged.Resources["bar"].Endpoints["get"].Path)
+
+	// Sub-resource endpoints are rewritten too.
+	assert.Equal(t,
+		"/api/v2/bar/{id}/comments",
+		merged.Resources["bar"].SubResources["comments"].Endpoints["list"].Path,
+	)
+}
+
+// TestMergeSpecsLeavesPathsUnchangedWhenBaseURLsAgree guards the idempotent
+// case: when every spec's base URL is identical, no path rewriting fires and
+// the merged BaseURL keeps the shared path prefix.
+func TestMergeSpecsLeavesPathsUnchangedWhenBaseURLsAgree(t *testing.T) {
+	t.Parallel()
+
+	specA := &spec.APISpec{
+		Name:    "a",
+		Version: "0.1.0",
+		BaseURL: "https://example.com/api/v2",
+		Resources: map[string]spec.Resource{
+			"foo": {
+				Endpoints: map[string]spec.Endpoint{
+					"list": {Method: "GET", Path: "/foo"},
+				},
+			},
+		},
+		Types: map[string]spec.TypeDef{},
+	}
+	specB := &spec.APISpec{
+		Name:    "b",
+		Version: "0.1.0",
+		BaseURL: "https://example.com/api/v2",
+		Resources: map[string]spec.Resource{
+			"bar": {
+				Endpoints: map[string]spec.Endpoint{
+					"list": {Method: "GET", Path: "/bar"},
+				},
+			},
+		},
+		Types: map[string]spec.TypeDef{},
+	}
+
+	merged := mergeSpecs([]*spec.APISpec{specA, specB}, "combo")
+
+	assert.Equal(t, "https://example.com/api/v2", merged.BaseURL)
+	assert.Equal(t, "/foo", merged.Resources["foo"].Endpoints["list"].Path)
+	assert.Equal(t, "/bar", merged.Resources["bar"].Endpoints["list"].Path)
+}
+
+// TestMergeSpecsLeavesPathsUnchangedWhenHostsDiffer guards the cross-host case
+// (out of scope for the path-rewrite optimization): when specs live on
+// different hosts, the merge falls back to specs[0].BaseURL and no path
+// rewriting fires. Cross-host runs were already broken; this just keeps the
+// pre-fix behavior so the path-rewrite path doesn't make it worse.
+func TestMergeSpecsLeavesPathsUnchangedWhenHostsDiffer(t *testing.T) {
+	t.Parallel()
+
+	specA := &spec.APISpec{
+		Name:    "a",
+		Version: "0.1.0",
+		BaseURL: "https://a.example.com",
+		Resources: map[string]spec.Resource{
+			"foo": {
+				Endpoints: map[string]spec.Endpoint{
+					"list": {Method: "GET", Path: "/foo"},
+				},
+			},
+		},
+		Types: map[string]spec.TypeDef{},
+	}
+	specB := &spec.APISpec{
+		Name:    "b",
+		Version: "0.1.0",
+		BaseURL: "https://b.example.com/api/v2",
+		Resources: map[string]spec.Resource{
+			"bar": {
+				Endpoints: map[string]spec.Endpoint{
+					"list": {Method: "GET", Path: "/bar"},
+				},
+			},
+		},
+		Types: map[string]spec.TypeDef{},
+	}
+
+	merged := mergeSpecs([]*spec.APISpec{specA, specB}, "combo")
+
+	assert.Equal(t, "https://a.example.com", merged.BaseURL)
+	assert.Equal(t, "/foo", merged.Resources["foo"].Endpoints["list"].Path)
+	assert.Equal(t, "/bar", merged.Resources["bar"].Endpoints["list"].Path)
+}
+
 func TestNormalizeHTTPTransportAllowsBrowserChromeH3(t *testing.T) {
 	t.Parallel()
 
diff --git a/internal/cli/root.go b/internal/cli/root.go
index 77671268..85bf7748 100644
--- a/internal/cli/root.go
+++ b/internal/cli/root.go
@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"net/url"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -715,11 +716,13 @@ func mergeSpecs(specs []*spec.APISpec, name string) *spec.APISpec {
 		return specs[0]
 	}
 
+	mergedBaseURL, perSpecPathPrefix := planMultiSpecBaseURL(specs)
+
 	merged := &spec.APISpec{
 		Name:        name,
 		Description: "Combined CLI for multiple API services",
 		Version:     specs[0].Version,
-		BaseURL:     specs[0].BaseURL,
+		BaseURL:     mergedBaseURL,
 		BasePath:    specs[0].BasePath,
 		Auth:        specs[0].Auth,
 		Config: spec.ConfigSpec{
@@ -730,7 +733,7 @@ func mergeSpecs(specs []*spec.APISpec, name string) *spec.APISpec {
 		Types:     map[string]spec.TypeDef{},
 	}
 
-	for _, s := range specs {
+	for i, s := range specs {
 		if merged.SpecSource == "" || merged.SpecSource == "official" {
 			switch s.SpecSource {
 			case "sniffed":
@@ -747,7 +750,11 @@ func mergeSpecs(specs []*spec.APISpec, name string) *spec.APISpec {
 			merged.HTTPTransport = strongerHTTPTransport(merged.HTTPTransport, candidateTransport)
 		}
 
+		prefix := perSpecPathPrefix[i]
 		for resourceName, resource := range s.Resources {
+			if prefix != "" {
+				resource = prefixResourceEndpointPaths(resource, prefix)
+			}
 			key := resourceName
 			if _, exists := merged.Resources[key]; exists {
 				key = s.Name + "-" + resourceName
@@ -771,6 +778,98 @@ func mergeSpecs(specs []*spec.APISpec, name string) *spec.APISpec {
 	return merged
 }
 
+// planMultiSpecBaseURL decides how to reconcile the BaseURL field across
+// multiple input specs. The returned perSpecPathPrefix slice has one entry per
+// spec; a non-empty entry tells the caller to prepend that prefix to every
+// endpoint path in that spec. When every spec lives on the same scheme+host
+// but their path components diverge, the merged BaseURL collapses to the bare
+// host and each spec's path component is returned for folding into its
+// endpoints — this rescues the "spec A at https://x.com, spec B at
+// https://x.com/api/v2" case where the old collapse silently dropped spec B's
+// /api/v2 prefix and 404'd every B command. When hosts disagree (a separate,
+// out-of-scope multi-host problem) or every spec shares the same BaseURL, the
+// merged BaseURL stays specs[0].BaseURL and every prefix is empty.
+func planMultiSpecBaseURL(specs []*spec.APISpec) (mergedBaseURL string, perSpecPathPrefix []string) {
+	perSpecPathPrefix = make([]string, len(specs))
+
+	hosts := make([]string, len(specs))
+	paths := make([]string, len(specs))
+	for i, s := range specs {
+		hosts[i], paths[i] = splitBaseURL(s.BaseURL)
+	}
+
+	commonHost := hosts[0]
+	if commonHost == "" {
+		return specs[0].BaseURL, perSpecPathPrefix
+	}
+	for _, h := range hosts[1:] {
+		if h != commonHost {
+			return specs[0].BaseURL, perSpecPathPrefix
+		}
+	}
+
+	// All specs share a host. If every spec also shares the same path, no
+	// rewriting is needed — the merged BaseURL keeps the shared prefix.
+	allSamePath := true
+	for _, p := range paths[1:] {
+		if p != paths[0] {
+			allSamePath = false
+			break
+		}
+	}
+	if allSamePath {
+		return specs[0].BaseURL, perSpecPathPrefix
+	}
+
+	copy(perSpecPathPrefix, paths)
+	fmt.Fprintf(os.Stderr, "[multi-spec] base URL host %q shared; folding per-spec path prefixes into endpoint paths\n", commonHost)
+	return commonHost, perSpecPathPrefix
+}
+
+// splitBaseURL splits an absolute http(s) URL into its scheme+host root and
+// its path component. Returns ("", "") for empty or non-absolute inputs so
+// callers fall through to the existing "specs[0] wins" behavior. The path
+// component is trimmed of its trailing slash so the caller can prepend it to
+// an endpoint Path (which already starts with "/") without double slashes.
+func splitBaseURL(raw string) (host, path string) {
+	raw = strings.TrimSpace(raw)
+	if raw == "" {
+		return "", ""
+	}
+	parsed, err := url.Parse(raw)
+	if err != nil || parsed.Scheme == "" || parsed.Host == "" {
+		return "", ""
+	}
+	host = parsed.Scheme + "://" + parsed.Host
+	path = strings.TrimRight(parsed.Path, "/")
+	return host, path
+}
+
+// prefixResourceEndpointPaths returns a copy of resource with prefix prepended
+// to every endpoint Path (including sub-resources). Endpoints that already
+// declare an absolute BaseURL override are left alone — their path is
+// resolved against that override at runtime, not the spec-level BaseURL, so
+// folding the prefix in would double-resolve.
+func prefixResourceEndpointPaths(resource spec.Resource, prefix string) spec.Resource {
+	out := resource
+	if len(resource.Endpoints) > 0 {
+		out.Endpoints = make(map[string]spec.Endpoint, len(resource.Endpoints))
+		for name, ep := range resource.Endpoints {
+			if ep.BaseURL == "" {
+				ep.Path = prefix + ep.Path
+			}
+			out.Endpoints[name] = ep
+		}
+	}
+	if len(resource.SubResources) > 0 {
+		out.SubResources = make(map[string]spec.Resource, len(resource.SubResources))
+		for name, sub := range resource.SubResources {
+			out.SubResources[name] = prefixResourceEndpointPaths(sub, prefix)
+		}
+	}
+	return out
+}
+
 func strongerHTTPTransport(current, candidate string) string {
 	if httpTransportPriority(candidate) > httpTransportPriority(current) {
 		return candidate

← a9e9d006 fix(cli): allow runtime override of OAuth2 OIDC URLs (#970)  ·  back to Cli Printing Press  ·  feat(cli): add cliutil.ExtractNumber/ExtractInt for JSON-str 182395ca →