diff --git a/webhook_server/libs/handlers/runner_handler.py b/webhook_server/libs/handlers/runner_handler.py index 721d1d42..cc75743c 100644 --- a/webhook_server/libs/handlers/runner_handler.py +++ b/webhook_server/libs/handlers/runner_handler.py @@ -735,6 +735,151 @@ async def is_branch_exists(self, branch: str) -> bool: return False raise + async def _find_signoff_source( + self, + pull_request: PullRequest, + ) -> tuple[Any, str | None]: + """Find a commit with a Signed-off-by trailer and return it with the sign-off email. + + Checks the merge commit first (squash-merge). If the merge commit has no + Signed-off-by, falls back to scanning the PR's individual commits in + reverse order (regular merge). + + Returns (source_commit, signoff_email) or (None, None) if no sign-off found. + """ + # Try the merge commit first (squash-merge case) + merge_sha = await github_api_call( + lambda: pull_request.merge_commit_sha, logger=self.logger, log_prefix=self.log_prefix + ) + if merge_sha: + merge_commit = await github_api_call( + self.github_webhook.repository.get_commit, merge_sha, logger=self.logger, log_prefix=self.log_prefix + ) + commit_msg = await github_api_call( + lambda: merge_commit.commit.message, logger=self.logger, log_prefix=self.log_prefix + ) + signoff_match = re.findall(r"Signed-off-by:\s*(.+?)\s*<([^>]+)>", commit_msg) + if signoff_match: + return merge_commit, signoff_match[-1][1] + + # Fall back to PR commits (regular merge case) + commits = await github_api_call( + lambda: list(pull_request.get_commits()), logger=self.logger, log_prefix=self.log_prefix + ) + for commit in reversed(commits): + commit_msg = await github_api_call( + lambda c=commit: c.commit.message, logger=self.logger, log_prefix=self.log_prefix + ) + signoff_match = re.findall(r"Signed-off-by:\s*(.+?)\s*<([^>]+)>", commit_msg) + if signoff_match: + return commit, signoff_match[-1][1] + + return None, None + + async def _restore_original_author_for_cherry_pick( + self, + pull_request: PullRequest, + git_cmd: str, + github_token: str, + ) -> bool: + """Amend cherry-picked commit to restore the original PR author for DCO compliance. + + GitHub squash-merges rewrite the author email to the noreply format + (e.g., 86722603+user@users.noreply.github.com). When git cherry-pick + replays such a commit, the DCO check fails because the author email + no longer matches the Signed-off-by trailer. + + This method first checks the merge commit (via ``pull_request.merge_commit_sha``) + for a Signed-off-by trailer (squash-merge case). If not found, it falls back + to scanning the PR's individual commits (regular merge case). + + The author identity is built from the source commit's git author name + (preserved by GitHub) combined with the email from the Signed-off-by trailer. + + Returns True if the commit was amended, False if no amendment was needed or possible. + """ + try: + # Try merge commit first (squash-merge), fall back to PR commits (regular merge) + source_commit, signoff_email = await self._find_signoff_source(pull_request) + if not source_commit or not signoff_email: + self.logger.debug(f"{self.log_prefix} No Signed-off-by found, skipping author restore") + return False + + # Author name from the source commit's git author (GitHub preserves the display name) + # Author email from the Signed-off-by trailer (commit email may be noreply) + author_name = await github_api_call( + lambda: source_commit.commit.author.name, logger=self.logger, log_prefix=self.log_prefix + ) + author_email = signoff_email + + author_spec = f"{author_name} <{author_email}>" + redact_list = [github_token, author_spec, author_email, author_name] + + # Check if the cherry-picked commit author already matches (both name and email) + rc, current_author_info, _ = await run_command( + command=f"{git_cmd} log -1 --format=%an%n%ae", + log_prefix=self.log_prefix, + redact_secrets=redact_list, + mask_sensitive=self.github_webhook.mask_sensitive, + ) + if not rc: + self.logger.warning( + f"{self.log_prefix} Could not read current author info, proceeding with author amend" + ) + else: + info_lines = current_author_info.strip().splitlines() + if len(info_lines) == 2 and info_lines[0] == author_name and info_lines[1] == author_email: + self.logger.debug(f"{self.log_prefix} Author already matches original PR commit, no amend needed") + return False + + # Read the current commit message to fix Signed-off-by trailers + rc, current_msg, _ = await run_command( + command=f"{git_cmd} log -1 --format=%B", + log_prefix=self.log_prefix, + redact_secrets=redact_list, + mask_sensitive=self.github_webhook.mask_sensitive, + ) + amended_msg: str | None = None + if not rc: + self.logger.warning(f"{self.log_prefix} Could not read commit message, amending author only") + else: + # Remove all existing Signed-off-by trailers and add the correct one + msg_lines = current_msg.rstrip().splitlines() + filtered_lines = [line for line in msg_lines if not re.match(r"Signed-off-by:\s*", line)] + while filtered_lines and not filtered_lines[-1].strip(): + filtered_lines.pop() + filtered_lines.append("") + filtered_lines.append(f"Signed-off-by: {author_name} <{author_email}>") + amended_msg = "\n".join(filtered_lines) + "\n" + + # Amend the commit author and optionally the message + msg_flag = f"-m {shlex.quote(amended_msg)}" if amended_msg else "--no-edit" + rc, _, err = await run_command( + command=f"{git_cmd} commit --amend --author={shlex.quote(author_spec)} {msg_flag}", + log_prefix=self.log_prefix, + redact_secrets=redact_list, + mask_sensitive=self.github_webhook.mask_sensitive, + ) + + if not rc: + redacted_err = _redact_secrets( + err, + redact_list, + mask_sensitive=self.github_webhook.mask_sensitive, + ) + self.logger.warning( + f"{self.log_prefix} Failed to amend cherry-pick author for DCO compliance: {redacted_err}" + ) + return False + + self.logger.info(f"{self.log_prefix} Restored original author on cherry-pick for DCO compliance") + return True + except asyncio.CancelledError: + raise + except Exception: + self.logger.exception(f"{self.log_prefix} Failed to restore original author for cherry-pick") + return False + async def _resolve_cherry_pick_with_ai( self, worktree_path: str, @@ -1025,6 +1170,13 @@ async def cherry_pick( return cherry_pick_had_conflicts = True + # Restore original PR author on cherry-pick for DCO compliance + await self._restore_original_author_for_cherry_pick( + pull_request=pull_request, + git_cmd=git_cmd, + github_token=github_token, + ) + # Push the branch rc, out, err = await run_command( command=push_command, diff --git a/webhook_server/tests/test_runner_handler.py b/webhook_server/tests/test_runner_handler.py index f0c729e1..4acc1b23 100644 --- a/webhook_server/tests/test_runner_handler.py +++ b/webhook_server/tests/test_runner_handler.py @@ -1,3 +1,4 @@ +import asyncio from collections.abc import AsyncGenerator, Generator from contextlib import asynccontextmanager from dataclasses import dataclass @@ -1035,7 +1036,14 @@ async def test_cherry_pick_command_failure(self, runner_handler: RunnerHandler, async def test_cherry_pick_success(self, runner_handler: RunnerHandler, mock_pull_request: Mock) -> None: """Test cherry_pick with successful execution.""" runner_handler.github_webhook.pypi = {"token": "dummy"} - with patch.object(runner_handler, "is_branch_exists", new=AsyncMock(return_value=Mock())): + with ( + patch.object( + runner_handler, + "_restore_original_author_for_cherry_pick", + new=AsyncMock(return_value=False), + ) as mock_restore_author, + patch.object(runner_handler, "is_branch_exists", new=AsyncMock(return_value=Mock())), + ): with patch.object(runner_handler.check_run_handler, "set_check_in_progress") as mock_set_progress: with patch.object(runner_handler.check_run_handler, "set_check_success") as mock_set_success: with patch.object(runner_handler, "_checkout_worktree") as mock_checkout: @@ -1060,6 +1068,7 @@ async def test_cherry_pick_success(self, runner_handler: RunnerHandler, mock_pul ), ): await runner_handler.cherry_pick(mock_pull_request, "main") + mock_restore_author.assert_awaited_once() mock_set_progress.assert_called_once() mock_set_success.assert_called_once() # Called twice: success comment + label warning (mock URL can't parse PR number) @@ -1187,7 +1196,10 @@ async def test_cherry_pick_calls_checkout_worktree_with_skip_merge( ) -> None: """Test cherry_pick passes skip_merge=True to _checkout_worktree.""" runner_handler.github_webhook.pypi = {"token": "dummy"} - with patch.object(runner_handler, "is_branch_exists", new=AsyncMock(return_value=Mock())): + with ( + patch.object(runner_handler, "_restore_original_author_for_cherry_pick", new=AsyncMock(return_value=False)), + patch.object(runner_handler, "is_branch_exists", new=AsyncMock(return_value=Mock())), + ): with patch.object(runner_handler.check_run_handler, "set_check_in_progress"): with patch.object(runner_handler.check_run_handler, "set_check_success"): with patch.object(runner_handler, "_checkout_worktree") as mock_checkout: @@ -1336,7 +1348,10 @@ async def cherry_pick_setup( ) -> AsyncGenerator[CherryPickMocks]: """Common setup for cherry-pick tests.""" runner_handler.github_webhook.pypi = {"token": "dummy"} - with patch.object(runner_handler, "is_branch_exists", new=AsyncMock(return_value=Mock())): + with ( + patch.object(runner_handler, "_restore_original_author_for_cherry_pick", new=AsyncMock(return_value=False)), + patch.object(runner_handler, "is_branch_exists", new=AsyncMock(return_value=Mock())), + ): with patch.object(runner_handler.check_run_handler, "set_check_in_progress") as mock_set_progress: with patch.object(runner_handler.check_run_handler, "set_check_success") as mock_set_success: with patch.object(runner_handler, "_checkout_worktree") as mock_checkout: @@ -1570,7 +1585,10 @@ async def run_command_side_effect(command: str, **kwargs: Any) -> tuple[bool, st return (True, "https://github.com/test-org/test-repo/pull/99", "") return (True, "success", "") - with patch.object(runner_handler, "is_branch_exists", new=AsyncMock(return_value=Mock())): + with ( + patch.object(runner_handler, "_restore_original_author_for_cherry_pick", new=AsyncMock(return_value=False)), + patch.object(runner_handler, "is_branch_exists", new=AsyncMock(return_value=Mock())), + ): with patch.object(runner_handler.check_run_handler, "set_check_in_progress"): with patch.object(runner_handler.check_run_handler, "set_check_success") as mock_set_success: with patch.object(runner_handler, "_checkout_worktree") as mock_checkout: @@ -1641,7 +1659,10 @@ async def run_command_side_effect(command: str, **kwargs: Any) -> tuple[bool, st return (True, "https://github.com/test-org/test-repo/pull/99", "") return (True, "success", "") - with patch.object(runner_handler, "is_branch_exists", new=AsyncMock(return_value=Mock())): + with ( + patch.object(runner_handler, "_restore_original_author_for_cherry_pick", new=AsyncMock(return_value=False)), + patch.object(runner_handler, "is_branch_exists", new=AsyncMock(return_value=Mock())), + ): with patch.object(runner_handler.check_run_handler, "set_check_in_progress"): with patch.object(runner_handler.check_run_handler, "set_check_success") as mock_set_success: with patch.object(runner_handler, "_checkout_worktree") as mock_checkout: @@ -1889,6 +1910,422 @@ async def run_command_side_effect(command: str, **kwargs: Any) -> tuple[bool, st mock_ai_cli.assert_not_called() +class TestRestoreOriginalAuthorForCherryPick: + """Test suite for _restore_original_author_for_cherry_pick method.""" + + @pytest.fixture + def mock_github_webhook(self) -> Mock: + """Create a mock GithubWebhook instance.""" + mock_webhook = Mock() + mock_webhook.hook_data = {"action": "opened"} + mock_webhook.logger = Mock() + mock_webhook.log_prefix = "[TEST]" + mock_webhook.repository = Mock() + mock_webhook.repository.clone_url = "https://github.com/test/repo.git" + mock_webhook.repository.owner.login = "test-owner" + mock_webhook.token = "test-token" # pragma: allowlist secret + mock_webhook.clone_repo_dir = "/tmp/test-repo" + mock_webhook.tox = {} + mock_webhook.pre_commit = False + mock_webhook.build_and_push_container = False + mock_webhook.pypi = {} + mock_webhook.conventional_title = "" + mock_webhook.container_repository_username = "" + mock_webhook.container_repository_password = "" # pragma: allowlist secret + mock_webhook.slack_webhook_url = "" + mock_webhook.repository_full_name = "test/repo" + mock_webhook.dockerfile = "" + mock_webhook.container_build_args = [] + mock_webhook.container_command_args = [] + mock_webhook.ctx = None + mock_webhook.custom_check_runs = [] + mock_webhook.ai_features = None + mock_webhook.config = Mock() + mock_webhook.config.get_value = Mock(return_value=None) + mock_webhook.mask_sensitive = False + return mock_webhook + + @pytest.fixture + def mock_owners_file_handler(self) -> Mock: + """Create a mock OwnersFileHandler instance.""" + return Mock() + + @pytest.fixture + def runner_handler(self, mock_github_webhook: Mock, mock_owners_file_handler: Mock) -> RunnerHandler: + """Create a RunnerHandler instance with mocked dependencies.""" + return RunnerHandler(mock_github_webhook, mock_owners_file_handler) + + @staticmethod + def _make_merged_pr( + merge_commit_msg: str, + author_name: str = "Test User", + merge_commit_sha: str = "abc123", + ) -> tuple[Mock, Mock]: + """Create a mock PullRequest with a merge commit. + + Returns (mock_pr, mock_merge_commit) so tests can configure the + repository.get_commit() return value. + """ + mock_pr = Mock() + mock_pr.merge_commit_sha = merge_commit_sha + + mock_merge_commit = Mock() + mock_merge_commit.commit.message = merge_commit_msg + mock_merge_commit.commit.author.name = author_name + + return mock_pr, mock_merge_commit + + @pytest.mark.asyncio + async def test_amend_when_email_mismatch(self, runner_handler: RunnerHandler) -> None: + """Merge commit has Signed-off-by, cherry-pick author differs — amend.""" + mock_pr, mock_merge_commit = self._make_merged_pr( + merge_commit_msg="[Storage] Update owners\n\nSigned-off-by: Jenia Peimer \n", + author_name="Jenia Peimer", + ) + runner_handler.github_webhook.repository.get_commit.return_value = mock_merge_commit + + async def run_command_side_effect(command: str, **kwargs: Any) -> tuple[bool, str, str]: + if "log -1 --format=%an%n%ae" in command: + return (True, "jpeimer\n86722603+jpeimer@users.noreply.github.com", "") + if "log -1 --format=%B" in command: + return (True, "[Storage] Update owners\n\nSigned-off-by: jpeimer \n", "") + if "commit --amend" in command: + assert "Jenia Peimer " in command + # Verify the -m message contains the corrected Signed-off-by trailer + assert "-m" in command + assert "Signed-off-by: Jenia Peimer " in command + assert "Signed-off-by: jpeimer " not in command + return (True, "success", "") + return (True, "", "") + + with ( + patch( + "webhook_server.libs.handlers.runner_handler.run_command", + new=AsyncMock(side_effect=run_command_side_effect), + ), + patch( + "asyncio.to_thread", + new=AsyncMock(side_effect=lambda fn, *a, **kw: fn(*a, **kw) if a or kw else fn()), + ), + ): + result = await runner_handler._restore_original_author_for_cherry_pick( + pull_request=mock_pr, + git_cmd="git --work-tree=/tmp/test --git-dir=/tmp/test/.git", + github_token="test-token", # pragma: allowlist secret + ) + assert result is True + + @pytest.mark.asyncio + async def test_no_amend_when_author_matches(self, runner_handler: RunnerHandler) -> None: + """Merge commit author matches cherry-pick author (name and email) — no amend.""" + mock_pr, mock_merge_commit = self._make_merged_pr( + merge_commit_msg="feat: something\n\nSigned-off-by: Test User \n", + author_name="Test User", + ) + runner_handler.github_webhook.repository.get_commit.return_value = mock_merge_commit + + async def run_command_side_effect(command: str, **kwargs: Any) -> tuple[bool, str, str]: + if "log -1 --format=%an%n%ae" in command: + return (True, "Test User\ntest@example.com", "") + return (True, "", "") + + with ( + patch( + "webhook_server.libs.handlers.runner_handler.run_command", + new=AsyncMock(side_effect=run_command_side_effect), + ), + patch( + "asyncio.to_thread", + new=AsyncMock(side_effect=lambda fn, *a, **kw: fn(*a, **kw) if a or kw else fn()), + ), + ): + result = await runner_handler._restore_original_author_for_cherry_pick( + pull_request=mock_pr, + git_cmd="git --work-tree=/tmp/test --git-dir=/tmp/test/.git", + github_token="test-token", # pragma: allowlist secret + ) + assert result is False + + @pytest.mark.asyncio + async def test_no_signoff_in_merge_commit(self, runner_handler: RunnerHandler) -> None: + """Merge commit has no Signed-off-by, PR commits also have none — skip.""" + mock_pr, mock_merge_commit = self._make_merged_pr( + merge_commit_msg="feat: something\n\nNo sign-off here.\n", + ) + runner_handler.github_webhook.repository.get_commit.return_value = mock_merge_commit + pr_commit = Mock() + pr_commit.commit.message = "feat: no sign-off" + mock_pr.get_commits.return_value = [pr_commit] + + with patch( + "asyncio.to_thread", + new=AsyncMock(side_effect=lambda fn, *a, **kw: fn(*a, **kw) if a or kw else fn()), + ): + result = await runner_handler._restore_original_author_for_cherry_pick( + pull_request=mock_pr, + git_cmd="git --work-tree=/tmp/test --git-dir=/tmp/test/.git", + github_token="test-token", # pragma: allowlist secret + ) + assert result is False + + @pytest.mark.asyncio + async def test_amend_failure(self, runner_handler: RunnerHandler) -> None: + """Amend command fails — return False without blocking cherry-pick.""" + mock_pr, mock_merge_commit = self._make_merged_pr( + merge_commit_msg="feat: test\n\nSigned-off-by: User \n", + author_name="User", + ) + runner_handler.github_webhook.repository.get_commit.return_value = mock_merge_commit + + async def run_command_side_effect(command: str, **kwargs: Any) -> tuple[bool, str, str]: + if "log -1 --format=%an%n%ae" in command: + return (True, "noreply\nnoreply@github.com", "") + if "log -1 --format=%B" in command: + return (True, "feat: test\n\nSigned-off-by: noreply \n", "") + if "commit --amend" in command: + return (False, "", "error: could not amend") + return (True, "", "") + + with ( + patch( + "webhook_server.libs.handlers.runner_handler.run_command", + new=AsyncMock(side_effect=run_command_side_effect), + ), + patch( + "asyncio.to_thread", + new=AsyncMock(side_effect=lambda fn, *a, **kw: fn(*a, **kw) if a or kw else fn()), + ), + ): + result = await runner_handler._restore_original_author_for_cherry_pick( + pull_request=mock_pr, + git_cmd="git --work-tree=/tmp/test --git-dir=/tmp/test/.git", + github_token="test-token", # pragma: allowlist secret + ) + assert result is False + + @pytest.mark.asyncio + async def test_fallback_to_pr_commits(self, runner_handler: RunnerHandler) -> None: + """Merge commit has no Signed-off-by (regular merge) — falls back to PR commits.""" + # Merge commit without sign-off (regular merge message) + mock_pr = Mock() + mock_pr.merge_commit_sha = "merge123" + mock_merge_commit = Mock() + mock_merge_commit.commit.message = "Merge pull request #42 from user/branch" + runner_handler.github_webhook.repository.get_commit.return_value = mock_merge_commit + + # PR commit with sign-off + pr_commit = Mock() + pr_commit.commit.message = "feat: add feature\n\nSigned-off-by: Test User \n" + pr_commit.commit.author.name = "Test User" + mock_pr.get_commits.return_value = [pr_commit] + + async def run_command_side_effect(command: str, **kwargs: Any) -> tuple[bool, str, str]: + if "log -1 --format=%an%n%ae" in command: + return (True, "noreply\nnoreply@github.com", "") + if "log -1 --format=%B" in command: + return (True, "feat: add feature\n\nSigned-off-by: noreply \n", "") + if "commit --amend" in command: + assert "Test User " in command + return (True, "success", "") + return (True, "", "") + + with ( + patch( + "webhook_server.libs.handlers.runner_handler.run_command", + new=AsyncMock(side_effect=run_command_side_effect), + ), + patch( + "asyncio.to_thread", + new=AsyncMock(side_effect=lambda fn, *a, **kw: fn(*a, **kw) if a or kw else fn()), + ), + ): + result = await runner_handler._restore_original_author_for_cherry_pick( + pull_request=mock_pr, + git_cmd="git --work-tree=/tmp/test --git-dir=/tmp/test/.git", + github_token="test-token", # pragma: allowlist secret + ) + assert result is True + + @pytest.mark.asyncio + async def test_no_signoff_anywhere(self, runner_handler: RunnerHandler) -> None: + """No Signed-off-by in merge commit or PR commits — skip.""" + mock_pr = Mock() + mock_pr.merge_commit_sha = "merge123" + mock_merge_commit = Mock() + mock_merge_commit.commit.message = "Merge pull request #42 from user/branch" + runner_handler.github_webhook.repository.get_commit.return_value = mock_merge_commit + + pr_commit = Mock() + pr_commit.commit.message = "feat: no sign-off here" + mock_pr.get_commits.return_value = [pr_commit] + + with patch( + "asyncio.to_thread", + new=AsyncMock(side_effect=lambda fn, *a, **kw: fn(*a, **kw) if a or kw else fn()), + ): + result = await runner_handler._restore_original_author_for_cherry_pick( + pull_request=mock_pr, + git_cmd="git --work-tree=/tmp/test --git-dir=/tmp/test/.git", + github_token="test-token", # pragma: allowlist secret + ) + assert result is False + + @pytest.mark.asyncio + async def test_author_info_read_failure_still_amends(self, runner_handler: RunnerHandler) -> None: + """Cannot read current author info — log warning and proceed to amend.""" + mock_pr, mock_merge_commit = self._make_merged_pr( + merge_commit_msg="feat: test\n\nSigned-off-by: User \n", + author_name="User", + ) + runner_handler.github_webhook.repository.get_commit.return_value = mock_merge_commit + + async def run_command_side_effect(command: str, **kwargs: Any) -> tuple[bool, str, str]: + if "log -1 --format=%an%n%ae" in command: + return (False, "", "fatal: bad revision") + if "log -1 --format=%B" in command: + return (True, "feat: test\n\nSigned-off-by: noreply \n", "") + if "commit --amend" in command: + assert "User " in command + return (True, "success", "") + return (True, "", "") + + with ( + patch( + "webhook_server.libs.handlers.runner_handler.run_command", + new=AsyncMock(side_effect=run_command_side_effect), + ), + patch( + "asyncio.to_thread", + new=AsyncMock(side_effect=lambda fn, *a, **kw: fn(*a, **kw) if a or kw else fn()), + ), + ): + result = await runner_handler._restore_original_author_for_cherry_pick( + pull_request=mock_pr, + git_cmd="git --work-tree=/tmp/test --git-dir=/tmp/test/.git", + github_token="test-token", # pragma: allowlist secret + ) + assert result is True + runner_handler.github_webhook.logger.warning.assert_called() + + @pytest.mark.asyncio + async def test_signoff_name_mismatch_uses_merge_commit_author(self, runner_handler: RunnerHandler) -> None: + """Signed-off-by has username but merge commit author has full name — uses merge commit author.""" + mock_pr, mock_merge_commit = self._make_merged_pr( + merge_commit_msg="docs: update docs\n\nSigned-off-by: rnetser \n", + author_name="Ruth Netser", + ) + runner_handler.github_webhook.repository.get_commit.return_value = mock_merge_commit + + amend_commands: list[str] = [] + + async def run_command_side_effect(command: str, **kwargs: Any) -> tuple[bool, str, str]: + if "log -1 --format=%an%n%ae" in command: + return (True, "rnetser\nrnetser@redhat.com", "") + if "log -1 --format=%B" in command: + return ( + True, + "docs: update docs\n\nSigned-off-by: rnetser \n", + "", + ) + if "commit --amend" in command: + amend_commands.append(command) + return (True, "success", "") + return (True, "", "") + + with ( + patch( + "webhook_server.libs.handlers.runner_handler.run_command", + new=AsyncMock(side_effect=run_command_side_effect), + ), + patch( + "asyncio.to_thread", + new=AsyncMock(side_effect=lambda fn, *a, **kw: fn(*a, **kw) if a or kw else fn()), + ), + ): + result = await runner_handler._restore_original_author_for_cherry_pick( + pull_request=mock_pr, + git_cmd="git --work-tree=/tmp/test --git-dir=/tmp/test/.git", + github_token="test-token", # pragma: allowlist secret + ) + assert result is True + # Verify it used the merge commit author name, not the sign-off name + assert len(amend_commands) == 1 + assert "Ruth Netser " in amend_commands[0] + + @pytest.mark.asyncio + async def test_message_read_failure_amends_author_only(self, runner_handler: RunnerHandler) -> None: + """Cannot read commit message — fall back to amending author only (--no-edit).""" + mock_pr, mock_merge_commit = self._make_merged_pr( + merge_commit_msg="feat: test\n\nSigned-off-by: User \n", + author_name="User", + ) + runner_handler.github_webhook.repository.get_commit.return_value = mock_merge_commit + + async def run_command_side_effect(command: str, **kwargs: Any) -> tuple[bool, str, str]: + if "log -1 --format=%an%n%ae" in command: + return (True, "noreply\nnoreply@github.com", "") + if "log -1 --format=%B" in command: + return (False, "", "fatal: could not read") + if "commit --amend" in command: + assert "--no-edit" in command + assert "User " in command + return (True, "success", "") + return (True, "", "") + + with ( + patch( + "webhook_server.libs.handlers.runner_handler.run_command", + new=AsyncMock(side_effect=run_command_side_effect), + ), + patch( + "asyncio.to_thread", + new=AsyncMock(side_effect=lambda fn, *a, **kw: fn(*a, **kw) if a or kw else fn()), + ), + ): + result = await runner_handler._restore_original_author_for_cherry_pick( + pull_request=mock_pr, + git_cmd="git --work-tree=/tmp/test --git-dir=/tmp/test/.git", + github_token="test-token", # pragma: allowlist secret + ) + assert result is True + runner_handler.github_webhook.logger.warning.assert_called() + + @pytest.mark.asyncio + async def test_api_failure_returns_false(self, runner_handler: RunnerHandler) -> None: + """github_api_call raises exception — return False (best-effort, don't block cherry-pick).""" + mock_pr = Mock() + mock_pr.merge_commit_sha = "abc123" + + with patch( + "webhook_server.libs.handlers.runner_handler.github_api_call", + new=AsyncMock(side_effect=RuntimeError("GitHub API unavailable")), + ): + result = await runner_handler._restore_original_author_for_cherry_pick( + pull_request=mock_pr, + git_cmd="git --work-tree=/tmp/test --git-dir=/tmp/test/.git", + github_token="test-token", # pragma: allowlist secret + ) + assert result is False + runner_handler.github_webhook.logger.exception.assert_called() + + @pytest.mark.asyncio + async def test_cancelled_error_is_reraised(self, runner_handler: RunnerHandler) -> None: + """asyncio.CancelledError is re-raised, not swallowed.""" + mock_pr = Mock() + mock_pr.merge_commit_sha = "abc123" + + with patch( + "webhook_server.libs.handlers.runner_handler.github_api_call", + new=AsyncMock(side_effect=asyncio.CancelledError()), + ): + with pytest.raises(asyncio.CancelledError): + await runner_handler._restore_original_author_for_cherry_pick( + pull_request=mock_pr, + git_cmd="git --work-tree=/tmp/test --git-dir=/tmp/test/.git", + github_token="test-token", # pragma: allowlist secret + ) + + class TestCheckConfig: """Test suite for CheckConfig dataclass."""