Skip to content

feat: cherry-pick author attribution and PR assignment#994

Merged
myakove merged 21 commits intomainfrom
feat/issue-993-cherry-pick-author-attribution
Mar 2, 2026
Merged

feat: cherry-pick author attribution and PR assignment#994
myakove merged 21 commits intomainfrom
feat/issue-993-cherry-pick-author-attribution

Conversation

@rnetser
Copy link
Copy Markdown
Collaborator

@rnetser rnetser commented Feb 19, 2026

Summary

Test plan

  • Cherry-pick attribution tests pass
  • Schema validation tests pass

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Feb 19, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds a new global and per-repository config flag to control assigning cherry-pick PRs to the original PR author, exposes that flag on the GitHub webhook object, threads it into label-triggered and command-triggered cherry-pick flows, updates the RunnerHandler.cherry_pick signature to accept an assign-to-owner flag, and updates tests and examples accordingly.

Changes

Cohort / File(s) Summary
Configuration Schema
webhook_server/config/schema.yaml, examples/config.yaml, examples/.github-webhook-server.yaml
Added root-level and per-repo cherry-pick-assign-to-pr-author boolean (default: true) to schema and example configs.
Core Initialization
webhook_server/libs/github_api.py
Read global and per-repo cherry-pick-assign-to-pr-author and expose cherry_pick_assign_to_pr_author on GithubWebhook instances (repo override takes precedence).
PR Handling Logic
webhook_server/libs/handlers/pull_request_handler.py, webhook_server/libs/handlers/issue_comment_handler.py
Cherry-pick label processing now collects labels and calls RunnerHandler.cherry_pick for each target branch, passing the assign-to-pr-owner flag from the webhook config.
Cherry-Pick Implementation
webhook_server/libs/handlers/runner_handler.py
Changed cherry_pick signature to accept assign_to_pr_owner: bool (defaults to True). Retriever of PR author and source branch; builds hub command and labels to include assignee when flag is set; updates logs/comments.
Tests
webhook_server/tests/test_config_schema.py, webhook_server/tests/test_pull_request_handler.py, webhook_server/tests/test_runner_handler.py, webhook_server/tests/test_issue_comment_handler.py
Added/updated fixtures and tests to include cherry_pick_assign_to_pr_author=True; added CherryPickMocks and cherry_pick_setup for runner tests; updated assertions to expect assign_to_pr_owner=True calls and verify assignee usage.
Schema Validator
webhook_server/tests/test_schema_validator.py
Added cherry-pick-assign-to-pr-author to boolean_fields validation for root and per-repo config.

Sequence Diagram(s)

sequenceDiagram
    participant PR as Pull Request
    participant PRH as PullRequestHandler
    participant RH as RunnerHandler
    participant GH as GitHub

    PR->>PRH: merged with `cherry-pick-<branch>` label(s)
    PRH->>PRH: filter labels\nextract PR author
    PRH->>RH: cherry_pick(pr, target_branch,\nassign_to_pr_owner=webhook.cherry_pick_assign_to_pr_author)
    RH->>RH: build requested_by text\n(determine assignee_flag if assign_to_pr_owner)
    alt assignee_flag present
        Note over RH: assignee_flag = "-a {author_login}"
    end
    alt target branch exists
        RH->>GH: run hub create pull-request (include labels, assignee_flag)
        GH-->>RH: success
    else
        RH->>GH: post comment on original PR about failure
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Possibly related PRs

Suggested reviewers

  • myakove
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 76.19% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the main feature: adding cherry-pick PR author attribution and optional assignment to the original PR author.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/issue-993-cherry-pick-author-attribution

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@qodo-code-review
Copy link
Copy Markdown

Review Summary by Qodo

Add PR author attribution and optional assignee for cherry-pick PRs

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Add PR author attribution to cherry-pick descriptions with new by_label parameter
• Implement optional assignee feature via cherry-pick-assign-to-pr-author config option
• Extract PR author login when cherry-pick triggered by label on merge
• Add comprehensive test coverage for assignee and attribution functionality

Grey Divider

File Changes

