← back to Cli Printing Press
fix(cli): preserve operation-routing path params (#567)
18542129e8ec10d67735c227efbad8772bac9704 · 2026-05-03 22:34:04 -0700 · Trevin Chow
Files touched
M internal/generator/generator_test.goM internal/generator/templates/sync.go.tmplM internal/openapi/parser.goM internal/openapi/parser_test.go
Diff
commit 18542129e8ec10d67735c227efbad8772bac9704
Author: Trevin Chow <trevin@trevinchow.com>
Date: Sun May 3 22:34:04 2026 -0700
fix(cli): preserve operation-routing path params (#567)
---
internal/generator/generator_test.go | 103 ++++++++++++++++++++++++++++++
internal/generator/templates/sync.go.tmpl | 11 ++--
internal/openapi/parser.go | 8 ++-
internal/openapi/parser_test.go | 89 ++++++++++++++++++++++++++
4 files changed, 205 insertions(+), 6 deletions(-)
diff --git a/internal/generator/generator_test.go b/internal/generator/generator_test.go
index 9c2df7c8..0e60c48a 100644
--- a/internal/generator/generator_test.go
+++ b/internal/generator/generator_test.go
@@ -4907,6 +4907,109 @@ func TestGeneratedSyncIDFieldOverridesAndProbes(t *testing.T) {
runGoCommand(t, outputDir, "test", "./internal/store/...", "-run", "TestUpsertBatch_TemplatedIDFieldOverrideWins|TestUpsertBatch_GenericFallbackList|TestUpsertBatch_ExtractFailuresReturnedForPerItemMisses")
}
+func TestGenerateOperationRoutingPathParamDefault(t *testing.T) {
+ t.Parallel()
+
+ apiSpec := &spec.APISpec{
+ Name: "routing",
+ Version: "0.1.0",
+ BaseURL: "https://api.example.com",
+ Auth: spec.AuthConfig{Type: "none"},
+ Config: spec.ConfigSpec{
+ Format: "toml",
+ Path: "~/.config/routing-pp-cli/config.toml",
+ },
+ Resources: map[string]spec.Resource{
+ "graphql": {
+ Description: "GraphQL endpoints",
+ SubResources: map[string]spec.Resource{
+ "followers": {
+ Description: "Followers",
+ Endpoints: map[string]spec.Endpoint{
+ "get": {
+ Method: "GET",
+ Path: "/graphql/{pathQueryId}/Followers",
+ Description: "Get followers",
+ Params: []spec.Param{
+ {Name: "pathQueryId", Type: "string", PathParam: true, Default: "followers123", Description: "Path query id"},
+ {Name: "variables", Type: "string", Required: true},
+ },
+ Response: spec.ResponseDef{Type: "array"},
+ },
+ },
+ },
+ },
+ },
+ },
+ }
+
+ outputDir := filepath.Join(t.TempDir(), naming.CLI(apiSpec.Name))
+ require.NoError(t, New(apiSpec, outputDir).Generate())
+
+ cliGo, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "graphql_followers_get.go"))
+ require.NoError(t, err)
+ cliContent := string(cliGo)
+ assert.Regexp(t,
+ regexp.MustCompile(`cmd\.Flags\(\)\.StringVar\(&flagPathQueryId,\s*"path-query-id",\s*"followers123",\s*"Path query id"\)`),
+ cliContent,
+ "defaulted operation-routing path param should be user-overridable")
+ assert.Contains(t, cliContent,
+ `path = replacePathParam(path, "pathQueryId", fmt.Sprintf("%v", flagPathQueryId))`,
+ "generated command must substitute the path template before calling the API")
+
+ mcpGo, err := os.ReadFile(filepath.Join(outputDir, "internal", "mcp", "tools.go"))
+ require.NoError(t, err)
+ assert.Regexp(t,
+ regexp.MustCompile(`makeAPIHandler\("GET",\s*"/graphql/\{pathQueryId\}/Followers",\s*\[]string\{[^}]*"pathQueryId"`),
+ string(mcpGo),
+ "MCP handler must receive the routing path param so it can substitute the URL")
+}
+
+func TestGenerateSyncRejectsUnknownResourcePath(t *testing.T) {
+ t.Parallel()
+
+ apiSpec := &spec.APISpec{
+ Name: "syncpaths",
+ Version: "0.1.0",
+ BaseURL: "https://api.example.com",
+ Auth: spec.AuthConfig{Type: "none"},
+ Config: spec.ConfigSpec{
+ Format: "toml",
+ Path: "~/.config/syncpaths-pp-cli/config.toml",
+ },
+ Resources: map[string]spec.Resource{
+ "widgets": {
+ Description: "Widgets",
+ Endpoints: map[string]spec.Endpoint{
+ "list": {
+ Method: "GET",
+ Path: "/widgets",
+ Description: "List widgets",
+ Response: spec.ResponseDef{Type: "array"},
+ Pagination: &spec.Pagination{CursorParam: "after", LimitParam: "limit"},
+ },
+ },
+ },
+ },
+ }
+
+ outputDir := filepath.Join(t.TempDir(), naming.CLI(apiSpec.Name))
+ require.NoError(t, New(apiSpec, outputDir).Generate())
+
+ syncGo, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "sync.go"))
+ require.NoError(t, err)
+ syncContent := string(syncGo)
+ assert.Contains(t, syncContent,
+ `func syncResourcePath(resource string) (string, error)`,
+ "syncResourcePath should report unsupported resource names explicitly")
+ assert.Contains(t, syncContent,
+ `return "", fmt.Errorf("unknown sync resource %q", resource)`,
+ "sync must not invent REST-style paths for unknown or GraphQL-only resources")
+ assert.NotContains(t, syncContent,
+ `return "/" + resource`,
+ "sync must not request fake /<resource> paths")
+}
+
func TestGenerateGraphQLCompiles(t *testing.T) {
t.Parallel()
diff --git a/internal/generator/templates/sync.go.tmpl b/internal/generator/templates/sync.go.tmpl
index 7cfff962..791d03a4 100644
--- a/internal/generator/templates/sync.go.tmpl
+++ b/internal/generator/templates/sync.go.tmpl
@@ -314,7 +314,10 @@ func syncResource(c interface {
fmt.Fprintf(os.Stderr, `{"event":"sync_start","resource":"%s"}`+"\n", resource)
}
- path := syncResourcePath(resource)
+ path, err := syncResourcePath(resource)
+ if err != nil {
+ return syncResult{Resource: resource, Err: err, Duration: time.Since(started)}
+ }
var totalCount int
// Resume cursor from sync_state (unless --full cleared it)
@@ -882,16 +885,16 @@ func defaultSyncResources() []string {
// syncResourcePath maps resource names to their actual API endpoint paths.
// For REST APIs this is typically "/<resource>". For non-REST APIs (e.g., Steam)
// this preserves the actual endpoint path like "/ISteamApps/GetAppList/v2".
-func syncResourcePath(resource string) string {
+func syncResourcePath(resource string) (string, error) {
paths := map[string]string{
{{- range .SyncableResources}}
"{{.Name}}": "{{.Path}}",
{{- end}}
}
if p, ok := paths[resource]; ok {
- return p
+ return p, nil
}
- return "/" + resource
+ return "", fmt.Errorf("unknown sync resource %q", resource)
}
{{- if .DependentSyncResources}}
diff --git a/internal/openapi/parser.go b/internal/openapi/parser.go
index 1d7541f1..a931f271 100644
--- a/internal/openapi/parser.go
+++ b/internal/openapi/parser.go
@@ -1570,7 +1570,7 @@ func filterGlobalParams(resources map[string]spec.Resource) {
seen := map[string]struct{}{}
for _, param := range endpoint.Params {
- if param.Positional {
+ if isPathSubstitutionParam(param) {
continue
}
if _, ok := seen[param.Name]; ok {
@@ -1607,7 +1607,7 @@ func filterGlobalParams(resources map[string]spec.Resource) {
walkResourceEndpoints(resources, func(endpoint *spec.Endpoint) {
filtered := endpoint.Params[:0]
for _, param := range endpoint.Params {
- if !param.Positional {
+ if !isPathSubstitutionParam(param) {
if _, ok := globalParams[param.Name]; ok {
continue
}
@@ -1628,6 +1628,10 @@ func filterGlobalParams(resources map[string]spec.Resource) {
}
}
+func isPathSubstitutionParam(param spec.Param) bool {
+ return param.Positional || param.PathParam
+}
+
func walkResourceEndpoints(resources map[string]spec.Resource, fn func(endpoint *spec.Endpoint)) {
resourceNames := make([]string, 0, len(resources))
for name := range resources {
diff --git a/internal/openapi/parser_test.go b/internal/openapi/parser_test.go
index 54307c92..961cdcef 100644
--- a/internal/openapi/parser_test.go
+++ b/internal/openapi/parser_test.go
@@ -730,6 +730,95 @@ func TestReclassifyPathParamDefaults(t *testing.T) {
assert.Equal(t, "PICK", params[2].Default, "enum default should be first value")
}
+func TestParsePreservesDefaultedPathParamsDuringGlobalFilter(t *testing.T) {
+ data := []byte(`
+openapi: 3.0.0
+info:
+ title: GraphQL Routing API
+ version: 1.0.0
+servers:
+ - url: https://api.example.com
+paths:
+ /graphql/{pathQueryId}/Followers:
+ get:
+ operationId: getFollowers
+ parameters:
+ - in: path
+ name: pathQueryId
+ required: true
+ schema:
+ type: string
+ default: followers123
+ - in: query
+ name: variables
+ required: true
+ schema:
+ type: string
+ responses:
+ "200":
+ description: ok
+ /graphql/{pathQueryId}/Following:
+ get:
+ operationId: getFollowing
+ parameters:
+ - in: path
+ name: pathQueryId
+ required: true
+ schema:
+ type: string
+ default: following123
+ - in: query
+ name: variables
+ required: true
+ schema:
+ type: string
+ responses:
+ "200":
+ description: ok
+ /graphql/{pathQueryId}/Likes:
+ get:
+ operationId: getLikes
+ parameters:
+ - in: path
+ name: pathQueryId
+ required: true
+ schema:
+ type: string
+ default: likes123
+ - in: query
+ name: variables
+ required: true
+ schema:
+ type: string
+ responses:
+ "200":
+ description: ok
+`)
+
+ parsed, err := Parse(data)
+ require.NoError(t, err)
+
+ for _, path := range []string{
+ "/graphql/{pathQueryId}/Followers",
+ "/graphql/{pathQueryId}/Following",
+ "/graphql/{pathQueryId}/Likes",
+ } {
+ endpoint := findEndpoint(t, parsed, path)
+ var routingParam *spec.Param
+ for i := range endpoint.Params {
+ if endpoint.Params[i].Name == "pathQueryId" {
+ routingParam = &endpoint.Params[i]
+ break
+ }
+ }
+ if assert.NotNil(t, routingParam, "pathQueryId should survive global-param filtering") {
+ assert.True(t, routingParam.PathParam, "defaulted path param should remain a URL substitution flag")
+ assert.False(t, routingParam.Positional, "defaulted path param should stay flag-shaped")
+ assert.NotNil(t, routingParam.Default, "operation-specific query id default should be preserved")
+ }
+ }
+}
+
func TestCleanSpecName(t *testing.T) {
tests := []struct {
title string
← e588bf7e fix(cli): parse epoch Retry-After headers (#565)
·
back to Cli Printing Press
·
fix(skills): tighten publish and polish workflows (#568) 847f7c5b →