fix: restore original author on cherry-pick for DCO compliance#1073
Merged
fix: restore original author on cherry-pick for DCO compliance#1073
Conversation
GitHub squash-merges rewrite the commit author email to the noreply format (e.g., 86722603+user@users.noreply.github.com), but preserve the original email in the Signed-off-by trailer. When git cherry-pick replays such a commit, the email mismatch causes DCO checks to fail. Add _restore_original_author_for_cherry_pick() that fetches the original PR commits, finds the Signed-off-by trailer, and amends the cherry-picked commit author to match. Only amends when the original PR had a sign-off. Called after successful cherry-pick (clean or AI-resolved) before push. Closes: #1072 Assisted-by: Claude <noreply@anthropic.com> Signed-off-by: rnetser <rnetser@redhat.com>
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds methods to locate a PR's Changes
Sequence DiagramsequenceDiagram
participant WH as Webhook Handler
participant RH as RunnerHandler
participant GHA as GitHub API
participant GIT as Git CLI
WH->>RH: cherry_pick(pull_request, target_branch, ...)
RH->>GIT: git cherry-pick <commit>
alt conflict
RH->>RH: resolve conflicts (AI/manual)
RH->>GIT: git cherry-pick --continue
end
RH->>GHA: GET /repos/.../pulls/<orig>/commits
GHA-->>RH: commits[]
RH->>RH: scan commits newest→oldest for Signed-off-by
RH->>GIT: git log -1 --format=%an%n%ae
GIT-->>RH: current_author_name/email
alt sign-off found AND emails differ
RH->>GIT: git log -1 --format=%B
GIT-->>RH: commit_message
RH->>GIT: git commit --amend --author="Name <email>" [-m / -F / --no-edit]
GIT-->>RH: amend result (success/fail)
else no sign-off or emails match
RH->>RH: skip amend
end
RH->>GIT: git push origin <branch>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Collaborator
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Container Operations
Cherry-pick Operations
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
AI Features
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 785-808: The code logs and embeds the full real author identity
when amending commits (see run_command usage building f"{git_cmd} commit --amend
--no-edit --author={shlex.quote(f'{author_name} <{author_email}>')}" and the
subsequent logger.info), which risks leaking personal emails; change this by (1)
adding author_email (and optionally author_name) to the run_command
redact_secrets list so command traces do not contain the raw identity, (2) avoid
emitting the full "Name <email>" in logger.info/logger.warning — instead log a
redacted form (e.g., mask the local-part of the email or only log the domain) or
simply log "Restored original author" without the literal, and (3) optionally
build an author_cmd variable for the commit string and a separate
redacted_author string for logs to keep the commit and logs consistent
(referencing the commit amend call and logger.info in runner_handler.py).
- Around line 760-809: Wrap the external PyGithub calls (the call to
pull_request.get_commits() via github_api_call and the per-commit access to
c.commit.message) in try/except that catches Exception, immediately re-raises
asyncio.CancelledError, and for other exceptions uses self.logger.exception(...)
to log the full traceback and then return False so the cherry-pick flow
continues (best-effort author restore) instead of propagating the error; update
the code around commits = await github_api_call(...) and the loop that reads
commit_msg (referencing pull_request.get_commits(), c.commit.message,
github_api_call, and cherry_pick()/the surrounding method) to implement this
behavior.
In `@webhook_server/tests/test_runner_handler.py`:
- Around line 1038-1041: The test stubs out
_restore_original_author_for_cherry_pick but never asserts it was awaited;
update the patch to assign the AsyncMock to a local variable (e.g., mock_restore
= AsyncMock(return_value=False) and patch.object(runner_handler,
"_restore_original_author_for_cherry_pick", new=mock_restore)), call the code
path (await runner_handler.cherry_pick(...)), then assert the hook was actually
awaited with mock_restore.assert_awaited_once() or
assert_awaited_with(expected_args) to prove cherry_pick awaited the restore
hook.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 035a90c9-e333-45e7-b297-7abf8bec4ecb
📒 Files selected for processing (2)
webhook_server/libs/handlers/runner_handler.pywebhook_server/tests/test_runner_handler.py
…r, assert hook - Wrap PyGithub calls in try/except so author restore is best-effort and does not abort cherry-pick on API failure - Redact author name/email from run_command logs and simplify INFO message - Assert _restore_original_author_for_cherry_pick is awaited in cherry-pick success test Assisted-by: Claude <noreply@anthropic.com> Signed-off-by: rnetser <rnetser@redhat.com>
Collaborator
Author
|
/build-and-push-container |
Collaborator
|
New container for ghcr.io/myk-org/github-webhook-server:pr-1073 published |
Collaborator
Author
|
/verified |
Collaborator
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
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 804-811: The warning log at self.logger.warning uses the raw
stderr returned from run_command (err) which can contain sensitive author info;
before logging, redact/mask secrets (github_token, author_spec, author_email,
author_name) from err using the same redaction/masking logic used by run_command
(or a shared redact utility) and then log the redacted message (e.g., compute
redacted_err = redact(err, [github_token, author_spec, author_email,
author_name]) and use that in the f"{self.log_prefix} Failed to amend..." call);
update the code paths around run_command, self.logger.warning, and any helper
used for redaction to ensure stderr is never logged unredacted.
In `@webhook_server/tests/test_runner_handler.py`:
- Around line 1912-2185: Add two tests for
RunnerHandler._restore_original_author_for_cherry_pick that cover
github_api_call failure paths: (1) patch
"webhook_server.libs.handlers.runner_handler.github_api_call" to raise a regular
Exception when calling pull_request.get_commits (use the existing runner_handler
and _make_pr_with_commits helpers), assert the method returns False and that
runner_handler.github_webhook.logger.exception was called; (2) patch
github_api_call to raise asyncio.CancelledError and assert the CancelledError is
propagated (re-raised). Ensure both tests target the same function name
_restore_original_author_for_cherry_pick and verify behavior per
logger.exception and return False for the non-cancelled error path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1cfb4e4d-7a01-450d-882d-84be721a11d7
📒 Files selected for processing (2)
webhook_server/libs/handlers/runner_handler.pywebhook_server/tests/test_runner_handler.py
- Redact author identity from stderr before logging on amend failure - Add test for github_api_call exception → returns False (best-effort) - Add test for asyncio.CancelledError re-raise Assisted-by: Claude <noreply@anthropic.com> Signed-off-by: rnetser <rnetser@redhat.com>
|
Tip For best results, initiate chat on the files or code changes.
🧠 Learnings usedIf you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
Collaborator
Author
|
/build-and-push-container |
Collaborator
|
New container for ghcr.io/myk-org/github-webhook-server:pr-1073 published |
Collaborator
Author
|
/verified |
Collaborator
|
Is this flow happen on any cherry pick or just on PR with dco? |
…fix/cherry-pick-dco-author-mismatch
Collaborator
|
Clean rebase detected — no code changes compared to previous head ( |
…fix/cherry-pick-dco-author-mismatch
Collaborator
|
Clean rebase detected — no code changes compared to previous head ( |
Collaborator
|
/lgtm |
myakove-bot
reviewed
Apr 26, 2026
Collaborator
|
Successfully removed PR tag: ghcr.io/myk-org/github-webhook-server:pr-1073. |
Collaborator
|
New container for ghcr.io/myk-org/github-webhook-server:latest published |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When the webhook server cherry-picks a squash-merged commit, the DCO check on the cherry-pick PR fails because:
86722603+user@users.noreply.github.com)Signed-off-bytrailer preserves the original email (e.g.,user@redhat.com)git cherry-pickreplays the commit with the mismatched emailsSigned-off-byemail matches the commit author email → failsThis requires manual sign-off approval for every cherry-picked PR.
Solution
Add
_restore_original_author_for_cherry_pick()toRunnerHandlerthat:pull_request.get_commits()Signed-off-bytrailer (last commit first)Testing
TestRestoreOriginalAuthorForCherryPickcovering all pathsReferences
Closes: #1072
Assisted-by: Claude noreply@anthropic.com
Summary by CodeRabbit
Bug Fixes
Tests