Skip to content

[rust-guard] Rust Guard: Eliminate redundant Value clone in extract_mcp_response + PolicyScopeEntry clone in first_matching_scope #5074

@github-actions

Description

@github-actions

🦀 Rust Guard Improvement Report

Improvement 1: Change extract_mcp_response return type to Cow<'_, Value>

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

Problem

extract_mcp_response currently returns an owned Value, and its fallthrough path (the common case when the response is already unwrapped — not wrapped in content[0].text) unconditionally calls response.clone():

// labels/mod.rs, line ~145
pub(crate) fn extract_mcp_response(response: &Value) -> Value {
    // ...try to parse content[0].text...
    // fallthrough — response is already unwrapped:
    response.clone()   // ← clones the entire JSON Value (potentially MBs)
}

This function is called at the very start of both label_response_paths and label_response_items on every label_response invocation. For responses that are already parsed (the majority in production), the entire JSON payload is cloned for no reason.

Suggested Change

Return Cow<'_, Value> so the fallthrough borrows instead of clones. Callers already only read through actual_response.

Before

pub(crate) fn extract_mcp_response(response: &Value) -> Value {
    if let Some(content) = response.get("content").and_then(|v| v.as_array()) {
        if let Some(first) = content.first() {
            if let Some(text) = first.get("text").and_then(|v| v.as_str()) {
                if let Ok(parsed) = serde_json::from_str::<Value>(text) {
                    return parsed;
                }
            }
        }
    }
    response.clone()
}

After

use std::borrow::Cow;

pub(crate) fn extract_mcp_response(response: &Value) -> Cow<'_, Value> {
    if let Some(content) = response.get("content").and_then(|v| v.as_array()) {
        if let Some(first) = content.first() {
            if let Some(text) = first.get("text").and_then(|v| v.as_str()) {
                if let Ok(parsed) = serde_json::from_str::<Value>(text) {
                    return Cow::Owned(parsed);
                }
            }
        }
    }
    Cow::Borrowed(response)   // ← zero allocation on the common path
}

Callers (response_paths.rs:47, response_items.rs:39, backend.rs:400):

  • let actual_response = extract_mcp_response(response); — no call-site change needed; Cow<'_, Value> derefs to Value for all .get(), .as_array(), .is_object() calls automatically.
  • The one spot that does actual_response.clone() (response_items.rs:131) becomes actual_response.as_ref().clone().

Why This Matters

Every label_response call that handles a non-MCP-wrapped response (e.g. plain JSON from the gateway in routed mode) currently deep-copies the full payload. For a collection of 30 PRs or 100 search results this can be hundreds of kilobytes of avoidable allocation. WASM heaps are bounded, so reducing peak allocation pressure matters.


Improvement 2: Return &PolicyScopeEntry reference from first_matching_scope

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

Problem

first_matching_scope ends with .cloned(), cloning the found PolicyScopeEntry (which contains three Option<String> fields: scope_owner, scope_repo, scope_label) even though the single call site reads only three borrowed values:

// helpers.rs ~line 232
fn first_matching_scope(owner: &str, repo: &str, ctx: &PolicyContext) -> Option<PolicyScopeEntry> {
    ctx.scopes
        .iter()
        .find(|scope| { /* predicate */ })
        .cloned()   // ← clones Option<String> × 3 + String
}

// call site in policy_private_scope_label (~line 812):
if let Some(matched_scope) = first_matching_scope(resource_owner, resource_repo, ctx) {
    match matched_scope.scope_kind {          // Copy — fine
        ScopeKind::Owner =>
            private_scope_label(matched_scope.scope_owner.as_deref().unwrap_or("")),
        ScopeKind::Repo | ScopeKind::RepoPrefix =>
            private_scope_label(&matched_scope.scope_label),
        ...
    }
}

policy_private_scope_label is called for every private item in a collection response (one call per PR/issue/commit in label_response_paths), so this clone accumulates.

Suggested Change

Return a reference and let the call site borrow field values.

Before

fn first_matching_scope(owner: &str, repo: &str, ctx: &PolicyContext) -> Option<PolicyScopeEntry> {
    ctx.scopes
        .iter()
        .find(|scope| {
            let scoped_owner = scope.scope_owner.as_deref().unwrap_or("");
            let scoped_repo = scope.scope_repo.as_deref().unwrap_or("");
            repo_matches_scope(scope.scope_kind, owner, repo, scoped_owner, scoped_repo)
        })
        .cloned()
}

After

fn first_matching_scope<'a>(
    owner: &str,
    repo: &str,
    ctx: &'a PolicyContext,
) -> Option<&'a PolicyScopeEntry> {
    ctx.scopes.iter().find(|scope| {
        let scoped_owner = scope.scope_owner.as_deref().unwrap_or("");
        let scoped_repo = scope.scope_repo.as_deref().unwrap_or("");
        repo_matches_scope(scope.scope_kind, owner, repo, scoped_owner, scoped_repo)
    })
}

The call site policy_private_scope_label requires only minor adjustments — scope_kind is Copy, and scope_owner/scope_label are accessed via .as_deref() / as_str(), which work identically on references:

// Before: matched_scope.scope_owner.as_deref()
// After:  matched_scope.scope_owner.as_deref()   ← no change needed

Why This Matters

Eliminates three String clones (scope_owner, scope_repo, scope_label) on every call to policy_private_scope_label. In a response with 30 private PRs, this function is called 30 times — that's 90 avoided String allocations per response. WASM memory is not GC'd; every dropped allocation is a manual dealloc, so reducing churn is meaningful.


Codebase Health Summary

  • Total Rust files: 9
  • Total lines: 13,542
  • Areas analyzed: lib.rs, labels/mod.rs, labels/helpers.rs, labels/backend.rs, labels/response_paths.rs, labels/response_items.rs, labels/tool_rules.rs, labels/constants.rs, tools.rs
  • Areas with no further improvements: tools.rs (comprehensive const arrays + tests added), labels/constants.rs (scope_names/policy_integrity constants complete)

Generated by Rust Guard Improver • Run: §25312523666

Generated by Rust Guard Improver · ● 1.6M ·

  • expires on May 11, 2026, 9:59 AM UTC

Metadata

Metadata

Assignees

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