🦀 Rust Guard Improvement Report

Improvement 1: Eliminate Needless Clone in Cow::Owned(org.clone())

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

Problem

In the set_secret | delete_secret | set_variable | delete_variable match arm, the local String org is moved into Cow::Owned(org.clone()) while a .clone() is made unnecessarily. At that point org still needs to be borrowed for the two expressions on the adjacent lines:

secrecy = private_scope_label(&org);        // borrows &org
baseline_scope = Cow::Owned(org.clone());   // clones org, then moves the clone into Cow
integrity = writer_integrity(&org, ctx);    // borrows &org again

Since private_scope_label and writer_integrity both accept &str, all borrows can be satisfied before ownership is moved. Swapping the order of the three statements lets us write Cow::Owned(org) (a move, no clone), eliminating an unnecessary heap allocation on this code path.

Suggested Change

Reorder the three assignments so the borrow-only usages of org come first, and the move into Cow::Owned comes last.

Before

} else if !org.is_empty() {
    secrecy = private_scope_label(&org);
    baseline_scope = Cow::Owned(org.clone());
    integrity = writer_integrity(&org, ctx);
}

After

} else if !org.is_empty() {
    secrecy = private_scope_label(&org);
    integrity = writer_integrity(&org, ctx);
    baseline_scope = Cow::Owned(org); // move — no clone needed
}

Why This Matters

• Removes a heap allocation (a String clone) on the org-scoped secret/variable write path.
• Makes the ownership intent explicit: once the two borrow-only uses are done, org is moved rather than redundantly cloned.
• Consistent with how the prior suggestion (2026-07-16) eliminated a similar Cow::Owned(owner.as_str()) pattern elsewhere.

Improvement 2: Add list_releases Test for Private Repository Secrecy

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

Problem

The existing list_releases_produces_merged_integrity_per_item test only covers a public repository (empty secrecy). There is no test verifying that a private repository gets the correct private:<owner>/<repo> secrecy label on both per-item entries and default_labels.

The production code calls repo_visibility_secrecy(&arg_owner, &arg_repo, &default_repo, ctx) (line ~469), which derives privacy from the backend cache or in-memory policy. For tests, privacy is injected via the cache. The missing test leaves the secrecy-for-private-releases branch untested.

Suggested Change

Add a test that primes the repo-visibility cache for a private repository and verifies that list_releases attaches a private:<repo> secrecy label.

Before

Only a public-repo test exists:

#[test]
fn list_releases_produces_merged_integrity_per_item() {
    let tool_args = json!({"owner": "octocat", "repo": "hello"});
    // ... (public repo, empty secrecy checked)
}

After

Add alongside the existing test:

#[test]
fn list_releases_private_repo_gets_private_secrecy() {
    use crate::labels::backend::set_cached_repo_visibility;
    use crate::labels::constants::label_constants;

    // Prime cache: mark octocat/private-repo as private
    set_cached_repo_visibility("octocat/private-repo", true);

    let tool_args = json!({"owner": "octocat", "repo": "private-repo"});
    let items = json!([{"tag_name": "v1.0.0", "name": "Secret release"}]);
    let response = json!({
        "content": [{
            "type": "text",
            "text": serde_json::to_string(&items).expect("should serialize")
        }]
    });

    let result = label_response_paths("list_releases", &tool_args, &response, &ctx())
        .expect("list_releases should produce path labels");

    let private_label = format!("{}octocat/private-repo", label_constants::PRIVATE_PREFIX);
    let merged_label  = format!("{}octocat/private-repo", label_constants::MERGED_PREFIX);

    // Per-item labels
    for entry in &result.labeled_paths {
        assert!(
            entry.labels.secrecy.contains(&private_label),
            "private repo release should carry private secrecy; got {:?}",
            entry.labels.secrecy
        );
        assert!(
            entry.labels.integrity.contains(&merged_label),
            "release should have merged integrity; got {:?}",
            entry.labels.integrity
        );
    }

    // default_labels
    let dl = result.default_labels.as_ref().expect("default_labels should be set");
    assert!(dl.secrecy.contains(&private_label), "default_labels secrecy should be private");
}

Why This Matters

• Closes a coverage gap: the per-item secrecy derivation for list_releases on private repos is entirely untested.
• Prevents future regressions if the repo_visibility_secrecy call is accidentally removed or short-circuited in this arm.
• Mirrors the pattern already used by search_repositories_private_gets_secrecy_public_gets_empty for the search arm.

Codebase Health Summary

• Total Rust files: 10
• Total lines: 19,739
• Areas analyzed: lib.rs, tools.rs, labels/mod.rs, labels/backend.rs, labels/helpers.rs, labels/constants.rs, labels/response_items.rs, labels/response_paths.rs, labels/tool_rules.rs
• Areas with no further improvements: labels/constants.rs (desc_prefix constants fully in place), tools.rs visibility (pub→pub(crate) addressed in prior runs)

Generated by Rust Guard Improver • Run: §30621789716

Generated by Rust Guard Improver · sonnet46 · 138.8 AIC · ⊞ 7.8K · ◷

• expires on Aug 7, 2026, 10:06 AM UTC