Skip to content

[rust-guard] Rust Guard: Eliminate ctx.clone() in label_agent + NormalizedPolicy &'static str fields #4380

@github-actions

Description

@github-actions

🦀 Rust Guard Improvement Report

Improvement 1: Eliminate ctx.clone() in label_agent

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

Problem

At lib.rs:560, set_runtime_policy_context(ctx.clone()) clones the entire PolicyContext before immediately using ctx in the integrity match block (lines 562–567). PolicyContext holds eight Vec<String> / String fields — including trusted_bots, blocked_users, approval_labels, trusted_users, endorsement_reactions, disapproval_reactions, disapproval_integrity, and endorser_min_integrity. Cloning it deep-copies all of those on every label_agent call.

The only reason for the clone is that ctx is borrowed again immediately afterwards. A simple reorder removes the need entirely.

Suggested Change

Move the integrity computation before the set_runtime_policy_context call so ctx can be moved (not cloned).

Before

// lib.rs:560-567
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),
};

After

// compute integrity first — borrows ctx, does not need the global to be set yet
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),
};
// now move ctx — no clone needed
set_runtime_policy_context(ctx);

Why This Matters

label_agent is the first FFI call on every gateway request. Cloning the entire PolicyContext (all those Vec<String> heap allocations) on every call is pure overhead. The reorder is semantically correct because integrity computations (none_integrity, reader_integrity, etc.) take &PolicyContext and never read from the global. The global is only needed by label_resource / label_response, which are separate FFI calls that happen after label_agent returns.


Improvement 2: Change NormalizedPolicy fields from String to &'static str

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

Problem

NormalizedPolicy in lib.rs:334-338 stores scope_kind and min_integrity as owned String values, even though both are always compile-time constants:

  • min_integrity is set from integrity_floor.as_str().to_string() where as_str() already returns &'static str (line 571).
  • scope_kind is set from normalized_scope_kind() which either returns scope_kind.to_string() (from Display) or "Composite".to_string() — both are static strings (line 570).

ScopeKind has a Display impl but no as_str() -> &'static str method, so normalized_scope_kind is forced to allocate.

Suggested Change

  1. Add as_str() -> &'static str to ScopeKind in labels/helpers.rs.
  2. Change normalized_scope_kind in lib.rs to return &'static str.
  3. Change NormalizedPolicy.scope_kind and min_integrity from String to &'static str.
  4. Remove the .to_string() call at line 571.

Before

// labels/helpers.rs — ScopeKind only has Display, no as_str()
impl std::fmt::Display for ScopeKind {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        let s = match self {
            ScopeKind::All => "All",
            // ...
        };
        f.write_str(s)
    }
}

// lib.rs:334-338
#[derive(Debug, Serialize)]
struct NormalizedPolicy {
    scope_kind: String,
    #[serde(rename = "min-integrity")]
    min_integrity: String,
}

// lib.rs:486-492
fn normalized_scope_kind(scopes: &[PolicyScopeEntry]) -> String {
    if scopes.len() == 1 {
        scopes[0].scope_kind.to_string()  // allocates
    } else {
        "Composite".to_string()           // allocates
    }
}

// lib.rs:569-572
let normalized_policy = NormalizedPolicy {
    scope_kind: scope_kind_str,
    min_integrity: integrity_floor.as_str().to_string(),  // allocates
};

After

// labels/helpers.rs — add as_str() alongside Display
impl ScopeKind {
    pub fn as_str(self) -> &'static str {
        match self {
            ScopeKind::All => "All",
            ScopeKind::Public => "Public",
            ScopeKind::Owner => "Owner",
            ScopeKind::Repo => "Repo",
            ScopeKind::RepoPrefix => "RepoPrefix",
        }
    }
}

// lib.rs:334-338
#[derive(Debug, Serialize)]
struct NormalizedPolicy {
    scope_kind: &'static str,
    #[serde(rename = "min-integrity")]
    min_integrity: &'static str,
}

// lib.rs:486-492
fn normalized_scope_kind(scopes: &[PolicyScopeEntry]) -> &'static str {
    if scopes.len() == 1 {
        scopes[0].scope_kind.as_str()   // zero allocation
    } else {
        "Composite"                      // zero allocation
    }
}

// lib.rs:569-572
let normalized_policy = NormalizedPolicy {
    scope_kind: scope_kind_str,
    min_integrity: integrity_floor.as_str(),  // no .to_string()
};

Why This Matters

Both values are always static strings. Allocating heap Strings for them on every label_agent call is unnecessary. Changing to &'static str also makes the type signature self-documenting: the fields can only hold compile-time constant values, which is exactly what they do. The pattern mirrors what was already done for LabelResourceOutput.operation and LabelAgentOutput.difc_mode.


Codebase Health Summary

  • Total Rust files: 9 source files
  • Total lines: ~12,794
  • Areas analyzed: All source files (lib.rs, tools.rs, labels/mod.rs, labels/helpers.rs, labels/backend.rs, labels/tool_rules.rs, labels/response_items.rs, labels/response_paths.rs, labels/constants.rs)
  • Previously implemented: scopes.clone() eliminated (Apr 17), operation/difc_mode &'static str (Apr 20), is_any_trusted_actor helper (Apr 21), get_issue_author_association_with_callback delegate (Apr 22)

Generated by Rust Guard Improver • Run: §24828196012

Generated by Rust Guard Improver · ● 2.7M ·

  • expires on Apr 30, 2026, 9:52 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