Skip to content

fix: guard notification senders when matchedRuleIds are omitted#2

Merged
baires merged 3 commits intomainfrom
codex/fix-notification-guard
Apr 17, 2026
Merged

fix: guard notification senders when matchedRuleIds are omitted#2
baires merged 3 commits intomainfrom
codex/fix-notification-guard

Conversation

@baires
Copy link
Copy Markdown
Contributor

@baires baires commented Apr 17, 2026

What changed

  • normalize evaluation payloads from the API before the notification senders run
  • default missing reviewer arrays to empty arrays
  • derive matchedRuleIds from evidence[].ruleId when the API omits the explicit field
  • add a regression test covering the partial evaluation shape seen from the live backend

Why

The live MergeWire backend returned evaluation evidence and reviewers, but omitted matchedRuleIds for the demo-repo validation PRs. The action successfully posted PR comments and requested reviewers, then crashed in the Slack sender on matchedRuleIds.length, which prevented Slack and webhook delivery.

Validation

  • npm test
  • npm run typecheck
  • npm run release

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a normalizeEvaluation function to the API client to ensure that evaluation results have consistent array structures for evidence, matched rule IDs, and requested reviewers. It also includes logic to derive matchedRuleIds from the evidence list if the API omits them, along with a new test case to verify this functionality. Feedback was provided to ensure that matchedRuleIds contains unique values by using a Set, preventing duplicates that might arise from multiple evidence items sharing the same rule ID.

Comment thread src/api-client.ts Outdated
@baires baires merged commit 9e22133 into main Apr 17, 2026
1 check passed
@baires baires deleted the codex/fix-notification-guard branch April 17, 2026 02:28
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.

1 participant