refactor(graphql): scalar-leaf default selection + caller-controlled select

Replaces the baked-in recursive selection with two pieces:

- Default selection is now scalar/enum leaves of the return type only. This is
  always valid (leaves need no sub-selection) and small enough to stay under a
  server's query-complexity budget. Composite fields, and fields whose required
  arguments cannot be synthesized, are no longer emitted bare or argument-less.
- Tools expose an optional `select` input carrying a GraphQL selection set,
  spliced into the operation at invoke time, so callers request nested/list data
  (and supply nested required arguments) per call instead of inheriting one
  frozen, arbitrarily-bounded selection.

The old depth cap and first-12-fields truncation are gone: depth/breadth are now
the caller's choice via `select`, bounded by the server's own complexity limits.

Author: RhysSullivan

.changeset/graphql-valid-selections.md

@@ -2,4 +2,8 @@
 "@executor-js/plugin-graphql": patch
 ---
 
-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.
+Fix the GraphQL plugin generating invalid operations against large schemas, and make field selection caller-controlled instead of a baked-in guess.
+
+Previously each tool froze a recursive, depth- and count-bounded selection at sync time. Against a rich schema (GitLab) this produced invalid GraphQL (composite fields with no sub-selection, nested fields missing required arguments) so every call over a rich return type failed, and the arbitrary bound silently truncated which fields came back.
+
+Generated tools now default to selecting only scalar/enum leaf fields of the return type (always valid, always within a server's query-complexity budget), and expose an optional `select` input carrying a GraphQL selection set so a caller can request nested or list data per call (including supplying nested required arguments). Fixes #1146.

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

@@ -71,10 +71,23 @@ export const invoke = Effect.fn("GraphQL.invoke")(function* (
     Object.assign(variables, args.variables);
   }
 
+  // A caller-supplied `select` overrides the default scalar-leaf selection: it is
+  // spliced into the field's selection set so nested/list data can be requested
+  // per call. `select` is a control input, not a GraphQL variable, so it never
+  // enters `variables`. Falls back to the stored default operation when absent or
+  // when the binding predates the prefix/suffix split.
+  const customSelect = typeof args.select === "string" ? args.select.trim() : "";
+  const operationString =
+    customSelect.length > 0 &&
+    operation.operationPrefix != null &&
+    operation.operationSuffix != null
+      ? `${operation.operationPrefix} { ${customSelect} }${operation.operationSuffix}`
+      : operation.operationString;
+
   let request = HttpClientRequest.post(requestEndpoint).pipe(
     HttpClientRequest.setHeader("Content-Type", "application/json"),
     HttpClientRequest.bodyJsonUnsafe({
-      query: operation.operationString,
+      query: operationString,
       variables: Object.keys(variables).length > 0 ? variables : undefined,
     }),
   );

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

@@ -889,68 +889,106 @@ describe("graphqlPlugin detect URL-token fallback", () => {
 
 // Issue #1146: against a large, real-world schema (GitLab) the auto-generated
 // operations were invalid GraphQL and every call against a rich object type
-// failed validation. These drive the real plugin (live introspection -> operation
-// generation -> invocation) against a GitLab-shaped graphql-yoga server, which
-// validates with graphql-js exactly like the @emulators/gitlab surface. A clean
-// `ok: true` therefore proves the generated operation is valid GraphQL.
+// failed validation. The plugin now defaults to a scalar-leaf selection (always
+// valid, always cheap) and lets the caller pass an explicit `select` for nested
+// or list data. These drive the real plugin (live introspection -> generation ->
+// invocation) against a GitLab-shaped graphql-yoga server, which validates with
+// graphql-js exactly like the @emulators/gitlab surface, so a clean `ok: true`
+// proves the operation that went over the wire is valid GraphQL.
 describe("graphqlPlugin generates valid operations against rich schemas (#1146)", () => {
   const gitlabServer = serveGraphqlTestServer({ schema: makeGitlab1146Schema() });
 
   const lastQuery = (requests: { readonly payload: { readonly query?: string } }[]): string =>
     requests[requests.length - 1]?.payload.query ?? "";
 
-  it.effect("query.metadata: drops the nested field whose required argument it cannot fill", () =>
+  const setup = (slug: string) =>
     Effect.gen(function* () {
       const server = yield* gitlabServer;
       const executor = yield* makeExecutor();
-
-      yield* executor.graphql.addIntegration({ endpoint: server.endpoint, slug: "gitlab" });
+      yield* executor.graphql.addIntegration({ endpoint: server.endpoint, slug });
       yield* createOrgConnection(executor, {
-        integration: "gitlab",
+        integration: slug,
         name: "main",
         template: "none",
         value: "unused",
       });
+      yield* server.clearRequests;
+      return { server, executor };
+    });
 
+  it.effect("default selection is scalar leaves only: valid, and never bare composites", () =>
+    Effect.gen(function* () {
+      const { server, executor } = yield* setup("gitlab_default");
+
+      // metadata: scalar leaves only. `featureFlags` (required arg) and `kas`
+      // (composite) are omitted rather than emitted invalidly.
+      const meta = yield* executor.execute(
+        toolAddr("gitlab_default", "main", "query.metadata"),
+        {},
+      );
+      expect(meta).toMatchObject({ ok: true });
+      const metaQuery = lastQuery(yield* server.requests);
+      expect(metaQuery).toContain("version");
+      expect(metaQuery).not.toContain("featureFlags");
+      expect(metaQuery).not.toContain("kas");
+
+      // currentUser: scalar leaves only. No `mergeRequests` (composite) bare.
       yield* server.clearRequests;
-      const result = yield* executor.execute(toolAddr("gitlab", "main", "query.metadata"), {});
+      const user = yield* executor.execute(
+        toolAddr("gitlab_default", "main", "query.currentUser"),
+        {},
+      );
+      expect(user).toMatchObject({ ok: true });
+      const userQuery = lastQuery(yield* server.requests);
+      expect(userQuery).toContain("active");
+      expect(userQuery).not.toContain("mergeRequests");
+    }),
+  );
+
+  it.effect("a caller-supplied `select` fetches nested/list data and stays valid", () =>
+    Effect.gen(function* () {
+      const { server, executor } = yield* setup("gitlab_select");
+
+      const result = yield* executor.execute(
+        toolAddr("gitlab_select", "main", "query.currentUser"),
+        { select: "active mergeRequests { count nodes { id title author { name } } }" },
+      );
 
-      // Valid GraphQL: the server accepts and resolves it (no validation errors).
       expect(result).toMatchObject({ ok: true });
-      // `featureFlags(names: [String!]!)` has a required arg the generator cannot
-      // supply, so it is omitted rather than emitted without arguments.
       const query = lastQuery(yield* server.requests);
-      expect(query).not.toContain("featureFlags");
-      // Fields it CAN select are still present (no over-pruning).
-      expect(query).toContain("kas {");
+      expect(query).toContain("nodes {");
+      expect(query).toContain("author {");
     }),
   );
 
-  it.effect("query.currentUser: never emits a composite field without a sub-selection", () =>
+  it.effect("`select` can supply a nested field's required argument", () =>
     Effect.gen(function* () {
-      const server = yield* gitlabServer;
-      const executor = yield* makeExecutor();
+      const { server, executor } = yield* setup("gitlab_ff");
 
-      yield* executor.graphql.addIntegration({ endpoint: server.endpoint, slug: "gitlab2" });
-      yield* createOrgConnection(executor, {
-        integration: "gitlab2",
-        name: "main",
-        template: "none",
-        value: "unused",
+      const result = yield* executor.execute(toolAddr("gitlab_ff", "main", "query.metadata"), {
+        select: 'version featureFlags(names: ["flag_a"]) { name enabled }',
       });
 
-      yield* server.clearRequests;
-      const result = yield* executor.execute(toolAddr("gitlab2", "main", "query.currentUser"), {});
-
       expect(result).toMatchObject({ ok: true });
-      const query = lastQuery(yield* server.requests);
-      // The connection is traversed (proves recursion still works)...
-      expect(query).toContain("mergeRequests {");
-      expect(query).toContain("nodes {");
-      // ...but composite leaves past the depth cap (`author`, `assignees`) are
-      // dropped instead of being emitted bare, which the server would reject.
-      expect(query).not.toContain("author");
-      expect(query).not.toContain("assignees");
+      expect(lastQuery(yield* server.requests)).toContain("featureFlags(names:");
+    }),
+  );
+
+  it.effect("an invalid `select` surfaces the server's validation error verbatim", () =>
+    Effect.gen(function* () {
+      const { executor } = yield* setup("gitlab_bad");
+
+      // `author` is a composite emitted bare: the plugin passes the selection
+      // through and surfaces the server's rejection rather than silently fixing
+      // or swallowing it.
+      const result = yield* executor.execute(toolAddr("gitlab_bad", "main", "query.currentUser"), {
+        select: "mergeRequests { nodes { id author } }",
+      });
+
+      expect(result).toMatchObject({
+        ok: false,
+        error: { code: "graphql_errors" },
+      });
     }),
   );
 });

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

@@ -218,12 +218,6 @@ const unwrapTypeName = (ref: IntrospectionTypeRef): string => {
   return "Unknown";
 };
 
-// Bound the auto-generated selection so a huge schema (GitLab has 4000+ types)
-// does not produce an unbounded operation: cap recursion depth and the number
-// of fields emitted per level.
-const MAX_SELECTION_DEPTH = 2;
-const MAX_FIELDS_PER_LEVEL = 12;
-
 // Composite (output) types require a sub-selection; leaves (scalars/enums) must
 // not have one. Anything we cannot resolve in the type map is treated as a leaf
 // (custom scalars live in the map as SCALAR; truly-unknown types are rare).
@@ -235,58 +229,48 @@ const isCompositeType = (
   return kind === "OBJECT" || kind === "INTERFACE" || kind === "UNION";
 };
 
-// A nested field whose argument is non-null without a default cannot be selected
-// by the generator: it has no value to pass and emitting the field without the
-// argument is invalid (e.g. GitLab's `metadata.featureFlags(names:)`). Required
-// root-field arguments are different: those are threaded as operation variables
-// (and surfaced on the tool's input schema) in `buildOperationStringForField`.
+// A field whose argument is non-null without a default cannot be selected by the
+// generator: it has no value to pass and emitting the field without the argument
+// is invalid (e.g. GitLab's `metadata.featureFlags(names:)`). Root-field required
+// arguments are different: those are threaded as operation variables (and
+// surfaced on the tool's input schema) in `buildOperationStringForField`.
 const hasRequiredArgWithoutDefault = (field: IntrospectionField): boolean =>
   field.args.some(
     (arg: IntrospectionInputValue) => arg.type.kind === "NON_NULL" && arg.defaultValue == null,
   );
 
-const buildSelectionSet = (
+// Build the DEFAULT selection set for a field's return type: every scalar/enum
+// leaf the generator can select without arguments. It deliberately does NOT
+// recurse into composite fields or guess at nested selections, for two reasons:
+//   - A real schema (GitLab has 4000+ types) makes any recursive auto-expansion
+//     either arbitrary (which N fields? how deep?) or so large the server
+//     rejects it for exceeding its query-complexity budget.
+//   - A bounded-but-arbitrary selection silently freezes a partial view at sync
+//     time. Instead, callers that want nested or list data pass an explicit
+//     `select` (see buildOperationStringForField / invoke), so the choice of
+//     deeper fields is the caller's, not a guess baked into the tool.
+// The result is always valid: selecting only leaves never needs a sub-selection,
+// and a composite type with no selectable leaves falls back to `__typename`.
+const buildDefaultSelectionSet = (
   ref: IntrospectionTypeRef,
   types: ReadonlyMap<string, IntrospectionType>,
-  depth: number,
-  seen: Set<string>,
 ): string => {
-  if (depth > MAX_SELECTION_DEPTH) return "";
-
-  const leafName = unwrapTypeName(ref);
-  if (seen.has(leafName)) return "";
-
-  const objectType = types.get(leafName);
-  if (!objectType?.fields) return "";
-
-  const kind = objectType.kind;
-  if (kind === "SCALAR" || kind === "ENUM") return "";
-
-  seen.add(leafName);
-
-  const subFields: string[] = [];
-  for (const f of objectType.fields) {
-    if (subFields.length >= MAX_FIELDS_PER_LEVEL) break;
-    if (f.name.startsWith("__")) continue;
-    // Cannot synthesize a value for a required nested argument: skip the field
-    // rather than emit an invalid selection.
-    if (hasRequiredArgWithoutDefault(f)) continue;
-
-    if (isCompositeType(f.type, types)) {
-      // A composite field MUST have a sub-selection. When the recursion yields
-      // nothing (depth cap, cycle guard, or every child skipped) we drop the
-      // field instead of emitting it bare (which the server rejects).
-      const sub = buildSelectionSet(f.type, types, depth + 1, seen);
-      if (sub) subFields.push(`${f.name} ${sub}`);
-    } else {
-      // Scalar / enum leaf: select it directly.
-      subFields.push(f.name);
-    }
-  }
-
-  seen.delete(leafName);
-
-  return subFields.length > 0 ? `{ ${subFields.join(" ")} }` : "";
+  const objectType = types.get(unwrapTypeName(ref));
+  if (!objectType?.fields) return ""; // scalar / enum / unknown: no selection
+  if (objectType.kind === "SCALAR" || objectType.kind === "ENUM") return "";
+
+  const leaves = objectType.fields
+    .filter(
+      (f: IntrospectionField) =>
+        !f.name.startsWith("__") &&
+        !hasRequiredArgWithoutDefault(f) &&
+        !isCompositeType(f.type, types),
+    )
+    .map((f: IntrospectionField) => f.name);
+
+  // A composite type MUST have a non-empty selection; `__typename` is a leaf
+  // that exists on every composite, so it is a safe minimal fallback.
+  return leaves.length > 0 ? `{ ${leaves.join(" ")} }` : "{ __typename }";
 };
 
 // Name every generated operation: some servers reject anonymous operations, and
@@ -295,11 +279,23 @@ const buildSelectionSet = (
 const operationNameForField = (fieldName: string): string =>
   fieldName.charAt(0).toUpperCase() + fieldName.slice(1);
 
+interface BuiltOperation {
+  /** The operation with the default (scalar-leaf) selection. */
+  readonly operationString: string;
+  /** Everything up to (not including) the field's selection set:
+   *  `query Op($a: T) { field(a: $a)`. A caller-supplied `select` is wrapped as
+   *  `{ <select> }` and spliced between this prefix and the suffix at invoke
+   *  time, so the selection can be chosen per call without re-introspecting. */
+  readonly operationPrefix: string;
+  /** Closes the operation: ` }`. */
+  readonly operationSuffix: string;
+}
+
 const buildOperationStringForField = (
   kind: GraphqlOperationKind,
   field: IntrospectionField,
   types: ReadonlyMap<string, IntrospectionType>,
-): string => {
+): BuiltOperation => {
   const opType = kind === "query" ? "query" : "mutation";
   const opName = operationNameForField(field.name);
 
@@ -309,12 +305,16 @@ const buildOperationStringForField = (
   });
 
   const argPasses = field.args.map((arg) => `${arg.name}: $${arg.name}`);
-  const selectionSet = buildSelectionSet(field.type, types, 0, new Set());
+  const defaultSelection = buildDefaultSelectionSet(field.type, types);
 
   const varDefsStr = varDefs.length > 0 ? `(${varDefs.join(", ")})` : "";
   const argPassStr = argPasses.length > 0 ? `(${argPasses.join(", ")})` : "";
 
-  return `${opType} ${opName}${varDefsStr} { ${field.name}${argPassStr}${selectionSet ? ` ${selectionSet}` : ""} }`;
+  const operationPrefix = `${opType} ${opName}${varDefsStr} { ${field.name}${argPassStr}`;
+  const operationSuffix = ` }`;
+  const operationString = `${operationPrefix}${defaultSelection ? ` ${defaultSelection}` : ""}${operationSuffix}`;
+
+  return { operationString, operationPrefix, operationSuffix };
 };
 
 interface PreparedOperation {
@@ -324,6 +324,28 @@ interface PreparedOperation {
   readonly binding: OperationBinding;
 }
 
+// Surface an optional `select` input on every tool so a caller can choose the
+// return fields per call. The default operation selects only scalar leaves; to
+// fetch nested or list data the caller passes a GraphQL selection set here, which
+// is spliced into the operation at invoke time. A real field argument named
+// `select` (rare) is left untouched.
+const withSelectInput = (inputSchema: unknown, returnTypeName: string): unknown => {
+  const base =
+    inputSchema && typeof inputSchema === "object"
+      ? (inputSchema as Record<string, unknown>)
+      : { type: "object", properties: {} };
+  const properties = {
+    ...((base.properties as Record<string, unknown> | undefined) ?? {}),
+  };
+  if (!("select" in properties)) {
+    properties.select = {
+      type: "string",
+      description: `Optional GraphQL selection set for the \`${returnTypeName}\` return type. Overrides the default, which selects only scalar fields. Provide the fields to return, with sub-selections for nested objects and arguments where required, e.g. "id name items { id title }". Omit for the default.`,
+    };
+  }
+  return { ...base, type: "object", properties };
+};
+
 const prepareOperations = (
   fields: readonly ExtractedField[],
   introspection: IntrospectionResult,
@@ -360,21 +382,31 @@ const prepareOperations = (
 
     const key = `${extracted.kind}.${extracted.fieldName}`;
     const entry = fieldMap.get(key);
-    const operationString = entry
+    const built = entry
       ? buildOperationStringForField(entry.kind, entry.field, typeMap)
-      : `${extracted.kind} ${operationNameForField(extracted.fieldName)} { ${extracted.fieldName} }`;
+      : {
+          operationString: `${extracted.kind} ${operationNameForField(extracted.fieldName)} { ${extracted.fieldName} }`,
+          operationPrefix: undefined,
+          operationSuffix: undefined,
+        };
 
     const binding = OperationBinding.make({
       kind: extracted.kind,
       fieldName: extracted.fieldName,
-      operationString,
+      operationString: built.operationString,
       variableNames: extracted.arguments.map((a) => a.name),
+      ...(built.operationPrefix !== undefined && built.operationSuffix !== undefined
+        ? { operationPrefix: built.operationPrefix, operationSuffix: built.operationSuffix }
+        : {}),
     });
 
     return {
       toolName,
       description,
-      inputSchema: Option.getOrUndefined(extracted.inputSchema),
+      inputSchema: withSelectInput(
+        Option.getOrUndefined(extracted.inputSchema),
+        extracted.returnTypeName,
+      ),
       binding,
     };
   });