Skip to content

Reproduce #1146: GraphQL plugin emits invalid operations against large schemas#1150

Open
RhysSullivan wants to merge 2 commits into
mainfrom
repro-graphql-1146
Open

Reproduce #1146: GraphQL plugin emits invalid operations against large schemas#1150
RhysSullivan wants to merge 2 commits into
mainfrom
repro-graphql-1146

Conversation

@RhysSullivan

Copy link
Copy Markdown
Owner

Refs #1146.

Adds a plugin-level reproduction test for the GraphQL plugin generating invalid operations against large, production-shaped schemas.

What it does

buildSelectionSet freezes one machine-built selection set per root field at connect time. Against a non-trivial schema that frozen operation is invalid GraphQL, so the server rejects it on validation before any data returns, and every synced tool for that field fails on every call.

The test boots a local GitLab GraphQL emulator from @executor-js/emulate (createEmulator({ service: "gitlab" })), which carries GitLab's full real schema (4000+ types) with genuine graphql-js parse, validate, and introspect. No network and no vendored fixture. It introspects that schema, then drives the real product flow (addIntegration then connections.create then execute) and reads the upstream graphql-js errors out of the tool result.

It asserts the two distinct malformation mechanisms:

  1. A composite field is printed with no sub-selection (the depth cutoff and the cycle guard both empty a nested selection, and the parent still prints the field name bare). graphql-js: "... must have a selection of subfields".
  2. A field that requires an argument is printed without it (nested args are never threaded). graphql-js: 'argument "X" of type "Y!" is required, but it was not provided'.

Two cases:

  • Headline query.currentUser, one real root field that trips both mechanisms in a single call.
  • A systemic sweep over generated query.* tools whose root field takes no required top-level argument, so an empty input is valid and any failure is the generated selection's fault. More than 10 of them are rejected as invalid by the two validation regexes.

Status

The test is green: it currently asserts the operations are rejected. Once the selection-set generator is fixed (bound depth without emitting bare composites; thread nested required arguments, or omit fields it cannot satisfy), these calls should return success and the expectations flip to assert ok: true.

…emas

Adds a plugin-level repro test that boots the GitLab GraphQL emulator from
@executor-js/emulate (createEmulator service gitlab) on an explicit port,
introspects its real 4000+ type schema through the effect/unstable/http
HttpClient, and drives the product flow addIntegration, connections.create,
execute. Asserts the generated operations are rejected by graphql-js for two
distinct reasons: composite fields printed without a sub-selection, and
required nested arguments omitted. Headline currentUser case plus a systemic
sweep over no-required-arg query tools. Both green; flips to assert success
once the selection-set generator is fixed.
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 26, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
executor-marketing 97c13c2 Commit Preview URL

Branch Preview URL
Jun 26 2026, 08:11 PM

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 26, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
executor-cloud 97c13c2 Jun 26 2026, 08:13 PM

@greptile-apps

greptile-apps Bot commented Jun 26, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds a plugin-level reproduction test for issue #1146, where buildSelectionSet generates invalid GraphQL operations against large schemas by emitting bare composite fields and dropping nested required arguments. The test uses @executor-js/emulate to boot GitLab's full real schema (4000+ types) in-process and drives the complete product flow through addIntegrationconnections.createexecute, asserting the currently broken behavior so the assertions can be flipped to ok: true once the selection-set generator is fixed.

  • Headline case (query.currentUser): exercises both failure mechanisms in a single call — a composite field printed with no sub-selection ("must have a selection of subfields") and a selected field missing its required argument.
  • Systemic sweep: iterates all generated query.* tools whose root field takes no required top-level argument, confirming more than 10 are rejected as invalid by the two validation patterns and that currentUser is not an outlier.
  • Dependency bump: @executor-js/emulate is added as a devDependency to the graphql plugin package and bumped from 0.7.50.7.6 in the lockfile.

Confidence Score: 5/5

Safe to merge — the change is test-only, adds no production code, and cannot break any existing behavior.

All three changed files are test infrastructure: a new reproduction test, its devDependency declaration, and the lockfile update. No production logic is touched. The test correctly documents the existing broken behavior with inverted assertions and is designed to be flipped once the fix lands.

No files require special attention. The test file has a minor Effect.callback cleanup gap noted, but it does not affect correctness under normal test execution.

