fix: correct label parsing for hyphenated GitHub usernames (#1061)

## Summary

- Label parsing used `split("-")[-1]` which broke GitHub usernames
containing hyphens (e.g., `Ahmad-Hafe` was parsed as `Hafe`), causing
LGTM/approved/changes-requested labels to not be recognized
- Fixed by using prefix-length slicing (`_label[len(PREFIX):]`) to
correctly extract the full username
- Switched substring `in` checks to `startswith` for consistency with
the prefix-based extraction and other patterns in the file

## Bug Details

The `can-be-merged` check counted only 1 LGTM instead of 2 when one
reviewer had a hyphenated username. For example, label `lgtm-Ahmad-Hafe`
was split as `["lgtm", "Ahmad", "Hafe"]` and only `"Hafe"` was extracted
— which matched no reviewer.

Affected all three label types: `lgtm-`, `approved-`,
`changes-requested-`.

Bug existed since commit c90382a (2023-05-24).

## Test plan

- [x] Added tests for hyphenated usernames across all 3 label types
(lgtm, approved, changes-requested)
- [x] Added tests verifying substring labels are not falsely matched
(startswith fix)
- [x] All 1491 tests pass
- [x] 90.62% coverage (meets 90% threshold)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Improved label parsing for pull request approvals to correctly
recognize hyphenated usernames in LGTM, approval, and change request
labels.

* **Tests**
* Added test coverage for label parsing with hyphenated usernames and
prefix-matching validation.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Author: rnetser

webhook_server/libs/handlers/pull_request_handler.py

@@ -1593,16 +1593,17 @@ async def _check_if_pr_approved(self, labels: list[str]) -> str:
 
         if self.github_webhook.minimum_lgtm:
             for _label in labels:
-                reviewer = _label.split(LABELS_SEPARATOR)[-1]
-                if LGTM_BY_LABEL_PREFIX.lower() in _label.lower() and reviewer in all_reviewers_without_pr_owner:
-                    lgtm_count += 1
-                    if reviewer in all_reviewers_without_pr_owner_and_lgtmed:
-                        all_reviewers_without_pr_owner_and_lgtmed.remove(reviewer)
+                if _label.lower().startswith(LGTM_BY_LABEL_PREFIX.lower()):
+                    reviewer = _label[len(LGTM_BY_LABEL_PREFIX) :]
+                    if reviewer in all_reviewers_without_pr_owner:
+                        lgtm_count += 1
+                        if reviewer in all_reviewers_without_pr_owner_and_lgtmed:
+                            all_reviewers_without_pr_owner_and_lgtmed.remove(reviewer)
         self.logger.debug(f"{self.log_prefix} lgtm_count: {lgtm_count}")
 
         for _label in labels:
-            if APPROVED_BY_LABEL_PREFIX.lower() in _label.lower():
-                approved_by.append(_label.split(LABELS_SEPARATOR)[-1])
+            if _label.lower().startswith(APPROVED_BY_LABEL_PREFIX.lower()):
+                approved_by.append(_label[len(APPROVED_BY_LABEL_PREFIX) :])
         self.logger.debug(f"{self.log_prefix} approved_by: {approved_by}")
 
         missing_approvers = list(set(self.owners_file_handler.all_pull_request_approvers.copy()))
@@ -1658,8 +1659,8 @@ def _check_labels_for_can_be_merged(self, labels: list[str]) -> str:
         failure_output = ""
 
         for _label in labels:
-            if CHANGED_REQUESTED_BY_LABEL_PREFIX.lower() in _label.lower():
-                change_request_user = _label.split(LABELS_SEPARATOR)[-1]
+            if _label.lower().startswith(CHANGED_REQUESTED_BY_LABEL_PREFIX.lower()):
+                change_request_user = _label[len(CHANGED_REQUESTED_BY_LABEL_PREFIX) :]
                 if change_request_user in self.owners_file_handler.all_pull_request_approvers:
                     failure_output += "PR has changed requests from approvers\n"
                     self.logger.debug(f"{self.log_prefix} Found changed request by {change_request_user}")

webhook_server/tests/test_pull_request_handler.py

@@ -1680,6 +1680,64 @@ async def test_check_if_pr_approved_commented(self, pull_request_handler: PullRe
             result = await pull_request_handler._check_if_pr_approved(labels=[f"{COMMENTED_BY_LABEL_PREFIX}reviewer1"])
             assert result == ""  # Empty string means no errors
 
+    @pytest.mark.asyncio
+    async def test_check_if_pr_approved_lgtm_hyphenated_username(
+        self, pull_request_handler: PullRequestHandler
+    ) -> None:
+        """Test that LGTM labels with hyphenated usernames are parsed correctly."""
+        hyphenated_user = "Ahmad-Hafe"
+        with (
+            patch.object(
+                pull_request_handler.owners_file_handler,
+                "owners_data_for_changed_files",
+                _owners_data_coroutine(),
+            ),
+            patch.object(pull_request_handler.github_webhook, "minimum_lgtm", 1),
+            patch.object(pull_request_handler.owners_file_handler, "all_pull_request_approvers", []),
+            patch.object(pull_request_handler.owners_file_handler, "root_approvers", []),
+            patch.object(pull_request_handler.owners_file_handler, "root_reviewers", []),
+            patch.object(pull_request_handler.owners_file_handler, "all_pull_request_reviewers", [hyphenated_user]),
+            patch.object(pull_request_handler.github_webhook, "parent_committer", "someone-else"),
+        ):
+            result = await pull_request_handler._check_if_pr_approved(
+                labels=[f"{LGTM_BY_LABEL_PREFIX}{hyphenated_user}"]
+            )
+            assert result == ""  # Hyphenated username should be recognized as a valid reviewer
+
+    @pytest.mark.asyncio
+    async def test_check_if_pr_approved_approved_hyphenated_username(
+        self, pull_request_handler: PullRequestHandler
+    ) -> None:
+        """Test that approved labels with hyphenated usernames are parsed correctly."""
+        hyphenated_user = "Ahmad-Hafe"
+        with (
+            patch.object(
+                pull_request_handler.owners_file_handler,
+                "owners_data_for_changed_files",
+                _owners_data_coroutine(),
+            ),
+            patch.object(pull_request_handler.github_webhook, "minimum_lgtm", 0),
+            patch.object(pull_request_handler.owners_file_handler, "all_pull_request_approvers", [hyphenated_user]),
+            patch.object(pull_request_handler.owners_file_handler, "root_approvers", [hyphenated_user]),
+            patch.object(pull_request_handler.owners_file_handler, "root_reviewers", []),
+            patch.object(pull_request_handler.owners_file_handler, "all_pull_request_reviewers", []),
+        ):
+            result = await pull_request_handler._check_if_pr_approved(
+                labels=[f"{APPROVED_BY_LABEL_PREFIX}{hyphenated_user}"]
+            )
+            assert result == ""  # Hyphenated username should be recognized as an approver
+
+    def test_check_labels_for_can_be_merged_changes_requested_hyphenated_username(
+        self, pull_request_handler: PullRequestHandler
+    ) -> None:
+        """Test that changes-requested labels with hyphenated usernames are parsed correctly."""
+        hyphenated_user = "Ahmad-Hafe"
+        with patch.object(pull_request_handler.owners_file_handler, "all_pull_request_approvers", [hyphenated_user]):
+            result = pull_request_handler._check_labels_for_can_be_merged(
+                labels=[f"{CHANGED_REQUESTED_BY_LABEL_PREFIX}{hyphenated_user}"]
+            )
+            assert "PR has changed requests from approvers" in result
+
     def test_check_labels_for_can_be_merged_approved(self, pull_request_handler: PullRequestHandler) -> None:
         # Mock the logic to return empty string (no errors) when appropriate
         with patch.object(pull_request_handler, "_check_if_pr_approved", return_value=""):
@@ -1710,6 +1768,77 @@ def test_check_labels_for_can_be_merged_not_approved(self, pull_request_handler:
             result = pull_request_handler._check_labels_for_can_be_merged(labels=["other-label"])
             assert result == ""  # Empty string means no errors
 
+    @pytest.mark.asyncio
+    async def test_check_if_pr_approved_lgtm_label_only_matches_prefix(
+        self, pull_request_handler: PullRequestHandler
+    ) -> None:
+        """Test that only labels starting with LGTM prefix are counted as LGTM."""
+        reviewer = "reviewer1"
+        with (
+            patch.object(
+                pull_request_handler.owners_file_handler,
+                "owners_data_for_changed_files",
+                _owners_data_coroutine(),
+            ),
+            patch.object(pull_request_handler.github_webhook, "minimum_lgtm", 1),
+            patch.object(pull_request_handler.owners_file_handler, "all_pull_request_approvers", []),
+            patch.object(pull_request_handler.owners_file_handler, "root_approvers", []),
+            patch.object(pull_request_handler.owners_file_handler, "root_reviewers", []),
+            patch.object(pull_request_handler.owners_file_handler, "all_pull_request_reviewers", [reviewer]),
+            patch.object(pull_request_handler.github_webhook, "parent_committer", "someone-else"),
+        ):
+            # Valid LGTM label starting with prefix should count
+            result = await pull_request_handler._check_if_pr_approved(labels=[f"{LGTM_BY_LABEL_PREFIX}{reviewer}"])
+            assert result == ""
+
+            # Label containing prefix in the middle should NOT count
+            result = await pull_request_handler._check_if_pr_approved(labels=[f"not-{LGTM_BY_LABEL_PREFIX}{reviewer}"])
+            assert "Missing lgtm from reviewers" in result
+
+    @pytest.mark.asyncio
+    async def test_check_if_pr_approved_approved_label_only_matches_prefix(
+        self, pull_request_handler: PullRequestHandler
+    ) -> None:
+        """Test that only labels starting with approved prefix are matched."""
+        approver = "approver1"
+        with (
+            patch.object(
+                pull_request_handler.owners_file_handler,
+                "owners_data_for_changed_files",
+                _owners_data_coroutine({"repo/OWNERS": {"approvers": [approver]}}),
+            ),
+            patch.object(pull_request_handler.github_webhook, "minimum_lgtm", 0),
+            patch.object(pull_request_handler.owners_file_handler, "all_pull_request_approvers", [approver]),
+            patch.object(pull_request_handler.owners_file_handler, "root_approvers", []),
+            patch.object(pull_request_handler.owners_file_handler, "root_reviewers", []),
+            patch.object(pull_request_handler.owners_file_handler, "all_pull_request_reviewers", []),
+        ):
+            result = await pull_request_handler._check_if_pr_approved(labels=[f"{APPROVED_BY_LABEL_PREFIX}{approver}"])
+            assert result == ""
+
+            result = await pull_request_handler._check_if_pr_approved(
+                labels=[f"not-{APPROVED_BY_LABEL_PREFIX}{approver}"]
+            )
+            assert "Missing approved from approvers" in result
+
+    def test_check_labels_for_can_be_merged_changes_requested_label_only_matches_prefix(
+        self, pull_request_handler: PullRequestHandler
+    ) -> None:
+        """Test that only labels starting with changes-requested prefix are matched."""
+        approver = "reviewer1"
+        with patch.object(pull_request_handler.owners_file_handler, "all_pull_request_approvers", [approver]):
+            # Valid changes-requested label should trigger the check
+            result = pull_request_handler._check_labels_for_can_be_merged(
+                labels=[f"{CHANGED_REQUESTED_BY_LABEL_PREFIX}{approver}"]
+            )
+            assert "PR has changed requests from approvers" in result
+
+            # Label containing prefix in the middle should NOT trigger
+            result = pull_request_handler._check_labels_for_can_be_merged(
+                labels=[f"not-{CHANGED_REQUESTED_BY_LABEL_PREFIX}{approver}"]
+            )
+            assert result == ""
+
     @pytest.mark.asyncio
     async def test_skip_if_pull_request_already_merged_merged(
         self, pull_request_handler: PullRequestHandler, mock_pull_request: Mock