Skip to content

test: add regression test for non-team-member /approve before valid approval#99

Merged
joshjohanning merged 1 commit into
mainfrom
fix/98-non-team-approve-regression-test
Apr 3, 2026
Merged

test: add regression test for non-team-member /approve before valid approval#99
joshjohanning merged 1 commit into
mainfrom
fix/98-non-team-approve-regression-test

Conversation

@joshjohanning

@joshjohanning joshjohanning commented Apr 3, 2026

Copy link
Copy Markdown
Owner

Summary

Adds a regression test confirming that when a non-team-member comments /approve before a valid team member does, the action correctly skips the unauthorized comment and finds the later authorized one.

This behavior is already correct in the current Node.js implementation (the break only fires inside the teamMembers.has(actor) check), but the earlier shell-script version (v2) had a real bug where the break executed on any /approve match regardless of authorization.

A separate hotfix for v2 is being submitted in parallel. #100

References #98

Copilot AI review requested due to automatic review settings April 3, 2026 19:34

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

Adds a regression test to the Node.js GitHub Action to ensure /approve is accepted when the first matching comment is from a non-team member but a later matching comment is from an authorized team member (covering the failure mode described in #98 and preventing v2’s earlier shell-script bug from resurfacing in the JS implementation).

Changes:

  • Added an integration-style test case where an unauthorized /approve comment appears before an authorized /approve.
  • Asserts the action reports approval based on the later authorized comment (by checking output and log content).
Show a summary per file
File Description
src/index.test.js Adds regression test ensuring the action skips unauthorized /approve comments and approves on a subsequent authorized one.

Copilot's findings

  • Files reviewed: 1/1 changed files
  • Comments generated: 0

@joshjohanning joshjohanning force-pushed the fix/98-non-team-approve-regression-test branch from 2e2f120 to 0e444ae Compare April 3, 2026 19:40
…pproval

Adds a test confirming that when a non-team-member comments /approve
before a valid team member, the action correctly skips the unauthorized
comment and finds the later authorized one.

This behavior was already correct in the Node.js rewrite, but was a real
bug in the earlier shell-script version (v2). Adding the test to prevent
regression.

Ref #98
@joshjohanning joshjohanning force-pushed the fix/98-non-team-approve-regression-test branch from 0e444ae to 46ae8c9 Compare April 3, 2026 19:40
@joshjohanning joshjohanning changed the title Add regression test for non-team-member /approve before valid approval test: add regression test for non-team-member /approve before valid approval Apr 3, 2026
@joshjohanning joshjohanning merged commit d2b94f4 into main Apr 3, 2026
4 checks passed
@joshjohanning joshjohanning deleted the fix/98-non-team-approve-regression-test branch April 3, 2026 19:41
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.

2 participants