Skip to content

fix(github-client): use buildHeaders() in graphql() to eliminate duplicated header logic#121

Merged
xlabtg merged 3 commits intoxlabtg:mainfrom
konard:issue-116-13a7cfc3f309
Apr 9, 2026
Merged

fix(github-client): use buildHeaders() in graphql() to eliminate duplicated header logic#121
xlabtg merged 3 commits intoxlabtg:mainfrom
konard:issue-116-13a7cfc3f309

Conversation

@konard
Copy link
Copy Markdown

@konard konard commented Apr 7, 2026

Summary

Fixes #116

  • Removed duplicated header construction (Accept, X-GitHub-Api-Version, Content-Type, User-Agent, Authorization) from graphql() in lib/github-client.js
  • Replaced it with a single call to the shared buildHeaders() helper that already exists in the same closure
  • Any future updates to buildHeaders() (new headers, auth scheme changes, API version bumps) will now automatically apply to GraphQL requests

Changes

  • plugins/github-dev-assistant/lib/github-client.jsgraphql() now calls buildHeaders() instead of manually constructing headers
  • plugins/github-dev-assistant/tests/github-client-graphql.test.js — new regression test suite verifying that graphql() sends correct Authorization header, all standard GitHub headers, omits Authorization when no token set, and serializes query/variables correctly

How to reproduce the issue

Before this fix, updating buildHeaders() (e.g. changing the API version or adding a new required header) would have no effect on graphql() requests since it had its own copy of the header logic.

Test plan

  • graphql() sends Authorization: Bearer <token> from sdk.secrets
  • graphql() sends all standard GitHub API headers via buildHeaders()
  • graphql() omits Authorization when no token is configured
  • graphql() posts to https://api.github.com/graphql with correct body
  • All existing tests continue to pass (npm test: 219 pass, 0 fail)
  • Lint clean (npm run lint: 0 errors)

🤖 Generated with Claude Code

konard and others added 2 commits April 7, 2026 11:09
Adding .gitkeep for PR creation (default mode).
This file will be removed when the task is complete.

Issue: xlabtg#116
…ating header logic

Resolves xlabtg#116: graphql() was manually constructing headers (including
Authorization) instead of calling the shared buildHeaders() helper.
This meant any future change to buildHeaders() (new headers, auth
scheme, API version) would silently not apply to GraphQL requests.

Also adds a dedicated test suite (tests/github-client-graphql.test.js)
that verifies graphql() sends the correct Authorization header, standard
GitHub headers, omits Authorization when no token is set, and serializes
query/variables correctly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@konard konard changed the title [WIP] Security: graphql() method duplicates auth header logic instead of reusing buildHeaders() fix(github-client): use buildHeaders() in graphql() to eliminate duplicated header logic Apr 7, 2026
@konard konard marked this pull request as ready for review April 7, 2026 11:12
@konard
Copy link
Copy Markdown
Author

konard commented Apr 7, 2026

🤖 Solution Draft Log

This log file contains the complete execution trace of the AI solution draft process.

💰 Cost estimation:

  • Public pricing estimate: $0.389981
  • Calculated by Anthropic: $0.389981 USD
  • Difference: $0.000000 (+0.00%)

📊 Context and tokens usage:

  • Context window: 38.6K / 1M input tokens (4%), 5.2K / 64K output tokens (8%)

Total: 29.7K + 668.4K cached input tokens, 5.2K output tokens, $0.389981 cost

🤖 Models used:

  • Tool: Anthropic Claude Code
  • Requested: sonnet
  • Model: Claude Sonnet 4.6 (claude-sonnet-4-6)

📎 Log file uploaded as Gist (426KB)


Now working session is ended, feel free to review and add any feedback on the solution draft.

@konard
Copy link
Copy Markdown
Author

konard commented Apr 7, 2026

✅ Ready to merge

This pull request is now ready to be merged:

  • All CI checks have passed
  • No merge conflicts
  • No pending changes

Monitored by hive-mind with --auto-restart-until-mergeable flag

@xlabtg xlabtg merged commit 04e63c6 into xlabtg:main Apr 9, 2026
6 checks passed
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.

Security: graphql() method duplicates auth header logic instead of reusing buildHeaders()

2 participants