Skip to content

Commit b389607

Browse files
committed
fix(graphql): generate valid operations against large schemas
The selection-set builder emitted composite fields with no sub-selection (when it hit the depth cap or cycle guard) and selected nested fields whose required arguments it could not fill. Against a rich schema like GitLab's, every generated tool over a non-trivial return type failed server-side validation. buildSelectionSet now skips nested fields with a required argument it cannot supply, and drops composite fields whose sub-selection comes back empty instead of printing them bare. Adds a regression test that drives the plugin end to end against a GitLab-shaped graphql-js server. Fixes #1146
1 parent eb9ed89 commit b389607

4 files changed

Lines changed: 192 additions & 7 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@executor-js/plugin-graphql": patch
3+
---
4+
5+
Fix the GraphQL plugin generating invalid operations against large schemas. The auto-generated selection set no longer emits composite (object/connection) fields without a sub-selection, and no longer selects nested fields whose required arguments it cannot supply. Generated tools now validate against rich schemas (such as GitLab's) instead of failing on every call against a rich return type.

packages/plugins/graphql/src/sdk/plugin.test.ts

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import { endpointForTelemetry } from "./invoke";
2828
import { introspect } from "./introspect";
2929
import type { IntrospectionResult } from "./introspect";
3030
import {
31+
makeGitlab1146Schema,
3132
makeGreetingGraphqlSchema,
3233
serveGraphqlFailureTestServer,
3334
serveGraphqlTestServer,
@@ -885,3 +886,71 @@ describe("graphqlPlugin detect URL-token fallback", () => {
885886
}),
886887
);
887888
});
889+
890+
// Issue #1146: against a large, real-world schema (GitLab) the auto-generated
891+
// operations were invalid GraphQL and every call against a rich object type
892+
// failed validation. These drive the real plugin (live introspection -> operation
893+
// generation -> invocation) against a GitLab-shaped graphql-yoga server, which
894+
// validates with graphql-js exactly like the @emulators/gitlab surface. A clean
895+
// `ok: true` therefore proves the generated operation is valid GraphQL.
896+
describe("graphqlPlugin generates valid operations against rich schemas (#1146)", () => {
897+
const gitlabServer = serveGraphqlTestServer({ schema: makeGitlab1146Schema() });
898+
899+
const lastQuery = (requests: { readonly payload: { readonly query?: string } }[]): string =>
900+
requests[requests.length - 1]?.payload.query ?? "";
901+
902+
it.effect("query.metadata: drops the nested field whose required argument it cannot fill", () =>
903+
Effect.gen(function* () {
904+
const server = yield* gitlabServer;
905+
const executor = yield* makeExecutor();
906+
907+
yield* executor.graphql.addIntegration({ endpoint: server.endpoint, slug: "gitlab" });
908+
yield* createOrgConnection(executor, {
909+
integration: "gitlab",
910+
name: "main",
911+
template: "none",
912+
value: "unused",
913+
});
914+
915+
yield* server.clearRequests;
916+
const result = yield* executor.execute(toolAddr("gitlab", "main", "query.metadata"), {});
917+
918+
// Valid GraphQL: the server accepts and resolves it (no validation errors).
919+
expect(result).toMatchObject({ ok: true });
920+
// `featureFlags(names: [String!]!)` has a required arg the generator cannot
921+
// supply, so it is omitted rather than emitted without arguments.
922+
const query = lastQuery(yield* server.requests);
923+
expect(query).not.toContain("featureFlags");
924+
// Fields it CAN select are still present (no over-pruning).
925+
expect(query).toContain("kas {");
926+
}),
927+
);
928+
929+
it.effect("query.currentUser: never emits a composite field without a sub-selection", () =>
930+
Effect.gen(function* () {
931+
const server = yield* gitlabServer;
932+
const executor = yield* makeExecutor();
933+
934+
yield* executor.graphql.addIntegration({ endpoint: server.endpoint, slug: "gitlab2" });
935+
yield* createOrgConnection(executor, {
936+
integration: "gitlab2",
937+
name: "main",
938+
template: "none",
939+
value: "unused",
940+
});
941+
942+
yield* server.clearRequests;
943+
const result = yield* executor.execute(toolAddr("gitlab2", "main", "query.currentUser"), {});
944+
945+
expect(result).toMatchObject({ ok: true });
946+
const query = lastQuery(yield* server.requests);
947+
// The connection is traversed (proves recursion still works)...
948+
expect(query).toContain("mergeRequests {");
949+
expect(query).toContain("nodes {");
950+
// ...but composite leaves past the depth cap (`author`, `assignees`) are
951+
// dropped instead of being emitted bare, which the server would reject.
952+
expect(query).not.toContain("author");
953+
expect(query).not.toContain("assignees");
954+
}),
955+
);
956+
});

packages/plugins/graphql/src/sdk/plugin.ts

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import {
3838
type IntrospectionResult,
3939
type IntrospectionType,
4040
type IntrospectionField,
41+
type IntrospectionInputValue,
4142
type IntrospectionTypeRef,
4243
} from "./introspect";
4344
import { extract } from "./extract";
@@ -217,13 +218,40 @@ const unwrapTypeName = (ref: IntrospectionTypeRef): string => {
217218
return "Unknown";
218219
};
219220

