Reproduce #1146: GraphQL plugin emits invalid operations against large schemas#1150
Reproduce #1146: GraphQL plugin emits invalid operations against large schemas#1150RhysSullivan wants to merge 2 commits into
Conversation
…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.
Deploying with
|
| 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 |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
executor-cloud | 97c13c2 | Jun 26 2026, 08:13 PM |
| 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))); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
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.
| 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))); | |
| }); | |
| }); |
| // 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", | ||
| }); |
There was a problem hiding this comment.
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.
Cloudflare preview
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. |
@executor-js/cli
@executor-js/config
@executor-js/execution
@executor-js/sdk
@executor-js/codemode-core
@executor-js/runtime-quickjs
@executor-js/plugin-file-secrets
@executor-js/plugin-graphql
@executor-js/plugin-keychain
@executor-js/plugin-mcp
@executor-js/plugin-onepassword
@executor-js/plugin-openapi
executor
commit: |
Refs #1146.
Adds a plugin-level reproduction test for the GraphQL plugin generating invalid operations against large, production-shaped schemas.
What it does
buildSelectionSetfreezes 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 (addIntegrationthenconnections.createthenexecute) and reads the upstream graphql-js errors out of the tool result.It asserts the two distinct malformation mechanisms:
Two cases:
query.currentUser, one real root field that trips both mechanisms in a single call.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.