Skip to content

[rust-guard] Rust Guard: Deduplicate 4 inline search-query-fallback blocks into a shared helper #3740

@github-actions

Description

@github-actions

Improvement 1: Extract Repeated Search Query Fallback Into a Helper

Category: Code Duplication
File(s): guards/github-guard/rust-guard/src/labels/helpers.rs, labels/response_items.rs, labels/response_paths.rs
Effort: Medium (15–30 min)
Risk: Low

Problem

The same 7-line "extract owner/repo from tool_args, and if absent fall back to the search query" block appears 4 times verbatim — in response_items.rs (list_pull_requests arm, ~line 141; list_issues arm, ~line 264) and response_paths.rs (list_pull_requests arm, ~line 133; list_issues arm, ~line 255):

let (mut arg_owner, mut arg_repo, mut arg_repo_full) = extract_repo_info(tool_args);
// For search operations, extract repo from query when tool_args lacks owner/repo
if arg_owner.is_empty() || arg_repo.is_empty() {
    let query = tool_args.get("query").and_then(|v| v.as_str()).unwrap_or("");
    let (q_owner, q_repo, q_repo_id) = extract_repo_info_from_search_query(query);
    if !q_repo_id.is_empty() {
        arg_owner = q_owner;
        arg_repo = q_repo;
        arg_repo_full = q_repo_id;
    }
}

A nearly-identical private function, resolve_search_scope, already exists in tool_rules.rs but is scoped only to that module and has slightly different call-site semantics (caller pre-extracts owner/repo).

Suggested Change

Add a pub(crate) helper to helpers.rs that encapsulates the full pattern, and replace all 4 inline copies.

Before

// (repeated 4× across response_items.rs and response_paths.rs)
let (mut arg_owner, mut arg_repo, mut arg_repo_full) = extract_repo_info(tool_args);
if arg_owner.is_empty() || arg_repo.is_empty() {
    let query = tool_args.get("query").and_then(|v| v.as_str()).unwrap_or("");
    let (q_owner, q_repo, q_repo_id) = extract_repo_info_from_search_query(query);
    if !q_repo_id.is_empty() {
        arg_owner = q_owner;
        arg_repo = q_repo;
        arg_repo_full = q_repo_id;
    }
}

After

helpers.rs — add one new function:

/// Extract (owner, repo, repo_id) from tool_args, falling back to the
/// `query` field's `repo:` qualifier when the explicit fields are absent.
/// This is the canonical resolution for tools that accept either explicit
/// owner/repo args OR a free-text search query with a `repo:` scope.
pub(crate) fn extract_repo_scope_with_query_fallback(
    tool_args: &Value,
) -> (String, String, String) {
    let (owner, repo, repo_id) = extract_repo_info(tool_args);
    if owner.is_empty() || repo.is_empty() {
        let query = tool_args.get("query").and_then(|v| v.as_str()).unwrap_or("");
        let (q_owner, q_repo, q_repo_id) = extract_repo_info_from_search_query(query);
        if !q_repo_id.is_empty() {
            return (q_owner, q_repo, q_repo_id);
        }
    }
    (owner, repo, repo_id)
}

Each of the 4 call sites becomes:

let (arg_owner, arg_repo, arg_repo_full) = extract_repo_scope_with_query_fallback(tool_args);

Why This Matters

  • Eliminates 24 lines of identical boilerplate (4 × 6 lines)
  • Any future fix or semantic change (e.g., query precedence rules) only needs to be made once
  • The helper is easy to unit-test independently, unlike the inline code which requires exercising full match arms

Improvement 2: Merge Identical Blocked-Tool Match Arms in apply_tool_labels

Category: Code Duplication
File(s): guards/github-guard/rust-guard/src/labels/tool_rules.rs
Effort: Small (< 15 min)
Risk: Low

Problem

apply_tool_labels has two consecutive match arms for "blocked repository operations" that have identical bodies — both call only apply_repo_visibility_secrecy with the same arguments and serve the same purpose (classify the resource before label_resource blocks it via is_blocked_tool):

// === Repository Transfer (blocked: irreversible ownership change) ===
"transfer_repository" => {
    secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
}

// === Modifying Repository Operations (blocked: unsupported gh repo operations) ===
"archive_repository" | "unarchive_repository" | "rename_repository" => {
    secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
}

(labels/tool_rules.rs, lines ~211–227)

Suggested Change

Merge into a single arm.

Before

// === Repository Transfer (blocked: irreversible ownership change) ===
"transfer_repository" => {
    secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
}

// === Modifying Repository Operations (blocked: unsupported gh repo operations) ===
"archive_repository" | "unarchive_repository" | "rename_repository" => {
    secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
}

After

// === Blocked repository operations ===
// Applies repo-visibility secrecy before label_resource enforces the unconditional
// block via is_blocked_tool(). Covers: irreversible ownership changes
// (transfer_repository) and unsupported gh-repo operations (archive, unarchive,
// rename).
"transfer_repository"
| "archive_repository"
| "unarchive_repository"
| "rename_repository" => {
    secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
}

Why This Matters

  • Removes the silent duplication risk: a future maintainer adding logic to one arm would need to remember to mirror it in the other
  • All five blocked tools share the same pre-block classification policy; having a single arm makes that relationship explicit

Codebase Health Summary

  • Total Rust files: 10
  • Total lines: ~11,925
  • Areas analyzed: lib.rs, tools.rs, labels/mod.rs, labels/constants.rs, labels/helpers.rs, labels/backend.rs, labels/tool_rules.rs, labels/response_items.rs, labels/response_paths.rs
  • Areas with no further improvements: labels/constants.rs (clean)

Generated by Rust Guard Improver • Run: §24391991478

Generated by Rust Guard Improver · ● 1.2M ·

  • expires on Apr 21, 2026, 9:45 AM UTC

Metadata

Metadata

Assignees

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions