Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 40 additions & 12 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1126,18 +1126,46 @@ Users can interact with the webhook server through GitHub comments on pull reque

### Pull Request Commands

| Command | Description | Example |
| ------------------- | ----------------------------- | ------------------- |
| `/verified` | Mark PR as verified | `/verified` |
| `/verified cancel` | Remove verification | `/verified cancel` |
| `/hold` | Block PR merging | `/hold` |
| `/hold cancel` | Unblock PR merging | `/hold cancel` |
| `/wip` | Mark as work in progress | `/wip` |
| `/wip cancel` | Remove WIP status | `/wip cancel` |
| `/lgtm` | Approve changes | `/lgtm` |
| `/approve` | Approve PR | `/approve` |
| `/assign-reviewers` | Assign OWNERS-based reviewers | `/assign-reviewers` |
| `/check-can-merge` | Check merge readiness | `/check-can-merge` |
| Command | Description | Example |
| ------------------- | --------------------------------------------------- | ------------------- |
| `/verified` | Mark PR as verified | `/verified` |
| `/verified cancel` | Remove verification | `/verified cancel` |
| `/hold` | Block PR merging | `/hold` |
| `/hold cancel` | Unblock PR merging | `/hold cancel` |
| `/wip` | Mark as work in progress | `/wip` |
| `/wip cancel` | Remove WIP status | `/wip cancel` |
| `/lgtm` | Approve changes | `/lgtm` |
| `/approve` | Approve PR | `/approve` |
| `/assign-reviewers` | Assign OWNERS-based reviewers | `/assign-reviewers` |
| `/check-can-merge` | Check merge readiness | `/check-can-merge` |
| `/reprocess` | Trigger complete PR workflow reprocessing (OWNERS only) | `/reprocess` |

### Workflow Management

#### PR Reprocessing

The `/reprocess` command triggers complete PR workflow reprocessing from scratch, equivalent to reopening or synchronizing the PR.

**Permissions**: Requires user to be in repository OWNERS file (same as `/retest`)

**Use Cases**:
- Webhook delivery failed or was missed
- Processing interrupted mid-workflow
- OWNERS file changed and reviewers need reassignment
- Configuration changed and checks need re-evaluation
- PR got into inconsistent state and needs full reset

**Behavior**:
- Re-runs entire PR workflow including reviewer assignment, label updates, check queuing, and CI/CD tests
- Won't create duplicate welcome messages or tracking issues if they already exist
- Respects current repository configuration and OWNERS file

**Example**:

```bash
# Comment on the pull request
/reprocess
```

### Testing Commands