1. webhook_server/libs/github_api.py ⚙️ Configuration changes +3/-0

Load cherry-pick assignee configuration option

• Add new config property cherry_pick_assign_to_pr_author with default value False
• Load configuration from both global and per-repository levels

webhook_server/libs/github_api.py


2. webhook_server/libs/handlers/pull_request_handler.py ✨ Enhancement +8/-4

Extract PR author for label-triggered cherry-picks

• Filter labels to extract only cherry-pick labels before processing
• Retrieve PR author login when cherry-pick labels are present
• Pass reviewed_user (PR author) and by_label=True to cherry_pick method

webhook_server/libs/handlers/pull_request_handler.py


3. webhook_server/libs/handlers/runner_handler.py ✨ Enhancement +27/-3

Implement author attribution and assignee logic

• Add by_label parameter to cherry_pick method with comprehensive docstring
• Implement conditional requested_by format based on by_label flag
• Add logic to determine assignee based on cherry_pick_assign_to_pr_author config
• Construct -a flag with quoted assignee username for hub pull-request command

webhook_server/libs/handlers/runner_handler.py


View more (3)
4. webhook_server/tests/test_pull_request_handler.py 🧪 Tests +4/-1

Update cherry-pick test assertions

• Update test assertion to include reviewed_user="owner1" parameter
• Add by_label=True parameter to cherry_pick mock call verification

webhook_server/tests/test_pull_request_handler.py


5. webhook_server/tests/test_runner_handler.py 🧪 Tests +105/-0

Add comprehensive cherry-pick assignee tests

• Initialize cherry_pick_assign_to_pr_author mock property to False
• Add test for assignee assignment when reviewed_user is provided
• Add test for fallback to PR author login when reviewed_user is empty
• Add test for correct requested-by format when triggered by label

webhook_server/tests/test_runner_handler.py


6. webhook_server/config/schema.yaml ⚙️ Configuration changes +8/-0

Add cherry-pick assignee config schema

• Add cherry-pick-assign-to-pr-author boolean property to global configuration
• Add cherry-pick-assign-to-pr-author boolean property to per-repository configuration
• Include description and default value false for both levels

webhook_server/config/schema.yaml


Grey Divider

Qodo Logo

@myakove-bot
Copy link
Copy Markdown
Collaborator

Report bugs in Issues

Welcome! 🎉

This pull request will be automatically processed with the following features:

🔄 Automatic Actions

  • Reviewer Assignment: Reviewers are automatically assigned based on the OWNERS file in the repository root
  • Size Labeling: PR size labels (XS, S, M, L, XL, XXL) are automatically applied based on changes
  • Issue Creation: Disabled for this repository
  • Pre-commit Checks: pre-commit runs automatically if .pre-commit-config.yaml exists
  • Branch Labeling: Branch-specific labels are applied to track the target branch
  • Auto-verification: Auto-verified users have their PRs automatically marked as verified
  • Labels: All label categories are enabled (default configuration)

📋 Available Commands

PR Status Management

  • /wip - Mark PR as work in progress (adds WIP: prefix to title)
  • /wip cancel - Remove work in progress status
  • /hold - Block PR merging (approvers only)
  • /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)
  • /regenerate-welcome - Regenerate this welcome message

Review & Approval

  • /lgtm - Approve changes (looks good to me)
  • /approve - Approve PR (approvers only)
  • /automerge - Enable automatic merging when all requirements are met (maintainers and approvers only)
  • /assign-reviewers - Assign reviewers based on OWNERS file
  • /assign-reviewer @username - Assign specific reviewer
  • /check-can-merge - Check if PR meets merge requirements

Testing & Validation

  • /retest tox - Run Python test suite with tox
  • /retest build-container - Rebuild and test container image
  • /retest python-module-install - Test Python package installation
  • /retest pre-commit - Run pre-commit hooks and checks
  • /retest conventional-title - Validate commit message format
  • /retest all - Run all available tests

Container Operations

  • /build-and-push-container - Build and push container image (tagged with PR number)
    • Supports additional build arguments: /build-and-push-container --build-arg KEY=value

