[object Object]

← back to Cli Printing Press

fix(cli): four generator improvements from Redfin retro (#89)

6c3c8db8c28f9b595379346e351a2726a1ebcf62 · 2026-03-30 21:19:39 -0700 · Matt Van Horn

1. Env var naming: add envName template func that replaces hyphens with
   underscores before uppercasing. Fixes REDFIN-STINGRAY_CONFIG ->
   REDFIN_STINGRAY_CONFIG.

2. Promoted command collision: skip promoted commands when their name
   collides with a resource group command. Fixes duplicate stingray
   entries.

3. Verify arg inference: parse --help Use line to discover positional
   arg requirements, supply synthetic values. Fixes 52% verify pass
   rate on CLIs with custom commands requiring args.

4. JSONP/XSSI sanitization: strip common response prefixes ({}&&,
   )]}', BOM, etc.) before JSON parsing. Fixes Redfin's Stingray API
   responses.

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

Files touched

Diff

commit 6c3c8db8c28f9b595379346e351a2726a1ebcf62
Author: Matt Van Horn <mvanhorn@users.noreply.github.com>
Date:   Mon Mar 30 21:19:39 2026 -0700

    fix(cli): four generator improvements from Redfin retro (#89)
    
    1. Env var naming: add envName template func that replaces hyphens with
       underscores before uppercasing. Fixes REDFIN-STINGRAY_CONFIG ->
       REDFIN_STINGRAY_CONFIG.
    
    2. Promoted command collision: skip promoted commands when their name
       collides with a resource group command. Fixes duplicate stingray
       entries.
    
    3. Verify arg inference: parse --help Use line to discover positional
       arg requirements, supply synthetic values. Fixes 52% verify pass
       rate on CLIs with custom commands requiring args.
    
    4. JSONP/XSSI sanitization: strip common response prefixes ({}&&,
       )]}', BOM, etc.) before JSON parsing. Fixes Redfin's Stingray API
       responses.
    
    Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 internal/generator/generator.go             | 10 ++++
 internal/generator/generator_test.go        | 54 +++++++-------------
 internal/generator/templates/client.go.tmpl | 27 ++++++++++
 internal/generator/templates/config.go.tmpl |  4 +-
 internal/pipeline/runtime.go                | 78 +++++++++++++++++++++++++++--
 5 files changed, 132 insertions(+), 41 deletions(-)

diff --git a/internal/generator/generator.go b/internal/generator/generator.go
index 79bc212b..91e56660 100644
--- a/internal/generator/generator.go
+++ b/internal/generator/generator.go
@@ -96,6 +96,7 @@ func New(s *spec.APISpec, outputDir string) *Generator {
 		"currentYear":        func() string { return strconv.Itoa(time.Now().Year()) },
 		"modulePath":         func() string { return naming.CLI(s.Name) },
 		"kebab":              toKebab,
+		"envName":            func(s string) string { return strings.ToUpper(strings.ReplaceAll(s, "-", "_")) },
 	}
 	return g
 }
