fix(cli): stop GraphQL false-positives on description prose (#4486)

* fix(cli): stop GraphQL false-positives on description prose

Treat authored internal YAML and OpenAPI as those formats even when
description text contains "type <word>" or "type Query". Report the
structured-spec parse error instead of a GraphQL root-type miss.

Closes #4451

Co-authored-by: Trevin Chow <tmchow@users.noreply.github.com>

* fix(cli): keep GraphQL SDL with name/resources fields

Require a YAML scalar name and a nested resources mapping so unindented
GraphQL fields named name and resources are not classified as internal YAML.

Refs: #4451

Co-authored-by: Trevin Chow <tmchow@users.noreply.github.com>

* fix(cli): treat flow-style resources as internal YAML

Accept resources: { ... } as a YAML mapping so a block-scalar description
that happens to contain type Query still parses as internal YAML, not GraphQL.

Refs: #4451

Co-authored-by: Trevin Chow <tmchow@users.noreply.github.com>

* fix(cli): require a YAML key after resources

An indented closing brace after GraphQL `resources: [Type]!` no longer
counts as an internal-YAML mapping. Flow-style `resources: {payments:`
and block `resources:\n  payments:` still do.

Refs: #4451

Co-authored-by: Trevin Chow <tmchow@users.noreply.github.com>

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Trevin Chow <tmchow@users.noreply.github.com>
This commit is contained in:
Trevin Chow
2026-09-01 06:18:12 -07:00
committed by GitHub
parent 0282159de3
commit c6d48489fd
12 changed files with 439 additions and 44 deletions
+7 -12
View File
@@ -152,25 +152,20 @@ func detectFormat(data []byte, path string) string {
s := string(data)
lowerPath := strings.ToLower(path)
// GraphQL SDL detection.
if strings.HasSuffix(lowerPath, ".graphql") || strings.HasSuffix(lowerPath, ".gql") {
return "graphql"
}
if strings.Contains(s, "type Query") || strings.Contains(s, "type Mutation") {
return "graphql"
}
// OpenAPI detection.
if strings.Contains(s, "openapi:") || strings.Contains(s, "\"openapi\"") ||
strings.Contains(s, "swagger:") || strings.Contains(s, "\"swagger\"") {
if openapi.IsOpenAPI(data) {
return "openapi"
}
// Internal spec detection.
if spec.LooksLikeInternalYAML(data) {
return "internal"
}
if graphql.IsGraphQLSDL(data) {
return "graphql"
}
if strings.Contains(s, "base_url:") || strings.Contains(s, "resources:") {
return "internal"
}
// Default to OpenAPI.
return "openapi"
}
+1 -9
View File
@@ -6,7 +6,6 @@ import (
"os"
"strings"
"github.com/mvanhorn/cli-printing-press/v4/internal/graphql"
"github.com/mvanhorn/cli-printing-press/v4/internal/openapi"
"github.com/mvanhorn/cli-printing-press/v4/internal/pipeline"
"github.com/mvanhorn/cli-printing-press/v4/internal/spec"
@@ -96,14 +95,7 @@ func parsePublicParamAuditSpec(specFiles []string, cliName string, opts openapi.
return nil, &ExitError{Code: ExitSpecError, Err: fmt.Errorf("reading spec %s: %w", specFile, err)}
}
var apiSpec *spec.APISpec
if openapi.IsOpenAPI(data) {
apiSpec, err = parseOpenAPISpec(specFile, data, opts)
} else if graphql.IsGraphQLSDL(data) {
apiSpec, err = graphql.ParseSDLBytes(specFile, data)
} else {
apiSpec, err = spec.ParseBytes(data)
}
apiSpec, err := parseSpecBytes(specFile, data, opts)
if err != nil {
return nil, &ExitError{Code: ExitSpecError, Err: fmt.Errorf("parsing spec %s: %w", specFile, err)}
}
+5 -15
View File
@@ -24,8 +24,6 @@ import (
"github.com/mvanhorn/cli-printing-press/v4/internal/devicespec"
"github.com/mvanhorn/cli-printing-press/v4/internal/docspec"
"github.com/mvanhorn/cli-printing-press/v4/internal/generator"
"github.com/mvanhorn/cli-printing-press/v4/internal/googlediscovery"
"github.com/mvanhorn/cli-printing-press/v4/internal/graphql"
"github.com/mvanhorn/cli-printing-press/v4/internal/llm"
"github.com/mvanhorn/cli-printing-press/v4/internal/llmpolish"
"github.com/mvanhorn/cli-printing-press/v4/internal/naming"
@@ -420,19 +418,11 @@ func newGenerateCmd() *cobra.Command {
specRawBytes = append(specRawBytes, data)
var apiSpec *spec.APISpec
if openapi.IsOpenAPI(data) {
apiSpec, err = parseOpenAPISpec(specFile, data, openapi.ParseOptions{
Lenient: lenient,
StrictRefs: strictRefs,
AuthPreference: openAPIParseAuthPref,
})
} else if graphql.IsGraphQLSDL(data) {
apiSpec, err = graphql.ParseSDLBytes(specFile, data)
} else if googlediscovery.IsDiscovery(data) {
apiSpec, err = googlediscovery.Parse(specFile, data)
} else {
apiSpec, err = spec.ParseBytes(data)
}
apiSpec, err = parseSpecBytes(specFile, data, openapi.ParseOptions{
Lenient: lenient,
StrictRefs: strictRefs,
AuthPreference: openAPIParseAuthPref,
})
if err != nil {
return &ExitError{Code: ExitSpecError, Err: fmt.Errorf("parsing spec %s: %w", specFile, err)}
}
+28
View File
@@ -0,0 +1,28 @@
package cli
import (
"github.com/mvanhorn/cli-printing-press/v4/internal/googlediscovery"
"github.com/mvanhorn/cli-printing-press/v4/internal/graphql"
"github.com/mvanhorn/cli-printing-press/v4/internal/openapi"
"github.com/mvanhorn/cli-printing-press/v4/internal/spec"
)
// parseSpecBytes routes spec bytes to the parser for the format the author
// supplied. Internal YAML and OpenAPI win over GraphQL SDL detection so a
// failed structured-spec parse is reported as that failure, not as a
// GraphQL root-type miss triggered by description prose.
func parseSpecBytes(specFile string, data []byte, opts openapi.ParseOptions) (*spec.APISpec, error) {
if openapi.IsOpenAPI(data) {
return parseOpenAPISpec(specFile, data, opts)
}
if spec.LooksLikeInternalYAML(data) {
return spec.ParseBytes(data)
}
if graphql.IsGraphQLSDL(data) {
return graphql.ParseSDLBytes(specFile, data)
}
if googlediscovery.IsDiscovery(data) {
return googlediscovery.Parse(specFile, data)
}
return spec.ParseBytes(data)
}
+188
View File
@@ -0,0 +1,188 @@
package cli
import (
"os"
"path/filepath"
"testing"
"github.com/mvanhorn/cli-printing-press/v4/internal/openapi"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
const internalYAMLTypeProse = `name: typeprose
description: Payments API
version: 0.1.0
base_url: https://api.example.com
auth:
type: none
config:
format: toml
path: ~/.config/typeprose-pp-cli/config.toml
resources:
payments:
description: Manage payments
endpoints:
list:
method: GET
path: /payments
description: List payments
params:
- name: payment_type
type: string
description: Free-text payment type label. The accepted type Query and scalar value set is undocumented.
`
const internalYAMLTypeProseBroken = `name: typeprose
description: Payments API
version: 0.1.0
base_url: https://api.example.com
auth:
type: none
resources:
payments:
endpoints:
list:
method: GET
path: /payments
params:
- name: payment_type
description: Free-text payment type label. The accepted type Query and scalar value set is undocumented.
resources:
other:
endpoints:
list:
method: GET
path: /other
`
func TestParseSpecBytesInternalYAMLWithTypeProse(t *testing.T) {
t.Parallel()
parsed, err := parseSpecBytes("internal-spec.yaml", []byte(internalYAMLTypeProse), openapi.ParseOptions{})
require.NoError(t, err)
require.NotNil(t, parsed)
assert.Equal(t, "typeprose", parsed.Name)
require.Contains(t, parsed.Resources, "payments")
assert.Equal(t, "Free-text payment type label. The accepted type Query and scalar value set is undocumented.",
parsed.Resources["payments"].Endpoints["list"].Params[0].Description)
}
func TestParseSpecBytesInternalYAMLStructuralErrorNotGraphQL(t *testing.T) {
t.Parallel()
_, err := parseSpecBytes("internal-spec.yaml", []byte(internalYAMLTypeProseBroken), openapi.ParseOptions{})
require.Error(t, err)
assert.Contains(t, err.Error(), "spec structural error")
assert.Contains(t, err.Error(), "duplicate top-level key")
assert.NotContains(t, err.Error(), "GraphQL")
assert.NotContains(t, err.Error(), "no GraphQL root operation types found")
}
func TestParseSpecBytesOpenAPIErrorNotGraphQL(t *testing.T) {
t.Parallel()
broken := []byte(`openapi: "3.0.3"
info:
title: Broken Payments
description: Free-text payment type label mentioning type Query
paths: {}
`)
_, err := parseSpecBytes("openapi.yaml", broken, openapi.ParseOptions{})
require.Error(t, err)
assert.NotContains(t, err.Error(), "no GraphQL root operation types found")
}
func TestParseSpecBytesFlowStyleResourcesWithTypeProse(t *testing.T) {
t.Parallel()
data := []byte(`name: flowprose
description: |
type Query {
ignored: String
}
version: 0.1.0
base_url: https://api.example.com
auth:
type: none
resources: {payments: {description: Manage payments, endpoints: {list: {method: GET, path: /payments}}}}
`)
parsed, err := parseSpecBytes("internal-spec.yaml", data, openapi.ParseOptions{})
require.NoError(t, err)
require.NotNil(t, parsed)
assert.Equal(t, "flowprose", parsed.Name)
require.Contains(t, parsed.Resources, "payments")
}
func TestParseSpecBytesGraphQLFieldsWithIndentedCloser(t *testing.T) {
t.Parallel()
sdl := []byte("type Query {\nname: String\nresources: [Widget!]!\nwidget(id: ID!): Widget!\n }\n\ntype Widget {\n id: ID!\n name: String!\n}\n")
parsed, err := parseSpecBytes("schema.graphql", sdl, openapi.ParseOptions{})
require.NoError(t, err)
require.NotNil(t, parsed)
assert.NotEmpty(t, parsed.GraphQLEndpointPath)
}
func TestParseSpecBytesGraphQLFieldsNamedNameAndResources(t *testing.T) {
t.Parallel()
sdl := []byte("type Query {\nname: String\nresources: [Widget!]!\nwidget(id: ID!): Widget!\n}\n\ntype Widget {\n id: ID!\n name: String!\n}\n")
parsed, err := parseSpecBytes("schema.graphql", sdl, openapi.ParseOptions{})
require.NoError(t, err)
require.NotNil(t, parsed)
assert.NotEmpty(t, parsed.GraphQLEndpointPath)
assert.NotContains(t, parsed.Resources, "payments")
}
func TestParseSpecBytesGraphQLSDLStillParses(t *testing.T) {
t.Parallel()
sdl := []byte("type Query {\n widget(id: ID!): Widget!\n}\n\ntype Widget {\n id: ID!\n name: String!\n}\n")
parsed, err := parseSpecBytes("schema.graphql", sdl, openapi.ParseOptions{})
require.NoError(t, err)
require.NotNil(t, parsed)
assert.NotEmpty(t, parsed.GraphQLEndpointPath)
}
func TestGenerateCmdAcceptsInternalYAMLTypeProse(t *testing.T) {
t.Parallel()
dir := t.TempDir()
specPath := filepath.Join(dir, "internal-spec.yaml")
outputDir := filepath.Join(dir, "typeprose")
require.NoError(t, os.WriteFile(specPath, []byte(internalYAMLTypeProse), 0o644))
cmd := newGenerateCmd()
cmd.SetArgs([]string{
"--spec", specPath,
"--output", outputDir,
"--validate=false",
"--force",
})
require.NoError(t, cmd.Execute())
assert.DirExists(t, outputDir)
}
func TestGenerateCmdReportsInternalYAMLErrorNotGraphQL(t *testing.T) {
t.Parallel()
dir := t.TempDir()
specPath := filepath.Join(dir, "internal-spec.yaml")
outputDir := filepath.Join(dir, "typeprose")
require.NoError(t, os.WriteFile(specPath, []byte(internalYAMLTypeProseBroken), 0o644))
cmd := newGenerateCmd()
cmd.SetArgs([]string{
"--spec", specPath,
"--output", outputDir,
"--validate=false",
"--force",
})
err := cmd.Execute()
require.Error(t, err)
assert.Contains(t, err.Error(), specPath)
assert.Contains(t, err.Error(), "spec structural error")
assert.NotContains(t, err.Error(), "no GraphQL root operation types found")
assert.NoDirExists(t, outputDir)
}
+27 -8
View File
@@ -1,6 +1,7 @@
package graphql
import (
"bytes"
"fmt"
"os"
"path/filepath"
@@ -27,6 +28,13 @@ var (
// mistaken for the schema definition.
schemaBlockRE = regexp.MustCompile(`(?s)\bschema\b[^{:}]*\{(.*?)\}`)
schemaOpRE = regexp.MustCompile(`(?m)^\s*(query|mutation|subscription)\s*:\s*([A-Za-z_][A-Za-z0-9_]*)`)
// Line-anchored SDL constructs. Substring matches like `type Query` or
// `type ` inside YAML/OpenAPI description prose must not count.
sdlQueryTypeRE = regexp.MustCompile(`(?m)^\s*(?:extend\s+)?type\s+Query\b`)
sdlMutationTypeRE = regexp.MustCompile(`(?m)^\s*(?:extend\s+)?type\s+Mutation\b`)
sdlTypeDeclRE = regexp.MustCompile(`(?m)^\s*(?:extend\s+)?type\s+[A-Za-z_]`)
sdlScalarDeclRE = regexp.MustCompile(`(?m)^\s*scalar\s+[A-Za-z_]`)
sdlSchemaBlockRE = regexp.MustCompile(`(?ms)^\s*schema\b[^{:}]*\{(.*?)\}`)
)
type gqlType struct {
@@ -63,19 +71,30 @@ func ParseSDLBytes(source string, data []byte) (*spec.APISpec, error) {
// IsGraphQLSDL checks if the data looks like a GraphQL schema.
func IsGraphQLSDL(data []byte) bool {
// Authored structured specs win even when description prose mentions
// `type Query`, `type <word>`, or `scalar`. Falling through would hide
// the real OpenAPI/internal-YAML parse error behind a GraphQL message.
if spec.LooksLikeInternalYAML(data) || looksLikeOpenAPIDocument(data) {
return false
}
s := string(data)
if strings.Contains(s, "type Query") || strings.Contains(s, "type Mutation") {
if sdlQueryTypeRE.MatchString(s) || sdlMutationTypeRE.MatchString(s) {
return true
}
// A `schema { query: ... }` block that maps a root operation is an
// unambiguous GraphQL schema definition even when the schema aliases its
// roots and defines no scalars. Requiring the operation mapping (not just
// the keyword) keeps a literal "schema {" inside an OpenAPI description
// from being misclassified.
if m := schemaBlockRE.FindStringSubmatch(s); m != nil && schemaOpRE.MatchString(m[1]) {
// A line-anchored `schema { query: ... }` block that maps a root
// operation is an unambiguous GraphQL schema definition even when the
// schema aliases its roots and defines no scalars.
if m := sdlSchemaBlockRE.FindStringSubmatch(s); m != nil && schemaOpRE.MatchString(m[1]) {
return true
}
return strings.Contains(s, "type ") && strings.Contains(s, "scalar ")
return sdlTypeDeclRE.MatchString(s) && sdlScalarDeclRE.MatchString(s)
}
func looksLikeOpenAPIDocument(data []byte) bool {
return bytes.Contains(data, []byte(`"openapi"`)) ||
bytes.Contains(data, []byte(`"swagger"`)) ||
bytes.Contains(data, []byte("openapi:")) ||
bytes.Contains(data, []byte("swagger:"))
}
func parseSDLContent(source, raw string) (*spec.APISpec, error) {
+42
View File
@@ -166,9 +166,51 @@ func TestIsGraphQLSDLDetectsCustomRootSchema(t *testing.T) {
// Conventional and clearly-non-GraphQL inputs are unchanged.
assert.True(t, IsGraphQLSDL([]byte("type Query {\n me: User\n}\n")))
assert.True(t, IsGraphQLSDL([]byte("extend type Query {\n extra: String\n}\n")))
assert.True(t, IsGraphQLSDL([]byte("type User {\n name: String\n}\n\nscalar DateTime\n")))
assert.False(t, IsGraphQLSDL([]byte(`{"openapi":"3.0.0","paths":{"/x":{"get":{"responses":{}}}}}`)))
}
func TestIsGraphQLSDLIgnoresDescriptionProse(t *testing.T) {
t.Parallel()
internalWithTypeProse := []byte(`name: payments
base_url: https://api.example.com
resources:
payments:
endpoints:
list:
method: GET
path: /payments
params:
- name: payment_type
description: Free-text payment type label. The accepted type Query and scalar value set is undocumented.
`)
assert.False(t, IsGraphQLSDL(internalWithTypeProse), "internal YAML must not be GraphQL just because descriptions mention type/scalar")
openapiWithTypeProse := []byte(`openapi: 3.0.0
info:
title: Payments
description: Free-text payment type label mentioning type Query
paths:
/payments:
get:
responses:
"200":
description: OK
`)
assert.False(t, IsGraphQLSDL(openapiWithTypeProse), "OpenAPI must not be GraphQL because descriptions mention type Query")
gqlWithNameAndResourcesFields := []byte("type Query {\nname: String\nresources: [Widget!]!\n}\n\ntype Widget {\n id: ID!\n}\n")
assert.True(t, IsGraphQLSDL(gqlWithNameAndResourcesFields), "unindented GraphQL fields named name/resources are still SDL")
gqlWithIndentedCloser := []byte("type Query {\nname: String\nresources: [Widget!]!\n }\n\ntype Widget {\n id: ID!\n}\n")
assert.True(t, IsGraphQLSDL(gqlWithIndentedCloser), "indented closing brace after resources: [Type] is still SDL")
flowInternalWithSDLProse := []byte("name: payments\ndescription: |\n type Query {\n ignored: String\n }\nbase_url: https://api.example.com\nresources: {payments: {}}\n")
assert.False(t, IsGraphQLSDL(flowInternalWithSDLProse), "flow-style internal YAML stays internal even when a description block contains type Query")
}
func TestParseSDLMissingRootOperations(t *testing.T) {
// A schema block that aliases the query root to a type that is never
// defined has no discoverable operations; the parser must say so clearly
+3
View File
@@ -1452,6 +1452,9 @@ func detectSpecFormat(data []byte) string {
if openapi.IsOpenAPI(data) {
return "openapi3"
}
if spec.LooksLikeInternalYAML(data) {
return "internal"
}
if openapi.IsGraphQLSDL(data) {
return "graphql"
}
+5
View File
@@ -2276,6 +2276,11 @@ func TestDetectSpecFormat(t *testing.T) {
data: []byte("name: test\nbase_url: https://api.example.com"),
expected: "internal",
},
{
name: "internal spec with type prose in description",
data: []byte("name: payments\nbase_url: https://api.example.com\nresources:\n payments:\n endpoints:\n list:\n method: GET\n path: /payments\n params:\n - name: payment_type\n description: Free-text payment type label mentioning type Query and a scalar value\n"),
expected: "internal",
},
{
name: "empty",
data: []byte{},
+3
View File
@@ -566,6 +566,9 @@ func loadArchivedSpec(cliDir string) (*spec.APISpec, error) {
if openapi.IsOpenAPI(data) {
return openapi.ParseWithPathLenient(data, path)
}
if spec.LooksLikeInternalYAML(data) {
return spec.ParseBytes(data)
}
if graphql.IsGraphQLSDL(data) {
return graphql.ParseSDLBytes(path, data)
}
+40
View File
@@ -0,0 +1,40 @@
package spec
import (
"regexp"
)
var (
topLevelYAMLKeyRE = regexp.MustCompile(`(?m)^([A-Za-z_][A-Za-z0-9_-]*)\s*:`)
// GraphQL fields may be written unindented (`name: String`), so a bare
// key scan is not enough. An internal spec names the API with a scalar
// and nests resources as a YAML mapping (block key or nonempty flow
// `{payments:`). An indented `}` after `resources: [Type]!` is SDL.
yamlNameScalarRE = regexp.MustCompile(`(?m)^name:\s+(?:"[^"]+"|'[^']+'|[A-Za-z][A-Za-z0-9._-]*)\s*$`)
yamlResourcesMapRE = regexp.MustCompile(`(?m)^resources:\s*(?:\{[ \t]*[A-Za-z_]|(?:#.*)?\n[ \t]+[A-Za-z_][A-Za-z0-9_-]*\s*:)`)
)
// LooksLikeInternalYAML reports whether data is an authored internal YAML spec
// rather than OpenAPI or GraphQL SDL. Description prose is ignored because
// only top-level YAML structure is scanned.
func LooksLikeInternalYAML(data []byte) bool {
if len(data) == 0 {
return false
}
keys := topLevelYAMLKeys(data)
if keys["openapi"] || keys["swagger"] {
return false
}
if !keys["name"] || !keys["resources"] {
return false
}
return yamlNameScalarRE.Match(data) && yamlResourcesMapRE.Match(data)
}
func topLevelYAMLKeys(data []byte) map[string]bool {
found := make(map[string]bool)
for _, m := range topLevelYAMLKeyRE.FindAllSubmatch(data, -1) {
found[string(m[1])] = true
}
return found
}
+90
View File
@@ -0,0 +1,90 @@
package spec
import (
"testing"
"github.com/stretchr/testify/assert"
)
func TestLooksLikeInternalYAML(t *testing.T) {
t.Parallel()
tests := []struct {
name string
data []byte
want bool
}{
{
name: "minimal internal yaml",
data: []byte("name: payments\nbase_url: https://api.example.com\nresources:\n payments: {}\n"),
want: true,
},
{
name: "document start marker then name",
data: []byte("---\nname: payments\nresources:\n payments: {}\n"),
want: true,
},
{
name: "comment preamble",
data: []byte("# header\n\nname: payments\nresources:\n payments: {}\n"),
want: true,
},
{
name: "type and scalar only in description prose",
data: []byte("name: payments\nbase_url: https://api.example.com\nresources:\n payments:\n endpoints:\n list:\n method: GET\n path: /payments\n params:\n - name: payment_type\n description: Free-text payment type label. Also a scalar value.\n"),
want: true,
},
{
name: "missing resources",
data: []byte("name: payments\nbase_url: https://api.example.com\n"),
want: false,
},
{
name: "missing name",
data: []byte("base_url: https://api.example.com\nresources:\n payments: {}\n"),
want: false,
},
{
name: "openapi yaml",
data: []byte("openapi: 3.0.0\ninfo:\n title: Test\n"),
want: false,
},
{
name: "graphql sdl",
data: []byte("type Query {\n hello: String\n}\n"),
want: false,
},
{
name: "graphql fields named name and resources",
data: []byte("type Query {\nname: String\nresources: [Widget!]!\n}\n\ntype Widget {\n id: ID!\n}\n"),
want: false,
},
{
name: "graphql fields with indented closing brace",
data: []byte("type Query {\nname: String\nresources: [Widget!]!\n }\n\ntype Widget {\n id: ID!\n}\n"),
want: false,
},
{
name: "flow-style resources mapping",
data: []byte("name: payments\nbase_url: https://api.example.com\nresources: {payments: {endpoints: {list: {method: GET, path: /payments}}}}\n"),
want: true,
},
{
name: "flow-style resources with line-anchored type Query in description",
data: []byte("name: payments\ndescription: |\n type Query {\n ignored: String\n }\nresources: {payments: {}}\n"),
want: true,
},
{
name: "empty",
data: []byte{},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
assert.Equal(t, tt.want, LooksLikeInternalYAML(tt.data))
})
}
}