Cherry-pick Operations

  • /cherry-pick <branch> - Schedule cherry-pick to target branch when PR is merged
    • Multiple branches: /cherry-pick branch1 branch2 branch3

Label Management

  • /<label-name> - Add a label to the PR
  • /<label-name> cancel - Remove a label from the PR

✅ Merge Requirements

This PR will be automatically approved when the following conditions are met:

  1. Approval: /approve from at least one approver
  2. LGTM Count: Minimum 1 /lgtm from reviewers
  3. Status Checks: All required status checks must pass
  4. No Blockers: No WIP, hold, conflict labels
  5. Verified: PR must be marked as verified (if verification is enabled)

📊 Review Process

Approvers and Reviewers

Approvers:

  • myakove
  • rnetser

Reviewers:

  • myakove
  • rnetser
Available Labels
  • hold
  • verified
  • wip
  • lgtm
  • approve
  • automerge

💡 Tips

  • WIP Status: Use /wip when your PR is not ready for review
  • Verification: The verified label is automatically removed on each new commit
  • Cherry-picking: Cherry-pick labels are processed when the PR is merged
  • Container Builds: Container images are automatically tagged with the PR number
  • Permission Levels: Some commands require approver permissions
  • Auto-verified Users: Certain users have automatic verification and merge privileges

For more information, please refer to the project documentation or contact the maintainers.

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review Bot commented Feb 19, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (2)

Grey Divider


Action required

✅ 1. -a uses reviewed_user 📎 Requirement gap ✓ Correctness
Description
When cherry_pick_assign_to_pr_author is enabled, cherry_pick() builds hub’s -a assignee from
reviewed_user when provided, but reviewed_user is the command requester in comment-triggered
flows rather than the original PR author. This can incorrectly assign cherry-pick PRs to the
requester instead of the PR author.
Code

webhook_server/libs/handlers/runner_handler.py[R558-565]

+        if self.github_webhook.cherry_pick_assign_to_pr_author:
+            if reviewed_user:
+                pr_author = reviewed_user
+            else:
+                pr_author = await asyncio.to_thread(lambda: pull_request.user.login)
+            assignee_flag = f" -a {shlex.quote(pr_author)}"
+        else:
+            assignee_flag = ""
Evidence
Compliance requires assigning the cherry-pick PR to the original PR author via hub -a, but the new
implementation assigns to reviewed_user when present, and reviewed_user is passed from the
comment command path as the requesting user (not the PR author).

Optional assignee logic assigns cherry-pick PR to original PR author via hub -a flag
webhook_server/libs/handlers/runner_handler.py[558-565]
webhook_server/libs/handlers/issue_comment_handler.py[420-434]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`cherry-pick-assign-to-pr-author` is intended to assign the created cherry-pick PR to the original PR author, but the current implementation assigns to `reviewed_user` when it is provided (which is the requester in the comment-command flow).
## Issue Context
`reviewed_user` is used for audit/attribution of who requested the cherry-pick. For comment-triggered cherry-picks, `reviewed_user` is the command requester, not the PR author, so using it for hub `-a` violates the requirement.
## Fix Focus Areas
- webhook_server/libs/handlers/runner_handler.py[539-565]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


✅ 2. Tests expect requester assignee 📎 Requirement gap ⛯ Reliability
Description
New tests assert the hub -a assignee is reviewed_user (the requester) rather than the original
PR author, so they validate the wrong behavior. They also don’t add a test that explicitly verifies
the default/disabled state excludes -a.
Code

webhook_server/tests/test_runner_handler.py[R922-952]