@@ -965,6 +966,12 @@ func buildPromotedCommands(s *spec.APISpec) []PromotedCommand {
 	var promoted []PromotedCommand
 	usedNames := make(map[string]bool)
 
+	// Collect resource group names so promoted commands don't collide with them
+	resourceGroupNames := make(map[string]bool)
+	for name := range s.Resources {
+		resourceGroupNames[toKebab(name)] = true
+	}
+
 	for name, resource := range s.Resources {
 		// Find the primary GET endpoint: prefer GET without positional params, else first GET
 		var bestName string
@@ -1000,6 +1007,9 @@ func buildPromotedCommands(s *spec.APISpec) []PromotedCommand {
 		if builtinCommands[promotedName] {
 			continue
 		}
+		if resourceGroupNames[promotedName] {
+			continue
+		}
 		if usedNames[promotedName] {
 			continue
 		}
diff --git a/internal/generator/generator_test.go b/internal/generator/generator_test.go
index 8015cf00..fb8a0dd6 100644
--- a/internal/generator/generator_test.go
+++ b/internal/generator/generator_test.go
@@ -22,9 +22,9 @@ func TestGenerateProjectsCompile(t *testing.T) {
 		specPath      string
 		expectedFiles int
 	}{
-		{name: "stytch", specPath: filepath.Join("..", "..", "testdata", "stytch.yaml"), expectedFiles: 32},
-		{name: "clerk", specPath: filepath.Join("..", "..", "testdata", "clerk.yaml"), expectedFiles: 38},
-		{name: "loops", specPath: filepath.Join("..", "..", "testdata", "loops.yaml"), expectedFiles: 34},
+		{name: "stytch", specPath: filepath.Join("..", "..", "testdata", "stytch.yaml"), expectedFiles: 30},
+		{name: "clerk", specPath: filepath.Join("..", "..", "testdata", "clerk.yaml"), expectedFiles: 35},
+		{name: "loops", specPath: filepath.Join("..", "..", "testdata", "loops.yaml"), expectedFiles: 33},
 	}
 
 	for _, tt := range tests {
@@ -609,7 +609,7 @@ func TestToKebab(t *testing.T) {
 func TestBuildPromotedCommands(t *testing.T) {
 	t.Parallel()
 
-	t.Run("resource with list endpoint gets promoted", func(t *testing.T) {
+	t.Run("resource with list endpoint is NOT promoted (collides with resource group)", func(t *testing.T) {
 		t.Parallel()
 		s := &spec.APISpec{
 			Name:    "test",
@@ -624,13 +624,10 @@ func TestBuildPromotedCommands(t *testing.T) {
 			},
 		}
 		promoted := buildPromotedCommands(s)
-		require.Len(t, promoted, 1)
-		assert.Equal(t, "users", promoted[0].PromotedName)
-		assert.Equal(t, "users", promoted[0].ResourceName)
-		assert.Equal(t, "list", promoted[0].EndpointName)
+		assert.Empty(t, promoted, "promoted command should not be emitted when it collides with resource group name")
 	})
 
-	t.Run("ISteamUser resource becomes steam-user", func(t *testing.T) {
+	t.Run("ISteamUser resource is NOT promoted (collides with resource group)", func(t *testing.T) {
 		t.Parallel()
 		s := &spec.APISpec{
 			Name:    "test",
@@ -645,9 +642,7 @@ func TestBuildPromotedCommands(t *testing.T) {
 			},
 		}
 		promoted := buildPromotedCommands(s)
-		require.Len(t, promoted, 1)
-		assert.Equal(t, "steam-user", promoted[0].PromotedName)
-		assert.Equal(t, "ISteamUser", promoted[0].ResourceName)
+		assert.Empty(t, promoted, "promoted command should not be emitted when it collides with resource group name")
 	})
 
 	t.Run("resource named version is skipped (collides with built-in)", func(t *testing.T) {
@@ -687,7 +682,7 @@ func TestBuildPromotedCommands(t *testing.T) {
 		assert.Empty(t, promoted)
 	})
 
-	t.Run("prefers GET without positional params", func(t *testing.T) {
+	t.Run("prefers GET without positional params (collides with resource group)", func(t *testing.T) {
 		t.Parallel()
 		s := &spec.APISpec{
 			Name:    "test",
@@ -704,9 +699,7 @@ func TestBuildPromotedCommands(t *testing.T) {
 			},
 		}
 		promoted := buildPromotedCommands(s)
-		require.Len(t, promoted, 1)
-		assert.Equal(t, "list", promoted[0].EndpointName)
-		assert.Equal(t, "/items", promoted[0].Endpoint.Path)
+		assert.Empty(t, promoted, "promoted command collides with resource group name")
 	})
 
 	t.Run("all built-in names are skipped", func(t *testing.T) {
@@ -753,22 +746,13 @@ func TestGeneratedOutput_PromotedCommandExists(t *testing.T) {
 	gen := New(apiSpec, outputDir)
 	require.NoError(t, gen.Generate())
 
-	// Promoted command file should exist
+	// Promoted command file should NOT exist because "users" collides with the
+	// resource group command of the same name
 	promotedFile := filepath.Join(outputDir, "internal", "cli", "promoted_users.go")
-	assert.FileExists(t, promotedFile)
+	assert.NoFileExists(t, promotedFile)
 
-	// Read it and verify key content
-	content, err := os.ReadFile(promotedFile)
-	require.NoError(t, err)
-	contentStr := string(content)
-	assert.Contains(t, contentStr, "newUsersPromotedCmd")
-	assert.Contains(t, contentStr, `Use:   "users"`)
-	assert.Contains(t, contentStr, `/users`)
-
-	// Root.go should register the promoted command
-	rootContent, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "root.go"))
-	require.NoError(t, err)
-	assert.Contains(t, string(rootContent), "newUsersPromotedCmd")
+	// The resource group command should still exist
+	assert.FileExists(t, filepath.Join(outputDir, "internal", "cli", "users.go"))
 }
 
 func TestGeneratedOutput_PromotedCommandCompiles(t *testing.T) {
@@ -802,9 +786,9 @@ func TestGeneratedOutput_PromotedCommandCompiles(t *testing.T) {
 	gen := New(apiSpec, outputDir)
 	require.NoError(t, gen.Generate())
 
-	// Both promoted files should exist
-	assert.FileExists(t, filepath.Join(outputDir, "internal", "cli", "promoted_steam-user.go"))
-	assert.FileExists(t, filepath.Join(outputDir, "internal", "cli", "promoted_items.go"))
+	// Promoted files should NOT exist because they collide with resource group names
+	assert.NoFileExists(t, filepath.Join(outputDir, "internal", "cli", "promoted_i-steam-user.go"))
+	assert.NoFileExists(t, filepath.Join(outputDir, "internal", "cli", "promoted_items.go"))
 
 	// Must compile
 	runGoCommand(t, outputDir, "mod", "tidy")
@@ -842,8 +826,8 @@ func TestGeneratedOutput_PromotedCommandNotForBuiltins(t *testing.T) {
 
 	// "version" should NOT have a promoted command (collides with built-in)
 	assert.NoFileExists(t, filepath.Join(outputDir, "internal", "cli", "promoted_version.go"))
-	// "users" should have a promoted command
-	assert.FileExists(t, filepath.Join(outputDir, "internal", "cli", "promoted_users.go"))
+	// "users" should NOT have a promoted command (collides with resource group)
+	assert.NoFileExists(t, filepath.Join(outputDir, "internal", "cli", "promoted_users.go"))
 }
 
 func TestGeneratedHelpers_DeadCodeRemoved(t *testing.T) {
diff --git a/internal/generator/templates/client.go.tmpl b/internal/generator/templates/client.go.tmpl
index c0f13c5f..e68f836c 100644
--- a/internal/generator/templates/client.go.tmpl
+++ b/internal/generator/templates/client.go.tmpl
@@ -4,6 +4,7 @@
 package client
 
 import (
+	"bytes"
 	"crypto/sha256"
 	"encoding/hex"
 	"encoding/json"
@@ -355,6 +356,7 @@ func (c *Client) do(method, path string, params map[string]string, body any) (js
 		if err != nil {
 			return nil, 0, fmt.Errorf("reading response: %w", err)
 		}
+		respBody = sanitizeJSONResponse(respBody)
 
 		// Success
 		if resp.StatusCode < 400 {
@@ -535,6 +537,31 @@ func retryAfter(resp *http.Response) time.Duration {
 	return 5 * time.Second
 }
 
+// sanitizeJSONResponse strips known JSONP/XSSI prefixes and UTF-8 BOM from
+// response bodies so that downstream JSON parsing succeeds. For clean JSON
+// responses these checks are no-ops.
+func sanitizeJSONResponse(body []byte) []byte {
+	// UTF-8 BOM
+	body = bytes.TrimPrefix(body, []byte("\xEF\xBB\xBF"))
+
+	// JSONP/XSSI prefixes, ordered longest-first where prefixes overlap
+	prefixes := [][]byte{
+		[]byte(")]}'\n"),
+		[]byte(")]}'"),
+		[]byte("{}&&"),
+		[]byte("for(;;);"),
+		[]byte("while(1);"),
+	}
+	for _, p := range prefixes {
+		if bytes.HasPrefix(body, p) {
+			body = bytes.TrimPrefix(body, p)
+			body = bytes.TrimLeft(body, " \t\r\n")
+			break
+		}
+	}
+	return body
+}
+
 func truncateBody(b []byte) string {
 	s := string(b)
 	if len(s) > 200 {
diff --git a/internal/generator/templates/config.go.tmpl b/internal/generator/templates/config.go.tmpl
index 80150c12..fdbf3dd3 100644
--- a/internal/generator/templates/config.go.tmpl
+++ b/internal/generator/templates/config.go.tmpl
@@ -40,7 +40,7 @@ func Load(configPath string) (*Config, error) {
 	// Resolve config path
 	path := configPath
 	if path == "" {
-		path = os.Getenv("{{upper .Name}}_CONFIG")
+		path = os.Getenv("{{envName .Name}}_CONFIG")
 	}
 	if path == "" {
 		home, _ := os.UserHomeDir()
@@ -67,7 +67,7 @@ func Load(configPath string) (*Config, error) {
 {{- end}}
 
 	// Base URL override (used by printing-press verify to point at mock/test servers)
-	if v := os.Getenv("{{upper .Name}}_BASE_URL"); v != "" {
+	if v := os.Getenv("{{envName .Name}}_BASE_URL"); v != "" {
 		cfg.BaseURL = v
 	}
 
diff --git a/internal/pipeline/runtime.go b/internal/pipeline/runtime.go
index 7c9aba3a..9fc33812 100644
--- a/internal/pipeline/runtime.go
+++ b/internal/pipeline/runtime.go
@@ -109,6 +109,11 @@ func RunVerify(cfg VerifyConfig) (*VerifyReport, error) {
 	// 5. Discover commands
 	commands := discoverCommands(cfg.Dir)
 
+	// 5.5. Infer positional args from --help output
+	for i := range commands {
+		inferPositionalArgs(binaryPath, &commands[i])
+	}
+
 	// 6. Classify and run each command
 	for i := range commands {
 		classifyCommandKind(&commands[i], spec)
@@ -278,6 +283,64 @@ type discoveredCommand struct {
 	Args []string
 }
 
+// inferPositionalArgs runs `<binary> <cmd> --help`, parses the Usage line for
+// positional arg placeholders like <region> or [price], and maps them to
+// synthetic values. On any failure, it falls back to no extra args.
+func inferPositionalArgs(binary string, cmd *discoveredCommand) {
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+
+	helpCmd := exec.CommandContext(ctx, binary, cmd.Name, "--help")
+	out, err := helpCmd.CombinedOutput()
+	if err != nil {
+		return // fall back to no extra args
+	}
+
+	// Find the Usage line, e.g. "Usage:\n  cli-name pulse <region> [flags]"
+	usageRe := regexp.MustCompile(`(?m)^Usage:\s*\n\s+\S+\s+\S+(.*)$`)
+	m := usageRe.FindSubmatch(out)
+	if m == nil {
+		return
+	}
+	rest := string(m[1])
+
+	// Extract <arg> and [arg] placeholders (but not [flags] or [command])
+	placeholderRe := regexp.MustCompile(`[<\[]([a-zA-Z][\w-]*)[>\]]`)
+	matches := placeholderRe.FindAllStringSubmatch(rest, -1)
+	if len(matches) == 0 {
+		return
+	}
+
+	for _, match := range matches {
+		name := strings.ToLower(match[1])
+		// Skip cobra built-in placeholders
+		if name == "flags" || name == "command" {
+			continue
+		}
+		cmd.Args = append(cmd.Args, syntheticArgValue(name))
+	}
+}
+
+// syntheticArgValue maps a positional arg placeholder name to a synthetic test value.
+func syntheticArgValue(name string) string {
+	switch name {
+	case "region", "location", "city":
+		return "mock-city"
+	case "id", "property-id", "listing-id":
+		return "12345"
+	case "price", "amount":
+		return "500000"
+	case "zip", "zipcode":
+		return "94102"
+	case "url", "path":
+		return "/mock/path"
+	case "query", "search", "name":
+		return "mock-query"
+	default:
+		return "mock-value"
+	}
+}
+
 // classifyCommandKind determines if a command is read, write, local, or data-layer.
 func classifyCommandKind(cmd *discoveredCommand, spec *openAPISpec) {
 	name := cmd.Name
@@ -329,10 +392,18 @@ func runCommandTests(binary string, cmd discoveredCommand, mode string, env []st
 	// Get any required flags/args for this command
 	extraFlags := workflowTestFlags(cmd.Name)
 
+	// Build positional args + flags for test invocations
+	buildTestArgs := func(cmdName string, positionalArgs, flags []string, extra ...string) []string {
+		args := []string{cmdName}
+		args = append(args, positionalArgs...)
+		args = append(args, flags...)
+		args = append(args, extra...)
+		return args
+	}
+
 	// Test 2: --dry-run (skip for local/data-layer commands that don't make API calls)
 	if cmd.Kind != "local" && cmd.Kind != "data-layer" {
-		args := append([]string{cmd.Name}, extraFlags...)
-		args = append(args, "--dry-run")
+		args := buildTestArgs(cmd.Name, cmd.Args, extraFlags, "--dry-run")
 		err := runCLI(binary, args, env, 10*time.Second)
 		result.DryRun = err == nil
 	} else {
@@ -345,8 +416,7 @@ func runCommandTests(binary string, cmd discoveredCommand, mode string, env []st
 	} else if mode == "live" && cmd.Kind == "write" {
 		result.Execute = true // skip writes on live = pass (tested via dry-run)
 	} else {
-		args := append([]string{cmd.Name}, extraFlags...)
-		args = append(args, "--json")
+		args := buildTestArgs(cmd.Name, cmd.Args, extraFlags, "--json")
 		err := runCLI(binary, args, env, 15*time.Second)
 		result.Execute = err == nil
 	}

← 7f02e107 fix(cli): filter crowd-sniff auth env var hints by API name  ·  back to Cli Printing Press  ·  feat(skills): add /printing-press-polish standalone skill (# 03487972 →