Conversation
Add logGraphQL calls to the extractOwnerRepo and extractSearchQuery helper functions in internal/proxy/graphql.go to make the multi-step extraction process observable during debugging. - extractOwnerRepo: log values found in request variables - extractOwnerRepo: log values extracted via query regex fallback - extractSearchQuery: log search query found in variables - extractSearchQuery: log search query found inline in the query text - extractSearchQuery: log when no search query is found These logs activate only when DEBUG=proxy:graphql (or DEBUG=*) is set, adding zero overhead in production while enabling precise tracing of why a particular GraphQL request ends up with a specific owner/repo/query label. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds additional debug-level observability to GraphQL parsing in the proxy layer so operators can see how owner/repo and search query scope are derived when DEBUG=proxy:graphql is enabled.
Changes:
- Add debug logs in
extractOwnerRepowhen values are sourced from request variables and from the query regex fallback. - Add debug logs in
extractSearchQuerywhen the search query is found (variables or inline) and when no query is found.
Show a summary per file
| File | Description |
|---|---|
| internal/proxy/graphql.go | Adds targeted logGraphQL debug statements for owner/repo and search query extraction paths. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
internal/proxy/graphql.go:205
- Same concern for the inline match: logging the raw captured string without quoting can allow newlines/control characters to spill into logs. Prefer logging an escaped/quoted, truncated representation for safer/cleaner debug output.
if m := searchQueryArgPattern.FindStringSubmatch(query); m != nil {
logGraphQL.Printf("extractSearchQuery: found inline: %.80s", m[1])
return m[1]
- Files reviewed: 1/1 changed files
- Comments generated: 1
Comment on lines
197
to
199
| if v, ok := variables["query"].(string); ok && v != "" { | ||
| logGraphQL.Printf("extractSearchQuery: found in variables: %.80s", v) | ||
| return v |
There was a problem hiding this comment.
The search query value is user-controlled and may contain newlines/control characters; logging it with %s can create multi-line or hard-to-parse debug output. Consider quoting/escaping the truncated value (e.g., using a quoted format verb) to keep logs single-line and unambiguous.
This issue also appears on line 203 of the same file.
See below for a potential fix:
func truncateForLog(s string, maxRunes int) string {
if maxRunes <= 0 {
return ""
}
r := []rune(s)
if len(r) <= maxRunes {
return s
}
return string(r[:maxRunes])
}
// extractSearchQuery returns the search query argument from a GraphQL search
// query. It checks variables ($query) first, then inline query text.
func extractSearchQuery(query string, variables map[string]interface{}) string {
// Check variables for $query
if variables != nil {
if v, ok := variables["query"].(string); ok && v != "" {
logGraphQL.Printf("extractSearchQuery: found in variables: %q", truncateForLog(v, 80))
return v
}
}
// Parse inline: search(query:"repo:owner/name is:issue", ...)
if m := searchQueryArgPattern.FindStringSubmatch(query); m != nil {
logGraphQL.Printf("extractSearchQuery: found inline: %q", truncateForLog(m[1], 80))
Collaborator
|
@copilot apply changes based on the comments in this thread |
…lled search query values Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/985634c1-dde7-42d6-b1bb-258bd55d1f76 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Contributor
Done in 108772e. Added a 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 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.
Adds 5 targeted
logGraphQLdebug log calls to the helper functions ininternal/proxy/graphql.gothat extract owner, repo, and search query values from GraphQL requests.Changes
File:
internal/proxy/graphql.go(1 file, 5 insertions)extractOwnerRepoThis function tries three extraction strategies in order: request variables → query regex → raw JSON in query text. The new log calls make each step observable:
variablesmap (e.g.owner=%q repo=%q)queryRepoPatternregex fallbackextractSearchQueryWhy this helps
When debugging why a GraphQL request ends up with incorrect or missing owner/repo labels (e.g. the guard applies the wrong policy), operators can enable
DEBUG=proxy:graphqlto see exactly which code path populated each field. Without these logs, the extraction is a black box.Checklist
logGraphQLlogger (no new declaration needed)pkg:filenameconvention (proxy:graphql)MatchGraphQLentry/exit; new logs cover internal extraction steps)