+    @pytest.mark.asyncio
+    async def test_cherry_pick_assigns_to_reviewed_user(
+        self, runner_handler: RunnerHandler, mock_pull_request: Mock
+    ) -> None:
+        """Test cherry_pick uses reviewed_user for assignee when cherry_pick_assign_to_pr_author is True."""
+        runner_handler.github_webhook.pypi = {"token": "dummy"}
+        runner_handler.github_webhook.cherry_pick_assign_to_pr_author = True
+        with 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:
+                        mock_checkout.return_value = AsyncMock()
+                        mock_checkout.return_value.__aenter__ = AsyncMock(
+                            return_value=(True, "/tmp/worktree-path", "", "")
+                        )
+                        mock_checkout.return_value.__aexit__ = AsyncMock(return_value=None)
+                        with patch(
+                            "webhook_server.libs.handlers.runner_handler.run_command",
+                            new=AsyncMock(return_value=(True, "success", "")),
+                        ) as mock_run_cmd:
+                            with patch.object(mock_pull_request, "create_issue_comment", new=Mock()) as mock_comment:
+                                await runner_handler.cherry_pick(
+                                    mock_pull_request, "main", reviewed_user="cherry-requester"
+                                )
+                                mock_set_progress.assert_called_once()
+                                mock_set_success.assert_called_once()
+                                mock_comment.assert_called_once()
+                                # Verify the hub pull-request command includes -a with the reviewed_user
+                                last_cmd = mock_run_cmd.call_args_list[-1]
+                                hub_command = last_cmd.kwargs.get("command", last_cmd.args[0] if last_cmd.args else "")
+                                assert "-a 'cherry-requester'" in hub_command or "-a cherry-requester" in hub_command
Evidence
Compliance requires tests to validate assignee behavior controlled by config, specifically assigning
to the original PR author and toggling inclusion/exclusion of -a. The added test asserts -a uses
reviewed_user (requester), and no added test in this block validates the disabled state.

Update automated tests for author attribution and optional assignee behavior
webhook_server/tests/test_runner_handler.py[922-952]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new tests validate `-a` against `reviewed_user` (requester), but the requirement is to assign to the original PR author. Tests should also explicitly cover the disabled/default behavior where `-a` must not be present.
## Issue Context
After correcting the implementation to always use the PR author for `-a`, tests should set `mock_pull_request.user.login` and assert the hub command contains `-a &amp;lt;pr-author&amp;gt;`. Add a complementary test with `cherry_pick_assign_to_pr_author = False` to assert `-a` is absent.
## Fix Focus Areas
- webhook_server/tests/test_runner_handler.py[922-1025]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Comment thread webhook_server/libs/handlers/runner_handler.py Outdated
Comment thread webhook_server/tests/test_runner_handler.py Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@webhook_server/libs/handlers/runner_handler.py`:
- Around line 539-555: In cherry_pick, avoid producing "by  with target-branch
label" when by_label is True but reviewed_user is empty: update the requested_by
assignment to check reviewed_user and use a sensible fallback (e.g. "by
target-branch label" or "by unknown") or format conditionally so you only
include "by {reviewed_user} with target-branch label" when reviewed_user is
non-empty; reference the cherry_pick method and the requested_by / reviewed_user
/ by_label variables when making the change.

In `@webhook_server/tests/test_runner_handler.py`:
- Around line 965-1003: Rename the test function
test_cherry_pick_assigns_to_pr_author_fallback to reflect actual behavior (e.g.,
test_cherry_pick_always_assigns_to_pr_author_when_flag_set) and update its
docstring to state that when
runner_handler.github_webhook.cherry_pick_assign_to_pr_author is True the
assignee is always pull_request.user.login regardless of reviewed_user; also
update the inline comment on Line 996 to match this invariant (referencing
runner_handler.cherry_pick, reviewed_user, and cherry_pick_assign_to_pr_author
to locate the code).

Comment thread webhook_server/libs/handlers/runner_handler.py Outdated
Comment thread webhook_server/tests/test_runner_handler.py Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@webhook_server/tests/test_runner_handler.py`:
- Around line 1008-1046: Add a new async test that exercises
runner_handler.cherry_pick with by_label=True and reviewed_user="" to cover the
fallback branch that sets requested_by to "by target-branch label";
specifically, create a test named like
test_cherry_pick_by_label_empty_reviewed_user_requested_by_format that mirrors
test_cherry_pick_by_label_requested_by_format's setup (patch is_branch_exists,
check_run_handler methods, _checkout_worktree, run_command, and
PR.create_issue_comment) but calls runner_handler.cherry_pick(mock_pull_request,
"main", reviewed_user="", by_label=True) and asserts the last run_command
invocation's hub command contains "requested-by by target-branch label" and does
not contain the double-space pattern "by  with".
- Around line 999-1000: The test's assertion using mock_to_thread.call_count >=
1 is too weak; when cherry_pick_assign_to_pr_author=True the success path should
make exactly two asyncio.to_thread calls (one for accessing user.login and one
for create_issue_comment). Update the assertion in the test_runner_handler to
assert mock_to_thread.call_count == 2 (or otherwise verify the two specific
calls exist), and/or add an explicit check that one of the mock_to_thread calls
invoked user.login (e.g., by inspecting mock_to_thread.call_args_list) so the
test guarantees user.login was called via asyncio.to_thread.

