← back to Cli Printing Press
fix(generator): route receiver JSON helper through filters (#933)
e952e80794ac828dd2a35ff0db7ce91b834cb56c · 2026-05-10 17:33:13 -0700 · hnshah
* fix(generator): route receiver JSON helper through filters
* fix: remove unused printjson check helper
Files touched
M internal/generator/print_json_filtered_test.goM internal/generator/templates/root.go.tmplM internal/pipeline/printjson_filtered_check.goM internal/pipeline/printjson_filtered_check_test.goM skills/printing-press/SKILL.mdM testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/root.go
Diff
commit e952e80794ac828dd2a35ff0db7ce91b834cb56c
Author: hnshah <hnshah@gmail.com>
Date: Sun May 10 17:33:13 2026 -0700
fix(generator): route receiver JSON helper through filters (#933)
* fix(generator): route receiver JSON helper through filters
* fix: remove unused printjson check helper
---
internal/generator/print_json_filtered_test.go | 45 +++++++
internal/generator/templates/root.go.tmpl | 5 +-
internal/pipeline/printjson_filtered_check.go | 53 +-------
internal/pipeline/printjson_filtered_check_test.go | 134 ++-------------------
skills/printing-press/SKILL.md | 2 +-
.../printing-press-golden/internal/cli/root.go | 5 +-
6 files changed, 60 insertions(+), 184 deletions(-)
diff --git a/internal/generator/print_json_filtered_test.go b/internal/generator/print_json_filtered_test.go
index 7c223faa..765783f2 100644
--- a/internal/generator/print_json_filtered_test.go
+++ b/internal/generator/print_json_filtered_test.go
@@ -31,3 +31,48 @@ func TestPrintJSONFiltered_EmittedIntoHelpers(t *testing.T) {
require.Contains(t, src, "printOutputWithFlags(w, json.RawMessage(raw), flags)",
"printJSONFiltered must delegate to printOutputWithFlags so flag handling stays unified")
}
+
+// TestRootFlagsPrintJSONHonorsOutputFlags guards the receiver-style helper
+// agents naturally reach for when hand-authoring novel commands. It must route
+// through the same filtered output pipeline as generated endpoint commands so
+// --select, --compact, --csv, and --quiet cannot be silently bypassed.
+func TestRootFlagsPrintJSONHonorsOutputFlags(t *testing.T) {
+ t.Parallel()
+
+ apiSpec := minimalSpec("receiver-print")
+ outputDir := filepath.Join(t.TempDir(), "receiver-print-pp-cli")
+ require.NoError(t, New(apiSpec, outputDir).Generate())
+
+ testPath := filepath.Join(outputDir, "internal", "cli", "print_json_receiver_test.go")
+ require.NoError(t, os.WriteFile(testPath, []byte(`package cli
+
+import (
+ "bytes"
+ "strings"
+ "testing"
+
+ "github.com/spf13/cobra"
+)
+
+func TestRootFlagsPrintJSONHonorsSelect(t *testing.T) {
+ flags := &rootFlags{asJSON: true, selectFields: "id"}
+ cmd := &cobra.Command{}
+ var out bytes.Buffer
+ cmd.SetOut(&out)
+
+ if err := flags.printJSON(cmd, []map[string]any{{"id": "one", "name": "hidden"}}); err != nil {
+ t.Fatalf("printJSON returned error: %v", err)
+ }
+
+ got := out.String()
+ if !strings.Contains(got, "\"id\"") {
+ t.Fatalf("expected selected field in output, got %s", got)
+ }
+ if strings.Contains(got, "hidden") || strings.Contains(got, "\"name\"") {
+ t.Fatalf("printJSON bypassed --select filtering, got %s", got)
+ }
+}
+`), 0o644))
+
+ runGoCommand(t, outputDir, "test", "./internal/cli", "-run", "TestRootFlagsPrintJSONHonorsSelect", "-count=1")
+}
diff --git a/internal/generator/templates/root.go.tmpl b/internal/generator/templates/root.go.tmpl
index 73f759a2..927850dc 100644
--- a/internal/generator/templates/root.go.tmpl
+++ b/internal/generator/templates/root.go.tmpl
@@ -5,7 +5,6 @@ package cli
import (
"bytes"
- "encoding/json"
"fmt"
"io"
"os"
@@ -336,9 +335,7 @@ func (f *rootFlags) newClient() (*client.Client, error) {
}
func (f *rootFlags) printJSON(w *cobra.Command, v any) error {
- enc := json.NewEncoder(w.OutOrStdout())
- enc.SetIndent("", " ")
- return enc.Encode(v)
+ return printJSONFiltered(w.OutOrStdout(), v, f)
}
func (f *rootFlags) printTable(w *cobra.Command, headers []string, rows [][]string) error {
diff --git a/internal/pipeline/printjson_filtered_check.go b/internal/pipeline/printjson_filtered_check.go
index 6de58490..df50d879 100644
--- a/internal/pipeline/printjson_filtered_check.go
+++ b/internal/pipeline/printjson_filtered_check.go
@@ -3,14 +3,12 @@ package pipeline
import (
"os"
"path/filepath"
- "strings"
)
-// PrintJSONFilteredCheckResult flags hand-written novel commands that
-// emit JSON via flags.printJSON(cmd, v) instead of
-// printJSONFiltered(cmd.OutOrStdout(), v, flags). The former silently
-// drops --select, --compact, --csv, and --quiet — agents requesting
-// narrower output still receive the full payload.
+// PrintJSONFilteredCheckResult is retained for dogfood report compatibility.
+// Before #826 it flagged hand-written novel commands that called
+// flags.printJSON(cmd, v). That receiver-style helper now delegates through
+// printJSONFiltered, so the check only records how many CLI files were scanned.
type PrintJSONFilteredCheckResult struct {
Checked int `json:"checked"`
Findings []PrintJSONFilteredFinding `json:"findings,omitempty"`
@@ -24,16 +22,6 @@ type PrintJSONFilteredFinding struct {
Snippet string `json:"snippet"`
}
-// printJSONFilteredAntipattern is the literal call shape we flag. gofmt
-// normalizes spacing inside Go call expressions, so a substring match
-// is sufficient — no need for whitespace-flexible regex.
-const printJSONFilteredAntipattern = "flags.printJSON(cmd,"
-
-// printJSONFilteredSnippetMax bounds the snippet stored per finding so
-// a multi-hundred-char struct-literal call site doesn't bloat
-// dogfood.json. Truncated snippets get an ellipsis suffix.
-const printJSONFilteredSnippetMax = 120
-
func checkPrintJSONFiltered(cliDir string) PrintJSONFilteredCheckResult {
cliPkgDir := filepath.Join(cliDir, "internal", "cli")
if _, err := os.Stat(cliPkgDir); err != nil {
@@ -41,39 +29,8 @@ func checkPrintJSONFiltered(cliDir string) PrintJSONFilteredCheckResult {
}
result := PrintJSONFilteredCheckResult{}
- for _, path := range listGoFiles(cliPkgDir) {
- data, err := os.ReadFile(path)
- if err != nil {
- continue
- }
+ for range listGoFiles(cliPkgDir) {
result.Checked++
-
- // Walk line-by-line so each finding carries its own line number
- // and snippet. The number of files is small (low tens at most)
- // and per-file size is small (low thousands of bytes), so the
- // strings.Split allocation is in the noise vs. the surrounding
- // os.ReadFile cost.
- for lineIdx, line := range strings.Split(string(data), "\n") {
- if !strings.Contains(line, printJSONFilteredAntipattern) {
- continue
- }
- result.Findings = append(result.Findings, PrintJSONFilteredFinding{
- File: filepath.ToSlash(filepath.Join("internal", "cli", filepath.Base(path))),
- Line: lineIdx + 1,
- Snippet: truncateSnippet(strings.TrimSpace(line), printJSONFilteredSnippetMax),
- })
- }
}
return result
}
-
-// truncateSnippet caps s at maxRunes runes (UTF-8 safe — splitting
-// mid-rune would corrupt the dogfood.json output) and appends an
-// ellipsis when truncation occurred.
-func truncateSnippet(s string, maxRunes int) string {
- r := []rune(s)
- if len(r) <= maxRunes {
- return s
- }
- return string(r[:maxRunes]) + "…"
-}
diff --git a/internal/pipeline/printjson_filtered_check_test.go b/internal/pipeline/printjson_filtered_check_test.go
index 82a17c40..d12a7ee5 100644
--- a/internal/pipeline/printjson_filtered_check_test.go
+++ b/internal/pipeline/printjson_filtered_check_test.go
@@ -3,14 +3,14 @@ package pipeline
import (
"os"
"path/filepath"
- "strings"
"testing"
)
-// TestCheckPrintJSONFiltered_FlagsAntipattern asserts that the check
-// reports file + line + snippet for an offending call site. Without
-// these three fields a reviewer can't act on the finding.
-func TestCheckPrintJSONFiltered_FlagsAntipattern(t *testing.T) {
+// TestCheckPrintJSONFiltered_AllowsReceiverHelper guards the cleanup for #826:
+// flags.printJSON now delegates to printJSONFiltered in generated CLIs, so the
+// dogfood check must not keep warning on the receiver-style helper agents are
+// expected to use.
+func TestCheckPrintJSONFiltered_AllowsReceiverHelper(t *testing.T) {
t.Parallel()
cliDir := t.TempDir()
@@ -45,18 +45,8 @@ func newWidgetsCmd(flags *rootFlags) *cobra.Command {
if result.Checked != 1 {
t.Errorf("Checked = %d, want 1", result.Checked)
}
- if len(result.Findings) != 1 {
- t.Fatalf("Findings = %d, want 1: %+v", len(result.Findings), result.Findings)
- }
- f := result.Findings[0]
- if f.File != "internal/cli/widgets.go" {
- t.Errorf("File = %q, want internal/cli/widgets.go", f.File)
- }
- if f.Line != 10 {
- t.Errorf("Line = %d, want 10", f.Line)
- }
- if f.Snippet != "return flags.printJSON(cmd, rows)" {
- t.Errorf("Snippet = %q, want clean trimmed source line", f.Snippet)
+ if len(result.Findings) != 0 {
+ t.Fatalf("flags.printJSON is safe after #826; expected no findings, got: %+v", result.Findings)
}
}
@@ -97,37 +87,6 @@ func newWidgetsCmd(flags *rootFlags) *cobra.Command {
}
}
-// TestCheckPrintJSONFiltered_SkipsTestFiles confirms _test.go files
-// are excluded — regression-test fixtures may demonstrate the
-// antipattern intentionally.
-func TestCheckPrintJSONFiltered_SkipsTestFiles(t *testing.T) {
- t.Parallel()
-
- cliDir := t.TempDir()
- cliPkg := filepath.Join(cliDir, "internal", "cli")
- if err := os.MkdirAll(cliPkg, 0o755); err != nil {
- t.Fatalf("mkdir: %v", err)
- }
-
- const testGo = `package cli
-
-import "testing"
-
-func TestRegression(t *testing.T) {
- _ = flags.printJSON(cmd, struct{}{})
-}
-`
- if err := os.WriteFile(filepath.Join(cliPkg, "regression_test.go"), []byte(testGo), 0o644); err != nil {
- t.Fatalf("write regression_test.go: %v", err)
- }
-
- result := checkPrintJSONFiltered(cliDir)
-
- if len(result.Findings) != 0 {
- t.Fatalf("expected no findings (*_test.go skipped), got: %+v", result.Findings)
- }
-}
-
func TestCheckPrintJSONFiltered_SkipsWhenNoCliPkg(t *testing.T) {
t.Parallel()
@@ -137,82 +96,3 @@ func TestCheckPrintJSONFiltered_SkipsWhenNoCliPkg(t *testing.T) {
t.Errorf("expected Skipped=true when internal/cli is missing, got: %+v", result)
}
}
-
-// TestCheckPrintJSONFiltered_MultipleFindingsAcrossFiles confirms the
-// check accumulates findings across files and across multiple call
-// sites within a single file. Findings are also sorted by file path
-// (via listGoFiles), so output is deterministic across OSes.
-func TestCheckPrintJSONFiltered_MultipleFindingsAcrossFiles(t *testing.T) {
- t.Parallel()
-
- cliDir := t.TempDir()
- cliPkg := filepath.Join(cliDir, "internal", "cli")
- if err := os.MkdirAll(cliPkg, 0o755); err != nil {
- t.Fatalf("mkdir: %v", err)
- }
-
- const jobsGo = `package cli
-
-func A(flags *rootFlags, cmd *cobra.Command) error {
- return flags.printJSON(cmd, "first")
-}
-
-func B(flags *rootFlags, cmd *cobra.Command) error {
- return flags.printJSON(cmd, "second")
-}
-`
- const feedbackGo = `package cli
-
-func C(flags *rootFlags, cmd *cobra.Command) error {
- rows := []int{1, 2, 3}
- return flags.printJSON(cmd, rows)
-}
-`
- if err := os.WriteFile(filepath.Join(cliPkg, "jobs.go"), []byte(jobsGo), 0o644); err != nil {
- t.Fatalf("write jobs.go: %v", err)
- }
- if err := os.WriteFile(filepath.Join(cliPkg, "feedback.go"), []byte(feedbackGo), 0o644); err != nil {
- t.Fatalf("write feedback.go: %v", err)
- }
-
- result := checkPrintJSONFiltered(cliDir)
-
- if result.Checked != 2 {
- t.Errorf("Checked = %d, want 2", result.Checked)
- }
- if len(result.Findings) != 3 {
- t.Fatalf("Findings = %d, want 3: %+v", len(result.Findings), result.Findings)
- }
- // listGoFiles sorts alphabetically: feedback.go before jobs.go.
- if !strings.HasSuffix(result.Findings[0].File, "feedback.go") {
- t.Errorf("first finding should be from feedback.go, got %q", result.Findings[0].File)
- }
-}
-
-// TestTruncateSnippet covers the rune-safe truncation guard against
-// dogfood.json bloat. UTF-8 splitting at byte boundaries would corrupt
-// multi-byte characters; rune slicing keeps the output well-formed.
-func TestTruncateSnippet(t *testing.T) {
- t.Parallel()
-
- cases := []struct {
- name string
- in string
- max int
- want string
- }{
- {"under cap", "short line", 120, "short line"},
- {"exact cap", "abcde", 5, "abcde"},
- {"over cap", "abcdefghij", 5, "abcde…"},
- {"multibyte preserved", "café日本語abcde", 6, "café日本…"},
- }
- for _, tc := range cases {
- t.Run(tc.name, func(t *testing.T) {
- t.Parallel()
- got := truncateSnippet(tc.in, tc.max)
- if got != tc.want {
- t.Errorf("truncateSnippet(%q, %d) = %q, want %q", tc.in, tc.max, got, tc.want)
- }
- })
- }
-}
diff --git a/skills/printing-press/SKILL.md b/skills/printing-press/SKILL.md
index 68e6fbe1..b87b0af0 100644
--- a/skills/printing-press/SKILL.md
+++ b/skills/printing-press/SKILL.md
@@ -2149,7 +2149,7 @@ Priority 3 (polish):
After building each command in Priority 1 and Priority 2, verify these 10 principles are met. These map 1:1 to what Phase 4.9's agent readiness reviewer will check - apply them now so the review becomes a confirmation, not a catch-all.
1. **Non-interactive**: No TTY prompts, no `bufio.Scanner(os.Stdin)`, works in CI without a terminal
-2. **Structured output**: `--json` produces valid JSON, `--select` filters fields correctly. Hand-written novel commands that build a Go-typed slice/struct and emit JSON MUST call `printJSONFiltered(cmd.OutOrStdout(), v, flags)` instead of `flags.printJSON(cmd, v)`. The helper marshals the value and routes the bytes through `printOutputWithFlags`, picking up `--select`, `--compact`, `--csv`, and `--quiet` for free; direct `flags.printJSON` drops every one of those flags silently. Endpoint-mirror commands already use `printOutputWithFlags`; this rule covers hand-written novel commands. Verify with `<cli> <novel> --json --select <field> | jq 'keys'` returning only the requested fields.
+2. **Structured output**: `--json` produces valid JSON, `--select` filters fields correctly. Hand-written novel commands that build a Go-typed slice/struct and emit JSON should use the generated receiver-style helper, `flags.printJSON(cmd, v)`, or call `printJSONFiltered(cmd.OutOrStdout(), v, flags)` directly. Both route through `printOutputWithFlags`, picking up `--select`, `--compact`, `--csv`, and `--quiet` for free. Verify with `<cli> <novel> --json --select <field> | jq 'keys'` returning only the requested fields.
3. **Progressive help**: `--help` shows realistic examples with domain-specific values (not "abc123"). **Use `Example: strings.Trim(\`...\`, "\n")` (preserves leading 2-space indent) NOT `strings.TrimSpace(\`...\`)` (strips it).** TrimSpace makes the first example line unindented; dogfood's example-detection parser is tolerant of this in current versions, but the indented form renders correctly across every Cobra version and is the convention used by every generated command.
4. **Actionable errors**: Error messages name the specific flag/arg that's wrong and the correct usage
5. **Safe retries**: Mutation commands support `--dry-run`, idempotent where possible
diff --git a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/root.go b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/root.go
index 96aa19de..d8ce88a1 100644
--- a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/root.go
+++ b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/root.go
@@ -5,7 +5,6 @@ package cli
import (
"bytes"
- "encoding/json"
"fmt"
"io"
"os"
@@ -211,9 +210,7 @@ func (f *rootFlags) newClient() (*client.Client, error) {
}
func (f *rootFlags) printJSON(w *cobra.Command, v any) error {
- enc := json.NewEncoder(w.OutOrStdout())
- enc.SetIndent("", " ")
- return enc.Encode(v)
+ return printJSONFiltered(w.OutOrStdout(), v, f)
}
func (f *rootFlags) printTable(w *cobra.Command, headers []string, rows [][]string) error {
← 5f8dae13 fix(cli): reject reserved placeholder hosts in spec validati
·
back to Cli Printing Press
·
fix(cli): allow runtime override of OAuth2 OIDC URLs (#970) a9e9d006 →