[object Object]

← back to Cli Printing Press

fix(cli): always emit boolean body fields, no zero-guard (#1494)

9d3050b5a3a356eaceb0b9cb157bcf9dd69aa1e6 · 2026-05-15 23:31:01 -0700 · Trevin Chow

* fix(cli): always emit boolean body fields, no zero-guard

Booleans rendered by renderBodyMap previously sat behind an `if body* != false`
guard that omitted the field whenever the flag was unset. Many APIs server-default
boolean body fields to true (Freshservice notes private, Stripe automatic_*
enabled, etc.), so the omission path silently inverts the user's intent: a flag
defaulting to public posts as private.

Boolean params now always emit, mirroring the established `p.Type == "boolean"
|| p.Type == "bool"` pattern elsewhere in the generator (internal YAML specs use
"boolean", the OpenAPI parser normalizes to "bool"). String and integer scalars
keep their zero-guard, since their zero values ("" and 0) more commonly map to
"unset" on real APIs.

Closes #1298

* fix(cli): gate boolean body emission on cmd.Flags().Changed, not zero-guard

Greptile review on #1494 surfaced that "always emit booleans" trades the
user-false-drop bug for a worse one on PATCH endpoints: every untouched
boolean flag would silently send field=false on the wire and overwrite
server state. A user running `update-project --title "X"` would
unintentionally revert any previously-completed task to incomplete.

Switching to `cmd.Flags().Changed(flag)` distinguishes "user explicitly
set false" from "user did not touch the flag" and is correct for POST,
PUT, and PATCH. The Changed gate also threads cleanly through the
nested-object recursion: an untouched boolean leaf adds nothing to
the inner map, so `len(nestedMap) > 0` stays false and the parent
object key is not emitted.

Tests cover both type-form spellings (`boolean` from internal YAML
specs, `bool` from the OpenAPI parser) and the nested-object case.

Refs #1298

---------

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

Files touched

Diff

commit 9d3050b5a3a356eaceb0b9cb157bcf9dd69aa1e6
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Fri May 15 23:31:01 2026 -0700

    fix(cli): always emit boolean body fields, no zero-guard (#1494)
    
    * fix(cli): always emit boolean body fields, no zero-guard
    
    Booleans rendered by renderBodyMap previously sat behind an `if body* != false`
    guard that omitted the field whenever the flag was unset. Many APIs server-default
    boolean body fields to true (Freshservice notes private, Stripe automatic_*
    enabled, etc.), so the omission path silently inverts the user's intent: a flag
    defaulting to public posts as private.
    
    Boolean params now always emit, mirroring the established `p.Type == "boolean"
    || p.Type == "bool"` pattern elsewhere in the generator (internal YAML specs use
    "boolean", the OpenAPI parser normalizes to "bool"). String and integer scalars
    keep their zero-guard, since their zero values ("" and 0) more commonly map to
    "unset" on real APIs.
    
    Closes #1298
    
    * fix(cli): gate boolean body emission on cmd.Flags().Changed, not zero-guard
    
    Greptile review on #1494 surfaced that "always emit booleans" trades the
    user-false-drop bug for a worse one on PATCH endpoints: every untouched
    boolean flag would silently send field=false on the wire and overwrite
    server state. A user running `update-project --title "X"` would
    unintentionally revert any previously-completed task to incomplete.
    
    Switching to `cmd.Flags().Changed(flag)` distinguishes "user explicitly
    set false" from "user did not touch the flag" and is correct for POST,
    PUT, and PATCH. The Changed gate also threads cleanly through the
    nested-object recursion: an untouched boolean leaf adds nothing to
    the inner map, so `len(nestedMap) > 0` stays false and the parent
    object key is not emitted.
    
    Tests cover both type-form spellings (`boolean` from internal YAML
    specs, `bool` from the OpenAPI parser) and the nested-object case.
    
    Refs #1298
    
    ---------
    
    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
---
 internal/generator/body_map_test.go                | 51 ++++++++++++++++++++++
 internal/generator/generator.go                    | 15 +++++++
 .../internal/cli/projects_tasks_update-project.go  |  2 +-
 3 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/internal/generator/body_map_test.go b/internal/generator/body_map_test.go
index 97ff6507..68ff8da1 100644
--- a/internal/generator/body_map_test.go
+++ b/internal/generator/body_map_test.go
@@ -36,6 +36,28 @@ func TestBodyMap(t *testing.T) {
 				"\t\t\t\tbody[\"count\"] = bodyCount\n" +
 				"\t\t\t}\n",
 		},
+		{
+			// Booleans gate on cmd.Flags().Changed instead of the
+			// scalar zero-guard so user-set false reaches the wire
+			// AND untouched flags don't overwrite server state on
+			// PATCH endpoints. See issue #1298.
+			name:   "scalar boolean (internal spec form) gates on Changed, not zero-guard",
+			body:   []spec.Param{{Name: "private", Type: "boolean"}},
+			indent: "\t\t\t",
+			want: "\t\t\tif cmd.Flags().Changed(\"private\") {\n" +
+				"\t\t\t\tbody[\"private\"] = bodyPrivate\n" +
+				"\t\t\t}\n",
+		},
+		{
+			// The OpenAPI parser normalizes "boolean" -> "bool", so the
+			// renderer must match both forms or fail open on OpenAPI specs.
+			name:   "scalar bool (OpenAPI-normalized form) gates on Changed",
+			body:   []spec.Param{{Name: "enabled", Type: "bool"}},
+			indent: "\t\t\t",
+			want: "\t\t\tif cmd.Flags().Changed(\"enabled\") {\n" +
+				"\t\t\t\tbody[\"enabled\"] = bodyEnabled\n" +
+				"\t\t\t}\n",
+		},
 		{
 			name:   "object branch parses JSON and stores parsed value",
 			body:   []spec.Param{{Name: "metadata", Type: "object"}},
@@ -177,6 +199,35 @@ func TestBodyMap_NestedObject(t *testing.T) {
 	}
 }
 
+// TestBodyMap_NestedObject_BooleanLeaf verifies the boolean Changed
+// gate threads through the nested-object recursion: an untouched
+// boolean leaf adds nothing to the inner map, so len(nestedMap) > 0
+// stays false and the parent object key is not emitted. Without this,
+// every PATCH whose body declares a nested object with a boolean leaf
+// would silently send the parent with field=false on every call.
+func TestBodyMap_NestedObject_BooleanLeaf(t *testing.T) {
+	t.Parallel()
+	got := bodyMap([]spec.Param{{
+		Name: "settings",
+		Type: "object",
+		Fields: []spec.Param{
+			{Name: "private", Type: "boolean"},
+		},
+	}}, "\t")
+	want := "\t{\n" +
+		"\t\tnestedSettings := map[string]any{}\n" +
+		"\t\tif cmd.Flags().Changed(\"settings-private\") {\n" +
+		"\t\t\tnestedSettings[\"private\"] = bodySettingsPrivate\n" +
+		"\t\t}\n" +
+		"\t\tif len(nestedSettings) > 0 {\n" +
+		"\t\t\tbody[\"settings\"] = nestedSettings\n" +
+		"\t\t}\n" +
+		"\t}\n"
+	if got != want {
+		t.Errorf("bodyMap nested boolean leaf mismatch.\n got:\n%s\nwant:\n%s", got, want)
+	}
+}
+
 // TestBodyMap_NestedObject_PreservesScalarSiblings verifies that
 // nested and flat body params can coexist: nested produces a block,
 // scalars keep their existing if-then-set form.
diff --git a/internal/generator/generator.go b/internal/generator/generator.go
index ea9a3c79..396aa39a 100644
--- a/internal/generator/generator.go
+++ b/internal/generator/generator.go
@@ -3473,6 +3473,21 @@ func renderBodyMap(b *strings.Builder, body []spec.Param, indent, mapVar, identP
 			fmt.Fprintf(b, "%s}\n", indent)
 			continue
 		}
+		if p.Type == "boolean" || p.Type == "bool" {
+			// Booleans gate on cmd.Flags().Changed instead of a zero-guard.
+			// The zero-guard (body != false) drops user-set false values,
+			// letting the server's default (often true) silently invert
+			// intent. Unconditionally emitting flips the bug: PATCH bodies
+			// would carry "field: false" for every untouched flag and
+			// overwrite server state. Changed distinguishes "user set
+			// false" from "user did not touch the flag" and is correct
+			// for POST, PUT, and PATCH. Internal YAML specs use "boolean";
+			// the OpenAPI parser normalizes to "bool".
+			fmt.Fprintf(b, "%sif cmd.Flags().Changed(%q) {\n", indent, flag)
+			fmt.Fprintf(b, "%s\t%s[%q] = body%s\n", indent, mapVar, p.Name, ident)
+			fmt.Fprintf(b, "%s}\n", indent)
+			continue
+		}
 		fmt.Fprintf(b, "%sif body%s != %s {\n", indent, ident, zeroVal(p.Type))
 		fmt.Fprintf(b, "%s\t%s[%q] = body%s\n", indent, mapVar, p.Name, ident)
 		fmt.Fprintf(b, "%s}\n", indent)
diff --git a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/projects_tasks_update-project.go b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/projects_tasks_update-project.go
index 0412e27f..0d5eeb1f 100644
--- a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/projects_tasks_update-project.go
+++ b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/projects_tasks_update-project.go
@@ -59,7 +59,7 @@ func newProjectsTasksUpdateProjectCmd(flags *rootFlags) *cobra.Command {
 				body = jsonBody
 			} else {
 				body = map[string]any{}
-				if bodyCompleted != false {
+				if cmd.Flags().Changed("completed") {
 					body["completed"] = bodyCompleted
 				}
 				if bodyPriority != "" {

← 96fca081 feat(cli): expose PRINTING_PRESS_DOGFOOD env signal for long  ·  back to Cli Printing Press  ·  fix(cli): exempt vendor OpenAPI/Swagger source under .manusc dc0651ab →