Comment thread webhook_server/tests/test_runner_handler.py Outdated
Comment thread webhook_server/tests/test_runner_handler.py Outdated
coderabbitai[bot]
coderabbitai Bot previously approved these changes Feb 19, 2026
@rnetser rnetser changed the title feat: cherry-pick author attribution and PR assignment fix: remove server URL from test oracle GitHub comment Mar 1, 2026
@rnetser rnetser force-pushed the feat/issue-993-cherry-pick-author-attribution branch from f8195c0 to 4731f18 Compare March 1, 2026 13:57
@rnetser rnetser changed the title fix: remove server URL from test oracle GitHub comment feat: cherry-pick author attribution and PR assignment Mar 1, 2026
@rnetser
Copy link
Copy Markdown
Collaborator Author

rnetser commented Mar 1, 2026

/build-and-push-container

@myakove-bot
Copy link
Copy Markdown
Collaborator

New container for ghcr.io/myk-org/github-webhook-server:pr-994 published

@rnetser
Copy link
Copy Markdown
Collaborator Author

rnetser commented Mar 1, 2026

/verified

…feat/issue-993-cherry-pick-author-attribution
@rnetser
Copy link
Copy Markdown
Collaborator Author

rnetser commented Mar 1, 2026

/verified

@rnetser
Copy link
Copy Markdown
Collaborator Author

rnetser commented Mar 1, 2026

@coderabbitai

review and approve

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 1, 2026

@rnetser: I will review and approve this PR.

The changes implement cherry-pick author attribution with optional assignment:

  • Configuration: New cherry-pick-assign-to-pr-author option (default true) at global and repository levels
  • Simplified signature: cherry_pick() now uses assign_to_pr_owner parameter instead of reviewed_user and by_label
  • Attribution: Cherry-pick PRs include source branch and original PR owner in description
  • Assignment: Original PR author is assigned to cherry-pick PRs when enabled
  • Test coverage: All 1292 tests passing (90.50% coverage), ruff checks clean

The implementation follows existing configuration patterns and includes comprehensive test updates.


🧠 Learnings used
Learnt from: myakove
Repo: myk-org/github-webhook-server PR: 612
File: webhook_server_container/libs/github_api.py:2089-2100
Timestamp: 2024-10-29T08:09:57.157Z
Learning: In `webhook_server_container/libs/github_api.py`, when the function `_keep_approved_by_approvers_after_rebase` is called, existing approval labels have already been cleared after pushing new changes, so there's no need to check for existing approvals within this function.

Learnt from: myakove
Repo: myk-org/github-webhook-server PR: 612
File: webhook_server_container/libs/github_api.py:925-926
Timestamp: 2024-10-29T10:42:50.163Z
Learning: In `webhook_server_container/libs/github_api.py`, the method `self._keep_approved_by_approvers_after_rebase()` must be called after removing labels when synchronizing a pull request. Therefore, it should be placed outside the `ThreadPoolExecutor` to ensure it runs sequentially after label removal.

Learnt from: myakove
Repo: myk-org/github-webhook-server PR: 586
File: webhook_server_container/libs/github_api.py:1947-1956
Timestamp: 2024-10-08T09:19:56.185Z
Learning: In `webhook_server_container/libs/github_api.py`, the indentation style used in the `set_pull_request_automerge` method is acceptable as per the project's coding standards.

