Skip to content

[rust-guard] Rust Guard: eliminate scopes.clone() in label_agent + named DIFC_MODE constant #4008

@github-actions

Description

@github-actions

🦀 Rust Guard Improvement Report

Improvement 1: Eliminate scopes.clone() in label_agent by Reordering

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

Problem

In label_agent (lines 524–562), scopes is cloned when constructing PolicyContext, even though scopes continues to be used afterwards for computing secrecy, token, and the normalized scope kind. The clone is only needed because ctx is constructed before these values are computed.

let ctx = PolicyContext {
    scopes: scopes.clone(),   // ← unnecessary clone; scopes used below
    ...
};
set_runtime_policy_context(ctx.clone());

let secrecy: Vec<String> = scopes.iter()...;   // uses scopes
let token = scope_token(&scopes);              // uses scopes
let integrity = ... labels::none_integrity(&token, &ctx) ...;
let normalized_policy = NormalizedPolicy {
    scope_kind: normalized_scope_kind(&scopes), // uses scopes
    ...
};

PolicyScopeEntry contains heap-allocated String fields (scope_owner, scope_repo, scope_label), so each cloned entry allocates. The WASM environment benefits from avoiding unnecessary heap allocations.

Suggested Change

Move the computation of secrecy, token, and scope_kind before the PolicyContext constructor, then move scopes directly into ctx (no clone). The ctx.clone() for set_runtime_policy_context is unavoidable and stays unchanged.

Before

let ctx = PolicyContext {
    scopes: scopes.clone(),
    trusted_bots,
    blocked_users: policy.blocked_users,
    approval_labels: policy.approval_labels,
    trusted_users: policy.trusted_users,
    endorsement_reactions: policy.endorsement_reactions,
    disapproval_reactions: policy.disapproval_reactions,
    disapproval_integrity: policy.disapproval_integrity,
    endorser_min_integrity: policy.endorser_min_integrity,
};
set_runtime_policy_context(ctx.clone());

let secrecy: Vec<String> = scopes
    .iter()
    .filter_map(|scope| match scope.scope_kind {
        ScopeKind::All | ScopeKind::Public => None,
        ScopeKind::Owner | ScopeKind::Repo | ScopeKind::RepoPrefix => {
            if scope.scope_label.is_empty() {
                None
            } else {
                Some(format!("private:{}", scope.scope_label))
            }
        }
    })
    .collect();

let token = scope_token(&scopes);
let integrity = match integrity_floor {
    MinIntegrity::None => labels::none_integrity(&token, &ctx),
    MinIntegrity::Unapproved => labels::reader_integrity(&token, &ctx),
    MinIntegrity::Approved => labels::writer_integrity(&token, &ctx),
    MinIntegrity::Merged => labels::merged_integrity(&token, &ctx),
};

let difc_mode = "filter";

let normalized_policy = NormalizedPolicy {
    scope_kind: normalized_scope_kind(&scopes),
    min_integrity: integrity_floor.as_str().to_string(),
};

After

// Compute scope-derived values while `scopes` is still owned, before moving it into ctx.
let secrecy: Vec<String> = scopes
    .iter()
    .filter_map(|scope| match scope.scope_kind {
        ScopeKind::All | ScopeKind::Public => None,
        ScopeKind::Owner | ScopeKind::Repo | ScopeKind::RepoPrefix => {
            if scope.scope_label.is_empty() {
                None
            } else {
                Some(format!("private:{}", scope.scope_label))
            }
        }
    })
    .collect();

let token = scope_token(&scopes);
let scope_kind_str = normalized_scope_kind(&scopes);

let ctx = PolicyContext {
    scopes, // moved directly — no clone needed
    trusted_bots,
    blocked_users: policy.blocked_users,
    approval_labels: policy.approval_labels,
    trusted_users: policy.trusted_users,
    endorsement_reactions: policy.endorsement_reactions,
    disapproval_reactions: policy.disapproval_reactions,
    disapproval_integrity: policy.disapproval_integrity,
    endorser_min_integrity: policy.endorser_min_integrity,
};
set_runtime_policy_context(ctx.clone());

let integrity = match integrity_floor {
    MinIntegrity::None => labels::none_integrity(&token, &ctx),
    MinIntegrity::Unapproved => labels::reader_integrity(&token, &ctx),
    MinIntegrity::Approved => labels::writer_integrity(&token, &ctx),
    MinIntegrity::Merged => labels::merged_integrity(&token, &ctx),
};

let normalized_policy = NormalizedPolicy {
    scope_kind: scope_kind_str,
    min_integrity: integrity_floor.as_str().to_string(),
};

Why This Matters

Each PolicyScopeEntry contains three String fields (scope_owner, scope_repo, scope_label) that heap-allocate. Reordering eliminates the clone of the entire Vec<PolicyScopeEntry> with zero behavior change. In WASM, every avoided heap allocation reduces both peak memory use and GC pressure from the bump allocator.


Improvement 2: Replace Magic String "filter" with a Named Constant

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

Problem

Lines 559 and 568 of lib.rs use a bare string literal "filter" for the DIFC mode, creating an intermediate binding that exists solely to enable the .to_string() call:

let difc_mode = "filter";
// ...
difc_mode: difc_mode.to_string(),

This is a magic string — its meaning is implied only by the variable name. There is no named constant in lib.rs or labels/constants.rs that documents what the valid DIFC modes are or which one is canonical. If the value ever needs to change, there is a single string to update, but currently nothing signals its role to a reader.

Suggested Change

Declare a named constant at the top of lib.rs (near the other top-level constants at lines 24–26) and use it directly in the struct literal, eliminating the intermediate let.

Before

// (near top of lib.rs, existing constants)
const POLICY_SCOPE_ALL: &str = "all";
const POLICY_SCOPE_PUBLIC: &str = "public";

// ... inside label_agent ...
let difc_mode = "filter";

let normalized_policy = NormalizedPolicy { ... };

let output = LabelAgentOutput {
    agent: AgentLabels { secrecy, integrity },
    difc_mode: difc_mode.to_string(),
    normalized_policy,
};

After

// (near top of lib.rs, alongside existing constants)
const POLICY_SCOPE_ALL: &str = "all";
const POLICY_SCOPE_PUBLIC: &str = "public";
const DIFC_MODE: &str = "filter";

// ... inside label_agent — no intermediate let needed ...
let normalized_policy = NormalizedPolicy { ... };

let output = LabelAgentOutput {
    agent: AgentLabels { secrecy, integrity },
    difc_mode: DIFC_MODE.to_string(),
    normalized_policy,
};

Why This Matters

A named constant makes the value self-documenting and searchable. It matches the established pattern of POLICY_SCOPE_ALL and POLICY_SCOPE_PUBLIC already at the top of lib.rs, and it eliminates the superfluous one-line let binding whose only purpose was to name an anonymous literal.


Codebase Health Summary

  • Total Rust files: 10
  • Total lines: 12,439
  • Areas analyzed: lib.rs, labels/mod.rs, labels/helpers.rs, labels/tool_rules.rs, labels/constants.rs, tools.rs, labels/backend.rs, labels/response_items.rs, labels/response_paths.rs
  • Areas with no further improvements noted this run: tools.rs (well-structured, good test coverage), labels/constants.rs (clean)

Generated by Rust Guard Improver • Run: §24558457108

References:

Generated by Rust Guard Improver · ● 836.3K ·

  • expires on Apr 24, 2026, 9:53 AM UTC

Metadata

Metadata

Assignees

No one assigned

    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