fix(graphql): generate valid operations against large schemas (UsefulSoftwareCo#1146) (UsefulSoftwareCo#1173)

* 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 UsefulSoftwareCo#1146

* 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.

* feat(graphql): validate a caller-supplied `select` locally before invoking

graphql-js is already a runtime dependency, so parse-check the assembled
operation when a `select` is provided and fail fast with a
`graphql_invalid_selection` error instead of building a request. This rejects a
malformed selection (and any attempt to break out of the field's selection set)
with a precise local error, and never reaches the network for those.

Field- and argument-level validity stays with the upstream server (which returns
verbatim errors): a local GraphQLSchema would need a full introspection
snapshot, but the stored snapshot is the plugin's reduced shape, which
buildClientSchema cannot consume.

The select-to-operation assembly is extracted to a shared
`effectiveOperationString` so the validated string and the sent string are
identical.

Author: RhysSullivan

.changeset/graphql-valid-selections.md

@@ -0,0 +1,9 @@
+---
+"@executor-js/plugin-graphql": patch
+---
+
+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

@@ -21,6 +21,25 @@ export const endpointForTelemetry = (endpoint: string): string => {
   return url.toString();
 };
 
+/** The operation string to send for a call. A caller-supplied `select` overrides
+ *  the default scalar-leaf selection: it is spliced into the field's selection
+ *  set (`field { <select> }`) so nested/list data can be requested per call. Falls
+ *  back to the stored default operation when `select` is absent or the binding
+ *  predates the prefix/suffix split. `select` is a control input, never a GraphQL
+ *  variable. Shared with plugin.invokeTool so the validated string matches the
+ *  sent string exactly. */
+export const effectiveOperationString = (
+  operation: OperationBinding,
+  args: Record<string, unknown>,
+): string => {
+  const customSelect = typeof args.select === "string" ? args.select.trim() : "";
+  return customSelect.length > 0 &&
+    operation.operationPrefix != null &&
+    operation.operationSuffix != null
+    ? `${operation.operationPrefix} { ${customSelect} }${operation.operationSuffix}`
+    : operation.operationString;
+};
+
 // ---------------------------------------------------------------------------
 // Response helpers
 // ---------------------------------------------------------------------------
@@ -71,10 +90,15 @@ export const invoke = Effect.fn("GraphQL.invoke")(function* (
     Object.assign(variables, args.variables);
   }
 
+  // `select` (a control input, not a GraphQL variable) is applied here and never
+  // enters `variables`. plugin.invokeTool validates this same string before we
+  // reach the network.
+  const operationString = effectiveOperationString(operation, args);
+
   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

@@ -28,6 +28,7 @@ import { endpointForTelemetry } from "./invoke";
 import { introspect } from "./introspect";
 import type { IntrospectionResult } from "./introspect";
 import {
+  makeGitlab1146Schema,
   makeGreetingGraphqlSchema,
   serveGraphqlFailureTestServer,
   serveGraphqlTestServer,
@@ -885,3 +886,132 @@ 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. 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 ?? "";
+
+  const setup = (slug: string) =>
+    Effect.gen(function* () {
+      const server = yield* gitlabServer;
+      const executor = yield* makeExecutor();
+      yield* executor.graphql.addIntegration({ endpoint: server.endpoint, slug });
+      yield* createOrgConnection(executor, {
+        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 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 } } }" },
+      );
+
+      expect(result).toMatchObject({ ok: true });
+      const query = lastQuery(yield* server.requests);
+      expect(query).toContain("nodes {");
+      expect(query).toContain("author {");
+    }),
+  );
+
+  it.effect("`select` can supply a nested field's required argument", () =>
+    Effect.gen(function* () {
+      const { server, executor } = yield* setup("gitlab_ff");
+
+      const result = yield* executor.execute(toolAddr("gitlab_ff", "main", "query.metadata"), {
+        select: 'version featureFlags(names: ["flag_a"]) { name enabled }',
+      });
+
+      expect(result).toMatchObject({ ok: true });
+      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" },
+      });
+    }),
+  );
+
+  it.effect("rejects a malformed `select` locally, before any network call", () =>
+    Effect.gen(function* () {
+      const { server, executor } = yield* setup("gitlab_syntax");
+      // Warm the binding cache (the first invoke introspects to materialize
+      // bindings) so the malformed call below has no legitimate reason to hit the
+      // wire: if it does, validation failed to short-circuit.
+      yield* executor.execute(toolAddr("gitlab_syntax", "main", "query.currentUser"), {});
+      yield* server.clearRequests;
+
+      const result = yield* executor.execute(
+        toolAddr("gitlab_syntax", "main", "query.currentUser"),
+        { select: "active mergeRequests { nodes {" },
+      );
+
+      expect(result).toMatchObject({
+        ok: false,
+        error: { code: "graphql_invalid_selection" },
+      });
+      // Parse-check happens before the request is built, so nothing reached the server.
+      expect((yield* server.requests).length).toBe(0);
+    }),
+  );
+});