Conversation
Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/99a9af23-d3aa-4922-b49f-0cc207cab3aa Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Collaborator
|
@copilot are there other potential paths that the difc proxy should support? can you document all the paths it does support and why? |
Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/99a9af23-d3aa-4922-b49f-0cc207cab3aa Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix DIFC proxy returning 404 for GHES API paths
Fix GHES GraphQL path handling in DIFC proxy when upstream is Apr 16, 2026
/api/v3
Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/a3d9899a-0f54-47a9-b6ad-5846ecf1427e Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Contributor
Author
I documented the current supported proxy paths and rationale in Short answer: the proxy intentionally supports the read endpoints needed for repo-assist/gh workflows (REST route table + GraphQL on If you want additional specific read endpoints covered, I can add them explicitly to |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes GHES GraphQL forwarding in the DIFC proxy when the configured upstream GitHub API base is .../api/v3, ensuring GraphQL requests are sent to the correct GHES endpoint (/api/graphql) while keeping REST behavior unchanged.
Changes:
- Rewrites upstream URL construction for
POST /graphqlwhengithubAPIURLends with/api/v3to forward to/api/graphql. - Adds a unit test validating the
/graphql→/api/graphqlrewrite behavior (including a query-string case). - Updates proxy-mode documentation to describe REST path normalization and accepted GraphQL endpoint paths, including the GHES rewrite.
Show a summary per file
| File | Description |
|---|---|
| internal/proxy/proxy.go | Adds GHES-specific GraphQL forwarding rewrite from /graphql to /api/graphql when upstream base is /api/v3. |
| internal/proxy/proxy_test.go | Adds coverage for the GHES GraphQL rewrite in forwardToGitHub. |
| docs/PROXY_MODE.md | Documents REST normalization, fail-closed behavior for reads, GraphQL accepted paths, and GHES GraphQL rewrite. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 1
Comment on lines
399
to
+405
| url := s.githubAPIURL + path | ||
| pathOnly, query, hasQuery := strings.Cut(path, "?") | ||
| if strings.HasSuffix(s.githubAPIURL, "/api/v3") && pathOnly == "/graphql" { | ||
| url = strings.TrimSuffix(s.githubAPIURL, "/api/v3") + "/api/graphql" | ||
| if hasQuery { | ||
| url += "?" + query | ||
| } |
There was a problem hiding this comment.
GraphQL query-string preservation appears incomplete end-to-end: forwardToGitHub now preserves ?… when path includes it, but the proxy’s GraphQL forwarding path is hard-coded elsewhere as "/graphql" (without the original query), so real /graphql?foo=bar requests will still drop the query. Consider plumbing the original request’s RawQuery into the GraphQL forward path (or switching the GraphQL forwarding call sites to pass the full path argument) so this logic is actually exercised in production.
See below for a potential fix:
// path should be the original request URI path, including any raw query string.
func (s *Server) forwardToGitHub(ctx context.Context, method, path string, body io.Reader, contentType string, clientAuth string) (*http.Response, error) {
baseURL, err := url.Parse(s.githubAPIURL)
if err != nil {
return nil, fmt.Errorf("failed to parse GitHub API URL: %w", err)
}
requestURL, err := url.Parse(path)
if err != nil {
return nil, fmt.Errorf("failed to parse upstream path %q: %w", path, err)
}
pathOnly := requestURL.Path
if strings.HasSuffix(baseURL.Path, "/api/v3") && pathOnly == "/graphql" {
baseURL.Path = strings.TrimSuffix(baseURL.Path, "/api/v3") + "/api/graphql"
baseURL.RawQuery = requestURL.RawQuery
} else {
baseURL = baseURL.ResolveReference(requestURL)
}
targetURL := baseURL.String()
logProxy.Printf("forwarding %s %s → %s", method, path, targetURL)
req, err := http.NewRequestWithContext(ctx, method, targetURL, body)
if err != nil {
Collaborator
|
@copilot update the pr based on this review feedback |
Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/4809eb86-df75-49ee-94fe-225e87c3467d Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/4809eb86-df75-49ee-94fe-225e87c3467d Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/4809eb86-df75-49ee-94fe-225e87c3467d Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Collaborator
|
@copilot resolve the merge conflicts in this pull request |
Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/4809eb86-df75-49ee-94fe-225e87c3467d Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/4809eb86-df75-49ee-94fe-225e87c3467d Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Contributor
Author
Updated based on the review feedback in commit The proxy now preserves GraphQL query strings end-to-end by forwarding the original request path (including query) through both GraphQL forwarding call sites, and GHES rewrite handling now accepts Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Copilot
AI
changed the title
Fix GHES GraphQL path handling in DIFC proxy when upstream is
Fix GHES GraphQL path handling and end-to-end query preservation in DIFC proxy when upstream is Apr 16, 2026
/api/v3/api/v3
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Contributor
Author
Resolved merge conflicts and updated the PR with merge commit Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
This was referenced Apr 16, 2026
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
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.
The DIFC proxy was forwarding GraphQL requests to an invalid GHES endpoint when the upstream API base was configured as
.../api/v3, causingghCLI requests routed through the proxy (e.g./api/graphql) to fail with 404. This change normalizes GraphQL forwarding for GHES and ensures GraphQL query strings are preserved end-to-end while keeping existing REST behavior unchanged.GraphQL forwarding normalization for GHES
githubAPIURLends with/api/v3, GraphQL requests are forwarded to/api/graphqlinstead of/api/v3/graphql./graphql,/api/graphql,/api/v3/graphql).End-to-end query-string preservation
"/graphql"path, so query parameters are not dropped in production DIFC flow.Targeted coverage for rewritten GraphQL paths
/graphql→/api/graphql/graphql?foo=bar→/api/graphql?foo=bar/api/graphql→/api/graphql/api/v3/graphql?foo=bar→/api/graphql?foo=bar> [!WARNING]
>
>