Skip to content

Commit f010749

Browse files
authored
[Repo Assist] perf(rust-guard): add ORDER_LOW_TO_HIGH_PIPED const and direct tests for parse_integrity/scope_string (#5468)
🤖 *This PR was created by Repo Assist, an automated AI assistant.* ## Root Cause `parse_integrity` in `lib.rs` built an error message string at runtime via a 7-step iterator chain: ```rust policy_integrity::ORDER_HIGH_TO_LOW .iter() .rev() .copied() .collect::<Vec<_>>() .join("|") ``` This allocates a temporary `Vec<&str>` and a heap `String` each time an invalid `min-integrity` value is supplied, even though the result is the compile-time constant `"none|unapproved|approved|merged"`. Additionally, `parse_integrity` and `scope_string` had no direct unit tests — they were only covered indirectly through integration tests, leaving the error message text and None-fallback arms of `scope_string` unverified. ## Changes **`guards/github-guard/rust-guard/src/labels/constants.rs`** - Added `ORDER_LOW_TO_HIGH_PIPED: &str = "none|unapproved|approved|merged"` to the `policy_integrity` module — a compile-time constant replacing the runtime chain. - Added `order_low_to_high_piped_matches_order_high_to_low`: a consistency test that derives the piped string from `ORDER_HIGH_TO_LOW` at test time and asserts it equals `ORDER_LOW_TO_HIGH_PIPED`, preventing silent drift if integrity levels are ever added or reordered. **`guards/github-guard/rust-guard/src/lib.rs`** - Replaced the 7-step iterator chain in `parse_integrity` with `policy_integrity::ORDER_LOW_TO_HIGH_PIPED`. - Added `parse_integrity_accepts_all_valid_values`: verifies all four valid tokens are accepted. - Added `parse_integrity_rejects_unknown_value`: verifies the error message contains the full `ORDER_LOW_TO_HIGH_PIPED` constant string (not just individual tokens), locking down the exact error message content. - Added `scope_string_all_arms`: covers all 8 `ScopeKind` arms including None-fallback cases for Owner/Repo/RepoPrefix. ## Test Status **Rust tests**: ✅ 387/387 passed (`cargo test` in `guards/github-guard/rust-guard/`) **Go build/tests**: ⚠️ Infrastructure issue — `go 1.25.0` toolchain download blocked by network firewall (`proxy.golang.org` forbidden). No Go files were changed. > [!WARNING] > > Generated by <a href="https://github.com/github/gh-aw-mcpg/actions/runs/25673459949/agentic_workflow">Repo Assist</a> · ● 2.2M · <a href="https://github.com/search?q=repo%3Agithub%2Fgh-aw-mcpg+%22gh-aw-workflow-id%3A+repo-assist%22&type=pullrequests">◷</a> > > To install this <a href="https://github.com/githubnext/agentics/blob/851905c06e905bf362a9f6cc54f912e3df747d55/workflows/repo-assist.md">agentic workflow</a>, run > ``` > gh aw add githubnext/agentics@851905c > ```
2 parents 5403533 + f16ae7d commit f010749

2 files changed

Lines changed: 56 additions & 6 deletions

File tree

guards/github-guard/rust-guard/src/labels/constants.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,30 @@ pub mod policy_integrity {
2929
pub const MERGED: &str = "merged";
3030

3131
pub const ORDER_HIGH_TO_LOW: [&str; 4] = [MERGED, APPROVED, UNAPPROVED, NONE];
32+
/// Low-to-high order joined with `|`, ready for use in error messages.
33+
pub const ORDER_LOW_TO_HIGH_PIPED: &str = "none|unapproved|approved|merged";
34+
}
35+
36+
#[cfg(test)]
37+
mod tests {
38+
use super::policy_integrity;
39+
40+
/// Ensures ORDER_LOW_TO_HIGH_PIPED stays in sync with ORDER_HIGH_TO_LOW.
41+
/// If a new integrity level is added or reordered, this test will catch the drift.
42+
#[test]
43+
fn order_low_to_high_piped_matches_order_high_to_low() {
44+
let derived: String = policy_integrity::ORDER_HIGH_TO_LOW
45+
.iter()
46+
.rev()
47+
.copied()
48+
.collect::<Vec<_>>()
49+
.join("|");
50+
assert_eq!(
51+
derived,
52+
policy_integrity::ORDER_LOW_TO_HIGH_PIPED,
53+
"ORDER_LOW_TO_HIGH_PIPED is out of sync with ORDER_HIGH_TO_LOW"
54+
);
55+
}
3256
}
3357

3458
/// Canonical *reserved* scope token strings used for baseline and integrity scoping.

guards/github-guard/rust-guard/src/lib.rs

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -443,12 +443,7 @@ fn parse_integrity(value: &str) -> Result<MinIntegrity, String> {
443443
policy_integrity::MERGED => Ok(MinIntegrity::Merged),
444444
_ => Err(format!(
445445
"AllowOnly.min-integrity must be one of {}",
446-
policy_integrity::ORDER_HIGH_TO_LOW
447-
.iter()
448-
.rev()
449-
.copied()
450-
.collect::<Vec<_>>()
451-
.join("|")
446+
policy_integrity::ORDER_LOW_TO_HIGH_PIPED
452447
)),
453448
}
454449
}
@@ -1347,4 +1342,35 @@ mod tests {
13471342
assert_eq!(preview.len(), 500);
13481343
assert_eq!(preview.chars().count(), 250);
13491344
}
1345+
1346+
#[test]
1347+
fn parse_integrity_accepts_all_valid_values() {
1348+
assert_eq!(parse_integrity("none"), Ok(MinIntegrity::None));
1349+
assert_eq!(parse_integrity("unapproved"), Ok(MinIntegrity::Unapproved));
1350+
assert_eq!(parse_integrity("approved"), Ok(MinIntegrity::Approved));
1351+
assert_eq!(parse_integrity("merged"), Ok(MinIntegrity::Merged));
1352+
}
1353+
1354+
#[test]
1355+
fn parse_integrity_rejects_unknown_value() {
1356+
let err = parse_integrity("superuser").expect_err("unknown value must be rejected");
1357+
assert!(err.contains("must be one of"), "error should describe constraint: {err}");
1358+
assert!(
1359+
err.contains(policy_integrity::ORDER_LOW_TO_HIGH_PIPED),
1360+
"error should contain the full valid-options string \"{}\": {err}",
1361+
policy_integrity::ORDER_LOW_TO_HIGH_PIPED
1362+
);
1363+
}
1364+
1365+
#[test]
1366+
fn scope_string_all_arms() {
1367+
assert_eq!(scope_string(ScopeKind::All, None, None), POLICY_SCOPE_ALL);
1368+
assert_eq!(scope_string(ScopeKind::Public, None, None), POLICY_SCOPE_PUBLIC);
1369+
assert_eq!(scope_string(ScopeKind::Owner, Some("octocat"), None), "octocat");
1370+
assert_eq!(scope_string(ScopeKind::Owner, None, None), "");
1371+
assert_eq!(scope_string(ScopeKind::Repo, Some("o"), Some("r")), "o/r");
1372+
assert_eq!(scope_string(ScopeKind::Repo, None, None), "");
1373+
assert_eq!(scope_string(ScopeKind::RepoPrefix, Some("o"), Some("pfx")), "o/pfx*");
1374+
assert_eq!(scope_string(ScopeKind::RepoPrefix, None, None), "");
1375+
}
13501376
}

0 commit comments

Comments
 (0)