Skip to content

Commit a6f07a3

Browse files
authored
[Repo Assist] fix(rust-guard): remove dead .or_else() fallback in notifications arm (#8935)
🤖 *This is an automated pull request from Repo Assist.* Closes #8921 ## Root Cause In `guards/github-guard/rust-guard/src/labels/response_items.rs`, the `list_notifications | get_notification_details` arm had dead code: ```rust let items = actual_response.as_array().or_else(|| response.as_array()); ``` The `.or_else(|| response.as_array())` fallback is always a no-op because `extract_mcp_response` (which produces `actual_response`) returns either: - **`Cow::Owned(parsed)`** – `parsed` was successfully deserialized from `content[0].text`, so `actual_response.as_array()` is already `Some(_)`; the fallback is unreachable. - **`Cow::Borrowed(response)`** – `actual_response` *is* `response`, so `.or_else(|| response.as_array())` checks the identical pointer twice. This pattern is absent in every other arm of the same `match` block (they all call `collect_items_simple(&actual_response)` or `extract_items_slice(&actual_response, ...)` directly), making the notifications arm inconsistently defensive without benefit. ## Fix Remove the `.or_else(...)` chain, leaving the simpler: ```rust let items = actual_response.as_array(); ``` No behaviour change — the removed path was never executed. ## Tests Added The notifications arm previously had **zero test coverage**. Three tests are added: | Test | Covers | |------|--------| | `list_notifications_labels_each_item_as_private` | `private_user_label()` secrecy assignment for each notification | | `list_notifications_description_includes_id` | `"notification:{id}"` description format | | `get_notification_details_empty_array_returns_empty` | empty array input → empty result | ## Test Status ✅ **All 576 Rust tests pass** (up from 573 before adding the new tests): ``` test result: ok. 576 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.03s ``` ⚠️ Go unit tests blocked by network restrictions in this CI environment (pre-existing infrastructure issue — `proxy.golang.org` is inaccessible). This is a Rust-only change. --- *Generated by [Repo Assist](https://github.com/github/gh-aw-mcpg/actions/runs/28945131964)* > [!WARNING] > <details> > <summary>Firewall blocked 2 domains</summary> > > The following domains were blocked by the firewall during workflow execution: > > - `awmgmcpg` > - `proxy.golang.org` >> To allow these domains, add them to the `network.allowed` list in your workflow frontmatter: > > ```yaml > network: > allowed: > - defaults > - "awmgmcpg" > - "proxy.golang.org" > ``` > > See [Network Configuration](https://github.github.com/gh-aw/reference/network/) for more information. > > </details> > Generated by [Repo Assist](https://github.com/github/gh-aw-mcpg/actions/runs/28945131964) · 716.4 AIC · ⊞ 10.8K · [◷](https://github.com/search?q=repo%3Agithub%2Fgh-aw-mcpg+%22gh-aw-workflow-id%3A+repo-assist%22&type=pullrequests) > <sub>Comment <em>/repo-assist</em> to run again</sub> > <details> <summary><sub>Add this agentic workflow to your repo</sub></summary> To install this agentic workflow, run ``` gh aw add githubnext/agentics@851905c ``` </details> <!-- gh-aw-agentic-workflow: Repo Assist, engine: copilot, version: 1.0.68, model: claude-sonnet-4.6, id: 28945131964, workflow_id: repo-assist, run: https://github.com/github/gh-aw-mcpg/actions/runs/28945131964 --> <!-- gh-aw-workflow-id: repo-assist --> <!-- gh-aw-workflow-call-id: github/gh-aw-mcpg/repo-assist -->
2 parents 4817e70 + b41ba5e commit a6f07a3

1 file changed

Lines changed: 40 additions & 1 deletion

File tree

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

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ pub fn label_response_items(
392392

393393
// === Notifications - all are private ===
394394
"list_notifications" | "get_notification_details" => {
395-
let items = actual_response.as_array().or_else(|| response.as_array());
395+
let items = actual_response.as_array();
396396

397397
if let Some(items) = items {
398398
let notif_secrecy = private_user_label();
@@ -522,4 +522,43 @@ mod tests {
522522
assert_eq!(items.len(), 1);
523523
assert_eq!(items[0].get("number").and_then(|v| v.as_u64()), Some(42));
524524
}
525+
526+
/// Notifications are labelled as private with a `notification:{id}` description.
527+
#[test]
528+
fn list_notifications_labels_each_item_as_private() {
529+
let ctx = default_ctx();
530+
let response = json!([
531+
{"id": "1", "subject": {"title": "PR reviewed"}},
532+
{"id": "2", "subject": {"title": "Mention"}}
533+
]);
534+
let result = label_response_items("list_notifications", &json!({}), &response, &ctx);
535+
assert_eq!(result.len(), 2);
536+
let expected_secrecy = private_user_label();
537+
for item in &result {
538+
assert_eq!(
539+
item.labels.secrecy,
540+
expected_secrecy,
541+
"notification secrecy should be private:user"
542+
);
543+
}
544+
}
545+
546+
/// The description field is formatted as `notification:{id}` for each item.
547+
#[test]
548+
fn list_notifications_description_includes_id() {
549+
let ctx = default_ctx();
550+
let response = json!([{"id": "42"}]);
551+
let result = label_response_items("list_notifications", &json!({}), &response, &ctx);
552+
assert_eq!(result.len(), 1);
553+
assert_eq!(result[0].labels.description, "notification:42");
554+
}
555+
556+
/// An empty notifications array returns an empty result set.
557+
#[test]
558+
fn get_notification_details_empty_array_returns_empty() {
559+
let ctx = default_ctx();
560+
let result =
561+
label_response_items("get_notification_details", &json!({}), &json!([]), &ctx);
562+
assert!(result.is_empty());
563+
}
525564
}

0 commit comments

Comments
 (0)