Skip to content

[Repo Assist] refactor(guard): deduplicate label_response write paths and extract get_author_association#8343

Merged
lpcox merged 5 commits into
mainfrom
repo-assist/fix-rust-guard-dedup-8328-4e0d93d67886d79e
Jun 30, 2026
Merged

[Repo Assist] refactor(guard): deduplicate label_response write paths and extract get_author_association#8343
lpcox merged 5 commits into
mainfrom
repo-assist/fix-rust-guard-dedup-8328-4e0d93d67886d79e

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

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

Closes #8328

Problem

label_response in lib.rs had two near-identical 10-line write blocks — one in the path-based branch and one in the legacy item-based branch — that both reimplemented the output-write logic manually:

  • output_json.len() as u32 > output_size — the as u32 cast is unsound for outputs exceeding 4 GB
  • output_json.len() as i32 return — can produce a negative return value if output exceeds i32::MAX, which the host misinterprets as an error code

Meanwhile, try_write_json_output (already used in label_agent and label_resource) handles these casts safely via u32::try_from / i32::try_from. label_response was never updated to use it.

Similarly, has_author_association and author_association_floor in helpers.rs both duplicated the dual-alias lookup for author_association / authorAssociation.

Fix

lib.rs: Replace both write blocks with two-line try_write_json_output calls, mapping -1 (buffer too small / overflow) to 0 (label_response's "skip labeling" signal):

// path-based branch:
let n = try_write_json_output(&output_json, output_ptr, output_size, "label_response/path");
return if n < 0 { 0 } else { n };

// legacy branch:
let n = try_write_json_output(&output_json, output_ptr, output_size, "label_response");
if n < 0 { 0 } else { n }

helpers.rs: Extract a private get_author_association helper and delegate both callers to it:

fn get_author_association(item: &Value) -> Option<&str> {
    item.get("author_association")
        .and_then(|v| v.as_str())
        .or_else(|| item.get("authorAssociation").and_then(|v| v.as_str()))
}

pub fn has_author_association(item: &Value) -> bool {
    get_author_association(item).is_some()
}
// author_association_floor now calls get_author_association(item) directly

Impact

  • Removes ~32 lines of duplicated logic (net -32 lines)
  • Fixes unsafe truncation casts in both label_response branches
  • All three exported WASM functions now go through the same safe write helper
  • Single source of truth for the two-alias field lookup

Test Status

All 556 Rust unit tests pass (cargo test green). Go tests skipped due to sandbox network restrictions (infrastructure-only failure, not caused by these changes).

Generated by Repo Assist · 129.7 AIC · ⊞ 12.3K ·
Comment /repo-assist to run again

Add this agentic workflow to your repo

To install this agentic workflow, run

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

…et_author_association

Replace the two duplicate 10-line write blocks in label_response
(path-based and legacy branches) with calls to try_write_json_output,
mapping its -1 failure code to the 0 that label_response uses for
'skip labeling'.

Extract a private get_author_association helper in helpers.rs to
consolidate the dual-alias lookup (author_association / authorAssociation)
that was duplicated between has_author_association and author_association_floor.

Fixes:
- Removes unsafe as u32 / as i32 truncation casts (now uses try_from)
- All three exported WASM functions use the same safe write helper
- Single source of truth for the two-alias field lookup

Closes #8328

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lpcox lpcox marked this pull request as ready for review June 30, 2026 13:31
Copilot AI review requested due to automatic review settings June 30, 2026 13:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 refactors the Rust GitHub guard to reduce duplication and make label_response output writing consistent with other exported WASM entrypoints by routing through the existing safe try_write_json_output helper, and centralizes author_association alias lookup logic in one place.

Changes:

  • Refactors label_response to use try_write_json_output for both the path-based and legacy output write paths (avoids unsafe as u32/as i32 casts).
  • Extracts a private get_author_association helper and reuses it from has_author_association and author_association_floor.
  • Updates/expands doc comments around the author association extraction helper.
Show a summary per file
File Description
guards/github-guard/rust-guard/src/lib.rs Deduplicates label_response output writing via try_write_json_output and maps write failures to the existing “skip labeling” return convention.
guards/github-guard/rust-guard/src/labels/helpers.rs Deduplicates the author_association/authorAssociation lookup into a single helper and updates call sites.

Review details

Tip

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

  • Files reviewed: 2/2 changed files
  • Comments generated: 3
  • Review effort level: Low

Comment thread guards/github-guard/rust-guard/src/lib.rs Outdated
Comment thread guards/github-guard/rust-guard/src/lib.rs Outdated
Comment thread guards/github-guard/rust-guard/src/labels/helpers.rs Outdated
lpcox and others added 2 commits June 30, 2026 06:35
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
GitHub Advanced Security started work on behalf of lpcox June 30, 2026 13:36 View session
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
GitHub Advanced Security started work on behalf of lpcox June 30, 2026 13:36 View session
GitHub Advanced Security finished work on behalf of lpcox June 30, 2026 13:36
GitHub Advanced Security finished work on behalf of lpcox June 30, 2026 13:37
@lpcox

lpcox commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

@copilot resolve the merge conflicts in this pull request and fix failing ci checks https://github.com/github/gh-aw-mcpg/actions/runs/28448521427/job/84304139332?pr=8343

GitHub Advanced Security started work on behalf of lpcox June 30, 2026 13:39 View session
GitHub Advanced Security finished work on behalf of lpcox June 30, 2026 13:40
…-assist/fix-rust-guard-dedup-8328-4e0d93d67886d79e

# Conflicts:
#	guards/github-guard/rust-guard/src/lib.rs

Copilot AI commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request and fix failing ci checks https://github.com/github/gh-aw-mcpg/actions/runs/28448521427/job/84304139332?pr=8343

Merged main into this branch and resolved the conflict in guards/github-guard/rust-guard/src/lib.rs.

The CI failure was caused by a missing closing } for the label_response function — the multi-line expansion of the if n < 0 { 0 } else { n } block (added to address the reviewer logging feedback) consumed the } that had been closing the function body. The merge from main (commit 560e2c28) restores proper indentation and the function's closing brace. All 556 Rust unit tests pass locally.

Copilot finished work on behalf of lpcox June 30, 2026 13:47
Copilot AI requested a review from lpcox June 30, 2026 13:47
@lpcox lpcox merged commit ddfe7f9 into main Jun 30, 2026
24 checks passed
@lpcox lpcox deleted the repo-assist/fix-rust-guard-dedup-8328-4e0d93d67886d79e branch June 30, 2026 13:56
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: Deduplicate write paths in label_response and author_association lookup

3 participants