Important Files Changed

Filename Overview
packages/plugins/graphql/src/sdk/invalid-operations-large-schema.test.ts New regression test reproducing issue #1146; asserts current buggy behavior (inverted assertions). Well-structured with emulator teardown via Effect.acquireRelease. Minor: Effect.callback missing interruption cleanup.
packages/plugins/graphql/package.json Adds @executor-js/emulate ^0.7.6 as a devDependency to support the new reproduction test.
bun.lock Bumps @executor-js/emulate from 0.7.5 to 0.7.6 with updated integrity hash; no other dependency changes.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Test
    participant availablePort
    participant GitLabEmulator
    participant Executor
    participant graphqlPlugin

    Test->>availablePort: probe for free port
    availablePort-->>Test: port N
    Test->>GitLabEmulator: "createEmulator({ service: "gitlab", port: N })"
    GitLabEmulator-->>Test: emulator (url, close)
    Test->>Executor: createExecutor(config)
    Executor-->>Test: executor
    Test->>Executor: "graphql.addIntegration({ endpoint, slug })"
    Executor->>graphqlPlugin: introspect schema
    graphqlPlugin->>GitLabEmulator: POST /api/graphql (introspectionQuery)
    GitLabEmulator-->>graphqlPlugin: 4000+ type SDL
    graphqlPlugin->>graphqlPlugin: buildSelectionSet per root field (freezes operations)
    graphqlPlugin-->>Executor: tools registered
    Test->>Executor: connections.create(...)
    Executor-->>Test: connection
    Test->>Executor: "execute(toolAddr("query.currentUser"), {})"
    Executor->>graphqlPlugin: invoke frozen operation string
    graphqlPlugin->>GitLabEmulator: POST /api/graphql (frozen op)
    GitLabEmulator->>GitLabEmulator: graphql-js validate()
    GitLabEmulator-->>graphqlPlugin: errors: ["must have a selection...", "argument ... required"]
    graphqlPlugin-->>Executor: ToolError(code: graphql_errors)
    Executor-->>Test: "result { ok: false }"
    Test->>Test: assert ok:false + error messages (BUG encoded)
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Test
    participant availablePort
    participant GitLabEmulator
    participant Executor
    participant graphqlPlugin

    Test->>availablePort: probe for free port
    availablePort-->>Test: port N
    Test->>GitLabEmulator: "createEmulator({ service: "gitlab", port: N })"
    GitLabEmulator-->>Test: emulator (url, close)
    Test->>Executor: createExecutor(config)
    Executor-->>Test: executor
    Test->>Executor: "graphql.addIntegration({ endpoint, slug })"
    Executor->>graphqlPlugin: introspect schema
    graphqlPlugin->>GitLabEmulator: POST /api/graphql (introspectionQuery)
    GitLabEmulator-->>graphqlPlugin: 4000+ type SDL
    graphqlPlugin->>graphqlPlugin: buildSelectionSet per root field (freezes operations)
    graphqlPlugin-->>Executor: tools registered
    Test->>Executor: connections.create(...)
    Executor-->>Test: connection
    Test->>Executor: "execute(toolAddr("query.currentUser"), {})"
    Executor->>graphqlPlugin: invoke frozen operation string
    graphqlPlugin->>GitLabEmulator: POST /api/graphql (frozen op)
    GitLabEmulator->>GitLabEmulator: graphql-js validate()
    GitLabEmulator-->>graphqlPlugin: errors: ["must have a selection...", "argument ... required"]
    graphqlPlugin-->>Executor: ToolError(code: graphql_errors)
    Executor-->>Test: "result { ok: false }"
    Test->>Test: assert ok:false + error messages (BUG encoded)
Loading

Reviews (2): Last reviewed commit: "Dedupe @executor-js/emulate to 0.7.6 in ..." | Re-trigger Greptile

