[object Object]

← back to Cli Printing Press

feat(generator): owner precedence chain — preserve attribution on regen (#470)

d1a8a467bf0a64cd75f43b40fee270b22b8186b2 · 2026-05-01 16:24:24 -0700 · Trevin Chow

Replaces the single-tier `git config`-based owner resolution with a
layered precedence chain so regenerations against existing trees keep
their original attribution instead of silently flipping to whoever's
running the command.

New tiers (highest priority first):
  1. --owner CLI flag (caller sets spec.Owner before generator runs)
  2. .printing-press.json's owner field
  3. parsed `// Copyright YYYY <owner>.` line in internal/cli/root.go
  4. git config github.user
  5. sanitized git config user.name
  6. literal "USER"

Surfaced by the regen-merge sweep batch on the library: running
`printing-press generate` on my machine flipped Copyright lines from
hiten-shah to trevin-chow across 11 CLIs because the resolver only
read git config. PR #180 in the library repo restored the affected
attributions; this change prevents the recurrence.

CLI surface:
  --owner <name>  override owner attribution (highest priority)

Manifest schema:
  added `owner` field to .printing-press.json so subsequent regens
  can read what the previous run wrote.

Architecture choice:
  Two functions instead of one — resolveOwnerForExisting(outputDir)
  vs resolveOwnerForNew() — to avoid the "pass empty string to skip
  tiers" overload. The brand-new path doesn't carry an outputDir
  parameter at all.

Reuse note:
  Considered exporting pipeline.ReadCLIManifestOwner to mirror
  ReadCLIBinaryName, but pipeline already imports generator, so
  reading .printing-press.json from generator would create an import
  cycle. Inlined a minimal struct unmarshal instead.

Tests added (5 new):
  - manifest tier wins over copyright header
  - copyright tier when manifest is absent
  - manifest with absent owner field falls through to copyright
  - manifest with empty-string owner falls through to copyright
  - new-project path returns non-empty owner

Verification:
  - go test ./... — 2633 pass (5 new)
  - go vet, golangci-lint clean
  - golden verify passed (.printing-press.json fixtures updated)

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

Files touched

Diff

commit d1a8a467bf0a64cd75f43b40fee270b22b8186b2
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Fri May 1 16:24:24 2026 -0700

    feat(generator): owner precedence chain — preserve attribution on regen (#470)
    
    Replaces the single-tier `git config`-based owner resolution with a
    layered precedence chain so regenerations against existing trees keep
    their original attribution instead of silently flipping to whoever's
    running the command.
    
    New tiers (highest priority first):
      1. --owner CLI flag (caller sets spec.Owner before generator runs)
      2. .printing-press.json's owner field
      3. parsed `// Copyright YYYY <owner>.` line in internal/cli/root.go
      4. git config github.user
      5. sanitized git config user.name
      6. literal "USER"
    
    Surfaced by the regen-merge sweep batch on the library: running
    `printing-press generate` on my machine flipped Copyright lines from
    hiten-shah to trevin-chow across 11 CLIs because the resolver only
    read git config. PR #180 in the library repo restored the affected
    attributions; this change prevents the recurrence.
    
    CLI surface:
      --owner <name>  override owner attribution (highest priority)
    
    Manifest schema:
      added `owner` field to .printing-press.json so subsequent regens
      can read what the previous run wrote.
    
    Architecture choice:
      Two functions instead of one — resolveOwnerForExisting(outputDir)
      vs resolveOwnerForNew() — to avoid the "pass empty string to skip
      tiers" overload. The brand-new path doesn't carry an outputDir
      parameter at all.
    
    Reuse note:
      Considered exporting pipeline.ReadCLIManifestOwner to mirror
      ReadCLIBinaryName, but pipeline already imports generator, so
      reading .printing-press.json from generator would create an import
      cycle. Inlined a minimal struct unmarshal instead.
    
    Tests added (5 new):
      - manifest tier wins over copyright header
      - copyright tier when manifest is absent
      - manifest with absent owner field falls through to copyright
      - manifest with empty-string owner falls through to copyright
      - new-project path returns non-empty owner
    
    Verification:
      - go test ./... — 2633 pass (5 new)
      - go vet, golangci-lint clean
      - golden verify passed (.printing-press.json fixtures updated)
    
    Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 internal/cli/root.go                               | 13 ++-
 internal/generator/generator.go                    |  9 +-
 internal/generator/owner_test.go                   | 97 ++++++++++++++++++++++
 internal/generator/plan_generate.go                | 68 ++++++++++++++-
 internal/pipeline/climanifest.go                   |  8 +-
 .../cafe-bistro/.printing-press.json               |  1 +
 .../printing-press-golden/.printing-press.json     |  1 +
 7 files changed, 182 insertions(+), 15 deletions(-)

diff --git a/internal/cli/root.go b/internal/cli/root.go
index 67549416..cfc26a65 100644
--- a/internal/cli/root.go
+++ b/internal/cli/root.go
@@ -75,6 +75,7 @@ func Execute() error {
 func newGenerateCmd() *cobra.Command {
 	var specFiles []string
 	var cliName string
+	var owner string
 	var outputDir string
 	var validate bool
 	var refresh bool
@@ -143,7 +144,7 @@ func newGenerateCmd() *cobra.Command {
 				if err != nil {
 					return &ExitError{Code: ExitSpecError, Err: fmt.Errorf("parsing generated spec: %w", err)}
 				}
-				if err := applyGenerateSpecFlags(parsed, specSource, "docs", clientPattern, httpTransport); err != nil {
+				if err := applyGenerateSpecFlags(parsed, specSource, "docs", clientPattern, httpTransport, owner); err != nil {
 					return err
 				}
 
@@ -161,6 +162,7 @@ func newGenerateCmd() *cobra.Command {
 					APIName:       parsed.Name,
 					DocsURL:       docsURL,
 					OutputDir:     absOut,
+					Owner:         parsed.Owner,
 					Spec:          parsed,
 					NovelFeatures: novelFeatures,
 				}); err != nil {
@@ -282,7 +284,7 @@ func newGenerateCmd() *cobra.Command {
 				apiSpec = mergeSpecs(specs, cliName)
 			}
 
-			if err := applyGenerateSpecFlags(apiSpec, specSource, "", clientPattern, httpTransport); err != nil {
+			if err := applyGenerateSpecFlags(apiSpec, specSource, "", clientPattern, httpTransport, owner); err != nil {
 				return err
 			}
 
@@ -321,6 +323,7 @@ func newGenerateCmd() *cobra.Command {
 				SpecSrcs:      specFiles,
 				SpecURL:       specURL,
 				OutputDir:     absOut,
+				Owner:         apiSpec.Owner,
 				Spec:          apiSpec,
 				NovelFeatures: novelFeatures,
 			}); err != nil {
@@ -359,6 +362,7 @@ func newGenerateCmd() *cobra.Command {
 
 	cmd.Flags().StringSliceVar(&specFiles, "spec", nil, "Path or URL to API spec (can be repeated)")
 	cmd.Flags().StringVar(&cliName, "name", "", "CLI name (required when using multiple specs)")
+	cmd.Flags().StringVar(&owner, "owner", "", "Override owner attribution in generated copyright headers (highest priority; otherwise resolved from existing .printing-press.json, copyright header, or git config)")
 	cmd.Flags().StringVar(&outputDir, "output", "", "Output directory (default: ~/printing-press/library/<name>)")
 	cmd.Flags().BoolVar(&validate, "validate", true, "Run quality gates on the generated project")
 	cmd.Flags().BoolVar(&refresh, "refresh", false, "Refresh cached remote spec before generating")
@@ -439,7 +443,7 @@ func runGenerateProject(apiSpec *spec.APISpec, absOut string, opts generateProje
 	return novelFeatures, runGeneratePolishPass(opts.polish, apiSpec.Name, absOut), nil
 }
 
-func applyGenerateSpecFlags(apiSpec *spec.APISpec, specSource, defaultSpecSource, clientPattern, httpTransport string) error {
+func applyGenerateSpecFlags(apiSpec *spec.APISpec, specSource, defaultSpecSource, clientPattern, httpTransport, owner string) error {
 	if specSource != "" {
 		normalized, err := normalizeSpecSource(specSource)
 		if err != nil {
@@ -463,6 +467,9 @@ func applyGenerateSpecFlags(apiSpec *spec.APISpec, specSource, defaultSpecSource
 		}
 		apiSpec.HTTPTransport = normalized
 	}
+	if owner != "" {
+		apiSpec.Owner = owner
+	}
 	return nil
 }
 
diff --git a/internal/generator/generator.go b/internal/generator/generator.go
index 7789ebe5..85621e81 100644
--- a/internal/generator/generator.go
+++ b/internal/generator/generator.go
@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"net/url"
 	"os"
-	"os/exec"
 	"path/filepath"
 	"slices"
 	"sort"
@@ -155,13 +154,7 @@ type Generator struct {
 
 func New(s *spec.APISpec, outputDir string) *Generator {
 	if s.Owner == "" {
-		if out, err := exec.Command("git", "config", "github.user").Output(); err == nil && len(out) > 0 {
-			s.Owner = strings.TrimSpace(string(out))
-		} else if out, err := exec.Command("git", "config", "user.name").Output(); err == nil && len(out) > 0 {
-			s.Owner = strings.TrimSpace(string(out))
-		} else {
-			s.Owner = "USER"
-		}
+		s.Owner = resolveOwnerForExisting(outputDir)
 	}
 	// Sanitize owner for Go module path: lowercase, no spaces/special chars
 	s.Owner = strings.ToLower(s.Owner)
diff --git a/internal/generator/owner_test.go b/internal/generator/owner_test.go
new file mode 100644
index 00000000..33e8d71d
--- /dev/null
+++ b/internal/generator/owner_test.go
@@ -0,0 +1,97 @@
+package generator
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestResolveOwnerForExistingManifestWins pins tier 1 of the precedence
+// chain: a `.printing-press.json` with an `owner` field is the highest-
+// priority signal short of the explicit --owner flag (which the caller
+// sets via spec.Owner before resolveOwnerForExisting runs).
+func TestResolveOwnerForExistingManifestWins(t *testing.T) {
+	dir := t.TempDir()
+	manifest := `{"schema_version":1,"owner":"hiten-shah","cli_name":"foo-pp-cli","api_name":"foo"}`
+	require.NoError(t, os.WriteFile(filepath.Join(dir, ".printing-press.json"), []byte(manifest), 0o644))
+
+	// Even with a copyright header that disagrees, the manifest wins.
+	require.NoError(t, os.MkdirAll(filepath.Join(dir, "internal", "cli"), 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "internal", "cli", "root.go"),
+		[]byte("// Copyright 2026 someone-else. Licensed under Apache-2.0.\npackage cli\n"), 0o644))
+
+	assert.Equal(t, "hiten-shah", resolveOwnerForExisting(dir))
+}
+
+// TestResolveOwnerForExistingCopyrightFallback pins tier 2: when the
+// manifest is absent or the owner field is empty, parse the copyright
+// header in internal/cli/root.go.
+func TestResolveOwnerForExistingCopyrightFallback(t *testing.T) {
+	dir := t.TempDir()
+	require.NoError(t, os.MkdirAll(filepath.Join(dir, "internal", "cli"), 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "internal", "cli", "root.go"),
+		[]byte("// Copyright 2026 hiten-shah. Licensed under Apache-2.0. See LICENSE.\npackage cli\n"), 0o644))
+
+	assert.Equal(t, "hiten-shah", resolveOwnerForExisting(dir))
+}
+
+// TestResolveOwnerForExistingFallsThroughOnAbsentOwner pins that a
+// manifest without an `owner` field doesn't short-circuit the chain —
+// copyright parse gets a chance.
+func TestResolveOwnerForExistingFallsThroughOnAbsentOwner(t *testing.T) {
+	dir := t.TempDir()
+	manifest := `{"schema_version":1,"cli_name":"foo-pp-cli","api_name":"foo"}` // no owner field
+	require.NoError(t, os.WriteFile(filepath.Join(dir, ".printing-press.json"), []byte(manifest), 0o644))
+	require.NoError(t, os.MkdirAll(filepath.Join(dir, "internal", "cli"), 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "internal", "cli", "root.go"),
+		[]byte("// Copyright 2026 hiten-shah.\npackage cli\n"), 0o644))
+
+	assert.Equal(t, "hiten-shah", resolveOwnerForExisting(dir))
+}
+
+// TestResolveOwnerForExistingFallsThroughOnEmptyOwner pins the explicit
+// empty-string case: `{"owner":""}` is treated identically to the
+// absent-field case — both fall through to copyright parse. Without this
+// guarantee, a corrupted or partially-populated manifest would short-
+// circuit to an empty owner string.
+func TestResolveOwnerForExistingFallsThroughOnEmptyOwner(t *testing.T) {
+	dir := t.TempDir()
+	manifest := `{"schema_version":1,"owner":"","cli_name":"foo-pp-cli","api_name":"foo"}`
+	require.NoError(t, os.WriteFile(filepath.Join(dir, ".printing-press.json"), []byte(manifest), 0o644))
+	require.NoError(t, os.MkdirAll(filepath.Join(dir, "internal", "cli"), 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "internal", "cli", "root.go"),
+		[]byte("// Copyright 2026 hiten-shah.\npackage cli\n"), 0o644))
+
+	assert.Equal(t, "hiten-shah", resolveOwnerForExisting(dir))
+}
+
+// TestResolveOwnerForNew pins the brand-new project path: no existing
+// tree, falls through git config to USER. Result depends on the runner's
+// git config; we just check it returns a non-empty value.
+func TestResolveOwnerForNew(t *testing.T) {
+	assert.NotEmpty(t, resolveOwnerForNew())
+}
+
+// TestParseCopyrightOwnerHandlesMissingFile confirms the function returns
+// "" rather than panicking when root.go isn't there.
+func TestParseCopyrightOwnerHandlesMissingFile(t *testing.T) {
+	assert.Equal(t, "", parseCopyrightOwner(t.TempDir()))
+}
+
+// TestReadManifestOwnerHandlesMissingFile confirms the function returns
+// "" rather than panicking when the manifest isn't there.
+func TestReadManifestOwnerHandlesMissingFile(t *testing.T) {
+	assert.Equal(t, "", readManifestOwner(t.TempDir()))
+}
+
+// TestReadManifestOwnerHandlesMalformed confirms invalid JSON returns ""
+// rather than crashing the resolver.
+func TestReadManifestOwnerHandlesMalformed(t *testing.T) {
+	dir := t.TempDir()
+	require.NoError(t, os.WriteFile(filepath.Join(dir, ".printing-press.json"),
+		[]byte("not json at all{{{"), 0o644))
+	assert.Equal(t, "", readManifestOwner(dir))
+}
diff --git a/internal/generator/plan_generate.go b/internal/generator/plan_generate.go
index 67fdb2ee..4f48dd26 100644
--- a/internal/generator/plan_generate.go
+++ b/internal/generator/plan_generate.go
@@ -1,10 +1,12 @@
 package generator
 
 import (
+	"encoding/json"
 	"fmt"
 	"os"
 	"os/exec"
 	"path/filepath"
+	"regexp"
 	"sort"
 	"strconv"
 	"strings"
@@ -61,7 +63,7 @@ func GenerateFromPlan(planSpec *PlanSpec, outputDir string) error {
 		return fmt.Errorf("plan has no CLI name")
 	}
 
-	owner := resolveOwner()
+	owner := resolveOwnerForExisting(outputDir)
 
 	// Create directory structure
 	dirs := []string{
@@ -284,8 +286,48 @@ func partitionCommands(commands []PlanCommand) (topLevel []PlanCommand, parents
 	return filteredTopLevel, parents
 }
 
-// resolveOwner tries to determine the GitHub user/org for the module path.
-func resolveOwner() string {
+// resolveOwnerForExisting returns the owner attribution for a regeneration
+// against an existing tree at outputDir. Tiered so regens preserve original
+// attribution instead of silently flipping to whoever's running the
+// generator:
+//  1. .printing-press.json's `owner` field, if present and non-empty
+//  2. parsed `// Copyright YYYY <owner>` line in internal/cli/root.go
+//  3. resolveOwnerForNew() (git config / "USER")
+//
+// Reads .printing-press.json directly rather than calling
+// pipeline.ReadCLIManifestOwner because the pipeline package already
+// imports generator — adding the reverse direction would create a cycle.
+func resolveOwnerForExisting(outputDir string) string {
+	if owner := readManifestOwner(outputDir); owner != "" {
+		return owner
+	}
+	if owner := parseCopyrightOwner(outputDir); owner != "" {
+		return owner
+	}
+	return resolveOwnerForNew()
+}
+
+// readManifestOwner returns the `owner` field from
+// outputDir/.printing-press.json, or "" if the file is absent, malformed,
+// or the field is empty.
+func readManifestOwner(outputDir string) string {
+	data, err := os.ReadFile(filepath.Join(outputDir, ".printing-press.json"))
+	if err != nil {
+		return ""
+	}
+	var m struct {
+		Owner string `json:"owner"`
+	}
+	if err := json.Unmarshal(data, &m); err != nil {
+		return ""
+	}
+	return m.Owner
+}
+
+// resolveOwnerForNew returns the owner attribution for a brand-new project
+// (no existing tree to read from). Falls through git config in priority
+// order: github.user, sanitized user.name, literal "USER".
+func resolveOwnerForNew() string {
 	if out, err := exec.Command("git", "config", "github.user").Output(); err == nil && len(out) > 0 {
 		return strings.TrimSpace(string(out))
 	}
@@ -295,6 +337,26 @@ func resolveOwner() string {
 	return "USER"
 }
 
+// copyrightOwnerRe matches a Go source `// Copyright YYYY <owner>.` line.
+// The owner capture matches the same characters sanitizeOwner would emit
+// (lowercase letters, digits, `-`, `_`) plus uppercase to be lenient on
+// hand-edited files.
+var copyrightOwnerRe = regexp.MustCompile(`(?m)^//\s*Copyright\s+\d+\s+([A-Za-z0-9_-]+)\.`)
+
+// parseCopyrightOwner reads outputDir/internal/cli/root.go (the generator's
+// canonical copyright site) and returns the owner string from the
+// "// Copyright YYYY <owner>." header. Returns "" on any failure.
+func parseCopyrightOwner(outputDir string) string {
+	data, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "root.go"))
+	if err != nil {
+		return ""
+	}
+	if m := copyrightOwnerRe.FindSubmatch(data); m != nil {
+		return string(m[1])
+	}
+	return ""
+}
+
 // sanitizeOwner cleans up an owner string for use in Go module paths.
 func sanitizeOwner(s string) string {
 	s = strings.ToLower(s)
diff --git a/internal/pipeline/climanifest.go b/internal/pipeline/climanifest.go
index dcd96322..23ec7ec2 100644
--- a/internal/pipeline/climanifest.go
+++ b/internal/pipeline/climanifest.go
@@ -42,7 +42,11 @@ type CLIManifest struct {
 	DisplayName string `json:"display_name,omitempty"`
 	// CLIName is the executable/binary name (for example "espn-pp-cli").
 	// It does not track the slug-keyed library directory.
-	CLIName            string   `json:"cli_name"`
+	CLIName string `json:"cli_name"`
+	// Owner is the attribution recorded in generated copyright headers
+	// (for example "hiten-shah"). Persisted here so subsequent regens
+	// preserve attribution regardless of who's running the generator.
+	Owner              string   `json:"owner,omitempty"`
 	SpecURL            string   `json:"spec_url,omitempty"`
 	SpecPath           string   `json:"spec_path,omitempty"`
 	SpecFormat         string   `json:"spec_format,omitempty"`
@@ -261,6 +265,7 @@ type GenerateManifestParams struct {
 	SpecURL       string   // --spec-url: explicit provenance URL (when --spec is a local downloaded file)
 	DocsURL       string   // --docs URL, if used
 	OutputDir     string
+	Owner         string                 // resolved owner attribution (manifest preserve > copyright parse > git config)
 	Spec          *spec.APISpec          // parsed spec for MCP metadata (nil if unavailable)
 	NovelFeatures []NovelFeatureManifest // transcendence features from research (nil if unavailable)
 }
@@ -275,6 +280,7 @@ func WriteManifestForGenerate(p GenerateManifestParams) error {
 		PrintingPressVersion: version.Version,
 		APIName:              p.APIName,
 		CLIName:              naming.CLI(p.APIName),
+		Owner:                p.Owner,
 	}
 
 	// Populate spec_url / spec_path from the first spec source.
diff --git a/testdata/golden/expected/generate-golden-api-unicode/cafe-bistro/.printing-press.json b/testdata/golden/expected/generate-golden-api-unicode/cafe-bistro/.printing-press.json
index 293d4c0d..40d759c5 100644
--- a/testdata/golden/expected/generate-golden-api-unicode/cafe-bistro/.printing-press.json
+++ b/testdata/golden/expected/generate-golden-api-unicode/cafe-bistro/.printing-press.json
@@ -11,6 +11,7 @@
   "mcp_binary": "cafe-bistro-pp-mcp",
   "mcp_ready": "full",
   "mcp_tool_count": 1,
+  "owner": "printing-press-golden",
   "printing_press_version": "<PRINTING_PRESS_VERSION>",
   "schema_version": 1,
   "spec_checksum": "sha256:8cfba131c384836e8dde461511c9fd63cf2cbab7793553067765d515f2007aa1",
diff --git a/testdata/golden/expected/generate-golden-api/printing-press-golden/.printing-press.json b/testdata/golden/expected/generate-golden-api/printing-press-golden/.printing-press.json
index ac23345e..f7381875 100644
--- a/testdata/golden/expected/generate-golden-api/printing-press-golden/.printing-press.json
+++ b/testdata/golden/expected/generate-golden-api/printing-press-golden/.printing-press.json
@@ -12,6 +12,7 @@
   "mcp_public_tool_count": 1,
   "mcp_ready": "full",
   "mcp_tool_count": 8,
+  "owner": "printing-press-golden",
   "printing_press_version": "<PRINTING_PRESS_VERSION>",
   "schema_version": 1,
   "spec_checksum": "sha256:333b2c06c645b8f394fbbaaedf8126e06919529d7d9305c94c5ebcf1f5a897b5",

← abde446e feat(regenmerge): add TEMPLATED-BODY-DRIFT verdict to catch  ·  back to Cli Printing Press  ·  chore(skills): re-add context: fork to polish and output-rev 20ffaaa4 →