Commit ef8d7e6
authored
feat: cherry-pick retry/rebase commands + pre-commit auto-fix + security hardening (#1108)
<h3>PR Summary by Qodo</h3>
Add cherry-pick retry + PR rebase commands, pre-commit auto-fix, and
command guards
<code>✨ Enhancement</code> <code>🐞 Bug fix</code> <code>🧪 Tests</code>
<code>📝 Documentation</code> <code>🕐 40+ Minutes</code>
<img
src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg"
height="10%" alt="Grey Divider">
<h3>Walkthroughs</h3>
<details open>
<summary>User Description</summary>
<br/>
## Summary
### New commands (#1103)
- `/cherry-pick-retry <branch>` — Retry a failed cherry-pick on merged
PRs
- `/rebase` — Rebase any open PR onto its base branch (with bot-PR
ownership validation)
### Pre-commit auto-fix (#1089)
- Run pre-commit after cherry-pick, before push
- If files modified, commit fixes automatically
### Security hardening
- Add `is_user_valid_to_run_commands` guard to all 7 previously
unguarded commands: `/cherry-pick`, `/assign-reviewer`,
`/assign-reviewers`, `/check-can-merge`, `/verified`, `/wip`,
`/test-oracle`
- Verify bot ownership before closing cherry-pick PR on retry
Closes #1103
Closes #1089
</details>
<details open>
<summary>AI Description</summary>
<br/>
<pre>
• Add <b><i>/cherry-pick-retry</i></b> and <b><i>/rebase</i></b>
issue-comment commands to unblock bot-driven workflows.
• Run pre-commit after cherry-pick and auto-commit any formatter fixes
before pushing.
• Harden command execution by gating previously unguarded commands with
repo ownership checks.
</pre>
</details>
<details>
<summary>Diagram</summary>
<br/>
```mermaid
graph TD
A["Issue comment"] --> B["IssueCommentHandler"] --> C["OwnersFileHandler"]
B --> D["RunnerHandler"] --> E["Local git worktree"] --> F{{"GitHub API/remote"}}
D --> G["Pre-commit"] --> E
B --> H["PullRequestHandler"]
```
</details>
<details>
<summary>High-Level Assessment</summary>
<br/>
The following are alternative approaches to this PR:
<details>
<summary><b>1. Use GitHub's built-in 'Update branch' /
merge-upstream flow</b></summary>
- ➕ Avoids local git worktree + force-push complexity
- ➕ Leverages GitHub-native conflict reporting and permissions
- ➖ Not always available/enabled for all repos and branch protection
rules
- ➖ Doesn't cover all rebase semantics; may merge instead of rebase
</details>
<details>
<summary><b>2. Centralize command authorization via a
dispatcher/decorator</b></summary>
- ➕ Reduces repeated guard boilerplate per command
- ➕ Makes it harder to accidentally introduce unguarded commands in
future
- ➖ Refactor touches many call sites; higher churn and regression risk
- ➖ Less explicit per-command control (unless carefully designed)
</details>
<details>
<summary><b>3. Run formatting fixes only in CI and comment results (no
auto-commit)</b></summary>
- ➕ Avoids bot-authored commits and force-push interactions
- ➕ Keeps PR history purely authored by humans unless opted-in
- ➖ Doesn't unblock cherry-pick automation; still requires manual
fix+push
- ➖ Adds latency and back-and-forth for small formatting issues
</details>
**Recommendation:** The PR’s approach is appropriate for a bot-driven
maintenance workflow: rebase and cherry-pick actions need local git to
be deterministic, and the added authorization gates materially reduce
abuse risk. Consider a follow-up to centralize authorization in the
command dispatcher to prevent future unguarded commands and reduce
repetition.
</details>
<img
src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg"
height="10%" alt="Grey Divider">
<h3>File Changes</h3>
<details>
<summary><strong>Enhancement</strong> (3)</summary>
<blockquote>
<details>
<summary><strong>issue_comment_handler.py</strong> <code>Add
/cherry-pick-retry + /rebase dispatch and guard sensitive
commands</code> <code>+153/-0</code></summary>
<br/>
>Add /cherry-pick-retry + /rebase dispatch and guard sensitive commands
>
><pre>
>• Registers new command constants and routes
'/cherry-pick-retry' and '/rebase' to the
appropriate handlers. Adds 'is_user_valid_to_run_commands'
checks to previously unguarded commands (e.g., cherry-pick,
assign-reviewer(s), check-can-merge, verified, wip, test-oracle).
Implements 'process_cherry_pick_retry_command' to validate
merged state, require an existing cherry-pick label, close bot-owned
prior cherry-pick PRs referencing the original PR, then re-run
cherry-pick.
></pre>
>
><a
href='https://github.com/myk-org/github-webhook-server/pull/1108/files#diff-b868fb1872f7796f85f4d0e1dca51b2720e89a4af12baa480e21d8f22333de83'>webhook_server/libs/handlers/issue_comment_handler.py</a>
<hr/>
</details>
</blockquote>
<blockquote>
<details>
<summary><strong>runner_handler.py</strong> <code>Run pre-commit
auto-fix during cherry-pick and add PR rebase implementation</code>
<code>+200/-0</code></summary>
<br/>
>Run pre-commit auto-fix during cherry-pick and add PR rebase
implementation
>
><pre>
>• Enhances 'cherry_pick()' to optionally run pre-commit in
the cherry-pick worktree and, if hooks modified files, auto-stage and
commit the fixes before pushing. Adds 'rebase_pr()' which
rebases the PR head onto its base branch and force-pushes with lease;
for bot-owned PRs, it enforces that only the PR assignee (initiator) or
maintainers can run the rebase.
></pre>
>
><a
href='https://github.com/myk-org/github-webhook-server/pull/1108/files#diff-0cb54c95cafda12d8d169c7b03ac484738f4cf925c22f6e6b8b8c5db0730ce42'>webhook_server/libs/handlers/runner_handler.py</a>
<hr/>
</details>
</blockquote>
<blockquote>
<details>
<summary><strong>constants.py</strong> <code>Add command constants for
cherry-pick retry and rebase</code> <code>+2/-0</code></summary>
<br/>
>Add command constants for cherry-pick retry and rebase
>
><pre>
>• Defines 'COMMAND_CHERRY_PICK_RETRY_STR' and
'COMMAND_REBASE_STR' for consistent command parsing and
documentation.
></pre>
>
><a
href='https://github.com/myk-org/github-webhook-server/pull/1108/files#diff-9a2d73fb31266bc568369ee81f15b1ebb12d9703f412a0ab65cf7c5a5b98060f'>webhook_server/utils/constants.py</a>
<hr/>
</details>
</blockquote>
</details>
<details>
<summary><strong>Tests</strong> (2)</summary>
<blockquote>
<details>
<summary><strong>test_issue_comment_handler.py</strong> <code>Add tests
for new commands and authorization guards</code>
<code>+510/-0</code></summary>
<br/>
>Add tests for new commands and authorization guards
>
><pre>
>• Introduces async tests ensuring unauthorized users are blocked from
newly guarded commands. Adds coverage for '/cherry-pick-retry'
dispatch, argument validation, merged/label preconditions, bot-ownership
checks when closing existing cherry-pick PRs, and '/rebase'
dispatch/guard behavior.
></pre>
>
><a
href='https://github.com/myk-org/github-webhook-server/pull/1108/files#diff-cbfe12aa39c2b47d87a6f8d21da369eb9a1c64424c49d3584a1a220f68f05c18'>webhook_server/tests/test_issue_comment_handler.py</a>
<hr/>
</details>
</blockquote>
<blockquote>
<details>
<summary><strong>test_runner_handler.py</strong> <code>Add rebase_pr and
cherry-pick pre-commit auto-fix test coverage</code>
<code>+418/-0</code></summary>
<br/>
>Add rebase_pr and cherry-pick pre-commit auto-fix test coverage
>
><pre>
>• Adds a new 'TestRebasePr' suite covering non-open PR
rejection, bot-owned PR authorization (assignee/maintainer), worktree
failures, conflict handling with abort, and success paths. Adds
'TestCherryPickPreCommitAutoFix' to validate pre-commit
execution, auto-commit behavior when hooks modify files, and skipping
when disabled.
></pre>
>
><a
href='https://github.com/myk-org/github-webhook-server/pull/1108/files#diff-beb446b71d75b7a427d17e48e75971c270de4820cb9786a12af9e4f988887269'>webhook_server/tests/test_runner_handler.py</a>
<hr/>
</details>
</blockquote>
</details>
<details>
<summary><strong>Documentation</strong> (1)</summary>
<blockquote>
<details>
<summary><strong>pull_request_handler.py</strong> <code>Expose new
commands in the welcome/help message</code> <code>+5/-1</code></summary>
<br/>
>Expose new commands in the welcome/help message
>
><pre>
>• Extends the generated welcome section to document
'/cherry-pick-retry' and adds a new 'Branch
Management' section for '/rebase'. Ensures rebase help
text appears even when cherry-pick operations are not enabled.
></pre>
>
><a
href='https://github.com/myk-org/github-webhook-server/pull/1108/files#diff-8644dc42c86db802123c2ba72847dca72589fe19f330ecc70621af895a72fc8a'>webhook_server/libs/handlers/pull_request_handler.py</a>
<hr/>
</details>
</blockquote>
</details>
<img
src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg"
height="10%" alt="Grey Divider">
<a href="https://www.qodo.ai"><img
src="https://www.qodo.ai/wp-content/uploads/2025/03/qodo-logo.svg"
width="80" alt="Qodo Logo"></a>1 parent b8c21fc commit ef8d7e6
9 files changed
Lines changed: 1691 additions & 40 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
195 | 195 | | |
196 | 196 | | |
197 | 197 | | |
| 198 | + | |
| 199 | + | |
198 | 200 | | |
199 | 201 | | |
200 | 202 | | |
| |||
537 | 539 | | |
538 | 540 | | |
539 | 541 | | |
| 542 | + | |
| 543 | + | |
| 544 | + | |
| 545 | + | |
| 546 | + | |
| 547 | + | |
| 548 | + | |
| 549 | + | |
| 550 | + | |
| 551 | + | |
| 552 | + | |
| 553 | + | |
| 554 | + | |
| 555 | + | |
| 556 | + | |
| 557 | + | |
| 558 | + | |
| 559 | + | |
| 560 | + | |
| 561 | + | |
| 562 | + | |
| 563 | + | |
| 564 | + | |
| 565 | + | |
| 566 | + | |
540 | 567 | | |
541 | 568 | | |
542 | 569 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
24 | 24 | | |
25 | 25 | | |
26 | 26 | | |
| 27 | + | |
27 | 28 | | |
| 29 | + | |
28 | 30 | | |
29 | 31 | | |
30 | 32 | | |
| |||
160 | 162 | | |
161 | 163 | | |
162 | 164 | | |
| 165 | + | |
| 166 | + | |
163 | 167 | | |
164 | 168 | | |
165 | 169 | | |
| |||
219 | 223 | | |
220 | 224 | | |
221 | 225 | | |
| 226 | + | |
222 | 227 | | |
223 | 228 | | |
224 | 229 | | |
| |||
253 | 258 | | |
254 | 259 | | |
255 | 260 | | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
256 | 265 | | |
257 | 266 | | |
258 | 267 | | |
| |||
264 | 273 | | |
265 | 274 | | |
266 | 275 | | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
267 | 280 | | |
268 | 281 | | |
269 | 282 | | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
270 | 287 | | |
271 | 288 | | |
272 | 289 | | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
273 | 294 | | |
274 | 295 | | |
275 | 296 | | |
276 | 297 | | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
277 | 314 | | |
278 | 315 | | |
279 | 316 | | |
| |||
295 | 332 | | |
296 | 333 | | |
297 | 334 | | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
298 | 339 | | |
299 | 340 | | |
300 | 341 | | |
| |||
322 | 363 | | |
323 | 364 | | |
324 | 365 | | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
325 | 370 | | |
326 | 371 | | |
327 | 372 | | |
| |||
374 | 419 | | |
375 | 420 | | |
376 | 421 | | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
| 425 | + | |
377 | 426 | | |
378 | 427 | | |
379 | 428 | | |
| |||
526 | 575 | | |
527 | 576 | | |
528 | 577 | | |
529 | | - | |
| 578 | + | |
| 579 | + | |
530 | 580 | | |
531 | 581 | | |
532 | 582 | | |
| |||
558 | 608 | | |
559 | 609 | | |
560 | 610 | | |
| 611 | + | |
| 612 | + | |
| 613 | + | |
| 614 | + | |
| 615 | + | |
| 616 | + | |
| 617 | + | |
| 618 | + | |
| 619 | + | |
| 620 | + | |
| 621 | + | |
| 622 | + | |
| 623 | + | |
| 624 | + | |
| 625 | + | |
| 626 | + | |
| 627 | + | |
| 628 | + | |
| 629 | + | |
| 630 | + | |
| 631 | + | |
| 632 | + | |
| 633 | + | |
| 634 | + | |
| 635 | + | |
| 636 | + | |
| 637 | + | |
| 638 | + | |
| 639 | + | |
| 640 | + | |
| 641 | + | |
| 642 | + | |
| 643 | + | |
| 644 | + | |
| 645 | + | |
| 646 | + | |
| 647 | + | |
| 648 | + | |
| 649 | + | |
| 650 | + | |
| 651 | + | |
| 652 | + | |
| 653 | + | |
| 654 | + | |
| 655 | + | |
| 656 | + | |
| 657 | + | |
| 658 | + | |
| 659 | + | |
| 660 | + | |
| 661 | + | |
| 662 | + | |
| 663 | + | |
| 664 | + | |
| 665 | + | |
| 666 | + | |
| 667 | + | |
| 668 | + | |
| 669 | + | |
| 670 | + | |
| 671 | + | |
| 672 | + | |
| 673 | + | |
| 674 | + | |
| 675 | + | |
| 676 | + | |
| 677 | + | |
| 678 | + | |
| 679 | + | |
| 680 | + | |
| 681 | + | |
| 682 | + | |
| 683 | + | |
| 684 | + | |
| 685 | + | |
| 686 | + | |
| 687 | + | |
| 688 | + | |
| 689 | + | |
| 690 | + | |
| 691 | + | |
| 692 | + | |
| 693 | + | |
| 694 | + | |
| 695 | + | |
| 696 | + | |
| 697 | + | |
| 698 | + | |
| 699 | + | |
| 700 | + | |
| 701 | + | |
| 702 | + | |
| 703 | + | |
| 704 | + | |
| 705 | + | |
| 706 | + | |
| 707 | + | |
| 708 | + | |
| 709 | + | |
| 710 | + | |
| 711 | + | |
| 712 | + | |
| 713 | + | |
| 714 | + | |
| 715 | + | |
| 716 | + | |
| 717 | + | |
| 718 | + | |
| 719 | + | |
| 720 | + | |
| 721 | + | |
| 722 | + | |
| 723 | + | |
| 724 | + | |
| 725 | + | |
| 726 | + | |
| 727 | + | |
| 728 | + | |
| 729 | + | |
| 730 | + | |
| 731 | + | |
| 732 | + | |
| 733 | + | |
| 734 | + | |
| 735 | + | |
| 736 | + | |
| 737 | + | |
| 738 | + | |
| 739 | + | |
| 740 | + | |
| 741 | + | |
| 742 | + | |
| 743 | + | |
| 744 | + | |
| 745 | + | |
| 746 | + | |
| 747 | + | |
| 748 | + | |
| 749 | + | |
| 750 | + | |
| 751 | + | |
| 752 | + | |
| 753 | + | |
| 754 | + | |
| 755 | + | |
| 756 | + | |
| 757 | + | |
| 758 | + | |
| 759 | + | |
| 760 | + | |
| 761 | + | |
| 762 | + | |
| 763 | + | |
| 764 | + | |
| 765 | + | |
| 766 | + | |
| 767 | + | |
| 768 | + | |
| 769 | + | |
561 | 770 | | |
562 | 771 | | |
563 | 772 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
481 | 481 | | |
482 | 482 | | |
483 | 483 | | |
484 | | - | |
| 484 | + | |
485 | 485 | | |
486 | 486 | | |
487 | 487 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
795 | 795 | | |
796 | 796 | | |
797 | 797 | | |
| 798 | + | |
| 799 | + | |
| 800 | + | |
| 801 | + | |
798 | 802 | | |
799 | | - | |
| 803 | + | |
800 | 804 | | |
801 | 805 | | |
802 | 806 | | |
| |||
0 commit comments