[object Object]

← back to Cli Printing Press

fix(cli): promote write-endpoint params to body when body is empty (#791)

ff1353099491ede69491a2694763ec88c856eccc · 2026-05-12 17:43:14 -0700 · Adam Harris

* fix(cli): promote write-endpoint params to body when body is empty

POST/PUT/PATCH endpoints whose payload fields were declared under
`params:` (instead of `body:`) generated CLIs that registered flags
and validated `--required` correctly, but never serialized the values
into the request body. The body-assembly branch in
command_endpoint.go.tmpl iterates only Endpoint.Body, so the API
received `{}` and rejected the call with "missing required field".

Add a post-parse normalization pass that, for POST/PUT/PATCH endpoints
with empty `Body` and at least one non-path/non-positional `Param`,
relocates those Params into `Body`. Path placeholders and positional
args remain in `Params` so URL substitution still works. An explicit
`body:` block on the endpoint is preserved verbatim — author intent
wins. DELETE is excluded since the template has no body-assembly
branch for it.

The discovery surface was slack-pp-cli's `messages post_message`,
which was unable to send any message until the workaround of piping
the body via `--stdin` was used. The same shape affects
`update_message`, `schedule_message`, `delete_message`, and any future
internal-YAML CLI written this way.

Tests added cover POST/PUT/PATCH promotion, path-placeholder
preservation, explicit-body preservation, GET non-promotion, DELETE
non-promotion, and subresource walking. `scripts/golden.sh verify`
passes all 16 cases — existing test specs already use `body:`
correctly, so the promotion is a no-op for them.

Closes #790

* fix(cli): honor explicit body: [] as escape hatch for write-endpoint promotion

Detect whether the source spec declared a `body:` key via a custom
UnmarshalYAML/UnmarshalJSON on Endpoint that records presence on a new
BodySet sentinel. The params->body promotion pass now bails on BodySet,
so an explicit `body: []` opts an endpoint out of promotion (genuinely
bodyless POST with query params) while an absent `body:` still triggers
the fix from the original PR. Mixed `params:` + non-empty `body:`
continues to leave `params:` as query strings — now spelled out in the
function docstring so authors don't expect implicit merging.

Also makes the DELETE subtest assert on param names instead of count,
so a future YAML change does not silently break the assertion's
intended invariant.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Files touched

Diff

commit ff1353099491ede69491a2694763ec88c856eccc
Author: Adam Harris <adam@lastrev.com>
Date:   Tue May 12 17:43:14 2026 -0700

    fix(cli): promote write-endpoint params to body when body is empty (#791)
    
    * fix(cli): promote write-endpoint params to body when body is empty
    
    POST/PUT/PATCH endpoints whose payload fields were declared under
    `params:` (instead of `body:`) generated CLIs that registered flags
    and validated `--required` correctly, but never serialized the values
    into the request body. The body-assembly branch in
    command_endpoint.go.tmpl iterates only Endpoint.Body, so the API
    received `{}` and rejected the call with "missing required field".
    
    Add a post-parse normalization pass that, for POST/PUT/PATCH endpoints
    with empty `Body` and at least one non-path/non-positional `Param`,
    relocates those Params into `Body`. Path placeholders and positional
    args remain in `Params` so URL substitution still works. An explicit
    `body:` block on the endpoint is preserved verbatim — author intent
    wins. DELETE is excluded since the template has no body-assembly
    branch for it.
    
    The discovery surface was slack-pp-cli's `messages post_message`,
    which was unable to send any message until the workaround of piping
    the body via `--stdin` was used. The same shape affects
    `update_message`, `schedule_message`, `delete_message`, and any future
    internal-YAML CLI written this way.
    
    Tests added cover POST/PUT/PATCH promotion, path-placeholder
    preservation, explicit-body preservation, GET non-promotion, DELETE
    non-promotion, and subresource walking. `scripts/golden.sh verify`
    passes all 16 cases — existing test specs already use `body:`
    correctly, so the promotion is a no-op for them.
    
    Closes #790
    
    * fix(cli): honor explicit body: [] as escape hatch for write-endpoint promotion
    
    Detect whether the source spec declared a `body:` key via a custom
    UnmarshalYAML/UnmarshalJSON on Endpoint that records presence on a new
    BodySet sentinel. The params->body promotion pass now bails on BodySet,
    so an explicit `body: []` opts an endpoint out of promotion (genuinely
    bodyless POST with query params) while an absent `body:` still triggers
    the fix from the original PR. Mixed `params:` + non-empty `body:`
    continues to leave `params:` as query strings — now spelled out in the
    function docstring so authors don't expect implicit merging.
    
    Also makes the DELETE subtest assert on param names instead of count,
    so a future YAML change does not silently break the assertion's
    intended invariant.
    
    Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
    
    ---------
    
    Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 internal/spec/spec.go      |  95 ++++++++++++++++
 internal/spec/spec_test.go | 264 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 359 insertions(+)

diff --git a/internal/spec/spec.go b/internal/spec/spec.go
index eca8ae37..7c7b411a 100644
--- a/internal/spec/spec.go
+++ b/internal/spec/spec.go
@@ -1073,6 +1073,12 @@ type Endpoint struct {
 	// docs/SPEC-EXTENSIONS.md for the canonical schema.
 	Walker *WalkerConfig `yaml:"walker,omitempty" json:"walker,omitempty"`
 	Alias  string        `yaml:"-" json:"-"` // computed, not from YAML
+	// BodySet reports whether the source spec declared a `body:` key on this
+	// endpoint, distinct from an absent key. Populated by the custom
+	// UnmarshalYAML / UnmarshalJSON below. The params→body promotion pass
+	// reads this to honor an explicit empty `body: []` as an opt-out signal
+	// for write endpoints that genuinely take query params and no JSON body.
+	BodySet bool `yaml:"-" json:"-"`
 }
 
 // WalkerConfig declares a hierarchical-walk dependency for a child endpoint.
@@ -1095,6 +1101,31 @@ type WalkerConfig struct {
 	KeyParam string `yaml:"key_param,omitempty" json:"key_param,omitempty"`
 }
 
+func (e *Endpoint) UnmarshalYAML(value *yaml.Node) error {
+	type endpointAlias Endpoint
+	var out endpointAlias
+	if err := value.Decode(&out); err != nil {
+		return err
+	}
+	*e = Endpoint(out)
+	e.BodySet = yamlMappingHasKey(value, "body")
+	return nil
+}
+
+func (e *Endpoint) UnmarshalJSON(data []byte) error {
+	type endpointAlias Endpoint
+	var out endpointAlias
+	if err := json.Unmarshal(data, &out); err != nil {
+		return err
+	}
+	*e = Endpoint(out)
+	var raw map[string]json.RawMessage
+	if err := json.Unmarshal(data, &raw); err == nil {
+		_, e.BodySet = raw["body"]
+	}
+	return nil
+}
+
 func (e Endpoint) EffectiveResponseFormat() string {
 	if strings.TrimSpace(e.ResponseFormat) == "" {
 		return ResponseFormatJSON
@@ -1323,6 +1354,7 @@ func ParseBytes(data []byte) (*APISpec, error) {
 	}
 	s.expandOperations()
 	s.enrichPathParams()
+	s.promoteParamsToBodyForWriteEndpoints()
 	if err := s.validateReservedNames(); err != nil {
 		return nil, err
 	}
@@ -1568,6 +1600,69 @@ func enrichEndpointPathParams(e *Endpoint) {
 	}
 }
 
+// promoteParamsToBodyForWriteEndpoints fills Endpoint.Body for POST/PUT/PATCH
+// endpoints whose source spec did not declare a `body:` key by relocating
+// non-path, non-positional Params there. Internal YAML specs commonly list
+// write-endpoint payload fields under `params:` instead of `body:`. Without
+// this promotion, the generator declares flags and required-flag validation
+// for those params, but the body-assembly branch in command_endpoint.go.tmpl
+// iterates only Endpoint.Body — so the values never reach the request body
+// and the API rejects the call with "missing required field".
+//
+// Author intent wins when `body:` is present in the source, even if empty:
+//   - `body: [...]` (non-empty) preserves the explicit block; remaining
+//     `params:` entries are left as query parameters by design.
+//   - `body: []` (explicit empty) is the escape hatch for write endpoints
+//     that genuinely take only query parameters and carry no JSON body.
+//   - Mixed `params:` + non-empty `body:` is allowed but not auto-merged.
+//     The author is asserting that those `params:` entries are URL query
+//     parameters, not body fields. Authors who want them in the body must
+//     move them under `body:` themselves.
+func (s *APISpec) promoteParamsToBodyForWriteEndpoints() {
+	for resourceName, r := range s.Resources {
+		s.promoteResourceParamsToBody(&r)
+		s.Resources[resourceName] = r
+	}
+}
+
+func (s *APISpec) promoteResourceParamsToBody(r *Resource) {
+	if r.Endpoints != nil {
+		for endpointName, e := range r.Endpoints {
+			promoteEndpointParamsToBody(&e)
+			r.Endpoints[endpointName] = e
+		}
+	}
+	for subName, sub := range r.SubResources {
+		s.promoteResourceParamsToBody(&sub)
+		r.SubResources[subName] = sub
+	}
+}
+
+func promoteEndpointParamsToBody(e *Endpoint) {
+	switch strings.ToUpper(e.Method) {
+	case "POST", "PUT", "PATCH":
+	default:
+		return
+	}
+	if e.BodySet || len(e.Body) > 0 || len(e.Params) == 0 {
+		return
+	}
+	keep := make([]Param, 0, len(e.Params))
+	promote := make([]Param, 0, len(e.Params))
+	for _, p := range e.Params {
+		if p.PathParam || p.Positional {
+			keep = append(keep, p)
+			continue
+		}
+		promote = append(promote, p)
+	}
+	if len(promote) == 0 {
+		return
+	}
+	e.Params = keep
+	e.Body = promote
+}
+
 // expandOperations converts operations shorthand (e.g., [list, get, create])
 // into explicit Endpoint entries for each resource that has Operations set.
 // Explicit endpoints take precedence over generated ones.
diff --git a/internal/spec/spec_test.go b/internal/spec/spec_test.go
index 62eae607..7822326b 100644
--- a/internal/spec/spec_test.go
+++ b/internal/spec/spec_test.go
@@ -3872,3 +3872,267 @@ func TestWalkerConfig_YAMLRoundTrip(t *testing.T) {
 		assert.NotContains(t, string(data), "key_param")
 	})
 }
+
+func TestPromoteParamsToBodyForWriteEndpoints(t *testing.T) {
+	t.Parallel()
+
+	const header = `name: testapi
+base_url: https://api.example.com
+auth:
+  type: bearer_token
+  env_vars: [TESTAPI_TOKEN]
+resources:
+`
+
+	t.Run("POST endpoint with params and no body promotes to body", func(t *testing.T) {
+		t.Parallel()
+		input := header + `  messages:
+    description: Slack-style message endpoints
+    endpoints:
+      post_message:
+        method: POST
+        path: /chat.postMessage
+        description: Send a message
+        params:
+          - name: channel
+            type: string
+            required: true
+          - name: text
+            type: string
+            required: true
+          - name: thread_ts
+            type: string
+`
+		s, err := ParseBytes([]byte(input))
+		require.NoError(t, err)
+		ep := s.Resources["messages"].Endpoints["post_message"]
+		assert.Empty(t, ep.Params, "non-path params should have moved to Body")
+		require.Len(t, ep.Body, 3)
+		bodyNames := []string{ep.Body[0].Name, ep.Body[1].Name, ep.Body[2].Name}
+		assert.ElementsMatch(t, []string{"channel", "text", "thread_ts"}, bodyNames)
+	})
+
+	t.Run("POST endpoint preserves path placeholders in Params", func(t *testing.T) {
+		t.Parallel()
+		input := header + `  widgets:
+    description: Widget endpoints
+    endpoints:
+      activate:
+        method: POST
+        path: /widgets/{id}/activate
+        description: Activate a widget
+        params:
+          - name: reason
+            type: string
+            required: true
+`
+		s, err := ParseBytes([]byte(input))
+		require.NoError(t, err)
+		ep := s.Resources["widgets"].Endpoints["activate"]
+		require.Len(t, ep.Params, 1, "id placeholder should remain in Params")
+		assert.Equal(t, "id", ep.Params[0].Name)
+		assert.True(t, ep.Params[0].Positional)
+		require.Len(t, ep.Body, 1)
+		assert.Equal(t, "reason", ep.Body[0].Name)
+	})
+
+	t.Run("POST endpoint with explicit body is left untouched", func(t *testing.T) {
+		t.Parallel()
+		input := header + `  items:
+    description: Item endpoints
+    endpoints:
+      create:
+        method: POST
+        path: /items
+        description: Create item
+        params:
+          - name: org_id
+            type: string
+        body:
+          - name: name
+            type: string
+            required: true
+`
+		s, err := ParseBytes([]byte(input))
+		require.NoError(t, err)
+		ep := s.Resources["items"].Endpoints["create"]
+		require.Len(t, ep.Params, 1)
+		assert.Equal(t, "org_id", ep.Params[0].Name)
+		require.Len(t, ep.Body, 1)
+		assert.Equal(t, "name", ep.Body[0].Name)
+	})
+
+	t.Run("GET endpoint params are not promoted", func(t *testing.T) {
+		t.Parallel()
+		input := header + `  lookup:
+    description: Lookup endpoints
+    endpoints:
+      query:
+        method: GET
+        path: /lookup
+        description: Lookup
+        params:
+          - name: q
+            type: string
+            required: true
+          - name: limit
+            type: integer
+`
+		s, err := ParseBytes([]byte(input))
+		require.NoError(t, err)
+		ep := s.Resources["lookup"].Endpoints["query"]
+		require.Len(t, ep.Params, 2)
+		assert.Empty(t, ep.Body)
+	})
+
+	t.Run("PUT and PATCH are also promoted", func(t *testing.T) {
+		t.Parallel()
+		input := header + `  records:
+    description: Record endpoints
+    endpoints:
+      replace:
+        method: PUT
+        path: /records/{id}
+        description: Replace record
+        params:
+          - name: name
+            type: string
+      modify:
+        method: PATCH
+        path: /records/{id}
+        description: Patch record
+        params:
+          - name: status
+            type: string
+`
+		s, err := ParseBytes([]byte(input))
+		require.NoError(t, err)
+		put := s.Resources["records"].Endpoints["replace"]
+		require.Len(t, put.Body, 1)
+		assert.Equal(t, "name", put.Body[0].Name)
+
+		patch := s.Resources["records"].Endpoints["modify"]
+		require.Len(t, patch.Body, 1)
+		assert.Equal(t, "status", patch.Body[0].Name)
+	})
+
+	t.Run("DELETE is not promoted", func(t *testing.T) {
+		t.Parallel()
+		input := header + `  records:
+    description: Record endpoints
+    endpoints:
+      remove:
+        method: DELETE
+        path: /records/{id}
+        description: Delete record
+        params:
+          - name: cascade
+            type: boolean
+`
+		s, err := ParseBytes([]byte(input))
+		require.NoError(t, err)
+		ep := s.Resources["records"].Endpoints["remove"]
+		names := make([]string, len(ep.Params))
+		for i, p := range ep.Params {
+			names[i] = p.Name
+		}
+		assert.ElementsMatch(t, []string{"id", "cascade"}, names, "DELETE keeps the {id} placeholder enrichPathParams injected and the cascade query param")
+		assert.Empty(t, ep.Body, "DELETE keeps cascade as a query/flag, not body")
+	})
+
+	t.Run("subresource endpoints are walked", func(t *testing.T) {
+		t.Parallel()
+		input := header + `  channels:
+    description: Channel endpoints
+    sub_resources:
+      messages:
+        description: Channel messages
+        endpoints:
+          post:
+            method: POST
+            path: /channels/{channelId}/messages
+            description: Post message
+            params:
+              - name: text
+                type: string
+                required: true
+`
+		s, err := ParseBytes([]byte(input))
+		require.NoError(t, err)
+		ep := s.Resources["channels"].SubResources["messages"].Endpoints["post"]
+		require.Len(t, ep.Body, 1)
+		assert.Equal(t, "text", ep.Body[0].Name)
+	})
+
+	t.Run("explicit empty body: [] opts out of promotion", func(t *testing.T) {
+		t.Parallel()
+		input := header + `  pipelines:
+    description: Pipeline endpoints
+    endpoints:
+      trigger:
+        method: POST
+        path: /pipelines/trigger
+        description: Trigger a pipeline
+        params:
+          - name: dry_run
+            type: boolean
+        body: []
+`
+		s, err := ParseBytes([]byte(input))
+		require.NoError(t, err)
+		ep := s.Resources["pipelines"].Endpoints["trigger"]
+		assert.True(t, ep.BodySet, "explicit `body: []` should set BodySet")
+		assert.Empty(t, ep.Body, "explicit empty body stays empty")
+		require.Len(t, ep.Params, 1, "params stay as query params when author opted out")
+		assert.Equal(t, "dry_run", ep.Params[0].Name)
+	})
+
+	t.Run("mixed params and explicit body leaves params as query strings", func(t *testing.T) {
+		t.Parallel()
+		input := header + `  uploads:
+    description: Upload endpoints
+    endpoints:
+      create:
+        method: POST
+        path: /uploads
+        description: Create upload
+        params:
+          - name: idempotency_key
+            type: string
+        body:
+          - name: filename
+            type: string
+            required: true
+`
+		s, err := ParseBytes([]byte(input))
+		require.NoError(t, err)
+		ep := s.Resources["uploads"].Endpoints["create"]
+		assert.True(t, ep.BodySet)
+		require.Len(t, ep.Params, 1, "idempotency_key stays as query/flag, not silently moved into body")
+		assert.Equal(t, "idempotency_key", ep.Params[0].Name)
+		require.Len(t, ep.Body, 1)
+		assert.Equal(t, "filename", ep.Body[0].Name)
+	})
+
+	t.Run("absent body key leaves BodySet false and triggers promotion", func(t *testing.T) {
+		t.Parallel()
+		input := header + `  notes:
+    description: Note endpoints
+    endpoints:
+      create:
+        method: POST
+        path: /notes
+        description: Create note
+        params:
+          - name: title
+            type: string
+            required: true
+`
+		s, err := ParseBytes([]byte(input))
+		require.NoError(t, err)
+		ep := s.Resources["notes"].Endpoints["create"]
+		assert.False(t, ep.BodySet, "no body key in source -> BodySet false")
+		require.Len(t, ep.Body, 1, "title was promoted to body")
+		assert.Equal(t, "title", ep.Body[0].Name)
+	})
+}

← 90798403 chore(main): release 4.5.2 (#1230)  ·  back to Cli Printing Press  ·  chore(ci): enable greptile status checks (#1268) a8192b52 →