[rust-guard] Rust Guard: dedup PR fork-detection fallback and use OWNER constant in owner_type_from_repo_object

[github-actions]

🦀 Rust Guard Improvement Report

Improvement 1: Remove duplicated inline fork-detection fallback in response_items.rs

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

Problem

In labels/response_items.rs (around lines 189–203), the code calls is_forked_pr(item) and then, in the .or_else(...) fallback, re-implements the exact same base/head repo.full_name comparison logic that is_forked_pr (in labels/helpers.rs:1321) already performs. Since is_forked_pr already returns None only when either full_name is missing/empty (the same condition checked again inline), this fallback branch is dead code — it can never produce a different result than what is_forked_pr already computed.

let is_forked = is_forked_pr(item).or_else(|| {
    let base_full_name = item
        .get("base")
        .and_then(|b| b.get("repo"))
        .and_then(|r| r.get(field_names::FULL_NAME))
        .and_then(|v| v.as_str());
    let head_full_name = item
        .get("head")
        .and_then(|h| h.get("repo"))
        .and_then(|r| r.get(field_names::FULL_NAME))
        .and_then(|v| v.as_str());
    base_full_name
        .zip(head_full_name)
        .map(|(base, head)| !base.eq_ignore_ascii_case(head))
});

Suggested Change

Remove the redundant .or_else(...) closure entirely and call is_forked_pr(item) directly, since it already implements identical logic.

Before

let is_forked = is_forked_pr(item).or_else(|| {
    let base_full_name = item
        .get("base")
        .and_then(|b| b.get("repo"))
        .and_then(|r| r.get(field_names::FULL_NAME))
        .and_then(|v| v.as_str());
    let head_full_name = item
        .get("head")
        .and_then(|h| h.get("repo"))
        .and_then(|r| r.get(field_names::FULL_NAME))
        .and_then(|v| v.as_str());
    base_full_name
        .zip(head_full_name)
        .map(|(base, head)| !base.eq_ignore_ascii_case(head))
});

After

let is_forked = is_forked_pr(item);

Why This Matters

This removes ~13 lines of dead duplicated logic that can never execute differently than is_forked_pr, reducing WASM binary size slightly and eliminating a maintenance trap: if is_forked_pr's comparison logic ever changes (e.g., a future case-sensitivity fix), this inline copy would silently drift out of sync and produce inconsistent fork-detection results.

---

Improvement 2: Use field_names::OWNER constant instead of raw "owner" literal in backend.rs owner extraction

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

Problem

labels/backend.rs has three production (non-test) call sites that use the raw string literal "owner" for JSON field extraction (lines 1630, 1652, 1661 in owner_type_from_repo_object and repo_id_from_repo_object), even though field_names::OWNER already exists in labels/constants.rs and is used elsewhere in the same file's helper functions (e.g., field_names::FULL_NAME, field_names::LOGIN are already used a few lines below at line 1652-1655). This is an inconsistency: some field lookups in the same function use constants, others use raw literals.

Suggested Change

Replace the raw "owner" literals with field_names::OWNER in the three production call sites.

Before

fn owner_type_from_repo_object(item: &Value) -> Option<bool> {
    let owner_type = item
        .get("owner")
        .and_then(|o| o.get("type"))
        .and_then(|v| v.as_str())?;

    Some(owner_type.eq_ignore_ascii_case("Organization"))
}

    if let Some(owner_login) = item
        .get("owner")
        .and_then(|owner| owner.get(field_names::LOGIN))
        ...
    if let Some(owner_name) = item.get("owner").and_then(|v| v.as_str()) {

After

fn owner_type_from_repo_object(item: &Value) -> Option<bool> {
    let owner_type = item
        .get(field_names::OWNER)
        .and_then(|o| o.get("type"))
        .and_then(|v| v.as_str())?;

    Some(owner_type.eq_ignore_ascii_case("Organization"))
}

    if let Some(owner_login) = item
        .get(field_names::OWNER)
        .and_then(|owner| owner.get(field_names::LOGIN))
        ...
    if let Some(owner_name) = item.get(field_names::OWNER).and_then(|v| v.as_str()) {

Why This Matters

Keeps field-name access consistent (all via field_names::* constants) within the same function that already partially uses constants, avoiding a silent typo risk (e.g., "Owner" vs "owner") that would produce a wrong DIFC label with no compiler error — the exact rationale documented in constants.rs for why these constants exist in the first place.

---

Codebase Health Summary

• Total Rust files: 9 (lib.rs, permissions.rs, tools.rs, labels/{mod,backend,constants,helpers,response_items,response_paths,tool_rules}.rs)
• Total lines: ~20,649
• Areas analyzed: All modules, focusing on duplication, dead code, and type-safety (raw string literals vs. constants)
• Areas with no further improvements: None marked fully exhausted; ongoing incremental constant-adoption continues to surface small opportunities each week

---

Generated by Rust Guard Improver • Run: 32467016449

Generated by Rust Guard Improver · auto · 43 AIC · ⊞ 11.4K · ◷

• expires on Aug 28, 2026, 9:19 AM UTC

[github-actions]

🤖 This is an automated response from Repo Assist.

Thanks for the report! I implemented Improvement 2 (using field_names::OWNER instead of the raw "owner" literal in owner_type_from_repo_object and repo_id_from_repo_object) in a draft PR — build and full test suite (645 tests) pass.

I did not implement Improvement 1 (removing the is_forked_pr(item).or_else(...) fallback in response_items.rs): I tried it, and it broke the existing test list_pull_requests_preserves_empty_full_name_fork_semantics. is_forked_pr returns None when either full_name is an empty string, but the inline fallback deliberately treats two empty full_names as "same repo" (direct PR, not forked) — so it's not actually dead code, just intentionally different handling of the empty-string edge case. Leaving that logic in place to avoid an integrity-labeling regression.

Generated by Repo Assist · auto · 91.2 AIC · ⊞ 18.7K · ◷
_{Comment /repo-assist to run again}

_{Add this agentic workflow to your repo}

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@851905c06e905bf362a9f6cc54f912e3df747d55