🦀 Rust Guard Improvement Report

Improvement 1: Extract duplicated visibility-string ternary into a helper

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

Problem

The expression if is_private { "private" } else { "public" } is repeated verbatim in two places within is_repo_private_with_callback (lines 222 and 286), both used only for building log messages. This is a small duplication, but as more visibility-logging call sites get added (e.g. future retry paths), the pattern is likely to be copy-pasted again.

Suggested Change

Add a tiny private helper near is_repo_private_with_callback and use it at both call sites.

Before

crate::log_debug(&format!(
    "Repo visibility lookup cache hit for {}: {}",
    repo_id,
    if is_private { "private" } else { "public" }
));
...
crate::log_warn(&format!(
    "Repo visibility lookup result for {}: using cached {} after rate-limit error",
    repo_id,
    if is_private { "private" } else { "public" }
));

After

/// Renders a boolean visibility flag as its log-friendly string form.
fn visibility_str(is_private: bool) -> &'static str {
    if is_private { "private" } else { "public" }
}

crate::log_debug(&format!(
    "Repo visibility lookup cache hit for {}: {}",
    repo_id,
    visibility_str(is_private)
));
...
crate::log_warn(&format!(
    "Repo visibility lookup result for {}: using cached {} after rate-limit error",
    repo_id,
    visibility_str(is_private)
));

Why This Matters

Centralizing the mapping avoids future drift if the wording ever needs to change (e.g. localization, or adding an "unknown" state), and removes a small piece of copy-pasted logic flagged by duplication scans.

Improvement 2: Avoid unwrap() on CACHE_TEST_LOCK.lock() in test helper

Category: Type Safety / Test Robustness
File(s): guards/github-guard/rust-guard/src/labels/backend.rs (line ~120)
Effort: Small (< 15 min)
Risk: Low

Problem

lock_repo_visibility_cache_for_tests() calls CACHE_TEST_LOCK.lock().unwrap(). If any test panics while holding this mutex (e.g. an assertion failure inside one of the #[cfg(test)] blocks that call CACHE_TEST_LOCK.lock().unwrap() directly at lines 1163/1177), the Mutex becomes poisoned. Every subsequent test in the same test binary run that tries to lock it will then panic on .unwrap() with an unrelated "PoisonError" message, masking the real failure and causing cascading, confusing test failures.

Suggested Change

Use .unwrap_or_else(|poisoned| poisoned.into_inner()) (or PoisonError::into_inner) so a prior panic doesn't cascade into unrelated test failures — this is test-only code, so recovering the guard after poisoning is safe and simply lets subsequent tests still run and report their own real failure/success.

Before

#[cfg(test)]
pub(crate) fn lock_repo_visibility_cache_for_tests() -> MutexGuard<'static, ()> {
    CACHE_TEST_LOCK.lock().unwrap()
}

After

#[cfg(test)]
pub(crate) fn lock_repo_visibility_cache_for_tests() -> MutexGuard<'static, ()> {
    CACHE_TEST_LOCK.lock().unwrap_or_else(|poisoned| poisoned.into_inner())
}

The two inline call sites at lines 1163 and 1177 (CACHE_TEST_LOCK.lock().unwrap()) should be updated the same way, or better, refactored to call lock_repo_visibility_cache_for_tests() instead of locking the static directly.

Why This Matters

Test suites should fail loudly on the actual broken assertion, not on an unrelated poisoned-mutex panic in a different test that happened to run first. This is a common footgun with shared static Mutex test-serialization locks and is a one-line, zero-risk fix.

Codebase Health Summary

• Total Rust files: 9
• Total lines: ~20,071
• Areas analyzed: lib.rs, permissions.rs, tools.rs, labels/mod.rs, labels/backend.rs, labels/constants.rs, labels/helpers.rs, labels/response_paths.rs, labels/response_items.rs, labels/tool_rules.rs, Cargo.toml
• Areas with no further improvements: none yet (first run)

Generated by Rust Guard Improver • Run: 30995290899

Generated by Rust Guard Improver · auto · 43.9 AIC · ⊞ 10.8K · ◷

• expires on Aug 12, 2026, 9:59 AM UTC