[object Object]

← back to Cli Printing Press

fix(cli): synthesize undeclared path placeholders in OpenAPI parser (#1510)

bf9014d45ade16baacab07829c784448d19073c6 · 2026-05-16 03:15:10 -0700 · Trevin Chow

* fix(cli): synthesize undeclared path placeholders in OpenAPI parser (#1467)

Real-world OpenAPI specs (Tally, others) frequently include {placeholder}
tokens in a path template without declaring the corresponding parameter
at either the operation or path-item level. The YAML loader already
handles this via enrichEndpointPathParams; the OpenAPI parser did not.
With no matching Param entry the generator emits a literal
`/organizations/{organizationId}/invites` URL and every request returns
404 — no flag, no positional, no substitution call.

Export enrichPathParams as EnrichPathParams on APISpec and invoke it
from the OpenAPI parser just before Validate, so OpenAPI-parsed specs
get the same path-placeholder synthesis the YAML loader provides. The
existing case for already-declared params (e.g. {x} declared as query)
is reused too, which fixes a sibling case where a placeholder declared
in the wrong location still drove a 404.

* test(cli): scan by name instead of asserting exact Params count (#1467)

Addresses Greptile review feedback on #1510. require.Len(Params, 1)
would produce a confusing mismatch if any future enrichment step
synthesized an additional param. Scan for organizationId by name so the
assertion stays meaningful regardless of unrelated param growth.

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

Files touched

Diff

commit bf9014d45ade16baacab07829c784448d19073c6
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Sat May 16 03:15:10 2026 -0700

    fix(cli): synthesize undeclared path placeholders in OpenAPI parser (#1510)
    
    * fix(cli): synthesize undeclared path placeholders in OpenAPI parser (#1467)
    
    Real-world OpenAPI specs (Tally, others) frequently include {placeholder}
    tokens in a path template without declaring the corresponding parameter
    at either the operation or path-item level. The YAML loader already
    handles this via enrichEndpointPathParams; the OpenAPI parser did not.
    With no matching Param entry the generator emits a literal
    `/organizations/{organizationId}/invites` URL and every request returns
    404 — no flag, no positional, no substitution call.
    
    Export enrichPathParams as EnrichPathParams on APISpec and invoke it
    from the OpenAPI parser just before Validate, so OpenAPI-parsed specs
    get the same path-placeholder synthesis the YAML loader provides. The
    existing case for already-declared params (e.g. {x} declared as query)
    is reused too, which fixes a sibling case where a placeholder declared
    in the wrong location still drove a 404.
    
    * test(cli): scan by name instead of asserting exact Params count (#1467)
    
    Addresses Greptile review feedback on #1510. require.Len(Params, 1)
    would produce a confusing mismatch if any future enrichment step
    synthesized an additional param. Scan for organizationId by name so the
    assertion stays meaningful regardless of unrelated param growth.
    
    ---------
    
    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
---
 .../generator/path_param_positional_index_test.go  | 47 +++++++++++++++++++
 internal/openapi/parser.go                         |  9 ++++
 internal/openapi/parser_test.go                    | 52 ++++++++++++++++++++++
 internal/spec/spec.go                              |  6 +--
 4 files changed, 111 insertions(+), 3 deletions(-)

diff --git a/internal/generator/path_param_positional_index_test.go b/internal/generator/path_param_positional_index_test.go
index 1dec7867..a1401c38 100644
--- a/internal/generator/path_param_positional_index_test.go
+++ b/internal/generator/path_param_positional_index_test.go
@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/mvanhorn/cli-printing-press/v4/internal/naming"
+	"github.com/mvanhorn/cli-printing-press/v4/internal/openapi"
 	"github.com/mvanhorn/cli-printing-press/v4/internal/spec"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -390,3 +391,49 @@ func TestPositionalIndexHelper(t *testing.T) {
 	assert.Equal(t, -1, positionalIndex(multi, "missing"),
 		"unknown name should return -1, not panic")
 }
+
+// TestUndeclaredPathPlaceholderEmitsPositional pins the end-to-end repair for
+// real-world OpenAPI specs whose path templates carry a {placeholder} the
+// operation never declares in `parameters`. Without the spec-level enrichment,
+// the generator emits a literal `{organizationId}` URL segment and every
+// request to a parent-scoped resource 404s silently.
+func TestUndeclaredPathPlaceholderEmitsPositional(t *testing.T) {
+	t.Parallel()
+
+	yaml := `openapi: 3.0.0
+info:
+  title: Hierarchical
+  version: 1.0.0
+servers:
+  - url: https://api.example.com
+paths:
+  /organizations/{organizationId}/invites:
+    get:
+      summary: List organization invites
+      responses:
+        "200": {description: ok}
+  /organizations/{organizationId}/users:
+    get:
+      summary: List organization users
+      responses:
+        "200": {description: ok}
+`
+	apiSpec, err := openapi.Parse([]byte(yaml))
+	require.NoError(t, err)
+	apiSpec.Owner = "test-owner"
+	apiSpec.OwnerName = "Test"
+
+	outputDir := filepath.Join(t.TempDir(), naming.CLI(apiSpec.Name))
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	matches, err := filepath.Glob(filepath.Join(outputDir, "internal", "cli", "promoted_invites*.go"))
+	require.NoError(t, err)
+	require.Lenf(t, matches, 1, "expected one promoted_invites*.go, got %v", matches)
+	src, err := os.ReadFile(matches[0])
+	require.NoError(t, err)
+	body := string(src)
+	assert.Contains(t, body, `Use:         "invites <organizationId>"`,
+		"positional must appear in cobra Use so --help is honest")
+	assert.Contains(t, body, `replacePathParam(path, "organizationId", args[0])`,
+		"undeclared path placeholder must still drive URL substitution")
+}
diff --git a/internal/openapi/parser.go b/internal/openapi/parser.go
index 1cfee019..cbe55d86 100644
--- a/internal/openapi/parser.go
+++ b/internal/openapi/parser.go
@@ -462,6 +462,15 @@ func parseWithLocation(data []byte, lenient bool, location *url.URL) (*spec.APIS
 	result.RequiredHeaders, perEndpointHeaders = detectRequiredHeaders(doc, result.Auth)
 	applyHeaderOverrides(result, perEndpointHeaders)
 
+	// Synthesize Params entries for {placeholder} tokens in the path template
+	// that the operation never declared in `parameters` (or via a path-item /
+	// $ref shared parameter). Real-world specs frequently omit these — the
+	// path template is the source of truth, but without a matching Param
+	// entry the generator emits a URL with literal `{name}` segments and the
+	// request returns 404. Same pass the YAML loader uses, applied uniformly
+	// here so OpenAPI-parsed specs get the same guarantee.
+	result.EnrichPathParams()
+
 	if err := result.Validate(); err != nil {
 		return nil, fmt.Errorf("validating parsed spec: %w", err)
 	}
diff --git a/internal/openapi/parser_test.go b/internal/openapi/parser_test.go
index ad92360c..c4d4a964 100644
--- a/internal/openapi/parser_test.go
+++ b/internal/openapi/parser_test.go
@@ -1899,6 +1899,58 @@ paths:
 	}
 }
 
+// Real-world OpenAPI specs (Tally, others) frequently include {placeholder}
+// tokens in a path template without declaring the corresponding parameter at
+// either the operation or path-item level. The path template is then the only
+// source of truth for what's required. Without synthesizing a Param entry,
+// the generated CLI emits a literal `{organizationId}` URL segment and every
+// request 404s. Mirrors the same enrichment the internal YAML loader applies.
+func TestParseSynthesizesUndeclaredPathPlaceholders(t *testing.T) {
+	t.Parallel()
+	data := []byte(`
+openapi: 3.0.0
+info:
+  title: Hierarchical API
+  version: 1.0.0
+servers:
+  - url: https://api.example.com
+paths:
+  /organizations/{organizationId}/invites:
+    get:
+      summary: List organization invites
+      responses:
+        "200":
+          description: ok
+  /organizations/{organizationId}/users:
+    get:
+      summary: List organization users
+      responses:
+        "200":
+          description: ok
+`)
+
+	parsed, err := Parse(data)
+	require.NoError(t, err)
+
+	for _, path := range []string{
+		"/organizations/{organizationId}/invites",
+		"/organizations/{organizationId}/users",
+	} {
+		endpoint := findEndpoint(t, parsed, path)
+		var orgID *spec.Param
+		for i := range endpoint.Params {
+			if endpoint.Params[i].Name == "organizationId" {
+				orgID = &endpoint.Params[i]
+				break
+			}
+		}
+		if assert.NotNilf(t, orgID, "path %q must surface an organizationId param even when operation declared none", path) {
+			assert.True(t, orgID.Positional, "synthesized path placeholders must be positional")
+			assert.True(t, orgID.Required, "synthesized path placeholders must be required")
+		}
+	}
+}
+
 func TestCleanSpecName(t *testing.T) {
 	tests := []struct {
 		title string
diff --git a/internal/spec/spec.go b/internal/spec/spec.go
index 308aa79a..3b63bd00 100644
--- a/internal/spec/spec.go
+++ b/internal/spec/spec.go
@@ -1540,7 +1540,7 @@ func ParseBytes(data []byte) (*APISpec, error) {
 		return nil, fmt.Errorf("parsing yaml: %w", yamlErr)
 	}
 	s.expandOperations()
-	s.enrichPathParams()
+	s.EnrichPathParams()
 	s.promoteParamsToBodyForWriteEndpoints()
 	if err := s.validateReservedNames(); err != nil {
 		return nil, err
@@ -1697,7 +1697,7 @@ var pathParamRe = regexp.MustCompile(`\{([A-Za-z_][A-Za-z0-9_]*)\}`)
 
 var orGroupTokenRe = regexp.MustCompile(`\b[A-Z][A-Z0-9_]*\b`)
 
-// enrichPathParams walks every resource and sub-resource endpoint and ensures
+// EnrichPathParams walks every resource and sub-resource endpoint and ensures
 // each `{paramName}` placeholder in the endpoint path is represented in
 // Endpoint.Params with Positional: true, Required: true. The expandOperations
 // path already populates these for shorthand-generated endpoints; explicit
@@ -1713,7 +1713,7 @@ var orGroupTokenRe = regexp.MustCompile(`\b[A-Z][A-Z0-9_]*\b`)
 // Order is preserved: placeholders are appended in the order they appear in
 // the path so generated cobra `Args: cobra.ExactArgs(N)` sites and the
 // matching `replacePathParam(...args[i])` calls line up.
-func (s *APISpec) enrichPathParams() {
+func (s *APISpec) EnrichPathParams() {
 	for resourceName, r := range s.Resources {
 		s.enrichResourcePathParams(&r)
 		s.Resources[resourceName] = r

← c7e1b82b fix(cli): preserve body/content payload on single-object com  ·  back to Cli Printing Press  ·  feat(catalog): add OpenRouteService under new maps category 43c90ba0 →