Comment on lines +58 to +65
const availablePort = Effect.callback<number>((resume) => {
const probe = createServer();
probe.listen(0, "127.0.0.1", () => {
const address = probe.address();
const port = typeof address === "object" && address ? address.port : 0;
probe.close(() => resume(Effect.succeed(port)));
});
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The availablePort helper has no 'error' listener on the probe server. Node's net.Server is an EventEmitter, so if listen fails (e.g. the OS refuses to allocate a port) the 'error' event fires without a listener, which throws an uncaught exception and crashes the test process rather than cleanly failing. Adding an error handler that resumes the Effect with port 0 — which tells the emulator to let the OS pick its own free port — keeps the same intent while avoiding the crash path.

Suggested change
const availablePort = Effect.callback<number>((resume) => {
const probe = createServer();
probe.listen(0, "127.0.0.1", () => {
const address = probe.address();
const port = typeof address === "object" && address ? address.port : 0;
probe.close(() => resume(Effect.succeed(port)));
});
});
const availablePort = Effect.callback<number>((resume) => {
const probe = createServer();
probe.on("error", () => resume(Effect.succeed(0)));
probe.listen(0, "127.0.0.1", () => {
const address = probe.address();
const port = typeof address === "object" && address ? address.port : 0;
probe.close(() => resume(Effect.succeed(port)));
});
});

Comment on lines +195 to +216
// requires an argument is emitted without it.
expect(messages, "a selected field's required argument is dropped").toEqual(
expect.arrayContaining([expect.stringMatching(MISSING_REQUIRED_ARGUMENT)]),
);
}),
60000,
);

// Systemic: the failure is not one unlucky field. Sweep every generated
// query.* tool that takes no required top-level argument and count how many
// produce an operation the server rejects as invalid.
it.effect(
"the generator emits invalid operations across many of the schema's root query fields",
() =>
Effect.gen(function* () {
const emulator = yield* gitlabEmulator;
const executor = yield* makeExecutor();

yield* executor.graphql.addIntegration({
endpoint: graphqlEndpoint(emulator),
slug: "gitlab",
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Sequential sweep could race the 60 s timeout on slow machines. The for...of loop with yield* runs all 20+ tool calls in series. Each call goes through the full plugin stack (storage lookup, possible introspection cache hit, HTTP round-trip to the in-process emulator, graphql-js validation). Under a loaded CI runner the cumulative latency can push close to the timeout. Running the calls concurrently with Effect.forEach(..., { concurrency: "unbounded" }) would make the sweep much faster and leave headroom for the assertions.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

The frozen lockfile install in CI rejected the committed bun.lock because
the root resolution still pinned 0.7.5 while the graphql plugin requested
0.7.6. Regenerating dedupes the tree to a single 0.7.6 resolution.
@github-actions

Copy link
Copy Markdown
Contributor

Cloudflare preview

Console https://executor-preview-pr-1150.executor-e2e.workers.dev
MCP https://executor-preview-pr-1150.executor-e2e.workers.dev/mcp
Deployed commit 97c13c2

Sign-in is Cloudflare Access (one-time PIN to an allowed email). The preview has its own database and encryption key; it is destroyed when this PR closes.

@pkg-pr-new

pkg-pr-new Bot commented Jun 26, 2026

Copy link
Copy Markdown

Open in StackBlitz

@executor-js/cli

npm i https://pkg.pr.new/@executor-js/cli@1150

@executor-js/config

npm i https://pkg.pr.new/@executor-js/config@1150

@executor-js/execution

npm i https://pkg.pr.new/@executor-js/execution@1150

@executor-js/sdk

npm i https://pkg.pr.new/@executor-js/sdk@1150

@executor-js/codemode-core

npm i https://pkg.pr.new/@executor-js/codemode-core@1150

@executor-js/runtime-quickjs

npm i https://pkg.pr.new/@executor-js/runtime-quickjs@1150

@executor-js/plugin-file-secrets

npm i https://pkg.pr.new/@executor-js/plugin-file-secrets@1150

@executor-js/plugin-graphql

npm i https://pkg.pr.new/@executor-js/plugin-graphql@1150

@executor-js/plugin-keychain

npm i https://pkg.pr.new/@executor-js/plugin-keychain@1150

@executor-js/plugin-mcp

npm i https://pkg.pr.new/@executor-js/plugin-mcp@1150

@executor-js/plugin-onepassword

npm i https://pkg.pr.new/@executor-js/plugin-onepassword@1150

@executor-js/plugin-openapi

npm i https://pkg.pr.new/@executor-js/plugin-openapi@1150

executor

npm i https://pkg.pr.new/executor@1150

commit: 97c13c2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant