[object Object]

← back to Cli Printing Press

fix(cli): skip framework-auto-generated operationIds when deriving command names (#1367)

d697eb5ae9ee18804c77022996f71058c972982d · 2026-05-13 22:03:23 -0700 · Trevin Chow

* fix(cli): skip framework-auto-generated operationIds when deriving command names

When an OpenAPI spec uses framework-auto-generated operationIds (e.g.,
API Platform / Symfony emits api_paymentsubscriptions_get_collection on
GET /api/payment/subscriptions), the parser kept the concatenated path
tokens verbatim, producing unreadable command names like
`payment paymentsubscriptions-get-collection` and
`community communitycommunities-idmemberships-post` in --help, README,
and SKILL output.

Add a detector that flips when a token in the snake-cased operationId
equals the concatenation of two consecutive non-routing path segments.
On a hit, resolveEndpointName skips operationIDToName and falls back to
the existing verb + path-collision-suffix derivation, which produces
readable names like `payment list` and `community create`.

The signal is unambiguous evidence of auto-generation, so hand-curated
ids that merely share the api_ prefix (api_user_v1_create) and Google
Discovery dotted ids (run.projects.locations.services.create) keep the
existing name derivation path.

Closes #1256

* fix(cli): gate framework-operationId detector on verb suffix and exclude path-param tokens

Greptile P2 review on PR #1367 surfaced two regression paths in the
isFrameworkAutoGeneratedOperationID detector:

1. A hand-curated id like api_searchresults_list on /api/search/results
   would mis-trip the concat signal (search + results = searchresults
   matches the snake-cased operationId token) and regress to a generic
   `list` instead of the richer `searchresults-list` operationIDToName
   would produce.

2. Path-param names (e.g. {userId} normalising to "userid") silently
   contributed to concat candidates: userid + settings = useridsettings.
   A hand-curated id carrying that token would trip the bypass.

Fix both by tightening the detector to require two signals together:
(a) the snake-cased operationId ends with a framework verb suffix
(_post, _put, _patch, _delete, _get_collection, _get_subresource) and
(b) the existing concat-of-two-consecutive-non-routing-segments signal,
now with path-param segments excluded from the candidate list.

The systeme.io cases all trip via non-param adjacent pairs
(school+courses, community+communities, payment+subscriptions), so the
path-param exclusion is safe. Hand-curated ids ship English CRUD verbs
(list, create, search) and miss the verb-suffix gate.

Extends the table test with the two Greptile false-positive scenarios
plus a positive case for the _get_subresource composite suffix.

Files touched

Diff

commit d697eb5ae9ee18804c77022996f71058c972982d
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Wed May 13 22:03:23 2026 -0700

    fix(cli): skip framework-auto-generated operationIds when deriving command names (#1367)
    
    * fix(cli): skip framework-auto-generated operationIds when deriving command names
    
    When an OpenAPI spec uses framework-auto-generated operationIds (e.g.,
    API Platform / Symfony emits api_paymentsubscriptions_get_collection on
    GET /api/payment/subscriptions), the parser kept the concatenated path
    tokens verbatim, producing unreadable command names like
    `payment paymentsubscriptions-get-collection` and
    `community communitycommunities-idmemberships-post` in --help, README,
    and SKILL output.
    
    Add a detector that flips when a token in the snake-cased operationId
    equals the concatenation of two consecutive non-routing path segments.
    On a hit, resolveEndpointName skips operationIDToName and falls back to
    the existing verb + path-collision-suffix derivation, which produces
    readable names like `payment list` and `community create`.
    
    The signal is unambiguous evidence of auto-generation, so hand-curated
    ids that merely share the api_ prefix (api_user_v1_create) and Google
    Discovery dotted ids (run.projects.locations.services.create) keep the
    existing name derivation path.
    
    Closes #1256
    
    * fix(cli): gate framework-operationId detector on verb suffix and exclude path-param tokens
    
    Greptile P2 review on PR #1367 surfaced two regression paths in the
    isFrameworkAutoGeneratedOperationID detector:
    
    1. A hand-curated id like api_searchresults_list on /api/search/results
       would mis-trip the concat signal (search + results = searchresults
       matches the snake-cased operationId token) and regress to a generic
       `list` instead of the richer `searchresults-list` operationIDToName
       would produce.
    
    2. Path-param names (e.g. {userId} normalising to "userid") silently
       contributed to concat candidates: userid + settings = useridsettings.
       A hand-curated id carrying that token would trip the bypass.
    
    Fix both by tightening the detector to require two signals together:
    (a) the snake-cased operationId ends with a framework verb suffix
    (_post, _put, _patch, _delete, _get_collection, _get_subresource) and
    (b) the existing concat-of-two-consecutive-non-routing-segments signal,
    now with path-param segments excluded from the candidate list.
    
    The systeme.io cases all trip via non-param adjacent pairs
    (school+courses, community+communities, payment+subscriptions), so the
    path-param exclusion is safe. Hand-curated ids ship English CRUD verbs
    (list, create, search) and miss the verb-suffix gate.
    
    Extends the table test with the two Greptile false-positive scenarios
    plus a positive case for the _get_subresource composite suffix.
---
 internal/openapi/parser.go                     |  94 +++++++-
 internal/openapi/parser_framework_opid_test.go | 312 +++++++++++++++++++++++++
 2 files changed, 405 insertions(+), 1 deletion(-)

diff --git a/internal/openapi/parser.go b/internal/openapi/parser.go
index 8a0864c7..69597dc2 100644
--- a/internal/openapi/parser.go
+++ b/internal/openapi/parser.go
@@ -2360,7 +2360,11 @@ func tagDescriptionKeys(name string) []string {
 }
 
 func resolveEndpointName(method, path string, op *openapi3.Operation, existing map[string]spec.Endpoint, resourceName, basePath string, commonPrefix []string) string {
-	name := operationIDToName(operationID(op), resourceName, commonPrefix)
+	opID := operationID(op)
+	var name string
+	if !isFrameworkAutoGeneratedOperationID(opID, path) {
+		name = operationIDToName(opID, resourceName, commonPrefix)
+	}
 	if name == "" {
 		name = defaultEndpointName(method, path)
 	}
@@ -3998,6 +4002,94 @@ func operationIDToName(operationID, resourceName string, commonPrefix []string)
 	return strings.ReplaceAll(name, "_", "-")
 }
 
+// isFrameworkAutoGeneratedOperationID reports whether operationID was
+// auto-generated by a framework (API Platform / Symfony, etc.) that names
+// operations by concatenating path segments and appending the HTTP method.
+// Two signals must both fire:
+//
+//  1. The operationId, snake-cased and tokenized on underscores, ends with
+//     a framework verb suffix (_post, _put, _patch, _delete, _get_collection,
+//     _get_subresource). Hand-curated ids that ship CRUD verbs as English
+//     words (list, create, search) and Google Discovery dotted ids
+//     (run.projects.locations.services.create) miss this gate.
+//  2. The same token list contains a token equal to the concatenation
+//     (no separator) of two consecutive non-routing, non-path-param path
+//     segments. Path-param segments are excluded so a hand-curated id
+//     that happens to contain a "{paramName}+nextSegment" concat does not
+//     trip the detector.
+//
+// When both fire, callers skip operationIDToName so name derivation falls
+// back to verb + path-collision-suffix rather than emitting unreadable
+// command names like "paymentsubscriptions-get-collection".
+func isFrameworkAutoGeneratedOperationID(operationID, path string) bool {
+	if operationID == "" || path == "" {
+		return false
+	}
+	opTokens := strings.Split(toSnakeCase(operationID), "_")
+	if len(opTokens) < 2 {
+		return false
+	}
+	if !hasFrameworkVerbSuffix(opTokens) {
+		return false
+	}
+
+	segs := splitPath(path)
+	normSegs := make([]string, 0, len(segs))
+	for _, s := range segs {
+		if isVersionSegment(s) || isGenericAPIPrefix(s) || isPathParamSegment(s) {
+			continue
+		}
+		n := strings.ReplaceAll(toSnakeCase(s), "_", "")
+		if n == "" {
+			continue
+		}
+		normSegs = append(normSegs, n)
+	}
+	if len(normSegs) < 2 {
+		return false
+	}
+
+	// frameworkConcatMinLen guards against accidental matches on very short
+	// segment pairs. 8 chars is comfortably above any plausible single
+	// path segment and below every concatenation we have seen in the wild
+	// (paymentsubscriptions, communitycommunities, schoolenrollments).
+	const frameworkConcatMinLen = 8
+
+	for i := range len(normSegs) - 1 {
+		cand := normSegs[i] + normSegs[i+1]
+		if len(cand) < frameworkConcatMinLen {
+			continue
+		}
+		if slices.Contains(opTokens, cand) {
+			return true
+		}
+	}
+	return false
+}
+
+// hasFrameworkVerbSuffix reports whether opTokens ends with a verb suffix
+// that frameworks like API Platform emit as a marker of auto-generation:
+// the bare HTTP method (post/put/patch/delete) or one of the well-known
+// composite suffixes (get_collection, get_subresource). Hand-curated ids
+// ship English CRUD verbs (list, create, update) and do not match.
+func hasFrameworkVerbSuffix(opTokens []string) bool {
+	if len(opTokens) == 0 {
+		return false
+	}
+	last := opTokens[len(opTokens)-1]
+	switch last {
+	case "post", "put", "patch", "delete":
+		return true
+	}
+	if len(opTokens) >= 2 && opTokens[len(opTokens)-2] == "get" {
+		switch last {
+		case "collection", "subresource":
+			return true
+		}
+	}
+	return false
+}
+
 func googleOperationIDEndpointName(operationID, resourceName string) string {
 	chain, verb, ok := googleOperationIDResourceChain(operationID, true, true)
 	if !ok {
diff --git a/internal/openapi/parser_framework_opid_test.go b/internal/openapi/parser_framework_opid_test.go
new file mode 100644
index 00000000..31bc7d31
--- /dev/null
+++ b/internal/openapi/parser_framework_opid_test.go
@@ -0,0 +1,312 @@
+package openapi
+
+import (
+	"sort"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestParseFrameworkAutoGeneratedOperationIDs pins that endpoints from specs
+// using framework-auto-generated operationIds (API Platform / Symfony shape:
+// "api_<concatenated-path-tokens>_<verb>[_subresource]") receive readable
+// command names derived from the path rather than the unreadable concatenation
+// of path tokens that the operationId carries.
+//
+// Issue #1256.
+func TestParseFrameworkAutoGeneratedOperationIDs(t *testing.T) {
+	t.Parallel()
+
+	spec := []byte(`
+openapi: 3.0.3
+info:
+  title: Systeme Repro
+  version: 1.0.0
+servers:
+  - url: https://api.systeme.io
+paths:
+  /api/payment/subscriptions:
+    get:
+      operationId: api_paymentsubscriptions_get_collection
+      tags: ["Payment"]
+      responses:
+        "200":
+          description: ok
+          content:
+            application/json:
+              schema:
+                type: array
+                items:
+                  $ref: "#/components/schemas/PaymentSubscription"
+  /api/community/communities/{communityId}/memberships:
+    post:
+      operationId: api_communitycommunities_communityIdmemberships_post
+      tags: ["Community"]
+      parameters:
+        - name: communityId
+          in: path
+          required: true
+          schema: { type: string }
+      requestBody:
+        required: true
+        content:
+          application/json:
+            schema:
+              type: object
+              properties:
+                userId: { type: string }
+      responses:
+        "200":
+          description: ok
+  /api/school/courses/{courseId}/enrollments:
+    post:
+      operationId: api_schoolcourses_courseIdenrollments_post
+      tags: ["School"]
+      parameters:
+        - name: courseId
+          in: path
+          required: true
+          schema: { type: string }
+      requestBody:
+        required: true
+        content:
+          application/json:
+            schema:
+              type: object
+              properties:
+                userId: { type: string }
+      responses:
+        "200":
+          description: ok
+  /api/school/enrollments/{id}:
+    delete:
+      operationId: api_schoolenrollments_id_delete
+      tags: ["School"]
+      parameters:
+        - name: id
+          in: path
+          required: true
+          schema: { type: string }
+      responses:
+        "204":
+          description: no content
+  /api/school/enrollments:
+    get:
+      operationId: api_schoolenrollments_get_collection
+      tags: ["School"]
+      responses:
+        "200":
+          description: ok
+          content:
+            application/json:
+              schema:
+                type: array
+                items: { type: object }
+components:
+  schemas:
+    PaymentSubscription:
+      type: object
+      properties:
+        id: { type: string }
+`)
+	parsed, err := Parse(spec)
+	require.NoError(t, err)
+
+	tree := map[string][]string{}
+	for rname, r := range parsed.Resources {
+		names := make([]string, 0, len(r.Endpoints))
+		for ename := range r.Endpoints {
+			names = append(names, ename)
+		}
+		sort.Strings(names)
+		tree[rname] = names
+	}
+
+	// Resources are derived from path segments after the basePath, not from
+	// tag names, so "payment", "community", "school" are the expected keys.
+	require.Contains(t, tree, "payment")
+	require.Contains(t, tree, "community")
+	require.Contains(t, tree, "school")
+
+	for resource, endpoints := range tree {
+		for _, ep := range endpoints {
+			// The unreadable forms produced by the framework-id passthrough
+			// must never appear as command names.
+			for _, bad := range []string{
+				"paymentsubscriptions",
+				"communitycommunities",
+				"schoolcourses",
+				"schoolenrollments",
+				"communityidmemberships",
+				"courseidenrollments",
+				"idmemberships",
+				"idenrollments",
+			} {
+				assert.NotContains(t, ep, bad,
+					"resource %s endpoint %q must not retain framework-concatenated token %q", resource, ep, bad)
+			}
+		}
+	}
+
+	// Positive assertions: each endpoint must reduce to a recognizable
+	// CRUD verb (with optional path-collision suffix).
+	containsVerb := func(name string) bool {
+		for _, verb := range []string{"list", "get", "create", "update", "delete"} {
+			if name == verb || strings.HasPrefix(name, verb+"-") || strings.HasSuffix(name, "-"+verb) || strings.Contains(name, "-"+verb+"-") {
+				return true
+			}
+		}
+		return false
+	}
+	for resource, endpoints := range tree {
+		for _, ep := range endpoints {
+			assert.True(t, containsVerb(ep),
+				"resource %s endpoint %q should reduce to a recognizable CRUD verb after framework-id fallback", resource, ep)
+		}
+	}
+
+	// Negative: a hand-curated operationId should keep its derived name.
+	negSpec := []byte(`
+openapi: 3.0.3
+info:
+  title: Curated
+  version: 1.0.0
+servers:
+  - url: https://api.example.com
+paths:
+  /v1/customers:
+    get:
+      operationId: customersListAll
+      tags: ["Customers"]
+      responses:
+        "200":
+          description: ok
+          content:
+            application/json:
+              schema:
+                type: array
+                items: { type: object }
+`)
+	parsedNeg, err := Parse(negSpec)
+	require.NoError(t, err)
+	require.Contains(t, parsedNeg.Resources, "customers")
+	customers := parsedNeg.Resources["customers"]
+	negNames := make([]string, 0, len(customers.Endpoints))
+	for ename := range customers.Endpoints {
+		negNames = append(negNames, ename)
+	}
+	sort.Strings(negNames)
+	_, ok := customers.Endpoints["list-all"]
+	assert.True(t, ok, "hand-curated customersListAll must keep its operationId-derived name list-all; got endpoints: %v", negNames)
+}
+
+// TestIsFrameworkAutoGeneratedOperationID pins the detector boundary so
+// regressions on edge cases (Google Discovery, single-segment paths,
+// hand-curated operationIds) surface as table-test failures.
+func TestIsFrameworkAutoGeneratedOperationID(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		name        string
+		operationID string
+		path        string
+		want        bool
+	}{
+		{
+			name:        "API Platform list-collection on two-segment path",
+			operationID: "api_paymentsubscriptions_get_collection",
+			path:        "/api/payment/subscriptions",
+			want:        true,
+		},
+		{
+			name:        "API Platform with path param between segments",
+			operationID: "api_communitycommunities_communityIdmemberships_post",
+			path:        "/api/community/communities/{communityId}/memberships",
+			want:        true,
+		},
+		{
+			name:        "API Platform delete-by-id",
+			operationID: "api_schoolenrollments_id_delete",
+			path:        "/api/school/enrollments/{id}",
+			want:        true,
+		},
+		{
+			name:        "hand-curated Stripe-style operationId",
+			operationID: "customersListAll",
+			path:        "/v1/customers",
+			want:        false,
+		},
+		{
+			name:        "hand-curated Petstore operationId",
+			operationID: "addPet",
+			path:        "/v1/pet",
+			want:        false,
+		},
+		{
+			name:        "api_ prefix but no path-token concatenation",
+			operationID: "api_user_v1_create",
+			path:        "/api/user",
+			want:        false,
+		},
+		{
+			name:        "Google Discovery dotted operationId",
+			operationID: "run.projects.locations.services.create",
+			path:        "/v2/projects/{projectsId}/locations/{locationsId}/services",
+			want:        false,
+		},
+		{
+			name:        "Cal.com controller-prefixed operationId",
+			operationID: "BookingsController_2024-08-13_getBooking",
+			path:        "/v2/bookings/{uid}",
+			want:        false,
+		},
+		{
+			name:        "empty operationId",
+			operationID: "",
+			path:        "/api/foo/bar",
+			want:        false,
+		},
+		{
+			name:        "empty path",
+			operationID: "api_foo_bar_post",
+			path:        "",
+			want:        false,
+		},
+		{
+			// Hand-curated compound-noun id (e.g. searchresults) over a path
+			// whose segments happen to be the component words. Ends in
+			// English verb "list", not a framework HTTP-method suffix, so
+			// the verb-suffix gate filters it out.
+			name:        "compound-noun hand-curated id with English verb",
+			operationID: "api_searchresults_list",
+			path:        "/api/search/results",
+			want:        false,
+		},
+		{
+			// Path-param token (userid) must not contribute to concat
+			// candidates. Even though "userid"+"settings" = "useridsettings"
+			// could appear in a hand-curated id, path-param segments are
+			// excluded from normSegs so the candidate is never formed.
+			name:        "path-param adjacency does not contribute to concat",
+			operationID: "users_userid_settings_get",
+			path:        "/api/users/{userId}/settings",
+			want:        false,
+		},
+		{
+			// Sub-resource list (API Platform composite suffix).
+			name:        "API Platform get_subresource suffix",
+			operationID: "api_invoicelines_invoiceIdrefunds_get_subresource",
+			path:        "/api/invoice/lines/{invoiceId}/refunds",
+			want:        true,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := isFrameworkAutoGeneratedOperationID(tc.operationID, tc.path)
+			assert.Equal(t, tc.want, got)
+		})
+	}
+}

← ca1de8fa test(cli): pin args[] index for flag-exposed path param shap  ·  back to Cli Printing Press  ·  fix(cli): consume x-tenant-env-var so per-tenant sync paths 51b758c7 →