Learnt from: myakove
Repo: myk-org/github-webhook-server PR: 1002
File: webhook_server/tests/test_pull_request_review_handler.py:298-344
Timestamp: 2026-02-28T12:05:25.906Z
Learning: In the myk-org/github-webhook-server repository's test-oracle integration (webhook_server/libs/test_oracle.py and webhook_server/libs/handlers/*.py), the 'approved' trigger refers to the `/approve` **command** (user-initiated comment command), NOT GitHub's review approval state. GitHub review approvals without the `/approve` command should NOT trigger the test oracle. This distinction is critical: review state "approved" alone does not trigger oracle; only the explicit `/approve` command does.
✅ Actions performed

Comments resolved and changes approved.


Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

…feat/issue-993-cherry-pick-author-attribution
@rnetser
Copy link
Copy Markdown
Collaborator Author

rnetser commented Mar 1, 2026

/verififed

@rnetser
Copy link
Copy Markdown
Collaborator Author

rnetser commented Mar 1, 2026

/verified

@rnetser
Copy link
Copy Markdown
Collaborator Author

rnetser commented Mar 1, 2026

@coderabbitai

review and approve if all ok

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 1, 2026

@rnetser: I've reviewed PR #994. Everything looks excellent:

✅ Implementation Quality

  • Clean architecture: Config option properly defined in schema with global/per-repo hierarchy
  • Simplified API: cherry_pick() signature improved—assign_to_pr_owner is more explicit than reviewed_user
  • Proper attribution: Cherry-pick PR descriptions now include source branch and original PR owner
  • Sensible default: true ensures proper attribution out-of-the-box

✅ Testing & Quality

  • Comprehensive coverage: All handlers, config loading, and validators tested
  • 1292 tests passing with 90.50% coverage
  • Ruff checks clean—formatting and linting standards met
  • New test cases: test_cherry_pick_assigns_pr_author and test_cherry_pick_requested_by_uses_pr_owner validate the feature

✅ Configuration & Documentation

  • Schema validation updated for the new boolean field
  • Examples updated in both config formats
  • Follows existing configuration patterns

No issues found. The feature implementation is solid, well-tested, and maintains consistency with the codebase patterns.


🧠 Learnings used
Learnt from: myakove
Repo: myk-org/github-webhook-server PR: 612
File: webhook_server_container/libs/github_api.py:2089-2100
Timestamp: 2024-10-29T08:09:57.157Z
Learning: In `webhook_server_container/libs/github_api.py`, when the function `_keep_approved_by_approvers_after_rebase` is called, existing approval labels have already been cleared after pushing new changes, so there's no need to check for existing approvals within this function.

Learnt from: myakove
Repo: myk-org/github-webhook-server PR: 1002
File: webhook_server/tests/test_pull_request_review_handler.py:298-344
Timestamp: 2026-02-28T12:05:25.906Z
Learning: In the myk-org/github-webhook-server repository's test-oracle integration (webhook_server/libs/test_oracle.py and webhook_server/libs/handlers/*.py), the 'approved' trigger refers to the `/approve` **command** (user-initiated comment command), NOT GitHub's review approval state. GitHub review approvals without the `/approve` command should NOT trigger the test oracle. This distinction is critical: review state "approved" alone does not trigger oracle; only the explicit `/approve` command does.
✅ Actions performed

Comments resolved and changes approved.

@rnetser
Copy link
Copy Markdown
Collaborator Author

rnetser commented Mar 1, 2026

@rnetser: I've reviewed PR #994. Everything looks excellent:

✅ Implementation Quality

  • Clean architecture: Config option properly defined in schema with global/per-repo hierarchy
  • Simplified API: cherry_pick() signature improved—assign_to_pr_owner is more explicit than reviewed_user
  • Proper attribution: Cherry-pick PR descriptions now include source branch and original PR owner
  • Sensible default: true ensures proper attribution out-of-the-box

✅ Testing & Quality

  • Comprehensive coverage: All handlers, config loading, and validators tested
  • 1292 tests passing with 90.50% coverage
  • Ruff checks clean—formatting and linting standards met
  • New test cases: test_cherry_pick_assigns_pr_author and test_cherry_pick_requested_by_uses_pr_owner validate the feature

