Repository navigation
Add GitHub GraphQL capability and docs - #20
Merged
Merged
Conversation
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
📝 WalkthroughWalkthroughAdds GitHub GraphQL support across the stack: new GraphQL client, two GraphQL domain capabilities (query + docs), mock server GraphQL endpoint and tests, documentation updates clarifying REST + GraphQL coverage, and minor export changes. Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant Capability as GitHubGraphqlCapability
participant GQLClient as GitHubGraphqlClient
participant MockServer as Mock GitHub Server
Client->>Capability: handler({ query, variables })
Capability->>GQLClient: create via env (token, baseUrl)
Capability->>GQLClient: query({ query, variables, operationName })
GQLClient->>MockServer: POST /graphql (headers + body)
MockServer->>MockServer: parse & validate request
MockServer-->>GQLClient: 200 { data, errors?, extensions? } or 400/unsupported
GQLClient->>GQLClient: normalizeGraphqlResponse()
GQLClient-->>Capability: { status, data, errors, extensions }
Capability-->>Client: return normalized response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
kentcdodds
marked this pull request as ready for review
March 20, 2026 02:39
Contributor
|
🔎 Preview deployed: https://kody-pr-20.kentcdodds.workers.dev Worker: Mocks:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
mcp/github/github-graphql-client.test.ts (1)
15-32: Avoid buffering unused process output.
captureOutputstores all output in-memory, but callers discard the getter. This creates unnecessary unbounded memory growth risk in tests.♻️ Proposed fix
-function captureOutput(stream: ReadableStream<Uint8Array> | null) { - let output = '' - if (!stream) return () => output +function captureOutput(stream: ReadableStream<Uint8Array> | null) { + if (!stream) return const reader = stream.getReader() const decoder = new TextDecoder() void (async () => { try { while (true) { const { value, done } = await reader.read() if (done) break - if (value) output += decoder.decode(value) + if (value) void decoder.decode(value) } } catch { // ignore } })() - return () => output } @@ - void captureOutput(proc.stdout) - void captureOutput(proc.stderr) + captureOutput(proc.stdout) + captureOutput(proc.stderr)Also applies to: 80-81
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcp/github/github-graphql-client.test.ts` around lines 15 - 32, captureOutput currently accumulates all stream data into memory even when callers ignore the returned getter, causing unbounded buffering; update captureOutput (and the similar code at lines noted) so it only buffers when explicitly requested: add a boolean parameter like shouldBuffer (default false) or change the function to return a no-op getter by default and only create the reader/decoder/async loop when shouldBuffer is true or a buffering mode is requested; ensure symbols referenced are captureOutput, reader.getReader(), and decoder.decode() so you only instantiate the reader/decoder and start the async reader loop when buffering is enabled to avoid unnecessary memory growth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mcp/capabilities/coding/github-graphql.ts`:
- Around line 76-85: The normalizer currently drops non-GraphQL JSON error
payloads by returning errors: null when response.body contains a top-level
message (e.g., { "message": "Bad credentials" }); update the logic around the
destructured response.body (data, errors, extensions) to detect when errors is
null/undefined and response.body.message is present and, in that case, populate
errors with a single-element array like [{ message: response.body.message }] (or
otherwise surface the message in the returned object), while preserving status,
data (null if absent), and extensions; modify the block that throws on non-array
errors to still handle non-GraphQL payloads and return the constructed errors
array instead of dropping the message.
- Around line 8-10: The Zod schema for the GraphQL request allows
whitespace-only values because .min(1) runs before trimming; update the schema
fields named query and operationName in github-graphql.ts to call .trim() before
.min(1) (e.g., replace z.string().min(1) with z.string().trim().min(1)) so
whitespace-only inputs fail validation rather than being trimmed to empty at
runtime.
---
Nitpick comments:
In `@mcp/github/github-graphql-client.test.ts`:
- Around line 15-32: captureOutput currently accumulates all stream data into
memory even when callers ignore the returned getter, causing unbounded
buffering; update captureOutput (and the similar code at lines noted) so it only
buffers when explicitly requested: add a boolean parameter like shouldBuffer
(default false) or change the function to return a no-op getter by default and
only create the reader/decoder/async loop when shouldBuffer is true or a
buffering mode is requested; ensure symbols referenced are captureOutput,
reader.getReader(), and decoder.decode() so you only instantiate the
reader/decoder and start the async reader loop when buffering is enabled to
avoid unnecessary memory growth.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8ba33628-94bc-4cc2-a8de-df90dbeae19e
📒 Files selected for processing (14)
.env.exampledocs/agents/mock-api-servers.mddocs/agents/setup.mddocs/environment-variables.mddocs/setup-manifest.mdmcp/capabilities/coding/coding-capabilities.test.tsmcp/capabilities/coding/domain.tsmcp/capabilities/coding/github-graphql-api-docs.tsmcp/capabilities/coding/github-graphql.tsmcp/capabilities/coding/index.tsmcp/github/github-graphql-client.test.tsmcp/github/github-graphql-client.tsmock-servers/github/github-mock.test.tsmock-servers/github/worker.ts
Comment on lines
+8
to
+10
| query: z | ||
| .string() | ||
| .min(1) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Zod 4, does z.string().min(1)accept whitespace-only strings unless.trim()is applied before.min(1)?
💡 Result:
Yes, in Zod 4, z.string.min(1) accepts whitespace-only strings like " " because it checks the string length after any prior transforms, and without .trim, the length of " " is 1, which meets the minimum. To reject whitespace-only strings, apply .trim before .min(1), as in z.string.trim.min(1), which trims whitespace first, making " ".length become 0 and fail the check.
Citations:
- 1: z.string() validates empty strings colinhacks/zod#2466
- 2: https://stackoverflow.com/questions/77134910/how-can-i-remove-all-whitespace-in-zod
- 3: https://v4.zod.dev/api?id=strings
- 4: Min and max use spaces and characters. colinhacks/zod#2088
- 5: https://github.com/colinhacks/zod/blob/main/src/__tests__/string.test.ts
🏁 Script executed:
cat -n mcp/capabilities/coding/github-graphql.ts | sed -n '1,70p'Repository: kentcdodds/kody
Length of output: 2554
Whitespace-only query and operationName bypass schema validation.
query: ' ' and operationName: ' ' pass .min(1) checks because they have length ≥ 1, then get trimmed to '' at runtime (lines 56, 58) instead of failing fast during validation.
Apply .trim() before .min(1) in the schema:
Proposed fix
const inputSchema = z.object({
query: z
.string()
+ .trim()
.min(1)
.describe(
'GraphQL query or mutation string to send to https://api.github.com/graphql.',
),
variables: z
.record(z.string(), z.unknown())
.optional()
.describe('Optional GraphQL variables object (JSON).'),
operationName: z
.string()
+ .trim()
.min(1)
.optional()
.describe('Optional GraphQL operation name.'),
}) const response = await client.query({
- query: args.query.trim(),
+ query: args.query,
variables: args.variables,
- operationName: args.operationName?.trim() || undefined,
+ operationName: args.operationName || undefined,
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcp/capabilities/coding/github-graphql.ts` around lines 8 - 10, The Zod
schema for the GraphQL request allows whitespace-only values because .min(1)
runs before trimming; update the schema fields named query and operationName in
github-graphql.ts to call .trim() before .min(1) (e.g., replace
z.string().min(1) with z.string().trim().min(1)) so whitespace-only inputs fail
validation rather than being trimmed to empty at runtime.
Comment on lines
+76
to
+85
| const { data, errors, extensions } = response.body | ||
| if (errors != null && !Array.isArray(errors)) { | ||
| throw new Error('GitHub GraphQL response "errors" is not an array.') | ||
| } | ||
| return { | ||
| status: response.status, | ||
| data: data ?? null, | ||
| errors: errors ?? null, | ||
| extensions: extensions ?? null, | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For GitHub's GraphQL endpoint, what JSON body is returned on authentication failures (e.g., bad token)? Does it return messagewithout a GraphQLerrors array?
💡 Result:
For GitHub's GraphQL endpoint (https://api.github.com/graphql), authentication failures (e.g., bad or missing token) return an HTTP 401 Unauthorized response with a JSON body containing a top-level message field (e.g., "This endpoint requires you to be authenticated." or "Bad credentials") and a documentation_url, but no GraphQL errors array or data field. This is a standard GitHub API error format used before GraphQL processing, causing GraphQL clients to reject it as invalid (lacking data or errors). Examples from sources: - {"message": "This endpoint requires you to be authenticated.", "documentation_url": "https://docs.github.com/v3/#authentication"} - {"message": "Bad credentials", "documentation_url": "https://docs.github.com/rest"}
Citations:
- 1: https://stackoverflow.com/questions/64881780/github-graphql-api-authentication
- 2: GitHub API: Server did not return a GraphQL result graphql-python/gql#149
- 3: "This endpoint requires you to be authenticated" only to be told "You were already logged in" cli/cli#8729
- 4: https://stackoverflow.com/questions/44490426/receiving-401-bad-credentials-when-hitting-github-api-endpoint
- 5: https://docs.github.com/en/graphql/overview/rate-limits-and-node-limits-for-the-graphql-api
- 6: https://docs.github.com/en/rest/using-the-rest-api/troubleshooting-the-rest-api
- 7: https://docs.github.com/en/enterprise-server@2.21/rest/overview/resources-in-the-rest-api
🏁 Script executed:
# First, let's locate and read the GitHub GraphQL file to understand the full context
find . -name "github-graphql.ts" -type fRepository: kentcdodds/kody
Length of output: 102
🏁 Script executed:
# Read the file around the specified lines (76-85) to see the full context
if [ -f "./mcp/capabilities/coding/github-graphql.ts" ]; then
echo "=== File found, reading context (lines 60-100) ==="
cat -n ./mcp/capabilities/coding/github-graphql.ts | sed -n '60,100p'
else
echo "File not found at expected path, searching for it..."
git ls-files | grep -i "github-graphql"
fiRepository: kentcdodds/kody
Length of output: 986
Non-GraphQL JSON error payloads are being dropped.
If GitHub responds with a JSON body like { "message": "Bad credentials" } (which it does on authentication failures), the normalizer returns errors: null, losing the actionable error message. GitHub's GraphQL endpoint returns HTTP 401 responses with a top-level message field when authentication fails, but no GraphQL errors array.
🛠️ Proposed fix
const { data, errors, extensions } = response.body
if (errors != null && !Array.isArray(errors)) {
throw new Error('GitHub GraphQL response "errors" is not an array.')
}
+ const fallbackMessage =
+ typeof (response.body as { message?: unknown }).message === 'string'
+ ? (response.body as { message: string }).message
+ : null
return {
status: response.status,
data: data ?? null,
- errors: errors ?? null,
+ errors: errors ?? (fallbackMessage ? [{ message: fallbackMessage }] : null),
extensions: extensions ?? null,
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const { data, errors, extensions } = response.body | |
| if (errors != null && !Array.isArray(errors)) { | |
| throw new Error('GitHub GraphQL response "errors" is not an array.') | |
| } | |
| return { | |
| status: response.status, | |
| data: data ?? null, | |
| errors: errors ?? null, | |
| extensions: extensions ?? null, | |
| } | |
| const { data, errors, extensions } = response.body | |
| if (errors != null && !Array.isArray(errors)) { | |
| throw new Error('GitHub GraphQL response "errors" is not an array.') | |
| } | |
| const fallbackMessage = | |
| typeof (response.body as { message?: unknown }).message === 'string' | |
| ? (response.body as { message: string }).message | |
| : null | |
| return { | |
| status: response.status, | |
| data: data ?? null, | |
| errors: errors ?? (fallbackMessage ? [{ message: fallbackMessage }] : null), | |
| extensions: extensions ?? null, | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcp/capabilities/coding/github-graphql.ts` around lines 76 - 85, The
normalizer currently drops non-GraphQL JSON error payloads by returning errors:
null when response.body contains a top-level message (e.g., { "message": "Bad
credentials" }); update the logic around the destructured response.body (data,
errors, extensions) to detect when errors is null/undefined and
response.body.message is present and, in that case, populate errors with a
single-element array like [{ message: response.body.message }] (or otherwise
surface the message in the returned object), while preserving status, data (null
if absent), and extensions; modify the block that throws on non-array errors to
still handle non-GraphQL payloads and return the constructed errors array
instead of dropping the message.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Empty base URL falls through to broken
/graphqlendpoint- Added the same fallback default as the REST client so empty base URLs resolve to the GitHub API host.
- ✅ Fixed: Duplicated
formatRateLimitHintacross GitHub client files- Exported the shared helper from the REST client and reused it in the GraphQL client to remove duplication.
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Duplicated docs-path validation logic across capabilities
- Extracted shared
assertGithubDocsPathhelper and reused it for REST and GraphQL validations to remove duplicated logic.
- Extracted shared
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
mcp/capabilities/coding/github-graphql-api-docs.ts (1)
8-8: Consider extracting shared constants and schemas.
GITHUB_DOCS_ORIGIN,inputSchema, andoutputSchemaare duplicated between this file andgithub-rest-api-docs.ts. You could extract these to a shared module (e.g.,github-docs-shared.ts) to reduce duplication.♻️ Example shared module structure
// github-docs-shared.ts import { z } from 'zod' export const GITHUB_DOCS_ORIGIN = 'https://docs.github.com' export const createDocsInputSchema = (apiType: 'REST' | 'GraphQL', example: string) => z.object({ path: z .string() .min(1) .describe( `Path on docs.github.com for ${apiType} reference pages, starting with \`/{locale}/${apiType.toLowerCase()}/\` (no host). Example: \`${example}\`.`, ), }) export const docsOutputSchema = z.object({ status: z.number().describe('HTTP status from docs.github.com.'), contentType: z.string().nullable().describe('Response Content-Type header, if present.'), markdownTokenEstimate: z.string().nullable().describe('Optional x-markdown-tokens header when the response is markdown.'), body: z.string().describe('Response body (typically markdown when Content-Type is text/markdown).'), })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcp/capabilities/coding/github-graphql-api-docs.ts` at line 8, Extract the duplicated GITHUB_DOCS_ORIGIN, inputSchema, and outputSchema into a new shared module (e.g., github-docs-shared) and import them from both github-graphql-api-docs.ts and github-rest-api-docs.ts; specifically move the constant GITHUB_DOCS_ORIGIN and the two Zod schemas (currently named inputSchema and outputSchema) into the shared file, expose a helper like createDocsInputSchema(apiType, example) if you need API-specific descriptions, update both files to import GITHUB_DOCS_ORIGIN, the shared input schema (or factory) and docsOutputSchema, and remove the duplicated definitions from each file to eliminate redundancy.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@mcp/capabilities/coding/github-graphql-api-docs.ts`:
- Line 8: Extract the duplicated GITHUB_DOCS_ORIGIN, inputSchema, and
outputSchema into a new shared module (e.g., github-docs-shared) and import them
from both github-graphql-api-docs.ts and github-rest-api-docs.ts; specifically
move the constant GITHUB_DOCS_ORIGIN and the two Zod schemas (currently named
inputSchema and outputSchema) into the shared file, expose a helper like
createDocsInputSchema(apiType, example) if you need API-specific descriptions,
update both files to import GITHUB_DOCS_ORIGIN, the shared input schema (or
factory) and docsOutputSchema, and remove the duplicated definitions from each
file to eliminate redundancy.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3ee3ae66-9277-4727-838c-9eb25e59ad44
📒 Files selected for processing (3)
mcp/capabilities/coding/github-docs-path.tsmcp/capabilities/coding/github-graphql-api-docs.tsmcp/capabilities/coding/github-rest-api-docs.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing
Summary by CodeRabbit
New Features
Documentation
Tests
Refactor