rust-guard: sort BLOCKED_TOOLS, use binary_search, promote login to #[cfg(test)]#6913
Merged
Conversation
… to cfg(test) - Sort BLOCKED_TOOLS alphabetically to match WRITE_OPERATIONS/READ_WRITE_OPERATIONS convention - Switch is_blocked_tool to binary_search (O(log n)) instead of .contains() (O(n)) - Trim verbose redundant doc comment on is_blocked_tool to a concise one-liner - Add blocked_tools_are_sorted test mirroring write_operations_are_sorted - Change CollaboratorPermission.login from cfg_attr(not(test), allow(dead_code)) to #[cfg(test)], removing the field and its two allocations from production/WASM builds
Copilot
AI
changed the title
[WIP] Sort BLOCKED_TOOLS and promote CollaboratorPermission.login
rust-guard: sort BLOCKED_TOOLS, use binary_search, promote login to #[cfg(test)]
Jun 2, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aligns the Rust guard’s tool-classification and test-only metadata patterns by making BLOCKED_TOOLS match the existing sorted + binary_search approach used elsewhere, and by moving CollaboratorPermission.login fully behind #[cfg(test)] to avoid carrying the field in production builds.
Changes:
- Sort
BLOCKED_TOOLSand switchis_blocked_toolfrom.contains()tobinary_search, adding a sorted-order unit test. - Shorten the
BLOCKED_TOOLSdoc comment now that per-entry inline comments carry the detail. - Make
CollaboratorPermission.logintest-only and gate struct-literal initialization accordingly.
Show a summary per file
| File | Description |
|---|---|
| guards/github-guard/rust-guard/src/tools.rs | Sorts BLOCKED_TOOLS, uses binary_search, and adds a sorted-order test for correctness. |
| guards/github-guard/rust-guard/src/labels/backend.rs | Makes CollaboratorPermission.login test-only and updates construction sites to compile without the field in production. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 1
Comment on lines
+576
to
+580
| Some(CollaboratorPermission { | ||
| permission, | ||
| #[cfg(test)] | ||
| login, | ||
| }) |
Collaborator
|
@copilot address review feedback |
Contributor
Author
Done — the |
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.
BLOCKED_TOOLSwas the only slice intools.rswithout sorted order orbinary_search, andCollaboratorPermission.loginwas silencing a dead-code lint rather than actually removing the field from production builds.tools.rsBLOCKED_TOOLSalphabetically to matchWRITE_OPERATIONS/READ_WRITE_OPERATIONSis_blocked_tooltobinary_search(O(log n) vs O(n).contains())blocked_tools_are_sortedtest — mirrors the existing sorted-order tests for the other two sliceslabels/backend.rsCollaboratorPermission.loginfrom#[cfg_attr(not(test), allow(dead_code))]to#[cfg(test)]— removes the field and its allocations from the WASM binary entirely#[cfg(test)]: cache-hit path and API-call pathAll three "blocked/write/read-write" slices now follow the same sorted +
binary_search+ sorted-test pattern. Theloginfield now matches the#[cfg(test)]pattern already used by other test-only fields in the codebase.