Skip to content

[Repo Assist] perf(rust-guard): eliminate redundant Vec deep-clones in get_file_contents and list_releases#6990

Merged
lpcox merged 2 commits into
mainfrom
repo-assist/improve-rust-guard-vec-clones-20260604-36ef9f5683e14d42
Jun 4, 2026
Merged

[Repo Assist] perf(rust-guard): eliminate redundant Vec deep-clones in get_file_contents and list_releases#6990
lpcox merged 2 commits into
mainfrom
repo-assist/improve-rust-guard-vec-clones-20260604-36ef9f5683e14d42

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot commented Jun 4, 2026

🤖 This PR was created by Repo Assist, an automated AI assistant.

Closes #6982

What

In guards/github-guard/rust-guard/src/labels/response_paths.rs, two arms of label_response_paths used vec.clone().into() to create a SharedLabels (Arc<Vec<_>>), then separately moved the original Vec into a second SharedLabels for default_labels. This allocates the Vec twice — one deep clone plus one move into an Arc.

The fix converts the Vec directly into SharedLabels once (no clone), then uses O(1) Arc reference-count increments (SharedLabels::clone()) in the per-item loop and in default_labels.

This continues the clean-up applied earlier to list_commits (2026-06-03) and list_pull_requests/list_issues (2026-06-01), completing the pattern for all affected arms.

Changes

  • get_file_contents: Replace secrecy.clone().into() and file_integrity.clone().into() with direct .into(). Update default_labels to move the already-constructed SharedLabels instead of re-converting the originals.
  • list_releases: Replace default_secrecy.clone().into() with direct .into(). Update default_labels to move default_secrecy_shared.
  • New tests: Add 3 unit tests covering previously untested security-relevant branching:
    • get_file_contents_default_branch_gets_merged_integrity
    • get_file_contents_non_default_branch_gets_writer_integrity
    • list_releases_produces_merged_integrity_per_item

Test Status

Rust tests: ✅ All 456 tests pass (cargo test in guards/github-guard/rust-guard/)

Go build: ✅ go build ./... succeeds

Go tests: ⚠️ Infrastructure failure — testify@v1.11.1 and OTel SDK packages are not in the local module cache and the network proxy is blocked in this environment. This is pre-existing and unrelated to the Rust changes in this PR.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • proxy.golang.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "proxy.golang.org"

See Network Configuration for more information.

Generated by Repo Assist · sonnet46 3.7M ·

Add this agentic workflows to your repo

To install this agentic workflow, run

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

…tents and list_releases

Replace the `vec.clone().into()` pattern with `vec.into()` for
SharedLabels construction in the get_file_contents and list_releases
arms of label_response_paths. This continues the clean-up applied
earlier to list_commits, list_pull_requests, and list_issues.

The original code cloned the Vec into an Arc, then later consumed the
original via a second Arc allocation. The fix builds the Arc once and
uses O(1) refcount clones throughout the loop and default_labels.

Also add unit tests for get_file_contents (default-branch merged
integrity, non-default-branch writer integrity) and list_releases
(merged integrity per item, empty secrecy for public repos), covering
the security-relevant branching that was previously untested.

Closes #6982

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lpcox lpcox marked this pull request as ready for review June 4, 2026 14:39
Copilot AI review requested due to automatic review settings June 4, 2026 14:39
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR optimizes Rust guard response labeling by removing redundant deep clones when converting label vectors into SharedLabels (Arc-wrapped vectors), and adds unit tests to cover previously untested labeling branches for get_file_contents and list_releases.

Changes:

  • Avoid deep-cloning label Vecs by converting to SharedLabels once and reusing via cheap Arc clones.
  • Apply the optimization to get_file_contents and list_releases arms in label_response_paths.
  • Add three new unit tests validating integrity/secrecy behavior for these two tool responses.
Show a summary per file
File Description
guards/github-guard/rust-guard/src/labels/response_paths.rs Eliminates redundant Vec deep-clones in two labeling arms and adds targeted unit tests for integrity/secrecy branching.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 1/1 changed files
  • Comments generated: 1

Comment on lines +1026 to +1027
let merged_label = format!("{}octocat/hello", label_constants::MERGED_PREFIX);
let item_integrity = &result.labeled_paths[0].labels.integrity;
@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented Jun 4, 2026

@copilot address review feedback and fix failing ci check https://github.com/github/gh-aw-mcpg/actions/runs/26956017605/job/79543406045?pr=6990

Copy link
Copy Markdown
Contributor

Copilot AI commented Jun 4, 2026

@copilot address review feedback and fix failing ci check https://github.com/github/gh-aw-mcpg/actions/runs/26956017605/job/79543406045?pr=6990

Fixed in commit fix(rust-guard): remove unused merged_label variable in test — removed the unused merged_label binding that was causing the -D warnings CI failure. All 456 Rust lib tests pass with RUSTFLAGS=-D warnings.

Copilot AI requested a review from lpcox June 4, 2026 15:50
Copilot finished work on behalf of lpcox June 4, 2026 15:50
@lpcox lpcox merged commit 1e0f9bb into main Jun 4, 2026
16 checks passed
@lpcox lpcox deleted the repo-assist/improve-rust-guard-vec-clones-20260604-36ef9f5683e14d42 branch June 4, 2026 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[rust-guard] Rust Guard: Eliminate redundant Vec clones in response_paths.rs get_file_contents and list_releases

3 participants