Skip to content

[rust-guard] Rust Guard: Replace static-string String fields with &'static str in output structs + add response-labeling tests #4178

@github-actions

Description

@github-actions

🦀 Rust Guard Improvement Report

Improvement 1: Use &'static str for Always-Static Output Fields

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

Problem

Two serialization-only output structs contain String fields that are always populated from &'static str values, causing a heap allocation (and deallocation) on every call:

  • LabelResourceOutput::operation is always one of "read", "write", or "read-write" — three compile-time string literals stored in the operation: &'static str local variable, then cloned to String at line 716 via .to_string().
  • LabelAgentOutput::difc_mode is always DIFC_MODE (a &'static str constant), converted to String at line 570.

These are Serialize-only structs (never deserialized), so serde handles &'static str identically to String. There is no reason to heap-allocate.

Suggested Change

Change the two fields from String to &'static str and remove the .to_string() calls at their construction sites.

Before

#[derive(Debug, Serialize)]
struct LabelResourceOutput {
    resource: ResourceLabels,
    operation: String,              // always "read", "write", or "read-write"
}

#[derive(Debug, Serialize)]
struct LabelAgentOutput {
    agent: AgentLabels,
    difc_mode: String,              // always DIFC_MODE = "filter"
    normalized_policy: NormalizedPolicy,
}

// --- construction sites ---
// label_resource (~line 716):
let output = LabelResourceOutput {
    resource: ...,
    operation: operation.to_string(),   // <-- allocation
};

// label_agent (~line 570):
let output = LabelAgentOutput {
    agent: ...,
    difc_mode: DIFC_MODE.to_string(),   // <-- allocation
    normalized_policy: ...,
};

After

#[derive(Debug, Serialize)]
struct LabelResourceOutput {
    resource: ResourceLabels,
    operation: &'static str,
}

#[derive(Debug, Serialize)]
struct LabelAgentOutput {
    agent: AgentLabels,
    difc_mode: &'static str,
    normalized_policy: NormalizedPolicy,
}

// --- construction sites ---
// label_resource:
let output = LabelResourceOutput {
    resource: ...,
    operation,                  // no allocation — already &'static str
};

// label_agent:
let output = LabelAgentOutput {
    agent: ...,
    difc_mode: DIFC_MODE,       // no allocation — const &'static str
    normalized_policy: ...,
};

Why This Matters

In WASM, every heap allocation (and the corresponding deallocation) has measurable overhead due to the linear-memory allocator. label_resource and label_agent are called on every tool invocation. Eliminating even small redundant allocations improves throughput and reduces peak memory in the linear heap. The change is purely mechanical — no logic changes, no behavior changes, and serde::Serialize works identically on &'static str and String.


Improvement 2: Add Unit Tests for label_response_paths and label_response_items

Category: Test Coverage
File(s): guards/github-guard/rust-guard/src/labels/response_paths.rs, guards/github-guard/rust-guard/src/labels/response_items.rs
Effort: Medium (15–30 min)
Risk: Low

Problem

response_paths.rs and response_items.rs are the two most complex modules in the labeling pipeline — together they handle per-item DIFC label assignment for every collection response. Yet neither file contains a single unit test (confirmed: no #[cfg(test)] block in either file).

The label_response_paths function alone has five distinct branches (search_repositories, list_pull_requests / search_pull_requests, list_issues / search_issues, list_commits, and the _ => fallback), each with their own internal logic for computing secrecy and integrity. A regression in any branch would go undetected until an integration test (if one exists) catches it.

High-value test cases that are currently untested:

  1. search_repositories — private repo gets private secrecy, public gets empty secrecy
  2. list_pull_requests — merged PR gets merged:<repo> integrity, open PR gets lower integrity
  3. search_issues — repo scope extracted from query's repo: qualifier

Suggested Change

Add a #[cfg(test)] block at the bottom of response_paths.rs (matching the style used in helpers.rs and lib.rs) with at least these three tests:

#[cfg(test)]
mod tests {
    use super::*;
    use crate::labels::helpers::PolicyContext;
    use serde_json::json;

    fn ctx() -> PolicyContext {
        PolicyContext::default()
    }

    /// Private repos must receive a non-empty secrecy label;
    /// public repos must receive an empty secrecy vector.
    #[test]
    fn search_repositories_private_gets_secrecy_public_gets_empty() {
        let tool_args = json!({});
        let response = json!({
            "content": [{
                "type": "text",
                "text": serde_json::to_string(&json!({
                    "items": [
                        {"full_name": "octocat/private-repo", "private": true},
                        {"full_name": "octocat/public-repo",  "private": false}
                    ]
                })).unwrap()
            }]
        });

        let result = label_response_paths("search_repositories", &tool_args, &response, &ctx());
        let result = result.expect("should produce path labels");
        assert_eq!(result.labeled_paths.len(), 2);

        let private_entry = &result.labeled_paths[0];
        let public_entry  = &result.labeled_paths[1];

        assert!(!private_entry.labels.secrecy.is_empty(),
                "private repo should have non-empty secrecy");
        assert!(public_entry.labels.secrecy.is_empty(),
                "public repo should have empty secrecy");
    }

    /// A merged PR should receive merged-level integrity.
    #[test]
    fn list_pull_requests_merged_pr_gets_merged_integrity() {
        use crate::labels::constants::label_constants;
        let tool_args = json!({"owner": "octocat", "repo": "hello-world"});
        let pr = json!({
            "number": 1,
            "merged_at": "2024-01-01T00:00:00Z",
            "base": {"repo": {"full_name": "octocat/hello-world"}},
            "head": {"repo": {"full_name": "octocat/hello-world"}}
        });
        let response = json!({
            "content": [{"type": "text", "text": serde_json::to_string(&json!([pr])).unwrap()}]
        });

        let result = label_response_paths("list_pull_requests", &tool_args, &response, &ctx());
        let result = result.expect("should produce path labels");
        assert_eq!(result.labeled_paths.len(), 1);

        let entry = &result.labeled_paths[0];
        let merged_label = format!("{}octocat/hello-world", label_constants::MERGED_PREFIX);
        assert!(
            entry.labels.integrity.contains(&merged_label),
            "merged PR should have merged integrity; got {:?}",
            entry.labels.integrity
        );
    }

    /// label_response_paths returns None for an unrecognised tool name.
    #[test]
    fn unknown_tool_returns_none() {
        let result = label_response_paths(
            "unknown_tool",
            &json!({}),
            &json!({}),
            &ctx(),
        );
        assert!(result.is_none(), "unknown tool should produce no path labels");
    }
}

Why This Matters

response_paths.rs is the preferred (zero-copy) labeling path used in production for all collection responses. A silent regression here — e.g., a private repo accidentally getting empty secrecy — would leak private content to agents that shouldn't see it. These tests take ~20 minutes to write and permanently protect critical DIFC invariants.


Codebase Health Summary

  • Total Rust files: 9
  • Total lines: 12,495
  • Areas analyzed: lib.rs, tools.rs, labels/mod.rs, labels/helpers.rs, labels/tool_rules.rs, labels/backend.rs, labels/constants.rs, labels/response_items.rs, labels/response_paths.rs
  • Areas with no further improvements: labels/constants.rs, labels/backend.rs

Generated by Rust Guard Improver • Run: §24659616348

References:

Generated by Rust Guard Improver · ● 1.2M ·

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