Expand Down
9 changes: 9 additions & 0 deletions webhook_server/libs/handlers/issue_comment_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
COMMAND_ASSIGN_REVIEWERS_STR,
COMMAND_CHECK_CAN_MERGE_STR,
COMMAND_CHERRY_PICK_STR,
COMMAND_REPROCESS_STR,
COMMAND_RETEST_STR,
CONVENTIONAL_TITLE_STR,
HOLD_LABEL_STR,
Expand Down Expand Up @@ -160,6 +161,7 @@ async def user_commands(
) -> None:
available_commands: list[str] = [
COMMAND_RETEST_STR,
COMMAND_REPROCESS_STR,
COMMAND_CHERRY_PICK_STR,
COMMAND_ASSIGN_REVIEWERS_STR,
COMMAND_CHECK_CAN_MERGE_STR,
Expand Down Expand Up @@ -239,6 +241,13 @@ async def user_commands(
pull_request=pull_request, command_args=_args, reviewed_user=reviewed_user
)

elif _command == COMMAND_REPROCESS_STR:
if not await self.owners_file_handler.is_user_valid_to_run_commands(
pull_request=pull_request, reviewed_user=reviewed_user
):
return
await self.pull_request_handler.process_command_reprocess(pull_request=pull_request)

elif _command == BUILD_AND_PUSH_CONTAINER_STR:
if self.github_webhook.build_and_push_container:
await self.runner_handler.run_build_container(
Expand Down
100 changes: 100 additions & 0 deletions webhook_server/libs/handlers/pull_request_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ def _prepare_welcome_comment(self) -> str:
* `/hold cancel` - Unblock PR merging
* `/verified` - Mark PR as verified
* `/verified cancel` - Remove verification status
* `/reprocess` - Trigger complete PR workflow reprocessing (useful if webhook failed or configuration changed)

#### Review & Approval
* `/lgtm` - Approve changes (looks good to me)
Expand Down Expand Up @@ -1115,3 +1116,102 @@ async def skip_if_pull_request_already_merged(self, pull_request: PullRequest) -
return True

return False

async def _welcome_comment_exists(self, pull_request: PullRequest) -> bool:
"""Check if welcome message already exists for this PR."""

def check_comments() -> bool:
return any(
self.github_webhook.issue_url_for_welcome_msg in comment.body
for comment in pull_request.get_issue_comments()
)

return await asyncio.to_thread(check_comments)

async def _tracking_issue_exists(self, pull_request: PullRequest) -> bool:
"""Check if tracking issue already exists for this PR."""
expected_body = self._generate_issue_body(pull_request=pull_request)

def check_issues() -> bool:
return any(issue.body == expected_body for issue in self.repository.get_issues())

return await asyncio.to_thread(check_issues)

async def process_new_or_reprocess_pull_request(self, pull_request: PullRequest) -> None:
"""Process a new or reprocessed PR - handles welcome message, tracking issue, and full workflow.

This method extracts the core logic from the "opened" event handler to make it reusable
for both new PRs and the /reprocess command. It includes duplicate prevention checks.
"""
self.logger.step( # type: ignore[attr-defined]
f"{self.log_prefix} {format_task_fields('pr_initialization', 'pr_management', 'started')} "
f"Starting PR initialization workflow",
)

tasks: list[Coroutine[Any, Any, Any]] = []

# Add welcome message if it doesn't exist yet
if not await self._welcome_comment_exists(pull_request=pull_request):
self.logger.info(f"{self.log_prefix} Adding welcome message to PR")
welcome_msg = self._prepare_welcome_comment()
tasks.append(asyncio.to_thread(pull_request.create_issue_comment, body=welcome_msg))
else:
self.logger.info(f"{self.log_prefix} Welcome message already exists, skipping")

# Add tracking issue if it doesn't exist yet
if not await self._tracking_issue_exists(pull_request=pull_request):
self.logger.info(f"{self.log_prefix} Creating tracking issue for PR")
tasks.append(self.create_issue_for_new_pull_request(pull_request=pull_request))
else:
self.logger.info(f"{self.log_prefix} Tracking issue already exists, skipping")

# Always run these tasks
tasks.append(self.set_wip_label_based_on_title(pull_request=pull_request))
tasks.append(self.process_opened_or_synchronize_pull_request(pull_request=pull_request))

self.logger.step( # type: ignore[attr-defined]
f"{self.log_prefix} {format_task_fields('pr_initialization', 'pr_management', 'processing')} "
f"Executing initialization tasks",
)
results = await asyncio.gather(*tasks, return_exceptions=True)
for result in results:
if isinstance(result, Exception):
self.logger.error(f"{self.log_prefix} Async task failed: {result}")

# Set auto merge only after all initialization is done
self.logger.step( # type: ignore[attr-defined]
f"{self.log_prefix} {format_task_fields('pr_initialization', 'pr_management', 'processing')} "
f"Setting auto-merge configuration",
)
await self.set_pull_request_automerge(pull_request=pull_request)

self.logger.step( # type: ignore[attr-defined]
f"{self.log_prefix} {format_task_fields('pr_initialization', 'pr_management', 'completed')} "
f"PR initialization workflow completed",
)

async def process_command_reprocess(self, pull_request: PullRequest) -> None:
"""Handle /reprocess command - triggers full PR workflow from scratch."""
self.logger.step( # type: ignore[attr-defined]
f"{self.log_prefix} {format_task_fields('reprocess_command', 'pr_management', 'started')} "
f"Starting /reprocess command execution for PR #{pull_request.number}",
)

# Check if PR is already merged - skip if merged
if await asyncio.to_thread(lambda: pull_request.is_merged()):
self.logger.info(f"{self.log_prefix} PR is already merged, skipping reprocess")
self.logger.step( # type: ignore[attr-defined]
f"{self.log_prefix} {format_task_fields('reprocess_command', 'pr_management', 'completed')} "
f"/reprocess command completed (PR already merged - skipped)",
)
return

self.logger.info(f"{self.log_prefix} Executing full PR reprocessing workflow")

# Call the extracted reusable method
await self.process_new_or_reprocess_pull_request(pull_request=pull_request)

self.logger.step( # type: ignore[attr-defined]
f"{self.log_prefix} {format_task_fields('reprocess_command', 'pr_management', 'completed')} "
f"/reprocess command completed successfully",
)
147 changes: 147 additions & 0 deletions webhook_server/tests/test_issue_comment_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
COMMAND_ASSIGN_REVIEWERS_STR,
COMMAND_CHECK_CAN_MERGE_STR,
COMMAND_CHERRY_PICK_STR,
COMMAND_REPROCESS_STR,
COMMAND_RETEST_STR,
HOLD_LABEL_STR,
REACTIONS,
Expand Down Expand Up @@ -758,3 +759,149 @@ async def test_process_retest_command_async_task_exception(
pull_request=mock_pull_request, command_args="tox", reviewed_user="test-user"
)
mock_error.assert_called_once()

@pytest.mark.asyncio
async def test_user_commands_reprocess_command_registration(
self, issue_comment_handler: IssueCommentHandler
) -> None:
"""Test that reprocess command is in available_commands list."""
# Verify COMMAND_REPROCESS_STR is in the available_commands list
# by checking if the command is recognized (doesn't return early for unsupported command)
mock_pull_request = Mock()

with (
patch.object(
issue_comment_handler.owners_file_handler,
"is_user_valid_to_run_commands",
new=AsyncMock(return_value=True),
),
patch.object(
issue_comment_handler.pull_request_handler, "process_command_reprocess", new=AsyncMock()
) as mock_reprocess,
patch.object(issue_comment_handler, "create_comment_reaction", new=AsyncMock()),
):
await issue_comment_handler.user_commands(
pull_request=mock_pull_request,
command=COMMAND_REPROCESS_STR,
reviewed_user="test-user",
issue_comment_id=123,
)
# Command should be recognized and processed
mock_reprocess.assert_awaited_once_with(pull_request=mock_pull_request)

@pytest.mark.asyncio
async def test_user_commands_reprocess_authorized_user(self, issue_comment_handler: IssueCommentHandler) -> None:
"""Test reprocess command with authorized user (in OWNERS)."""
mock_pull_request = Mock()

with (
patch.object(
issue_comment_handler.owners_file_handler,
"is_user_valid_to_run_commands",
new=AsyncMock(return_value=True),
),
patch.object(
issue_comment_handler.pull_request_handler, "process_command_reprocess", new=AsyncMock()
) as mock_reprocess,
patch.object(issue_comment_handler, "create_comment_reaction", new=AsyncMock()) as mock_reaction,
):
await issue_comment_handler.user_commands(
pull_request=mock_pull_request,
command=COMMAND_REPROCESS_STR,
reviewed_user="approver1", # From fixture: all_pull_request_approvers
issue_comment_id=123,
)
# Verify user validation was called
issue_comment_handler.owners_file_handler.is_user_valid_to_run_commands.assert_awaited_once_with(
pull_request=mock_pull_request, reviewed_user="approver1"
)
# Verify reprocess handler was called
mock_reprocess.assert_awaited_once_with(pull_request=mock_pull_request)
# Verify reaction was added
mock_reaction.assert_awaited_once_with(
pull_request=mock_pull_request, issue_comment_id=123, reaction=REACTIONS.ok
)

@pytest.mark.asyncio
async def test_user_commands_reprocess_unauthorized_user(self, issue_comment_handler: IssueCommentHandler) -> None:
"""Test reprocess command with unauthorized user (not in OWNERS)."""
mock_pull_request = Mock()

with (
patch.object(
issue_comment_handler.owners_file_handler,
"is_user_valid_to_run_commands",
new=AsyncMock(return_value=False),
),
patch.object(
issue_comment_handler.pull_request_handler, "process_command_reprocess", new=AsyncMock()
) as mock_reprocess,
patch.object(issue_comment_handler, "create_comment_reaction", new=AsyncMock()) as mock_reaction,
):
await issue_comment_handler.user_commands(
pull_request=mock_pull_request,
command=COMMAND_REPROCESS_STR,
reviewed_user="unauthorized-user",
issue_comment_id=123,
)
# Verify user validation was called
issue_comment_handler.owners_file_handler.is_user_valid_to_run_commands.assert_awaited_once_with(
pull_request=mock_pull_request, reviewed_user="unauthorized-user"
)
# Verify reprocess handler was NOT called
mock_reprocess.assert_not_awaited()
# Reaction should still be added before permission check
mock_reaction.assert_awaited_once_with(
pull_request=mock_pull_request, issue_comment_id=123, reaction=REACTIONS.ok
)

@pytest.mark.asyncio
async def test_user_commands_reprocess_with_args(self, issue_comment_handler: IssueCommentHandler) -> None:
"""Test reprocess command with additional arguments (should ignore args)."""
mock_pull_request = Mock()

with (
patch.object(
issue_comment_handler.owners_file_handler,
"is_user_valid_to_run_commands",
new=AsyncMock(return_value=True),
),
patch.object(
issue_comment_handler.pull_request_handler, "process_command_reprocess", new=AsyncMock()
) as mock_reprocess,
patch.object(issue_comment_handler, "create_comment_reaction", new=AsyncMock()),
):
# Command with args (should be processed but args ignored)
await issue_comment_handler.user_commands(
pull_request=mock_pull_request,
command=f"{COMMAND_REPROCESS_STR} some-args",
reviewed_user="test-user",
issue_comment_id=123,
)
# Verify reprocess was called (args are ignored)
mock_reprocess.assert_awaited_once_with(pull_request=mock_pull_request)

@pytest.mark.asyncio
async def test_user_commands_reprocess_reaction_added(self, issue_comment_handler: IssueCommentHandler) -> None:
"""Test that reaction is added to comment for reprocess command."""
mock_pull_request = Mock()

with (
patch.object(
issue_comment_handler.owners_file_handler,
"is_user_valid_to_run_commands",
new=AsyncMock(return_value=True),
),
patch.object(issue_comment_handler.pull_request_handler, "process_command_reprocess", new=AsyncMock()),
patch.object(issue_comment_handler, "create_comment_reaction", new=AsyncMock()) as mock_reaction,
):
await issue_comment_handler.user_commands(
pull_request=mock_pull_request,
command=COMMAND_REPROCESS_STR,
reviewed_user="test-user",
issue_comment_id=456,
)
# Verify reaction was added with correct comment ID and reaction type
mock_reaction.assert_awaited_once_with(
pull_request=mock_pull_request, issue_comment_id=456, reaction=REACTIONS.ok
)
Loading