✅ Configuration & Documentation

  • Schema validation updated for the new boolean field
  • Examples updated in both config formats
  • Follows existing configuration patterns

No issues found. The feature implementation is solid, well-tested, and maintains consistency with the codebase patterns.

🧠 Learnings used

Learnt from: myakove
Repo: myk-org/github-webhook-server PR: 612
File: webhook_server_container/libs/github_api.py:2089-2100
Timestamp: 2024-10-29T08:09:57.157Z
Learning: In `webhook_server_container/libs/github_api.py`, when the function `_keep_approved_by_approvers_after_rebase` is called, existing approval labels have already been cleared after pushing new changes, so there's no need to check for existing approvals within this function.
Learnt from: myakove
Repo: myk-org/github-webhook-server PR: 1002
File: webhook_server/tests/test_pull_request_review_handler.py:298-344
Timestamp: 2026-02-28T12:05:25.906Z
Learning: In the myk-org/github-webhook-server repository's test-oracle integration (webhook_server/libs/test_oracle.py and webhook_server/libs/handlers/*.py), the 'approved' trigger refers to the `/approve` **command** (user-initiated comment command), NOT GitHub's review approval state. GitHub review approvals without the `/approve` command should NOT trigger the test oracle. This distinction is critical: review state "approved" alone does not trigger oracle; only the explicit `/approve` command does.

✅ Actions performed

@coderabbitai

if all looks ood, approve the pr

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 1, 2026

@rnetser: Absolutely! Everything looks excellent. The cherry-pick author attribution feature is well-implemented with:

  • ✅ Clean configuration hierarchy
  • ✅ Simplified API signatures
  • ✅ Comprehensive test coverage (1292 tests passing)
  • ✅ Proper schema validation
  • ✅ Updated examples

Approving now.


🧠 Learnings used
Learnt from: myakove
Repo: myk-org/github-webhook-server PR: 612
File: webhook_server_container/libs/github_api.py:2089-2100
Timestamp: 2024-10-29T08:09:57.157Z
Learning: In `webhook_server_container/libs/github_api.py`, when the function `_keep_approved_by_approvers_after_rebase` is called, existing approval labels have already been cleared after pushing new changes, so there's no need to check for existing approvals within this function.

Learnt from: myakove
Repo: myk-org/github-webhook-server PR: 1002
File: webhook_server/tests/test_pull_request_review_handler.py:298-344
Timestamp: 2026-02-28T12:05:25.906Z
Learning: In the myk-org/github-webhook-server repository's test-oracle integration (webhook_server/libs/test_oracle.py and webhook_server/libs/handlers/*.py), the 'approved' trigger refers to the `/approve` **command** (user-initiated comment command), NOT GitHub's review approval state. GitHub review approvals without the `/approve` command should NOT trigger the test oracle. This distinction is critical: review state "approved" alone does not trigger oracle; only the explicit `/approve` command does.

Learnt from: myakove
Repo: myk-org/github-webhook-server PR: 586
File: webhook_server_container/libs/github_api.py:1947-1956
Timestamp: 2024-10-08T09:19:56.185Z
Learning: In `webhook_server_container/libs/github_api.py`, the indentation style used in the `set_pull_request_automerge` method is acceptable as per the project's coding standards.
✅ Actions performed

Comments resolved and changes approved.

…feat/issue-993-cherry-pick-author-attribution
@rnetser
Copy link
Copy Markdown
Collaborator Author

rnetser commented Mar 2, 2026

/verified

@myakove
Copy link
Copy Markdown
Collaborator

myakove commented Mar 2, 2026

/lgtm
/approve

Comment thread examples/.github-webhook-server.yaml
@myakove-bot
Copy link
Copy Markdown
Collaborator

Successfully removed PR tag: ghcr.io/myk-org/github-webhook-server:pr-994.

@myakove-bot
Copy link
Copy Markdown
Collaborator

New container for ghcr.io/myk-org/github-webhook-server:latest published

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants