[object Object]

← back to Cli Printing Press

fix(cli): honor Spec.BasePath in generated client URL construction (#1108)

c6420747099278647d33c51c9f64df85d8584fdf · 2026-05-11 13:51:39 -0700 · Trevin Chow

* fix(cli): honor Spec.BasePath in generated client URL construction

The generator parsed and validated `base_path` on APISpec but no template
emitted it into the printed CLI, so specs declaring both `base_url` and
`base_path` (OneDev, Forgejo, Gitea, similar self-hosted apps) sent every
request to `${BaseURL}${path}` and returned 404 on every call.

Gate the emission on `Spec.BasePath != ""` via template conditionals so
existing specs without `base_path` produce byte-identical client.go and
config.go output. Generator goldens stay clean (17 cases pass).

Refs #926

* test(cli): make BasePath struct-field assertions whitespace-tolerant

CI's generated config.go had gofmt-aligned column widths that differed
from the template's literal indentation, breaking `assert.Contains` on
the exact whitespace pattern. Switch to regex assertions with `\s+`
between field-name, type, and tag so the test passes on either layout.

* fix(cli): reject base_path + client_pattern=proxy-envelope at spec validation

The proxy-envelope client pattern routes every request to the spec-level
BaseURL with the API path inside the JSON envelope (Service + Path fields).
URL-level prefixes don't apply — a BasePath would be silently dropped.
Surface the conflict at parse time alongside the existing resource-BaseURL
incompatibility guard so spec authors don't ship a CLI that 404s after the
first live request.

Files touched

Diff

commit c6420747099278647d33c51c9f64df85d8584fdf
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Mon May 11 13:51:39 2026 -0700

    fix(cli): honor Spec.BasePath in generated client URL construction (#1108)
    
    * fix(cli): honor Spec.BasePath in generated client URL construction
    
    The generator parsed and validated `base_path` on APISpec but no template
    emitted it into the printed CLI, so specs declaring both `base_url` and
    `base_path` (OneDev, Forgejo, Gitea, similar self-hosted apps) sent every
    request to `${BaseURL}${path}` and returned 404 on every call.
    
    Gate the emission on `Spec.BasePath != ""` via template conditionals so
    existing specs without `base_path` produce byte-identical client.go and
    config.go output. Generator goldens stay clean (17 cases pass).
    
    Refs #926
    
    * test(cli): make BasePath struct-field assertions whitespace-tolerant
    
    CI's generated config.go had gofmt-aligned column widths that differed
    from the template's literal indentation, breaking `assert.Contains` on
    the exact whitespace pattern. Switch to regex assertions with `\s+`
    between field-name, type, and tag so the test passes on either layout.
    
    * fix(cli): reject base_path + client_pattern=proxy-envelope at spec validation
    
    The proxy-envelope client pattern routes every request to the spec-level
    BaseURL with the API path inside the JSON envelope (Service + Path fields).
    URL-level prefixes don't apply — a BasePath would be silently dropped.
    Surface the conflict at parse time alongside the existing resource-BaseURL
    incompatibility guard so spec authors don't ship a CLI that 404s after the
    first live request.
---
 internal/generator/client_basepath_test.go  | 153 ++++++++++++++++++++++++++++
 internal/generator/templates/client.go.tmpl |  48 +++++++--
 internal/generator/templates/config.go.tmpl |  11 ++
 internal/spec/spec.go                       |   3 +
 internal/spec/spec_test.go                  |  25 +++++
 5 files changed, 234 insertions(+), 6 deletions(-)

diff --git a/internal/generator/client_basepath_test.go b/internal/generator/client_basepath_test.go
new file mode 100644
index 00000000..a040d4c2
--- /dev/null
+++ b/internal/generator/client_basepath_test.go
@@ -0,0 +1,153 @@
+package generator
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"regexp"
+	"strings"
+	"sync"
+	"testing"
+
+	"github.com/mvanhorn/cli-printing-press/v4/internal/spec"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestClientHonorsSpecBasePath checks that a spec declaring BaseURL plus a
+// separate BasePath produces a generated client that sends requests to
+// ${BaseURL}${BasePath}${path}. Some APIs split the host from the path mount
+// (e.g. base_url: https://host, base_path: /~api); previously that prefix was
+// dropped and every request 404'd.
+func TestClientHonorsSpecBasePath(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := minimalSpec("basepath-honored")
+	apiSpec.BasePath = "/~api"
+
+	outputDir := filepath.Join(t.TempDir(), "basepath-honored-pp-cli")
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	clientSrc, err := os.ReadFile(filepath.Join(outputDir, "internal", "client", "client.go"))
+	require.NoError(t, err)
+	client := string(clientSrc)
+
+	// Whitespace-insensitive: gofmt may align columns differently than the
+	// template's literal indentation, so match field shape via regex.
+	clientBasePathField := regexp.MustCompile(`(?m)^\s*BasePath\s+string\b`)
+	assert.Regexp(t, clientBasePathField, client,
+		"Client struct should expose a BasePath field when the spec declares one")
+	clientNewAssign := regexp.MustCompile(`BasePath:\s+normalizeBasePath\(cfg\.BasePath\)`)
+	assert.Regexp(t, clientNewAssign, client,
+		"New() should populate Client.BasePath from cfg.BasePath via the normalizer")
+	assert.Contains(t, client, "c.BaseURL + c.BasePath + path",
+		"do() should construct request URLs as BaseURL+BasePath+path")
+
+	cacheKeyBody := clientCacheKeyBody(t, client)
+	assert.Contains(t, cacheKeyBody, `"|base_path=" + c.BasePath`,
+		"cache key should include BasePath so a config change invalidates correctly")
+
+	configSrc, err := os.ReadFile(filepath.Join(outputDir, "internal", "config", "config.go"))
+	require.NoError(t, err)
+	config := string(configSrc)
+
+	configBasePathField := regexp.MustCompile("(?m)^\\s*BasePath\\s+string\\s+`toml:\"base_path\"`")
+	assert.Regexp(t, configBasePathField, config,
+		"Config struct should expose BasePath with a serialized tag matching the spec's config format")
+	assert.Contains(t, config, `BasePath: "/~api"`,
+		"Load() should seed cfg.BasePath from the spec default")
+	assert.Contains(t, config, `os.Getenv("BASEPATH_HONORED_BASE_PATH")`,
+		"Load() should accept an env-var override for BasePath")
+}
+
+// TestClientWithoutBasePathByteIdentical asserts the negative case: a spec
+// with no BasePath produces the same client.go and config.go content as
+// before this change (the conditional template guards must not leak when
+// .BasePath is empty).
+func TestClientWithoutBasePathByteIdentical(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := minimalSpec("no-basepath")
+
+	outputDir := filepath.Join(t.TempDir(), "no-basepath-pp-cli")
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	clientSrc, err := os.ReadFile(filepath.Join(outputDir, "internal", "client", "client.go"))
+	require.NoError(t, err)
+	client := string(clientSrc)
+	assert.NotContains(t, client, "BasePath",
+		"client.go must not emit BasePath when the spec doesn't declare base_path")
+	assert.NotContains(t, client, "normalizeBasePath",
+		"client.go must not emit the normalizeBasePath helper when unused")
+
+	configSrc, err := os.ReadFile(filepath.Join(outputDir, "internal", "config", "config.go"))
+	require.NoError(t, err)
+	config := string(configSrc)
+	assert.NotContains(t, config, "BasePath",
+		"config.go must not emit BasePath when the spec doesn't declare base_path")
+	assert.NotContains(t, config, "_BASE_PATH",
+		"config.go must not emit the BASE_PATH env override when BasePath is empty")
+}
+
+// TestClientBasePathLiveRequest compiles a printed CLI from a BasePath spec
+// against a stub server and asserts the request URL hits the prefix. This is
+// the runtime test: template content matching can drift; only the live HTTP
+// path proves the fix.
+func TestClientBasePathLiveRequest(t *testing.T) {
+	t.Parallel()
+
+	var (
+		mu       sync.Mutex
+		gotPaths []string
+	)
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		mu.Lock()
+		gotPaths = append(gotPaths, r.URL.Path)
+		mu.Unlock()
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"items": []}`))
+	}))
+	t.Cleanup(server.Close)
+
+	apiSpec := minimalSpec("bplive")
+	apiSpec.BaseURL = server.URL
+	apiSpec.BasePath = "/api/v1"
+	apiSpec.Auth = spec.AuthConfig{Type: "none"}
+	apiSpec.Resources = map[string]spec.Resource{
+		"things": {
+			Description: "Manage things",
+			Endpoints: map[string]spec.Endpoint{
+				"list": {Method: "GET", Path: "/things", Description: "List things"},
+			},
+		},
+	}
+
+	outputDir := filepath.Join(t.TempDir(), "bplive-pp-cli")
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	runGoCommand(t, outputDir, "mod", "tidy")
+	binaryPath := filepath.Join(outputDir, "bplive-pp-cli")
+	runGoCommand(t, outputDir, "build", "-o", binaryPath, "./cmd/bplive-pp-cli")
+
+	cmd := exec.Command(binaryPath, "things", "list", "--json")
+	cmd.Env = append(os.Environ(), "BPLIVE_BASE_URL="+server.URL)
+	out, err := cmd.CombinedOutput()
+	require.NoError(t, err, string(out))
+
+	// Confirm the response decodes; the actual shape comes from the stub.
+	var resp any
+	require.NoError(t, json.Unmarshal(out, &resp), string(out))
+
+	mu.Lock()
+	defer mu.Unlock()
+	require.NotEmpty(t, gotPaths, "stub server should have received at least one request")
+	for _, p := range gotPaths {
+		assert.True(t, strings.HasPrefix(p, "/api/v1/"),
+			"request path %q should start with the BasePath prefix /api/v1/", p)
+	}
+	assert.Contains(t, gotPaths, "/api/v1/things",
+		"the things.list endpoint should hit /api/v1/things, not /things")
+}
diff --git a/internal/generator/templates/client.go.tmpl b/internal/generator/templates/client.go.tmpl
index d134d50f..638dbfce 100644
--- a/internal/generator/templates/client.go.tmpl
+++ b/internal/generator/templates/client.go.tmpl
@@ -39,6 +39,11 @@ import (
 
 type Client struct {
 	BaseURL    string
+{{- if .BasePath}}
+	// BasePath sits between BaseURL and the request path; empty unless the
+	// spec declares base_path.
+	BasePath   string
+{{- end}}
 	Config     *config.Config
 	HTTPClient *http.Client
 	DryRun     bool
@@ -152,11 +157,11 @@ func (c *Client) baseURLForRequest() string {
 	{{- if $tier.BaseURL}}
 		return strings.TrimRight({{printf "%q" $tier.BaseURL}}, "/")
 	{{- else}}
-		return c.BaseURL
+		return c.BaseURL{{if $.BasePath}} + c.BasePath{{end}}
 	{{- end}}
 {{- end}}
 	default:
-		return c.BaseURL
+		return c.BaseURL{{if .BasePath}} + c.BasePath{{end}}
 	}
 }
 
@@ -302,6 +307,9 @@ func New(cfg *config.Config, timeout time.Duration, rateLimit float64) *Client {
 	sess.AttachTo(httpClient)
 	return &Client{
 		BaseURL:    strings.TrimRight(cfg.BaseURL, "/"),
+{{- if .BasePath}}
+		BasePath:   normalizeBasePath(cfg.BasePath),
+{{- end}}
 		Config:     cfg,
 		HTTPClient: httpClient,
 		cacheDir:   cacheDir,
@@ -325,6 +333,9 @@ func New(cfg *config.Config, timeout time.Duration, rateLimit float64) *Client {
 	httpClient := newHTTPClient(timeout, nil)
 	return &Client{
 		BaseURL:    strings.TrimRight(cfg.BaseURL, "/"),
+{{- if .BasePath}}
+		BasePath:   normalizeBasePath(cfg.BasePath),
+{{- end}}
 		Config:     cfg,
 		HTTPClient: httpClient,
 		cacheDir:   cacheDir,
@@ -377,6 +388,9 @@ func (c *Client) ProbeGet(path string) (int, error) {
 func (c *Client) cacheKey(path string, params map[string]string) string {
 	key := path
 	key += "|base_url=" + c.BaseURL
+{{- if .BasePath}}
+	key += "|base_path=" + c.BasePath
+{{- end}}
 {{- if .HasTierRouting}}
 	key += "|tier=" + c.requestTier + "|tier_base_url=" + c.baseURLForRequest()
 {{- end}}
@@ -636,13 +650,13 @@ func (c *Client) do(method, path string, params map[string]string, body any, hea
 	if isAbsoluteURL(path) {
 		targetURL, urlErr = buildURL("", path, endpointVars)
 	} else {
-		targetURL, urlErr = buildURL({{if .HasTierRouting}}requestBaseURL{{else}}c.BaseURL{{end}}, path, endpointVars)
+		targetURL, urlErr = buildURL({{if .HasTierRouting}}requestBaseURL{{else}}c.BaseURL{{if .BasePath}} + c.BasePath{{end}}{{end}}, path, endpointVars)
 	}
 	if urlErr != nil {
 		return nil, 0, urlErr
 	}
 {{- else}}
-		targetURL, urlErr := buildURL({{if .HasTierRouting}}requestBaseURL{{else}}c.BaseURL{{end}}, path, endpointVars)
+		targetURL, urlErr := buildURL({{if .HasTierRouting}}requestBaseURL{{else}}c.BaseURL{{if .BasePath}} + c.BasePath{{end}}{{end}}, path, endpointVars)
 	if urlErr != nil {
 		return nil, 0, urlErr
 	}
@@ -656,10 +670,10 @@ func (c *Client) do(method, path string, params map[string]string, body any, hea
 	if isAbsoluteURL(path) {
 		targetURL = path
 	} else {
-		targetURL = {{if .HasTierRouting}}requestBaseURL{{else}}c.BaseURL{{end}} + path
+		targetURL = {{if .HasTierRouting}}requestBaseURL{{else}}c.BaseURL{{if .BasePath}} + c.BasePath{{end}}{{end}} + path
 	}
 {{- else}}
-	targetURL := {{if .HasTierRouting}}requestBaseURL{{else}}c.BaseURL{{end}} + path
+	targetURL := {{if .HasTierRouting}}requestBaseURL{{else}}c.BaseURL{{if .BasePath}} + c.BasePath{{end}}{{end}} + path
 {{- end}}
 {{- end}}
 {{- end}}
@@ -1377,6 +1391,28 @@ func isAbsoluteURL(path string) bool {
 	return strings.HasPrefix(path, "https://") || strings.HasPrefix(path, "http://")
 }
 
+{{end -}}
+{{if .BasePath -}}
+// normalizeBasePath trims surrounding whitespace and trailing slashes, then
+// guarantees a single leading slash so concatenation with c.BaseURL produces
+// well-formed URLs whether the operator wrote "/api/v1", "api/v1/", or
+// "/api/v1/". Empty input stays empty so an unset override doesn't inject a
+// stray "/" into the request URL.
+func normalizeBasePath(p string) string {
+	p = strings.TrimSpace(p)
+	if p == "" {
+		return ""
+	}
+	p = strings.TrimRight(p, "/")
+	if p == "" {
+		return ""
+	}
+	if !strings.HasPrefix(p, "/") {
+		p = "/" + p
+	}
+	return p
+}
+
 {{end -}}
 // sanitizeJSONResponse strips known JSONP/XSSI prefixes and UTF-8 BOM from
 // response bodies so that downstream JSON parsing succeeds. For clean JSON
diff --git a/internal/generator/templates/config.go.tmpl b/internal/generator/templates/config.go.tmpl
index 9dd90532..aa7e7cd4 100644
--- a/internal/generator/templates/config.go.tmpl
+++ b/internal/generator/templates/config.go.tmpl
@@ -25,6 +25,9 @@ import (
 
 type Config struct {
 	BaseURL        string `{{configTag .Config.Format}}:"base_url"`
+{{- if .BasePath}}
+	BasePath       string `{{configTag .Config.Format}}:"base_path"`
+{{- end}}
 	AuthHeaderVal  string `{{configTag .Config.Format}}:"auth_header"`
 	Headers        map[string]string `{{configTag .Config.Format}}:"headers,omitempty"`
 	AuthSource     string `{{configTag .Config.Format}}:"-"`
@@ -78,6 +81,9 @@ func Load(configPath string) (*Config, error) {
 	cfg := &Config{
 {{- if .BaseURL}}
 		BaseURL: "{{.BaseURL}}",
+{{- end}}
+{{- if .BasePath}}
+		BasePath: "{{.BasePath}}",
 {{- end}}
 	}
 
@@ -163,6 +169,11 @@ func Load(configPath string) (*Config, error) {
 	if v := os.Getenv("{{envName .Name}}_BASE_URL"); v != "" {
 		cfg.BaseURL = v
 	}
+{{- if .BasePath}}
+	if v := os.Getenv("{{envName .Name}}_BASE_PATH"); v != "" {
+		cfg.BasePath = v
+	}
+{{- end}}
 {{- if and .HasAuthCommand .Auth.AuthorizationURL}}
 	if v := os.Getenv("{{envName .Name}}_AUTHORIZATION_URL"); v != "" {
 		cfg.AuthorizationURL = v
diff --git a/internal/spec/spec.go b/internal/spec/spec.go
index 93360638..468276cf 100644
--- a/internal/spec/spec.go
+++ b/internal/spec/spec.go
@@ -1731,6 +1731,9 @@ func (s *APISpec) Validate() error {
 	if s.ClientPattern == "proxy-envelope" && s.HasResourceBaseURLOverride() {
 		return fmt.Errorf("resource or endpoint base_url overrides are incompatible with client_pattern=proxy-envelope; the proxy POSTs every request to the spec-level BaseURL, so per-request overrides would be silently ignored")
 	}
+	if s.ClientPattern == "proxy-envelope" && s.BasePath != "" {
+		return fmt.Errorf("base_path is incompatible with client_pattern=proxy-envelope; the proxy routes via the envelope's Service/Path fields, not a URL-level prefix — fold the prefix into base_url instead")
+	}
 	for name, r := range s.Resources {
 		if len(r.Endpoints) == 0 && len(r.SubResources) == 0 {
 			return fmt.Errorf("resource %q has no endpoints", name)
diff --git a/internal/spec/spec_test.go b/internal/spec/spec_test.go
index f6693b39..62eae607 100644
--- a/internal/spec/spec_test.go
+++ b/internal/spec/spec_test.go
@@ -3290,6 +3290,31 @@ func TestValidateRejectsResourceBaseURLWithProxyEnvelope(t *testing.T) {
 	assert.Contains(t, err.Error(), "base_url")
 }
 
+// TestValidateRejectsBasePathWithProxyEnvelope — proxy-envelope routes via
+// the envelope's Service/Path fields, not a URL-level prefix; a BasePath
+// would be silently ignored by the proxy. Validate must fail-fast.
+func TestValidateRejectsBasePathWithProxyEnvelope(t *testing.T) {
+	t.Parallel()
+	s := &APISpec{
+		Name:          "proxypath",
+		Version:       "0.1.0",
+		BaseURL:       "https://proxy.example.com",
+		BasePath:      "/api/v1",
+		ClientPattern: "proxy-envelope",
+		Resources: map[string]Resource{
+			"items": {
+				Endpoints: map[string]Endpoint{
+					"list": {Method: "GET", Path: "/items", Description: "List"},
+				},
+			},
+		},
+	}
+	err := s.Validate()
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "proxy-envelope")
+	assert.Contains(t, err.Error(), "base_path")
+}
+
 // TestValidateAcceptsResourceBaseURLWithoutProxyEnvelope — the same
 // resource override is accepted when client_pattern is not the proxy
 // flavor. Negative cases (no resource override, proxy-envelope alone)

← cb3c0974 fix(cli): surface API body in sync_error events and add --pa  ·  back to Cli Printing Press  ·  fix(cli): emit default Accept */* header in generated HTTP c c626efca →