Skip to content

[rust-guard] Deduplicate item_number extraction and add direct commit_integrity coverage#7577

Merged
lpcox merged 2 commits into
mainfrom
copilot/rust-guard-extract-item-number-helper
Jun 15, 2026
Merged

[rust-guard] Deduplicate item_number extraction and add direct commit_integrity coverage#7577
lpcox merged 2 commits into
mainfrom
copilot/rust-guard-extract-item-number-helper

Conversation

Copilot AI commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

helpers.rs repeated the same number extraction chain in nine integrity/logging paths, and commit_integrity had no direct unit coverage despite enforcing multiple security-critical integrity floors/elevations.

  • Refactor: centralized issue/PR number extraction

    • Added a private helper in helpers.rs:
      • item_number(item: &Value) -> u64
    • Replaced all 9 duplicated number extraction sites (approval/promotion/demotion adjustments plus PR/issue blocked and collaborator-fallback logging paths) with item_number(item).
  • Tests: direct coverage for commit_integrity decision paths

    • Added focused unit tests in helpers.rs for:
      • blocked author -> blocked integrity
      • default-branch commit -> merged floor
      • public non-default commit -> none baseline floor
      • private commit -> writer floor
      • private + default-branch commit -> merged floor
      • public owner-authored commit -> writer floor
  • Example (deduped call site pattern)

#[inline]
fn item_number(item: &Value) -> u64 {
    item.get(field_names::NUMBER)
        .and_then(|v| v.as_u64())
        .unwrap_or(0)
}

// before: item.get(field_names::NUMBER).and_then(|v| v.as_u64()).unwrap_or(0)
let number = item_number(item);

GitHub Advanced Security started work on behalf of lpcox June 15, 2026 14:03 View session
GitHub Advanced Security finished work on behalf of lpcox June 15, 2026 14:04
GitHub Advanced Security started work on behalf of lpcox June 15, 2026 14:10 View session
GitHub Advanced Security finished work on behalf of lpcox June 15, 2026 14:11
Copilot AI changed the title [WIP] Extract item_number helper to eliminate 9x duplication in helpers.rs [rust-guard] Deduplicate item_number extraction and add direct commit_integrity coverage Jun 15, 2026
Copilot AI requested a review from lpcox June 15, 2026 14:13
Copilot finished work on behalf of lpcox June 15, 2026 14:13
@lpcox lpcox marked this pull request as ready for review June 15, 2026 14:35
Copilot AI review requested due to automatic review settings June 15, 2026 14:35

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 guard labeling helpers to remove repeated JSON extraction logic for issue/PR numbers in integrity-related logging, and adds direct unit tests for commit_integrity to cover key decision paths that affect integrity labeling.

Changes:

  • Added a private item_number(&Value) -> u64 helper to centralize number extraction for logging use cases.
  • Replaced nine duplicated number extraction chains across integrity promotion/demotion and logging paths with item_number(item).
  • Added unit tests that directly exercise commit_integrity outcomes across blocked-author, repo privacy, default-branch, and owner-authored scenarios.
Show a summary per file
File Description
guards/github-guard/rust-guard/src/labels/helpers.rs Deduplicates number extraction for logging and adds focused unit tests covering commit_integrity decision branches.

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: 0

@lpcox lpcox merged commit 9e49c9c into main Jun 15, 2026
46 checks passed
@lpcox lpcox deleted the copilot/rust-guard-extract-item-number-helper branch June 15, 2026 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants