← back to Cli Printing Press
fix(cli): index args[] by positional ordinal in path-param emit (#1211)
460d676ec54a9c231ff98f2047edbe61bffe2693 · 2026-05-12 08:58:16 -0700 · Trevin Chow
* fix(cli): index args[] by positional ordinal in path-param emit
Generator templates iterated Endpoint.Params with `range $i, $p` and
interpolated `$i` into both args[N] reads and len(args) guards inside
positional-only emission blocks. Cobra populates args from CLI
positionals in Positional-only declaration order, but Endpoint.Params
interleaves query and header params alongside positional ones. As soon
as a non-positional param sat before a positional path param (common
when an OpenAPI list endpoint declares limit/offset queries ahead of
the path param), the emitted index drifted off the Positional ordinal
and the command failed at runtime with "<name> is required" no matter
what positional the user passed. Verify never invokes path-param
commands with real positional values, so the mismatch shipped silently.
Route every args[] interpolation in command_endpoint.go.tmpl and
command_promoted.go.tmpl through a new positionalIndex(e, name) helper
that returns the Positional-only ordinal. The existing range topology
is preserved so emission order stays byte-stable on currently-correct
cases (golden harness 17/17 unchanged).
Fixes #1199
* docs(cli): document path-param args[] index drift learning
Captures the third recurrence of the same template-index/category drift
family. The 2026-05-05 doc on the MCP-side fix anticipated more sites
would surface; this learning records the args[]-indexing axis on
command_endpoint.go.tmpl and command_promoted.go.tmpl, the matched
positionalArgs / positionalIndex helper pair, and the why-comment
preservation rule that prevents future drive-by simplification.
Files touched
A docs/solutions/logic-errors/codegen-args-index-vs-positional-ordinal-2026-05-12.mdM internal/generator/generator.goA internal/generator/path_param_positional_index_test.goM internal/generator/templates/command_endpoint.go.tmplM internal/generator/templates/command_promoted.go.tmpl
Diff
commit 460d676ec54a9c231ff98f2047edbe61bffe2693
Author: Trevin Chow <trevin@trevinchow.com>
Date: Tue May 12 08:58:16 2026 -0700
fix(cli): index args[] by positional ordinal in path-param emit (#1211)
* fix(cli): index args[] by positional ordinal in path-param emit
Generator templates iterated Endpoint.Params with `range $i, $p` and
interpolated `$i` into both args[N] reads and len(args) guards inside
positional-only emission blocks. Cobra populates args from CLI
positionals in Positional-only declaration order, but Endpoint.Params
interleaves query and header params alongside positional ones. As soon
as a non-positional param sat before a positional path param (common
when an OpenAPI list endpoint declares limit/offset queries ahead of
the path param), the emitted index drifted off the Positional ordinal
and the command failed at runtime with "<name> is required" no matter
what positional the user passed. Verify never invokes path-param
commands with real positional values, so the mismatch shipped silently.
Route every args[] interpolation in command_endpoint.go.tmpl and
command_promoted.go.tmpl through a new positionalIndex(e, name) helper
that returns the Positional-only ordinal. The existing range topology
is preserved so emission order stays byte-stable on currently-correct
cases (golden harness 17/17 unchanged).
Fixes #1199
* docs(cli): document path-param args[] index drift learning
Captures the third recurrence of the same template-index/category drift
family. The 2026-05-05 doc on the MCP-side fix anticipated more sites
would surface; this learning records the args[]-indexing axis on
command_endpoint.go.tmpl and command_promoted.go.tmpl, the matched
positionalArgs / positionalIndex helper pair, and the why-comment
preservation rule that prevents future drive-by simplification.
---
...-args-index-vs-positional-ordinal-2026-05-12.md | 151 ++++++++++++
internal/generator/generator.go | 23 ++
.../generator/path_param_positional_index_test.go | 255 +++++++++++++++++++++
.../generator/templates/command_endpoint.go.tmpl | 25 +-
.../generator/templates/command_promoted.go.tmpl | 23 +-
5 files changed, 454 insertions(+), 23 deletions(-)
diff --git a/docs/solutions/logic-errors/codegen-args-index-vs-positional-ordinal-2026-05-12.md b/docs/solutions/logic-errors/codegen-args-index-vs-positional-ordinal-2026-05-12.md
new file mode 100644
index 00000000..fa74e5b0
--- /dev/null
+++ b/docs/solutions/logic-errors/codegen-args-index-vs-positional-ordinal-2026-05-12.md
@@ -0,0 +1,151 @@
+---
+title: "Codegen used full-Params index for args[] when Cobra populates from Positional-only declaration order"
+date: 2026-05-12
+category: logic-errors
+module: internal/generator/templates
+problem_type: logic_error
+component: tooling
+symptoms:
+ - "Generated path-param commands fail at runtime with '<name> is required' no matter what positional the user passes (e.g. keap-pp-cli tags contacts list-for-tag-id-using-get 1992 returns 'tagId is required')"
+ - "Bug reproduced in 3 of 3 published CLIs sampled (keap, supabase, learndash) — 5 broken commands total"
+ - "phase5-acceptance.json reports 9/9 verify pass on the affected CLI; verify does not invoke path-param commands with real positional values, so the runtime mismatch ships silently"
+ - "Sibling path-param commands without a trailing static URL segment (e.g. /v1/contacts/{email}) ship correctly, masking the bug shape from spot checks"
+root_cause: logic_error
+resolution_type: code_fix
+severity: high
+tags:
+ - generator-template
+ - path-params
+ - positional-args
+ - cobra
+ - args-indexing
+ - printed-cli
+ - command-endpoint-template
+ - command-promoted-template
+---
+
+# Codegen used full-Params index for args[] when Cobra populates from Positional-only declaration order
+
+## Problem
+
+`command_endpoint.go.tmpl` and `command_promoted.go.tmpl` iterated `Endpoint.Params` with `range $i, $p := .Endpoint.Params` and interpolated `$i` into both `args[$i]` reads and `if len(args) < N` guards inside positional-only emission blocks. Cobra populates the runtime `args` slice from CLI positional arguments in Positional-only declaration order, but `Endpoint.Params` interleaves query and header params alongside positional ones. As soon as a non-positional param sat before a positional path param (a common shape — OpenAPI list endpoints routinely declare `limit`/`offset` queries ahead of the path param), the emitted index drifted off the Positional ordinal and the command failed at runtime with `<name> is required`.
+
+The two-positional case where URL order coincides with Positional-only declaration order produced correct code by coincidence (`$i` equalled the Positional ordinal). The single-positional-with-trailing-segment case diverged and emitted a stale index.
+
+## Symptoms
+
+- `keap-pp-cli tags contacts list-for-tag-id-using-get 1992` → `Error: tagId is required` (template emitted `args[2]` / `len(args) < 3`; Cobra had `args = ["1992"]`).
+- Reproduced in three published CLIs:
+ - **keap** (3 broken commands): `tags_contacts_list-for-tag-id-using-get`, `tags_companies_list-for-tag-id-using-get`, `orders_transactions_list-for-order-using-get`.
+ - **supabase** (1): `projects_types_generate-typescript`.
+ - **learndash** (1): `users_course-progress_user-module-steps` (the first positional `args[0]` was correct; the second got the full-Params index `args[3]`).
+- After patching `args[N]` → `args[0]` and `len(args) < N+1` → `len(args) < 1` locally, the keap command returned the 78 contacts the user was asking about.
+- Verify reports 100% pass on the affected CLIs; the matrix exercises path-param commands without supplying real positional values, so the runtime mismatch is invisible to the scorer.
+
+## What Didn't Work
+
+Nothing went down a dead end. The retro report ([#1199](https://github.com/mvanhorn/cli-printing-press/issues/1199)) included diagnosis with high-confidence smoking-gun comparison (working `contacts/{email}/email-addresses` emit at `args[0]` versus broken `tags/{tagId}/contacts` emit at `args[2]` within the same generated CLI). Investigation went directly to the template iteration.
+
+The first design impulse — refactor the 10 affected template loops to `range positionalParams .Endpoint.Params` (filter to positional-only with new index) — was rejected because four of the loops interleave positional and non-positional emission in a single pass. Splitting into two ranges would shift the order of emitted map entries and break golden tests that froze byte-stable output for currently-correct cases.
+
+## Solution
+
+Introduce a template helper `positionalIndex(endpoint, name) -> int` that walks `Endpoint.Params` and returns the Positional-only ordinal of a named param. Route every `args[...]` interpolation and every `len(args) < ...` guard through it. Preserve the existing range structure (so emission order is byte-stable on currently-correct cases) and only swap the interpolated index value.
+
+**Helper** (`internal/generator/generator.go`):
+
+```go
+// positionalIndex returns the args[] slot a positional param fills at runtime.
+// Cobra populates args from CLI positionals in Positional-only declaration
+// order, but Endpoint.Params interleaves query/header params alongside path
+// params. The full-Params index drifts from the Positional ordinal as soon as
+// a non-positional param sits before a positional one (common when an OpenAPI
+// list endpoint declares query params before the path param) and the runtime
+// fails with "<name> is required" no matter what positional the user passes.
+// Returns -1 if name is not a positional param.
+func positionalIndex(e spec.Endpoint, name string) int {
+ idx := 0
+ for _, p := range e.Params {
+ if !p.Positional {
+ continue
+ }
+ if p.Name == name {
+ return idx
+ }
+ idx++
+ }
+ return -1
+}
+```
+
+Registered as a template func and used in both templates. The first positional block in each template binds `{{- $pi := positionalIndex $.Endpoint .Name}}` and reuses it (`add $pi 1` for the len-check threshold plus `args[$pi]` for the substitution); the four other sites per template call the helper inline because the value is used once.
+
+**Before** (`command_endpoint.go.tmpl`):
+
+```gotmpl
+{{- range $i, $p := .Endpoint.Params}}
+{{- if .Positional}}
+{{- if gt $i 0}}
+ if len(args) < {{add $i 1}} {
+ return usageErr(fmt.Errorf("{{.Name}} is required\n..."))
+ }
+{{- end}}
+{{- if pathContainsParam $.Endpoint.Path .Name}}
+ path = replacePathParam(path, "{{.Name}}", args[{{$i}}])
+{{- end}}
+{{- end}}
+{{- end}}
+```
+
+**After**:
+
+```gotmpl
+{{- range .Endpoint.Params}}
+{{- if .Positional}}
+{{- $pi := positionalIndex $.Endpoint .Name}}
+{{- if gt $pi 0}}
+ if len(args) < {{add $pi 1}} {
+ return usageErr(fmt.Errorf("{{.Name}} is required\n..."))
+ }
+{{- end}}
+{{- if pathContainsParam $.Endpoint.Path .Name}}
+ path = replacePathParam(path, "{{.Name}}", args[{{$pi}}])
+{{- end}}
+{{- end}}
+{{- end}}
+```
+
+The four interleaved-emission loops (htmlRequestParams, paginatedGet/resolvePaginatedRead maps, params map) keep their `range .Endpoint.Params` structure and only swap `args[{{$i}}]` for `args[{{positionalIndex $.Endpoint .Name}}]`. Both templates received the same treatment at five sites each (10 sites total).
+
+Regression coverage in `internal/generator/path_param_positional_index_test.go`:
+
+- `TestPathParamArgsIndexUsesPositionalOrdinal` pins three shapes against `command_endpoint.go.tmpl`: a single positional with two queries declared before it (the keap shape), two positionals with a query interleaved between (the learndash shape), and a regression baseline of two positionals in URL-matching order (the products/subscriptions shape that already worked).
+- `TestPromotedCommandPathParamArgsIndexUsesPositionalOrdinal` pins the same shape against `command_promoted.go.tmpl` (single-endpoint resource that gets promoted to a top-level command).
+- `TestPositionalIndexHelper` unit-tests the helper directly: positional ordinal lookup with queries interleaved, multi-positional ordinal lookup with a query between, unknown-name `-1` sentinel.
+
+`go test ./...` 3804 pass; `scripts/golden.sh verify` 17/17 PASS (byte-stable on existing fixtures because none of the frozen fixtures contained the bug shape); `go vet`, `golangci-lint`, `go fmt` clean.
+
+## Why This Works
+
+`Endpoint.Params` is a heterogeneous list — it carries every parameter the spec declares, regardless of where it lands at runtime (positional, flag, body, header). The template's range index reflects position in *that* list. Cobra's `args` slice, by contrast, reflects position in the Positional-only subsequence — the same subsequence the `Use:` line declares with `<a> <b> <c>`, which `positionalArgs(e)` already builds via the same filter-by-`Positional` pass. By extracting that filter into a callable helper (`positionalIndex`), the template can ask the same question the Use line implicitly answers and stay aligned with Cobra's runtime behavior.
+
+The fix keeps the existing loop topology — `range .Endpoint.Params` with `{{- if .Positional}}` gates — instead of switching to a positional-only iteration. That choice is deliberate: four of the loops interleave positional and non-positional emission in one pass, and splitting them would change the order of emitted map literal entries and break golden tests that froze byte-stable output for currently-correct cases. The helper lookup is O(N×M) per template render, but template rendering is offline code generation; the cost is negligible.
+
+The fix is local to one helper plus 10 template interpolations; no caller of `replacePathParam` or the generated handler functions needs to change.
+
+## Prevention
+
+- **This is now the third recurrence in the same template family.** [#171 WU-1](https://github.com/mvanhorn/cli-printing-press/issues/171) fixed an `Endpoint.Params` iteration miscategorization in `command_endpoint.go.tmpl`. [#578](https://github.com/mvanhorn/cli-printing-press/issues/578) fixed the same bug shape on the MCP side in `mcp_tools.go.tmpl` (see [logic-errors/mcp-handler-conflates-path-and-query-positional-params-2026-05-05.md](mcp-handler-conflates-path-and-query-positional-params-2026-05-05.md)). #1199 is the third instance: a different axis (args[] indexing rather than path-vs-query disambiguation) but the same root family — a generator-emitted handler computing an index/category from `Endpoint.Params`'s ordering rather than from a Positional-only or path-template authoritative source. Future template work in this area should default to suspicion.
+- **Reuse Positional-only ordinals through a named helper rather than ad-hoc range indices.** `positionalArgs(e)` (which builds the `Use:` string) and `positionalIndex(e, name)` (which routes `args[N]`) are now a matched pair; both filter on `p.Positional` in declaration order. Any new template emission that needs a Cobra-args slot should call `positionalIndex` rather than computing an index from a fresh range.
+- **Treat "passes verify" as necessary but not sufficient** for runtime correctness of positional-arg commands. The current verify matrix doesn't supply real positional values to path-param commands, so the failure shape this fix addresses is invisible to scorer-only gates. A printed-CLI dogfood pass that exercises path-param commands with real positionals catches this class of bug; consider that the minimum bar for releasing a CLI with non-trivial path-param surface area.
+- **Add golden coverage for new index-shaped emission.** The existing golden fixtures all happened to declare positional params first, so `$i` equalled the Positional ordinal and the byte-stable output was correct by coincidence. A new fixture with a non-positional param declared before a positional path param would lock the helper's behavior into the byte-stable contract. Tracked as a follow-up; the unit + integration tests in `path_param_positional_index_test.go` are the regression net today.
+- **Preserve the `positionalIndex` Go doc comment.** The 2026-05-05 retro on the sibling bug flagged that the "why" comment in the handler body was load-bearing — a future drive-by edit nearly recreated the bug by "simplifying" it. The same caution applies here: the comment on `positionalIndex` explains why `Endpoint.Params` ordering differs from Cobra args ordering, and stripping it invites recreation of the bug for the next reader who searches "why isn't `range $i` good enough?"
+
+## Related Issues
+
+- [#1199](https://github.com/mvanhorn/cli-printing-press/issues/1199) — original issue (this fix)
+- [#1192](https://github.com/mvanhorn/cli-printing-press/issues/1192) — sibling open issue: `replacePathParam` emits zero path-param encoding (same helper, different bug). Both indicate the path-param emission template is the right place to harden.
+- [#1198](https://github.com/mvanhorn/cli-printing-press/issues/1198) — scorer-side gap: verify doesn't exercise path-param positional binding (the reason this shipped silently). Independent fix.
+- [#578](https://github.com/mvanhorn/cli-printing-press/issues/578) / [#582](https://github.com/mvanhorn/cli-printing-press/pull/582) — second recurrence (MCP side, same template family).
+- [#171 WU-1](https://github.com/mvanhorn/cli-printing-press/issues/171) — first recurrence (CLI side, sibling axis).
+- [logic-errors/mcp-handler-conflates-path-and-query-positional-params-2026-05-05.md](mcp-handler-conflates-path-and-query-positional-params-2026-05-05.md) — solution doc for the second recurrence; shares the prevention rationale.
diff --git a/internal/generator/generator.go b/internal/generator/generator.go
index d50291ca..9e9fadd2 100644
--- a/internal/generator/generator.go
+++ b/internal/generator/generator.go
@@ -360,6 +360,7 @@ func New(s *spec.APISpec, outputDir string) *Generator {
"pathContainsParam": func(path, name string) bool {
return strings.Contains(path, "{"+name+"}")
},
+ "positionalIndex": positionalIndex,
"safeJoin": func(fields []string, sep string) string {
safe := make([]string, len(fields))
for i, f := range fields {
@@ -3697,6 +3698,28 @@ func positionalArgs(e spec.Endpoint) string {
return ""
}
+// positionalIndex returns the args[] slot a positional param fills at runtime.
+// Cobra populates args from CLI positionals in Positional-only declaration
+// order, but Endpoint.Params interleaves query/header params alongside path
+// params. The full-Params index drifts from the Positional ordinal as soon as
+// a non-positional param sits before a positional one (common when an OpenAPI
+// list endpoint declares query params before the path param) and the runtime
+// fails with "<name> is required" no matter what positional the user passes.
+// Returns -1 if name is not a positional param.
+func positionalIndex(e spec.Endpoint, name string) int {
+ idx := 0
+ for _, p := range e.Params {
+ if !p.Positional {
+ continue
+ }
+ if p.Name == name {
+ return idx
+ }
+ idx++
+ }
+ return -1
+}
+
func configTag(format string) string {
switch format {
case "toml":
diff --git a/internal/generator/path_param_positional_index_test.go b/internal/generator/path_param_positional_index_test.go
new file mode 100644
index 00000000..1068fb17
--- /dev/null
+++ b/internal/generator/path_param_positional_index_test.go
@@ -0,0 +1,255 @@
+package generator
+
+import (
+ "os"
+ "path/filepath"
+ "testing"
+
+ "github.com/mvanhorn/cli-printing-press/v4/internal/naming"
+ "github.com/mvanhorn/cli-printing-press/v4/internal/spec"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+)
+
+// TestPathParamArgsIndexUsesPositionalOrdinal exercises the codegen template's
+// args[] substitution for positional path params. The bug it pins: when a
+// non-positional query param sits before the positional path param in
+// Endpoint.Params, the template used to interpolate the full-Params index into
+// args[] (and into the len-check), making the generated command fail at
+// runtime with "<name> is required" regardless of what positional the user
+// passes. Fixed by indexing into args via the Positional-only ordinal.
+func TestPathParamArgsIndexUsesPositionalOrdinal(t *testing.T) {
+ t.Parallel()
+
+ apiSpec := &spec.APISpec{
+ Name: "argidx",
+ Version: "0.1.0",
+ BaseURL: "https://api.example.com",
+ Auth: spec.AuthConfig{Type: "none"},
+ Config: spec.ConfigSpec{
+ Format: "toml",
+ Path: "~/.config/argidx-pp-cli/config.toml",
+ },
+ Resources: map[string]spec.Resource{
+ "tags": {
+ Description: "Tags",
+ SubResources: map[string]spec.Resource{
+ "contacts": {
+ Description: "Contacts under a tag",
+ Endpoints: map[string]spec.Endpoint{
+ // list-for-tag: declares limit + offset (query) BEFORE the
+ // positional path param tagId. Without the fix, the template
+ // emitted args[2] (the full-Params index) instead of args[0]
+ // (the Positional ordinal).
+ "list-for-tag": {
+ Method: "GET",
+ Path: "/v1/tags/{tagId}/contacts",
+ Description: "List contacts for a tag",
+ Params: []spec.Param{
+ {Name: "limit", Type: "int"},
+ {Name: "offset", Type: "int"},
+ {Name: "tagId", Type: "string", Required: true, Positional: true},
+ },
+ },
+ },
+ },
+ },
+ },
+ "users": {
+ Description: "Users",
+ SubResources: map[string]spec.Resource{
+ "course-progress": {
+ Description: "Course progress",
+ Endpoints: map[string]spec.Endpoint{
+ // Two positional path params with a non-positional query
+ // interleaved between them. The Use line is `<id> <course>`
+ // (positional ordinals 0 and 1) even though Endpoint.Params
+ // has the query at index 1.
+ "steps": {
+ Method: "GET",
+ Path: "/users/{id}/course-progress/{course}/steps",
+ Description: "List steps for a course",
+ Params: []spec.Param{
+ {Name: "id", Type: "string", Required: true, Positional: true},
+ {Name: "fmt", Type: "string"},
+ {Name: "course", Type: "string", Required: true, Positional: true},
+ },
+ },
+ },
+ },
+ },
+ },
+ "products": {
+ Description: "Products",
+ SubResources: map[string]spec.Resource{
+ "subscriptions": {
+ Description: "Subscriptions",
+ Endpoints: map[string]spec.Endpoint{
+ // Currently-working baseline: two positional path params in
+ // URL-matching declaration order, no non-positional params
+ // before them. This case used to produce correct args[0]/
+ // args[1] and must continue to.
+ "list": {
+ Method: "GET",
+ Path: "/v1/products/{productId}/subscriptions/{subscriptionId}",
+ Description: "List subscriptions for a product",
+ Params: []spec.Param{
+ {Name: "productId", Type: "string", Required: true, Positional: true},
+ {Name: "subscriptionId", Type: "string", Required: true, Positional: true},
+ },
+ },
+ },
+ },
+ },
+ },
+ },
+ }
+
+ outputDir := filepath.Join(t.TempDir(), naming.CLI(apiSpec.Name))
+ require.NoError(t, New(apiSpec, outputDir).Generate())
+
+ read := func(rel string) string {
+ t.Helper()
+ data, err := os.ReadFile(filepath.Join(outputDir, rel))
+ require.NoErrorf(t, err, "read %s", rel)
+ return string(data)
+ }
+
+ // 1) Single positional with a non-positional query before it: args[0],
+ // no len-check (Cobra's MinimumNArgs(1) covers the first positional).
+ tagsList := read(filepath.Join("internal", "cli", "tags_contacts_list-for-tag.go"))
+ assert.Contains(t, tagsList,
+ `path = replacePathParam(path, "tagId", args[0])`,
+ "single positional path param must use args[0] regardless of declared query params before it")
+ assert.NotContains(t, tagsList,
+ `args[2]`,
+ "must not interpolate the full-Params index for the positional")
+ assert.NotContains(t, tagsList,
+ `if len(args) < 3`,
+ "must not emit a stale len-check derived from the full-Params index")
+
+ // 2) Two positional path params with a non-positional query interleaved:
+ // args[0]/args[1] in declaration order. The second positional gets a
+ // len-check at len(args) < 2 (its ordinal+1), not the full-Params index.
+ steps := read(filepath.Join("internal", "cli", "users_course-progress_steps.go"))
+ assert.Contains(t, steps,
+ `path = replacePathParam(path, "id", args[0])`,
+ "first positional must resolve to args[0]")
+ assert.Contains(t, steps,
+ `path = replacePathParam(path, "course", args[1])`,
+ "second positional must resolve to args[1] even with a non-positional param interleaved")
+ assert.Contains(t, steps,
+ `if len(args) < 2`,
+ "len-check for the second positional must use its ordinal+1")
+ assert.NotContains(t, steps,
+ `args[3]`,
+ "must not interpolate the full-Params index (3) for the second positional")
+ assert.NotContains(t, steps,
+ `if len(args) < 4`,
+ "must not emit a stale len-check derived from the full-Params index")
+
+ // 3) Regression baseline: pure positional declaration order matches URL
+ // order, no interleaved queries. Output unchanged from pre-fix behavior.
+ subs := read(filepath.Join("internal", "cli", "products_subscriptions_list.go"))
+ assert.Contains(t, subs,
+ `path = replacePathParam(path, "productId", args[0])`,
+ "baseline: first positional resolves to args[0]")
+ assert.Contains(t, subs,
+ `path = replacePathParam(path, "subscriptionId", args[1])`,
+ "baseline: second positional resolves to args[1]")
+ assert.Contains(t, subs,
+ `if len(args) < 2`,
+ "baseline: len-check for second positional unchanged")
+}
+
+// TestPromotedCommandPathParamArgsIndexUsesPositionalOrdinal covers the
+// promoted-command template (command_promoted.go.tmpl), which has the same
+// args[] indexing shape as the endpoint template but emits a different file
+// and emits the len-check unconditionally (no MinimumNArgs cover for the
+// first positional). The bug shape is identical: a single positional path
+// param with a non-positional query declared before it must resolve to
+// args[0] and len(args) < 1, not the full-Params index.
+func TestPromotedCommandPathParamArgsIndexUsesPositionalOrdinal(t *testing.T) {
+ t.Parallel()
+
+ apiSpec := &spec.APISpec{
+ Name: "promoarg",
+ Version: "0.1.0",
+ BaseURL: "https://api.example.com",
+ Auth: spec.AuthConfig{Type: "none"},
+ Config: spec.ConfigSpec{
+ Format: "toml",
+ Path: "~/.config/promoarg-pp-cli/config.toml",
+ },
+ Resources: map[string]spec.Resource{
+ // Single-endpoint resource — gets promoted to a top-level command.
+ // "limit" precedes "itemId" in Params so the full-Params index for
+ // itemId is 1, but its positional ordinal is 0.
+ "items": {
+ Description: "Items under a parent",
+ Endpoints: map[string]spec.Endpoint{
+ "list": {
+ Method: "GET",
+ Path: "/v1/items/{itemId}/children",
+ Description: "List children of an item",
+ Params: []spec.Param{
+ {Name: "limit", Type: "int"},
+ {Name: "itemId", Type: "string", Required: true, Positional: true},
+ },
+ },
+ },
+ },
+ },
+ }
+
+ 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_*.go"))
+ require.NoError(t, err)
+ require.Lenf(t, matches, 1, "expected exactly one promoted_*.go file, got %v", matches)
+ data, err := os.ReadFile(matches[0])
+ require.NoError(t, err)
+ src := string(data)
+
+ assert.Contains(t, src,
+ `path = replacePathParam(path, "itemId", args[0])`,
+ "promoted command: single positional path param must use args[0] regardless of query params declared before it")
+ assert.Contains(t, src,
+ `if len(args) < 1`,
+ "promoted command: len-check for the sole positional must derive from its ordinal+1, not the full-Params index")
+ assert.NotContains(t, src,
+ `args[1]`,
+ "promoted command: must not interpolate the full-Params index (1) for the positional")
+ assert.NotContains(t, src,
+ `if len(args) < 2`,
+ "promoted command: must not emit a stale len-check derived from the full-Params index")
+}
+
+func TestPositionalIndexHelper(t *testing.T) {
+ t.Parallel()
+
+ e := spec.Endpoint{
+ Params: []spec.Param{
+ {Name: "limit"},
+ {Name: "offset"},
+ {Name: "tagId", Positional: true},
+ },
+ }
+ assert.Equal(t, 0, positionalIndex(e, "tagId"),
+ "sole positional should resolve to ordinal 0 even when queries precede it")
+ assert.Equal(t, -1, positionalIndex(e, "limit"),
+ "non-positional param has no positional ordinal")
+
+ multi := spec.Endpoint{
+ Params: []spec.Param{
+ {Name: "id", Positional: true},
+ {Name: "fmt"},
+ {Name: "course", Positional: true},
+ },
+ }
+ assert.Equal(t, 0, positionalIndex(multi, "id"))
+ assert.Equal(t, 1, positionalIndex(multi, "course"))
+ assert.Equal(t, -1, positionalIndex(multi, "missing"),
+ "unknown name should return -1, not panic")
+}
diff --git a/internal/generator/templates/command_endpoint.go.tmpl b/internal/generator/templates/command_endpoint.go.tmpl
index 75c0b94c..912a7713 100644
--- a/internal/generator/templates/command_endpoint.go.tmpl
+++ b/internal/generator/templates/command_endpoint.go.tmpl
@@ -122,15 +122,16 @@ func new{{camel .FuncPrefix}}{{camel .EndpointName}}Cmd(flags *rootFlags) *cobra
{{- end}}
path := "{{.ResourceBaseURL}}{{.Endpoint.Path}}"{{/* ResourceBaseURL is empty unless the resource or endpoint declares an override; when set it makes path absolute and the client skips the c.BaseURL concat. */}}
-{{- range $i, $p := .Endpoint.Params}}
+{{- range .Endpoint.Params}}
{{- if .Positional}}
-{{- if gt $i 0}}
- if len(args) < {{add $i 1}} {
+{{- $pi := positionalIndex $.Endpoint .Name}}
+{{- if gt $pi 0}}
+ if len(args) < {{add $pi 1}} {
return usageErr(fmt.Errorf("{{.Name}} is required\nUsage: %s <%s>", cmd.CommandPath(), "{{.Name}}"))
}
{{- end}}
{{- if pathContainsParam $.Endpoint.Path .Name}}
- path = replacePathParam(path, "{{.Name}}", args[{{$i}}])
+ path = replacePathParam(path, "{{.Name}}", args[{{$pi}}])
{{- end}}
{{- end}}
{{- end}}
@@ -145,9 +146,9 @@ func new{{camel .FuncPrefix}}{{camel .EndpointName}}Cmd(flags *rootFlags) *cobra
{{- if .Endpoint.UsesHTMLResponse}}
htmlRequestParams := map[string]string{}
-{{- range $i, $p := .Endpoint.Params}}
+{{- range .Endpoint.Params}}
{{- if and .Positional (not (pathContainsParam $.Endpoint.Path .Name))}}
- htmlRequestParams["{{.Name}}"] = args[{{$i}}]
+ htmlRequestParams["{{.Name}}"] = args[{{positionalIndex $.Endpoint .Name}}]
{{- end}}
{{- if and (not .Positional) (not .PathParam)}}
if flag{{camel (paramIdent .)}} != {{zeroValForParam .Name .Type}} {
@@ -209,9 +210,9 @@ func new{{camel .FuncPrefix}}{{camel .EndpointName}}Cmd(flags *rootFlags) *cobra
{{- else if .Endpoint.Pagination}}
{{- if .HasStore}}
data, prov, err := resolvePaginatedRead(cmd.Context(), c, flags, "{{lower .ResourceName}}", path, map[string]string{
-{{- range $i, $p := .Endpoint.Params}}
+{{- range .Endpoint.Params}}
{{- if and .Positional (not (pathContainsParam $.Endpoint.Path .Name))}}
- "{{.Name}}": args[{{$i}}],
+ "{{.Name}}": args[{{positionalIndex $.Endpoint .Name}}],
{{- end}}
{{- if and (not .Positional) (not .PathParam)}}
"{{.Name}}": fmt.Sprintf("%v", flag{{camel (paramIdent .)}}),
@@ -220,9 +221,9 @@ func new{{camel .FuncPrefix}}{{camel .EndpointName}}Cmd(flags *rootFlags) *cobra
}, {{if .Endpoint.HeaderOverrides}}headerOverrides{{else}}nil{{end}}, flagAll, "{{.Endpoint.Pagination.CursorParam}}", "{{.Endpoint.Pagination.NextCursorPath}}", "{{.Endpoint.Pagination.HasMoreField}}")
{{- else}}
data, err := paginatedGet(c, path, map[string]string{
-{{- range $i, $p := .Endpoint.Params}}
+{{- range .Endpoint.Params}}
{{- if and .Positional (not (pathContainsParam $.Endpoint.Path .Name))}}
- "{{.Name}}": args[{{$i}}],
+ "{{.Name}}": args[{{positionalIndex $.Endpoint .Name}}],
{{- end}}
{{- if and (not .Positional) (not .PathParam)}}
"{{.Name}}": fmt.Sprintf("%v", flag{{camel (paramIdent .)}}),
@@ -232,9 +233,9 @@ func new{{camel .FuncPrefix}}{{camel .EndpointName}}Cmd(flags *rootFlags) *cobra
{{- end}}
{{- else}}
params := map[string]string{}
-{{- range $i, $p := .Endpoint.Params}}
+{{- range .Endpoint.Params}}
{{- if and .Positional (not (pathContainsParam $.Endpoint.Path .Name))}}
- params["{{.Name}}"] = args[{{$i}}]
+ params["{{.Name}}"] = args[{{positionalIndex $.Endpoint .Name}}]
{{- end}}
{{- if and (not .Positional) (not .PathParam)}}
if flag{{camel (paramIdent .)}} != {{zeroValForParam .Name .Type}} {
diff --git a/internal/generator/templates/command_promoted.go.tmpl b/internal/generator/templates/command_promoted.go.tmpl
index 620f58f2..aa6e4862 100644
--- a/internal/generator/templates/command_promoted.go.tmpl
+++ b/internal/generator/templates/command_promoted.go.tmpl
@@ -86,9 +86,10 @@ func new{{camel .PromotedName}}PromotedCmd(flags *rootFlags) *cobra.Command {
{{- end}}
path := "{{.ResourceBaseURL}}{{.Endpoint.Path}}"{{/* ResourceBaseURL is empty unless the resource or endpoint declares an override; when set it makes path absolute and the client skips the c.BaseURL concat. */}}
-{{- range $i, $p := .Endpoint.Params}}
+{{- range .Endpoint.Params}}
{{- if .Positional}}
- if len(args) < {{add $i 1}} {
+{{- $pi := positionalIndex $.Endpoint .Name}}
+ if len(args) < {{add $pi 1}} {
// JSON envelope: {error, usage}. Written first; the
// usageErr return preserves exit code 2 across modes.
if flags.asJSON {
@@ -102,7 +103,7 @@ func new{{camel .PromotedName}}PromotedCmd(flags *rootFlags) *cobra.Command {
return usageErr(fmt.Errorf("{{.Name}} is required\nUsage: %s <%s>", cmd.CommandPath(), "{{.Name}}"))
}
{{- if pathContainsParam $.Endpoint.Path .Name}}
- path = replacePathParam(path, "{{.Name}}", args[{{$i}}])
+ path = replacePathParam(path, "{{.Name}}", args[{{$pi}}])
{{- end}}
{{- end}}
{{- end}}
@@ -114,9 +115,9 @@ func new{{camel .PromotedName}}PromotedCmd(flags *rootFlags) *cobra.Command {
{{- if .Endpoint.UsesHTMLResponse}}
htmlRequestParams := map[string]string{}
-{{- range $i, $p := .Endpoint.Params}}
+{{- range .Endpoint.Params}}
{{- if and .Positional (not (pathContainsParam $.Endpoint.Path .Name))}}
- htmlRequestParams["{{.Name}}"] = args[{{$i}}]
+ htmlRequestParams["{{.Name}}"] = args[{{positionalIndex $.Endpoint .Name}}]
{{- end}}
{{- if and (not .Positional) (not .PathParam)}}
if flag{{camel (paramIdent .)}} != {{zeroValForParam .Name .Type}} {
@@ -129,9 +130,9 @@ func new{{camel .PromotedName}}PromotedCmd(flags *rootFlags) *cobra.Command {
{{- if .Endpoint.Pagination}}
{{- if .HasStore}}
data, prov, err := resolvePaginatedRead(cmd.Context(), c, flags, "{{lower .ResourceName}}", path, map[string]string{
-{{- range $i, $p := .Endpoint.Params}}
+{{- range .Endpoint.Params}}
{{- if and .Positional (not (pathContainsParam $.Endpoint.Path .Name))}}
- "{{.Name}}": args[{{$i}}],
+ "{{.Name}}": args[{{positionalIndex $.Endpoint .Name}}],
{{- end}}
{{- if and (not .Positional) (not .PathParam)}}
"{{.Name}}": fmt.Sprintf("%v", flag{{camel (paramIdent .)}}),
@@ -140,9 +141,9 @@ func new{{camel .PromotedName}}PromotedCmd(flags *rootFlags) *cobra.Command {
}, nil, flagAll, "{{.Endpoint.Pagination.CursorParam}}", "{{.Endpoint.Pagination.NextCursorPath}}", "{{.Endpoint.Pagination.HasMoreField}}")
{{- else}}
data, err := paginatedGet(c, path, map[string]string{
-{{- range $i, $p := .Endpoint.Params}}
+{{- range .Endpoint.Params}}
{{- if and .Positional (not (pathContainsParam $.Endpoint.Path .Name))}}
- "{{.Name}}": args[{{$i}}],
+ "{{.Name}}": args[{{positionalIndex $.Endpoint .Name}}],
{{- end}}
{{- if and (not .Positional) (not .PathParam)}}
"{{.Name}}": fmt.Sprintf("%v", flag{{camel (paramIdent .)}}),
@@ -153,9 +154,9 @@ func new{{camel .PromotedName}}PromotedCmd(flags *rootFlags) *cobra.Command {
{{- else}}
{{- if or (eq (upper .Endpoint.Method) "GET") (eq (upper .Endpoint.Method) "HEAD") (eq (upper .Endpoint.Method) "OPTIONS")}}
params := map[string]string{}
-{{- range $i, $p := .Endpoint.Params}}
+{{- range .Endpoint.Params}}
{{- if and .Positional (not (pathContainsParam $.Endpoint.Path .Name))}}
- params["{{.Name}}"] = args[{{$i}}]
+ params["{{.Name}}"] = args[{{positionalIndex $.Endpoint .Name}}]
{{- end}}
{{- if and (not .Positional) (not .PathParam)}}
if flag{{camel (paramIdent .)}} != {{zeroValForParam .Name .Type}} {
← 9df8c5a2 docs(skills): document x-mcp as the OpenAPI form of the mcp:
·
back to Cli Printing Press
·
fix(cli): resolve validation binary path with platform.Execu 7365fcc3 →