221+
// Bound the auto-generated selection so a huge schema (GitLab has 4000+ types)
222+
// does not produce an unbounded operation: cap recursion depth and the number
223+
// of fields emitted per level.
224+
const MAX_SELECTION_DEPTH = 2;
225+
const MAX_FIELDS_PER_LEVEL = 12;
226+
227+
// Composite (output) types require a sub-selection; leaves (scalars/enums) must
228+
// not have one. Anything we cannot resolve in the type map is treated as a leaf
229+
// (custom scalars live in the map as SCALAR; truly-unknown types are rare).
230+
const isCompositeType = (
231+
ref: IntrospectionTypeRef,
232+
types: ReadonlyMap<string, IntrospectionType>,
233+
): boolean => {
234+
const kind = types.get(unwrapTypeName(ref))?.kind;
235+
return kind === "OBJECT" || kind === "INTERFACE" || kind === "UNION";
236+
};
237+
238+
// A nested field whose argument is non-null without a default cannot be selected
239+
// by the generator: it has no value to pass and emitting the field without the
240+
// argument is invalid (e.g. GitLab's `metadata.featureFlags(names:)`). Required
241+
// root-field arguments are different: those are threaded as operation variables
242+
// (and surfaced on the tool's input schema) in `buildOperationStringForField`.
243+
const hasRequiredArgWithoutDefault = (field: IntrospectionField): boolean =>
244+
field.args.some(
245+
(arg: IntrospectionInputValue) => arg.type.kind === "NON_NULL" && arg.defaultValue == null,
246+
);
247+
220248
const buildSelectionSet = (
221249
ref: IntrospectionTypeRef,
222250
types: ReadonlyMap<string, IntrospectionType>,
223251
depth: number,
224252
seen: Set<string>,
225253
): string => {
226-
if (depth > 2) return "";
254+
if (depth > MAX_SELECTION_DEPTH) return "";
227255

228256
const leafName = unwrapTypeName(ref);
229257
if (seen.has(leafName)) return "";
@@ -236,13 +264,25 @@ const buildSelectionSet = (
236264

237265
seen.add(leafName);
238266

239-
const subFields = objectType.fields
240-
.filter((f: IntrospectionField) => !f.name.startsWith("__"))
241-
.slice(0, 12)
242-
.map((f: IntrospectionField) => {
267+
const subFields: string[] = [];
268+
for (const f of objectType.fields) {
269+
if (subFields.length >= MAX_FIELDS_PER_LEVEL) break;
270+
if (f.name.startsWith("__")) continue;
271+
// Cannot synthesize a value for a required nested argument: skip the field
272+
// rather than emit an invalid selection.
273+
if (hasRequiredArgWithoutDefault(f)) continue;
274+
275+
if (isCompositeType(f.type, types)) {
276+
// A composite field MUST have a sub-selection. When the recursion yields
277+
// nothing (depth cap, cycle guard, or every child skipped) we drop the
278+
// field instead of emitting it bare (which the server rejects).
243279
const sub = buildSelectionSet(f.type, types, depth + 1, seen);
244-
return sub ? `${f.name} ${sub}` : f.name;
245-
});
280+
if (sub) subFields.push(`${f.name} ${sub}`);
281+
} else {
282+
// Scalar / enum leaf: select it directly.
283+
subFields.push(f.name);
284+
}
285+
}
246286

247287
seen.delete(leafName);
248288

packages/plugins/graphql/src/testing/index.ts

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,77 @@ export const makeGreetingGraphqlSchema = (
251251
});
252252
};
253253

254+
// A small GitLab-shaped schema that reproduces the two failure modes from issue
255+
// #1146 when the plugin auto-generates selection sets against it:
256+
// 1. `metadata.featureFlags(names: [String!]!)` has a REQUIRED nested argument
257+
// the generator cannot fill.
258+
// 2. `currentUser` nests connections deep enough that composite leaf fields
259+
// (`author`, `assignees`) sit past the recursion depth cap, so a naive
260+
// generator emits them with no sub-selection.
261+
// graphql-yoga validates with graphql-js, exactly like the @emulators/gitlab
262+
// surface, so a generated operation that validates clean here is valid GraphQL.
263+
export const makeGitlab1146Schema = (): GraphQLSchema =>
264+
createSchema<GraphqlTestContext>({
265+
typeDefs: /* GraphQL */ `
266+
type Query {
267+
metadata: Metadata
268+
currentUser: User
269+
}
270+
271+
type Metadata {
272+
version: String
273+
revision: String
274+
enterprise: Boolean
275+
featureFlags(names: [String!]!): [FeatureFlag!]!
276+
kas: Kas
277+
}
278+
279+
type FeatureFlag {
280+
name: String
281+
enabled: Boolean
282+
}
283+
284+
type Kas {
285+
enabled: Boolean
286+
externalUrl: String
287+
}
288+
289+
type User {
290+
active: Boolean
291+
admin: Boolean
292+
mergeRequests: MergeRequestConnection
293+
}
294+
295+
type MergeRequestConnection {
296+
count: Int
297+
nodes: [MergeRequest!]
298+
pageInfo: PageInfo
299+
}
300+
301+
type PageInfo {
302+
hasNextPage: Boolean
303+
endCursor: String
304+
}
305+
306+
type MergeRequest {
307+
id: ID
308+
title: String
309+
author: Author
310+
assignees: AssigneeConnection
311+
}
312+
313+
type Author {
314+
name: String
315+
username: String
316+
}
317+
318+
type AssigneeConnection {
319+
count: Int
320+
nodes: [Author!]
321+
}
322+
`,
323+
});
324+
254325
export const TestLayers = {
255326
greeting: () => GraphqlTestServer.layer({ schema: makeGreetingGraphqlSchema() }),
256327
greetingWithOAuth: () =>

0 commit comments

Comments
 (0)