Add PRIVATE_BASE constant and deduplicate GraphQL traversal in rust-guard#3233
Merged
Add PRIVATE_BASE constant and deduplicate GraphQL traversal in rust-guard#3233
Conversation
lpcox
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves consistency and maintainability in the GitHub rust-guard labeling code by introducing a shared "private" secrecy constant and deduplicating repeated GraphQL node traversal logic. It also includes a small proxy-side behavior change to URL-escape the REST search_repositories query string.
Changes:
- Add
PRIVATE_BASE = "private"tolabel_constantsand replace hard-coded"private"label literals. - Deduplicate GraphQL collection traversal via a shared
find_graphql_nodes_with_pathhelper. - URL-escape the REST proxy
search_repositoriesqparameter.
Show a summary per file
| File | Description |
|---|---|
| internal/proxy/proxy.go | URL-escapes REST search_repositories query when building the upstream GitHub API path |
| guards/github-guard/rust-guard/src/labels/constants.rs | Adds PRIVATE_BASE to centralized label constants |
| guards/github-guard/rust-guard/src/labels/helpers.rs | Replaces raw "private" label usage; factors out GraphQL node traversal helper |
| guards/github-guard/rust-guard/src/labels/response_items.rs | Uses label_constants::PRIVATE_BASE instead of raw "private" label |
| guards/github-guard/rust-guard/src/labels/response_paths.rs | Uses label_constants::PRIVATE_BASE instead of raw "private" label |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 5/5 changed files
- Comments generated: 1
internal/proxy/proxy.go
Outdated
Comment on lines
+323
to
+327
| @@ -324,7 +324,7 @@ func (r *restBackendCaller) CallTool(ctx context.Context, toolName string, args | |||
| if pp, ok := argsMap["perPage"].(float64); ok { | |||
| perPage = fmt.Sprintf("%d", int(pp)) | |||
| } | |||
| apiPath = fmt.Sprintf("/search/repositories?q=%s&per_page=%s", query, perPage) | |||
| apiPath = fmt.Sprintf("/search/repositories?q=%s&per_page=%s", url.QueryEscape(query), perPage) | |||
There was a problem hiding this comment.
The PR description/title focuses on rust-guard label/constants + GraphQL traversal deduplication, but this hunk also changes REST proxy behavior (URL-escaping the search query). Please either (a) mention this Go change in the PR description or (b) split it into a separate PR so reviewers can track the behavioral change explicitly.
Improvement 1: Add PRIVATE_BASE constant to label_constants - Add `PRIVATE_BASE = "private"` constant alongside existing BLOCKED_BASE, READER_BASE, WRITER_BASE, MERGED_BASE constants - Replace 4 bare "private" string literals across helpers.rs, response_items.rs, and response_paths.rs with label_constants::PRIVATE_BASE Improvement 2: Eliminate GraphQL traversal duplication - Extract shared `find_graphql_nodes_with_path` helper that returns both the nodes array and its JSON Pointer path - Simplify `extract_items_array` GraphQL branch to a one-liner delegation - Rewrite `extract_graphql_nodes` to delegate entirely to the shared helper - Removes ~12 lines of verbatim-duplicated traversal logic Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
lpcox
force-pushed
the
copilot/rust-guard-add-private-base-constant
branch
from
5fa1874 to
2dca269
Compare
This was referenced Apr 6, 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.
Two small deduplication improvements in the Rust guard, as described in the original issue.
Improvement 1: Add
PRIVATE_BASEconstant tolabel_constantsThe bare
"private"string was used as a secrecy label value in 4 places across three files, butlabel_constantshad no constant for it — despite already definingBLOCKED_BASE,READER_BASE,WRITER_BASE, andMERGED_BASE.PRIVATE_BASE = "private"toconstants.rshelpers.rs,response_items.rs, andresponse_paths.rsImprovement 2: Eliminate GraphQL traversal duplication
extract_items_arrayandextract_graphql_nodesboth contained an identical ~12-line GraphQL node traversal loop. Extracted a sharedfind_graphql_nodes_with_pathhelper:extract_items_arraynow delegates its GraphQL branch to the helperextract_graphql_nodesbecomes a one-liner:find_graphql_nodes_with_path(response).map(|(arr, _)| arr)GRAPHQL_COLLECTION_FIELDSVerification
cargo test)make agent-finishedpasses (Go build + lint + all tests)