🦀 Rust Guard Improvement Report

Improvement 1: Extract duplicated governance_tools test array into a shared constant

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

Problem

The four-element array ["repository_ruleset_read", "custom_properties_read", "custom_properties_write", "create_repository_ruleset"] is copy-pasted verbatim in three separate test functions:

• apply_tool_labels_repo_governance_tools_are_scoped (line ~1718, as let governance_tools = [...])
• apply_tool_labels_governance_tools_are_repo_scoped (line ~1950, as &[...] inline)
• apply_tool_labels_governance_tools_are_org_and_enterprise_scoped (line ~1977, as &[...] inline)

This mirrors the exact pattern already fixed for the [labels, milestones, branches] method group on 2026-09-02 — the same duplication risk exists here: if a new governance tool (e.g. delete_repository_ruleset) is added to the production match arm at line 729/842, these three test lists must be updated in three places, and it's easy to miss one, silently leaving new tools untested.

Suggested Change

Add a pub(crate) const GOVERNANCE_TOOLS array (test-only, #[cfg(test)]) near the top of the mod tests block in tool_rules.rs, and reference it from all three test functions instead of repeating the literal array.

Before

    #[test]
    fn apply_tool_labels_repo_governance_tools_are_scoped() {
        // ...
        let governance_tools = [
            "repository_ruleset_read",
            "custom_properties_read",
            "custom_properties_write",
            "create_repository_ruleset",
        ];
        for tool in governance_tools {
        // ...

    #[test]
    fn apply_tool_labels_governance_tools_are_repo_scoped() {
        // ...
        for tool in &[
            "repository_ruleset_read",
            "custom_properties_read",
            "custom_properties_write",
            "create_repository_ruleset",
        ] {
        // ...

    #[test]
    fn apply_tool_labels_governance_tools_are_org_and_enterprise_scoped() {
        // ...
            for tool in &[
                "repository_ruleset_read",
                "custom_properties_read",
                "custom_properties_write",
                "create_repository_ruleset",
            ] {

After

#[cfg(test)]
mod tests {
    use super::*;
    // ... existing use statements ...

    /// Tools routed through `apply_governance_labels` (repo/org/enterprise-scoped
    /// governance metadata reads and writes). Shared across the three governance
    /// test functions below to avoid drift when a new governance tool is added.
    const GOVERNANCE_TOOLS: [&str; 4] = [
        "repository_ruleset_read",
        "custom_properties_read",
        "custom_properties_write",
        "create_repository_ruleset",
    ];

    #[test]
    fn apply_tool_labels_repo_governance_tools_are_scoped() {
        // ...
        for tool in GOVERNANCE_TOOLS {
        // ...

    #[test]
    fn apply_tool_labels_governance_tools_are_repo_scoped() {
        // ...
        for tool in &GOVERNANCE_TOOLS {
        // ...

    #[test]
    fn apply_tool_labels_governance_tools_are_org_and_enterprise_scoped() {
        // ...
            for tool in &GOVERNANCE_TOOLS {

Why This Matters

Eliminates a 3-way copy-pasted literal in test code. Future governance tools only need to be added to one array to get coverage in all three scope-variant tests (repo, org/enterprise, and public/private secrecy), removing a source of silent test-coverage gaps.

Improvement 2: Add field_names::COMMENT_NODE_ID constant for the raw "commentNodeID" literal

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

Problem

The raw string literal "commentNodeID" is used at 4 production sites (lines ~865, 875, 893, 903, all .get("commentNodeID") calls in the discussion_comment_write and pull_request_review_write match arms) and 4 more sites in tests (lines ~1392, 1424, 1448, 2964). This follows the same pattern already centralized for method, full_name, login, isError, etc. in field_names — a typo in any one copy (e.g. "commentNodeId" or "comment_node_id") would silently produce an empty node ID and mis-scope the secrecy/integrity labels for GraphQL-only discussion/PR-review mutations.

Suggested Change

Add pub const COMMENT_NODE_ID: &str = "commentNodeID"; to the field_names module in constants.rs, then replace all 4 production .get("commentNodeID") call sites in tool_rules.rs with .get(field_names::COMMENT_NODE_ID).

Before

// labels/constants.rs — field_names module
pub mod field_names {
    // ...
    pub const IS_ERROR: &str = "isError";
}

// labels/tool_rules.rs — discussion_comment_write arm
if repo_id.is_empty() {
    baseline_scope = Cow::Owned(format!(
        "node/{}",
        tool_args
            .get("commentNodeID")
            .and_then(|v| v.as_str())
            .unwrap_or("")
    ));
}
apply_dispatch_repo_labels(
    &owner,
    &repo,
    repo_id,
    tool_args
        .get("commentNodeID")
        .and_then(|v| v.as_str())
        .unwrap_or(""),
    &mut secrecy,
    &mut integrity,
    ctx,
);

After

// labels/constants.rs — field_names module
pub mod field_names {
    // ...
    pub const IS_ERROR: &str = "isError";
    pub const COMMENT_NODE_ID: &str = "commentNodeID";
}

// labels/tool_rules.rs — discussion_comment_write arm
if repo_id.is_empty() {
    baseline_scope = Cow::Owned(format!(
        "node/{}",
        tool_args
            .get(field_names::COMMENT_NODE_ID)
            .and_then(|v| v.as_str())
            .unwrap_or("")
    ));
}
apply_dispatch_repo_labels(
    &owner,
    &repo,
    repo_id,
    tool_args
        .get(field_names::COMMENT_NODE_ID)
        .and_then(|v| v.as_str())
        .unwrap_or(""),
    &mut secrecy,
    &mut integrity,
    ctx,
);

(Same replacement applies to the two .get("commentNodeID") calls in the pull_request_review_write arm just below.)

Why This Matters

Removes 4 duplicate raw-string field-name literals from production logic that directly drives DIFC label scoping for discussion/PR-review node-ID dispatch, matching the existing field_names centralization convention and eliminating a typo-based silent-failure risk.

Codebase Health Summary

• Total Rust files: 9
• Total lines: ~21,623 (lib.rs 1860, tools.rs 925, labels/mod.rs 6901, labels/backend.rs 1796, labels/helpers.rs 4739, labels/tool_rules.rs 3110, labels/response_items.rs 637, labels/response_paths.rs 1426, labels/constants.rs 229)
• Areas analyzed: labels/tool_rules.rs (governance test duplication, commentNodeID literals), labels/constants.rs (field_names candidates), labels/backend.rs & labels/mod.rs (unwrap() usage — no new unsafe patterns found beyond previously-reviewed call sites), Cargo.toml (release profile — no issues)
• Areas with no further improvements: none newly exhausted this run

Generated by Rust Guard Improver • Run: 34583399051

Generated by Rust Guard Improver · copilot · auto · 59.4 AIC · ⊞ 10.1K · ◷

• expires on Sep 18, 2026, 9:21 AM UTC