← back to Cli Printing Press
fix(cli): guard generated CLI build safety (#736)
1d26ae77985ef52df240018ad23913cb2602acc0 · 2026-05-08 11:45:13 -0700 · Trevin Chow
Files touched
M internal/crowdsniff/npm_test.goM internal/crowdsniff/specgen.goM internal/generator/generator.goM internal/generator/generator_test.goM internal/generator/templates/store.go.tmplM internal/generator/templates/sync.go.tmplM internal/naming/naming.goM internal/spec/spec.goM internal/spec/spec_test.go
Diff
commit 1d26ae77985ef52df240018ad23913cb2602acc0
Author: Trevin Chow <trevin@trevinchow.com>
Date: Fri May 8 11:45:13 2026 -0700
fix(cli): guard generated CLI build safety (#736)
---
internal/crowdsniff/npm_test.go | 3 ++
internal/crowdsniff/specgen.go | 16 +++++--
internal/generator/generator.go | 75 +++++++++++++++++++++++++++---
internal/generator/generator_test.go | 57 +++++++++++++++++++++++
internal/generator/templates/store.go.tmpl | 11 +----
internal/generator/templates/sync.go.tmpl | 20 ++------
internal/naming/naming.go | 32 ++++++++++++-
internal/spec/spec.go | 7 +++
internal/spec/spec_test.go | 60 ++++++++++++++++++++++++
9 files changed, 244 insertions(+), 37 deletions(-)
diff --git a/internal/crowdsniff/npm_test.go b/internal/crowdsniff/npm_test.go
index 0bc62ff8..3e19b57f 100644
--- a/internal/crowdsniff/npm_test.go
+++ b/internal/crowdsniff/npm_test.go
@@ -1311,6 +1311,9 @@ func TestBuildSpec_WithAuth(t *testing.T) {
apiSpec, err := BuildSpec("cal.com", "https://api.cal.com", endpoints, auth)
require.NoError(t, err)
+ assert.Equal(t, "cal-com", apiSpec.Name)
+ assert.Equal(t, "cal.com", apiSpec.DisplayName)
+ assert.Equal(t, "~/.config/cal-com-pp-cli/config.toml", apiSpec.Config.Path)
assert.Equal(t, []string{"CAL_COM_API_KEY"}, apiSpec.Auth.EnvVars)
})
}
diff --git a/internal/crowdsniff/specgen.go b/internal/crowdsniff/specgen.go
index a893edea..68886c30 100644
--- a/internal/crowdsniff/specgen.go
+++ b/internal/crowdsniff/specgen.go
@@ -6,6 +6,7 @@ import (
"strings"
"github.com/mvanhorn/cli-printing-press/v4/internal/discovery"
+ "github.com/mvanhorn/cli-printing-press/v4/internal/naming"
"github.com/mvanhorn/cli-printing-press/v4/internal/spec"
)
@@ -22,6 +23,14 @@ func BuildSpec(name, baseURL string, endpoints []AggregatedEndpoint, auth *Disco
if baseURL == "" {
return nil, fmt.Errorf("base_url is required")
}
+ displayName := strings.TrimSpace(name)
+ apiName := naming.Slug(displayName)
+ if apiName == "" {
+ return nil, fmt.Errorf("name must contain at least one letter or digit")
+ }
+ if apiName == displayName {
+ displayName = ""
+ }
resources := make(map[string]spec.Resource)
@@ -79,18 +88,19 @@ func BuildSpec(name, baseURL string, endpoints []AggregatedEndpoint, auth *Disco
authConfig := spec.AuthConfig{Type: "none"}
if auth != nil {
- authConfig = buildAuthConfig(name, auth)
+ authConfig = buildAuthConfig(apiName, auth)
}
apiSpec := &spec.APISpec{
- Name: name,
+ Name: apiName,
+ DisplayName: displayName,
Description: fmt.Sprintf("Discovered API spec for %s (crowd-sniff)", name),
Version: "0.1.0",
BaseURL: baseURL,
Auth: authConfig,
Config: spec.ConfigSpec{
Format: "toml",
- Path: fmt.Sprintf("~/.config/%s-pp-cli/config.toml", name),
+ Path: fmt.Sprintf("~/.config/%s-pp-cli/config.toml", apiName),
},
Resources: resources,
Types: map[string]spec.TypeDef{},
diff --git a/internal/generator/generator.go b/internal/generator/generator.go
index 4be33ea8..65be8a1f 100644
--- a/internal/generator/generator.go
+++ b/internal/generator/generator.go
@@ -310,12 +310,14 @@ func New(s *spec.APISpec, outputDir string) *Generator {
// `?limit=N` query param without honoring it; truncating client-
// side means the user-facing --limit flag works regardless.
// Surfaced by hackernews retro #350 finding F6.
- "endpointNeedsClientLimit": endpointNeedsClientLimit,
- "envName": naming.EnvPrefix,
- "safeName": safeSQLName,
- "isBackfillColumn": isStoreBackfillColumn,
- "hasBackfillColumns": hasStoreBackfillColumns,
- "backfillDecl": storeBackfillDecl,
+ "endpointNeedsClientLimit": endpointNeedsClientLimit,
+ "envName": naming.EnvPrefix,
+ "safeName": safeSQLName,
+ "resourceIDFieldOverrideEntries": resourceIDFieldOverrideEntries,
+ "criticalResourceEntries": criticalResourceEntries,
+ "isBackfillColumn": isStoreBackfillColumn,
+ "hasBackfillColumns": hasStoreBackfillColumns,
+ "backfillDecl": storeBackfillDecl,
"safeNameSuffix": func(name, suffix string) string {
return safeSQLName(name + suffix)
},
@@ -1963,6 +1965,67 @@ type visionRenderData struct {
AgentMoneyWorkflow AgentMoneyWorkflow
}
+type resourceIDFieldOverrideEntry struct {
+ Name string
+ Value string
+}
+
+type criticalResourceEntry struct {
+ Name string
+}
+
+func resourceIDFieldOverrideEntries(syncable []profiler.SyncableResource, dependent []profiler.DependentResource) []resourceIDFieldOverrideEntry {
+ overrides := map[string]string{}
+ for _, resource := range syncable {
+ if resource.IDField != "" {
+ overrides[resource.Name] = resource.IDField
+ }
+ }
+ for _, resource := range dependent {
+ if resource.IDField != "" {
+ overrides[resource.Name] = resource.IDField
+ }
+ }
+
+ names := make([]string, 0, len(overrides))
+ for name := range overrides {
+ names = append(names, name)
+ }
+ sort.Strings(names)
+
+ entries := make([]resourceIDFieldOverrideEntry, len(names))
+ for i, name := range names {
+ entries[i] = resourceIDFieldOverrideEntry{Name: name, Value: overrides[name]}
+ }
+ return entries
+}
+
+func criticalResourceEntries(syncable []profiler.SyncableResource, dependent []profiler.DependentResource) []criticalResourceEntry {
+ critical := map[string]bool{}
+ for _, resource := range syncable {
+ if resource.Critical {
+ critical[resource.Name] = true
+ }
+ }
+ for _, resource := range dependent {
+ if resource.Critical {
+ critical[resource.Name] = true
+ }
+ }
+
+ names := make([]string, 0, len(critical))
+ for name := range critical {
+ names = append(names, name)
+ }
+ sort.Strings(names)
+
+ entries := make([]criticalResourceEntry, len(names))
+ for i, name := range names {
+ entries[i] = criticalResourceEntry{Name: name}
+ }
+ return entries
+}
+
func (g *Generator) visionRenderData(schema []TableDef) visionRenderData {
gqlFieldPaths := map[string]string{}
for rName, r := range g.Spec.Resources {
diff --git a/internal/generator/generator_test.go b/internal/generator/generator_test.go
index 904ac7eb..4a187684 100644
--- a/internal/generator/generator_test.go
+++ b/internal/generator/generator_test.go
@@ -210,6 +210,63 @@ func TestGenerateNoUnscopedStoreOpen(t *testing.T) {
require.NoError(t, err, "walking generated CLI tree")
}
+func TestGenerateDedupesResourceRegistryMapEntries(t *testing.T) {
+ t.Parallel()
+
+ apiSpec := minimalSpec("resource-registry-dedupe")
+ apiSpec.Auth = spec.AuthConfig{Type: "none"}
+ apiSpec.Resources = map[string]spec.Resource{
+ "locations": {
+ Description: "Manage locations",
+ Endpoints: map[string]spec.Endpoint{
+ "list": {
+ Method: "GET",
+ Path: "/locations",
+ Response: spec.ResponseDef{Type: "array"},
+ Pagination: &spec.Pagination{CursorParam: "after", LimitParam: "limit"},
+ IDField: "id",
+ },
+ },
+ },
+ "contacts": {
+ Description: "Manage contacts",
+ Endpoints: map[string]spec.Endpoint{
+ "list": {
+ Method: "GET",
+ Path: "/contacts",
+ Response: spec.ResponseDef{Type: "array"},
+ Pagination: &spec.Pagination{CursorParam: "after", LimitParam: "limit"},
+ IDField: "id",
+ Critical: true,
+ },
+ "list_by_location": {
+ Method: "GET",
+ Path: "/locations/{locationId}/contacts",
+ Response: spec.ResponseDef{Type: "array"},
+ Pagination: &spec.Pagination{CursorParam: "after", LimitParam: "limit"},
+ IDField: "id",
+ Critical: true,
+ },
+ },
+ },
+ }
+
+ outputDir := filepath.Join(t.TempDir(), naming.CLI(apiSpec.Name))
+ gen := New(apiSpec, outputDir)
+ gen.VisionSet = VisionTemplateSet{Store: true, Sync: true}
+ require.NoError(t, gen.Generate())
+
+ storeSrc, err := os.ReadFile(filepath.Join(outputDir, "internal", "store", "store.go"))
+ require.NoError(t, err)
+ syncSrc, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "sync.go"))
+ require.NoError(t, err)
+
+ assert.Equal(t, 1, strings.Count(string(storeSrc), `"contacts": "id",`), "store.go should emit one ID override per resource")
+ assert.Equal(t, 1, strings.Count(string(syncSrc), `"contacts": "id",`), "sync.go should emit one ID override per resource")
+ assert.Equal(t, 1, strings.Count(string(syncSrc), `"contacts": true,`), "sync.go should emit one critical flag per resource")
+ runGoCommand(t, outputDir, "test", "./internal/cli", "./internal/store")
+}
+
// TestGenerateFreshnessHelperEmitted verifies that the cliutil freshness
// helper and auto-refresh wrapper are emitted when the spec opts into
// cache, and that the resulting CLI compiles end-to-end and its cliutil
diff --git a/internal/generator/templates/store.go.tmpl b/internal/generator/templates/store.go.tmpl
index b4e96594..3c485d79 100644
--- a/internal/generator/templates/store.go.tmpl
+++ b/internal/generator/templates/store.go.tmpl
@@ -723,15 +723,8 @@ func (s *Store) Upsert{{pascal .Name}}(data json.RawMessage) error {
// child path-item annotated with x-resource-id resolves the same as a flat
// path-item.
var resourceIDFieldOverrides = map[string]string{
-{{- range .SyncableResources}}
-{{- if .IDField}}
- {{printf "%q" .Name}}: {{printf "%q" .IDField}},
-{{- end}}
-{{- end}}
-{{- range .DependentSyncResources}}
-{{- if .IDField}}
- {{printf "%q" .Name}}: {{printf "%q" .IDField}},
-{{- end}}
+{{- range resourceIDFieldOverrideEntries .SyncableResources .DependentSyncResources}}
+ {{printf "%q" .Name}}: {{printf "%q" .Value}},
{{- end}}
}
diff --git a/internal/generator/templates/sync.go.tmpl b/internal/generator/templates/sync.go.tmpl
index 2b9ff84e..97e4cbea 100644
--- a/internal/generator/templates/sync.go.tmpl
+++ b/internal/generator/templates/sync.go.tmpl
@@ -1159,15 +1159,8 @@ func syncDependentResource(c interface {
// annotations on a child path-item are honored at runtime, not just on
// flat paths.
var resourceIDFieldOverrides = map[string]string{
-{{- range .SyncableResources}}
-{{- if .IDField}}
- {{printf "%q" .Name}}: {{printf "%q" .IDField}},
-{{- end}}
-{{- end}}
-{{- range .DependentSyncResources}}
-{{- if .IDField}}
- {{printf "%q" .Name}}: {{printf "%q" .IDField}},
-{{- end}}
+{{- range resourceIDFieldOverrideEntries .SyncableResources .DependentSyncResources}}
+ {{printf "%q" .Name}}: {{printf "%q" .Value}},
{{- end}}
}
@@ -1185,16 +1178,9 @@ var genericIDFieldFallbacks = []string{"id", "ID", "name", "uuid", "slug", "key"
// failed child sync flagged x-critical: true exits non-zero just like a
// flat-resource critical failure.
var criticalResources = map[string]bool{
-{{- range .SyncableResources}}
-{{- if .Critical}}
+{{- range criticalResourceEntries .SyncableResources .DependentSyncResources}}
{{printf "%q" .Name}}: true,
{{- end}}
-{{- end}}
-{{- range .DependentSyncResources}}
-{{- if .Critical}}
- {{printf "%q" .Name}}: true,
-{{- end}}
-{{- end}}
}
// extractID resolves an item's primary-key field. It consults the
diff --git a/internal/naming/naming.go b/internal/naming/naming.go
index f2893064..c221abf0 100644
--- a/internal/naming/naming.go
+++ b/internal/naming/naming.go
@@ -65,6 +65,31 @@ func HumanName(slug string) string {
return cases.Title(language.English).String(strings.ReplaceAll(slug, "-", " "))
}
+// Slug normalizes display-ish input into the slug grammar used for API names.
+func Slug(s string) string {
+ s = strings.ToLower(strings.TrimSpace(ASCIIFold(s)))
+ var b strings.Builder
+ lastHyphen := true
+ for _, r := range s {
+ switch {
+ case (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9'):
+ b.WriteRune(r)
+ lastHyphen = false
+ default:
+ if !lastHyphen && b.Len() > 0 {
+ b.WriteByte('-')
+ lastHyphen = true
+ }
+ }
+ }
+ return strings.Trim(b.String(), "-")
+}
+
+// IsSlug reports whether s matches the API slug grammar.
+func IsSlug(s string) bool {
+ return apiSlugRe.MatchString(s)
+}
+
// SnakeIdentifier collapses a free-form command spec into a snake_case Go
// identifier safe to use as an MCP tool name. "funding --who" → "funding_who",
// "FUNDING-TREND" → "funding_trend". Used by the generator's mcpToolName
@@ -327,10 +352,13 @@ func LibraryDirName(name string) string {
}
}
-// slugRe matches the slug grammar: lowercase alphanumeric + hyphens, must start
-// with an alphanumeric character. Accepts rerun suffixes like "dub-2".
+// slugRe is the legacy library-dir compatibility grammar: lowercase
+// alphanumeric + hyphens, must start with an alphanumeric character.
var slugRe = regexp.MustCompile(`^[a-z0-9][a-z0-9-]*$`)
+// apiSlugRe is stricter: hyphens must separate non-empty segments.
+var apiSlugRe = regexp.MustCompile(`^[a-z0-9]+(?:-[a-z0-9]+)*$`)
+
// IsValidLibraryDirName returns true if name is a valid library directory name.
// It accepts both legacy CLI directory names (e.g. "dub-pp-cli", "dub-pp-cli-2")
// and slug-keyed names (e.g. "dub", "cal-com", "dub-2"). It rejects empty strings,
diff --git a/internal/spec/spec.go b/internal/spec/spec.go
index 6fe78f1b..9dd81554 100644
--- a/internal/spec/spec.go
+++ b/internal/spec/spec.go
@@ -1496,6 +1496,13 @@ func (s *APISpec) Validate() error {
if s.Name == "" {
return fmt.Errorf("name is required")
}
+ if !naming.IsSlug(s.Name) {
+ suggestion := naming.Slug(s.Name)
+ if suggestion == "" {
+ return fmt.Errorf("spec name must be a kebab-case slug (got %q)", s.Name)
+ }
+ return fmt.Errorf("spec name must be a kebab-case slug (got %q); try %q", s.Name, suggestion)
+ }
// Note: s.Version holds the API version from the spec (for provenance).
// The CLI version is always hardcoded to "1.0.0" in the generated root.go
// template — it is independent of the API version.
diff --git a/internal/spec/spec_test.go b/internal/spec/spec_test.go
index 97496d47..8ce18939 100644
--- a/internal/spec/spec_test.go
+++ b/internal/spec/spec_test.go
@@ -125,6 +125,66 @@ resources:
assert.Equal(t, []string{"c"}, param.Aliases)
}
+func TestValidateNameRequiresKebabSlug(t *testing.T) {
+ baseSpec := func(name string) []byte {
+ return []byte(`
+name: ` + name + `
+base_url: https://api.example.com
+auth:
+ type: none
+resources:
+ items:
+ endpoints:
+ list:
+ method: GET
+ path: /items
+`)
+ }
+
+ tests := []struct {
+ name string
+ spec []byte
+ wantErr string
+ }{
+ {
+ name: "spaces",
+ spec: baseSpec("NSE India"),
+ wantErr: `spec name must be a kebab-case slug (got "NSE India"); try "nse-india"`,
+ },
+ {
+ name: "multi-word brand",
+ spec: baseSpec("Google Flights"),
+ wantErr: `spec name must be a kebab-case slug (got "Google Flights"); try "google-flights"`,
+ },
+ {
+ name: "trailing hyphen",
+ spec: baseSpec("google-flights-"),
+ wantErr: `spec name must be a kebab-case slug (got "google-flights-"); try "google-flights"`,
+ },
+ {
+ name: "doubled hyphen",
+ spec: baseSpec("google--flights"),
+ wantErr: `spec name must be a kebab-case slug (got "google--flights"); try "google-flights"`,
+ },
+ {
+ name: "valid slug",
+ spec: baseSpec("nse-india"),
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ _, err := ParseBytes(tt.spec)
+ if tt.wantErr == "" {
+ require.NoError(t, err)
+ return
+ }
+ require.Error(t, err)
+ assert.Contains(t, err.Error(), tt.wantErr)
+ })
+ }
+}
+
func TestParamPublicInputName(t *testing.T) {
assert.Equal(t, "address", Param{Name: "s", FlagName: "address"}.PublicInputName())
assert.Equal(t, "store_code", Param{Name: "store_code"}.PublicInputName())
← b4bdcf68 fix(cli): handle wrapped paginated responses (#731)
·
back to Cli Printing Press
·
fix(cli): preserve browser-sniffed request defaults (#737) 8721df4b →