-
Notifications
You must be signed in to change notification settings - Fork 29
feat: unified JSONL log file between proxy and gateway #2350
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
daeb1e3
feat: unified JSONL log file between proxy and gateway
lpcox d545bf8
feat: add REST backend enrichment to proxy
lpcox cafd95d
fix(proxy): pass tool_args to LabelResponse via request context
lpcox 90541dc
feat(proxy): add trusted-bots support and use stronger token for pre-…
lpcox 4c3c4ad
fix(proxy): use server token for enrichment calls
lpcox d119a9f
fix(workflow): add --trusted-bots flag to proxy start command
lpcox 9ab4d81
fix(workflow): include author field in pre-agent gh CLI calls
lpcox 24210e8
feat(proxy): inject author{login} and authorAssociation into GraphQL …
lpcox 794a0f9
fix(proxy): resolve staticcheck SA4011 ineffective break in switch
lpcox File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,143 @@ | ||
| package proxy | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "regexp" | ||
| "strings" | ||
| ) | ||
|
|
||
| // guardRequiredFields lists the GraphQL selection fields the DIFC guard needs | ||
| // for accurate integrity labeling. author{login} enables trusted-bot detection; | ||
| // authorAssociation provides the integrity level directly (MEMBER, CONTRIBUTOR, | ||
| // etc.) so the guard doesn't need extra enrichment REST round-trips. | ||
| var guardRequiredFields = []struct { | ||
| field string // field text to inject | ||
| present *regexp.Regexp // pattern that indicates the field is already selected | ||
| }{ | ||
| {"author{login}", regexp.MustCompile(`\bauthor\s*\{[^}]*\blogin\b`)}, | ||
| {"authorAssociation", regexp.MustCompile(`\bauthorAssociation\b`)}, | ||
| } | ||
|
|
||
| // allGuardFieldsPresent returns true if the query already contains every | ||
| // required guard field. | ||
| func allGuardFieldsPresent(query string) bool { | ||
| for _, f := range guardRequiredFields { | ||
| if !f.present.MatchString(query) { | ||
| return false | ||
| } | ||
| } | ||
| return true | ||
| } | ||
|
|
||
| // missingGuardFields returns the field strings not yet present in the query. | ||
| func missingGuardFields(query string) []string { | ||
| var missing []string | ||
| for _, f := range guardRequiredFields { | ||
| if !f.present.MatchString(query) { | ||
| missing = append(missing, f.field) | ||
| } | ||
| } | ||
| return missing | ||
| } | ||
|
|
||
| // InjectGuardFields rewrites a GraphQL request body to include fields | ||
| // required by the DIFC guard (e.g. author{login} for trusted-bot detection). | ||
| // Returns the (possibly modified) body. If injection is not needed or fails, | ||
| // the original body is returned unchanged. | ||
| func InjectGuardFields(body []byte, toolName string) []byte { | ||
| // Only rewrite for tools that need author info | ||
| switch toolName { | ||
| case "list_issues", "list_pull_requests", "issue_read", "pull_request_read", | ||
| "search_issues": | ||
| default: | ||
| return body | ||
| } | ||
|
|
||
| var gql GraphQLRequest | ||
| if err := json.Unmarshal(body, &gql); err != nil { | ||
| return body | ||
| } | ||
|
|
||
| if gql.Query == "" || allGuardFieldsPresent(gql.Query) { | ||
| return body | ||
| } | ||
|
|
||
| missing := missingGuardFields(gql.Query) | ||
| modified := injectFieldsIntoQuery(gql.Query, missing) | ||
| if modified == gql.Query { | ||
| return body | ||
| } | ||
|
|
||
| logGraphQL.Printf("injected %v into GraphQL query for %s", missing, toolName) | ||
|
|
||
| gql.Query = modified | ||
| out, err := json.Marshal(gql) | ||
| if err != nil { | ||
| return body | ||
| } | ||
| return out | ||
| } | ||
|
|
||
| // injectFieldsIntoQuery adds the given fields into the GraphQL query's node | ||
| // selection or fragment. Each field string (e.g. "author{login}", | ||
| // "authorAssociation") is comma-joined and injected as a single block. | ||
| func injectFieldsIntoQuery(query string, fields []string) string { | ||
| injection := strings.Join(fields, ",") | ||
|
|
||
| // Step 1: Check if the query uses a fragment spread in the nodes. | ||
| // Pattern: nodes { ...fragmentName } | ||
| fragmentInNodes := regexp.MustCompile(`nodes\s*\{\s*\.\.\.(\w+)`) | ||
| if m := fragmentInNodes.FindStringSubmatch(query); m != nil { | ||
| fragName := m[1] | ||
| return injectIntoFragment(query, fragName, injection) | ||
| } | ||
|
|
||
| // Step 2: No fragment — inject directly into nodes { ... } | ||
| nodesPattern := regexp.MustCompile(`(nodes\s*\{)`) | ||
| if nodesPattern.MatchString(query) { | ||
| return nodesPattern.ReplaceAllString(query, "${1}"+injection+",") | ||
| } | ||
|
|
||
| return query | ||
| } | ||
|
|
||
| // injectIntoFragment adds a field to the end of a named fragment definition. | ||
| // "fragment Name on Type { existing fields }" → "fragment Name on Type { existing fields field }" | ||
| func injectIntoFragment(query, fragName, field string) string { | ||
| // Match: fragment <name> on <Type> { ... } | ||
| // We need to find the closing brace of this specific fragment. | ||
| fragPrefix := "fragment " + fragName + " on " | ||
| idx := strings.Index(query, fragPrefix) | ||
| if idx == -1 { | ||
| return query | ||
| } | ||
|
|
||
| // Find the opening brace of the fragment body | ||
| braceStart := strings.Index(query[idx:], "{") | ||
| if braceStart == -1 { | ||
| return query | ||
| } | ||
| braceStart += idx | ||
|
|
||
| // Find the matching closing brace (handle nested braces) | ||
| depth := 0 | ||
| braceEnd := -1 | ||
| for i := braceStart; i < len(query); i++ { | ||
| if query[i] == '{' { | ||
| depth++ | ||
| } else if query[i] == '}' { | ||
| depth-- | ||
| if depth == 0 { | ||
| braceEnd = i | ||
| break | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if braceEnd == -1 { | ||
| return query | ||
| } | ||
|
|
||
| // Insert field before the closing brace | ||
| return query[:braceEnd] + "," + field + query[braceEnd:] | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing the proxy JSONL filename to match the gateway means two different processes/containers will append to the same JSONL file concurrently. The current JSONL logger uses json.Encoder.Encode (which can perform multiple writes) plus per-entry Sync(), so interleaving writes across processes can produce corrupted/invalid JSONL and the extra fsync contention can significantly slow proxy/gateway logging. Consider implementing cross-process safe append (e.g., marshal to bytes and do a single Write per entry, plus an OS-level file lock), or keep separate files and merge by timestamp during analysis.