-
Notifications
You must be signed in to change notification settings - Fork 198
Add /git:fix-cherrypick-robot-pr command for fixing cherrypick-robot PRs #180
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add /git:fix-cherrypick-robot-pr command for fixing cherrypick-robot PRs #180
Conversation
This command helps fix cherrypick-robot PRs that need manual intervention. When the robot creates a PR but fails due to verification errors, merge conflicts, or other issues it cannot handle, this command automates the process of creating a replacement PR with all necessary fixes. Features: - Extracts PR metadata (base branch, bug ID, commits) from robot PR - Analyzes error messages (user-provided or auto-detected from CI) - Creates a clean branch and cherry-picks all commits - Applies fixes for common issues: - JSON validation errors - Missing test annotations - Merge conflicts - Verification script failures - Creates replacement PR with proper formatting - Closes old robot PR with explanatory comment 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
WalkthroughThis PR adds a new Git plugin command for fixing cherry-pick robot PRs that require manual intervention. The changes include command registration in metadata files and comprehensive documentation describing the command's workflow, arguments, and usage examples. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 error, 1 inconclusive)
✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
Hi @wangke19. Thanks for your PR. I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Here is test results in local: |
- Replace hardcoded 'origin' and 'upstream' with dynamic discovery - Use 'git remote -v' and 'git config user.name' to identify remotes - Add fallback to common names if auto-discovery fails - Update Implementation section 3 and 6 with remote discovery logic - Update Notes section to document the remote discovery approach This addresses the code review feedback to avoid assuming git remote names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
plugins/git/commands/fix-robot-pr.md (1)
10-12: Add language specifications to all fenced code blocks for proper syntax highlighting.Per markdownlint (MD040), all fenced code blocks should declare a language. This ensures consistent rendering and syntax highlighting in documentation viewers.
Apply this diff to add language specifications:
## Synopsis -``` +```bash /git:fix-robot-pr <pr-url> [error-messages] -``` +``` ... ### 1. Extract Information from the Robot PR -Example: -```bash +Example: +```bash gh pr view https://github.com/openshift/origin/pull/30524 --json baseRefName,title,commits,number,statusCheckRollup -``` +``` ... ### 3. Discover Git Remotes and Create Branch -```bash +```bash # Discover the upstream remote (the main repository) ... git checkout -b cherry-pick-<issue-number>-to-<base-branch> $UPSTREAM_REMOTE/<base-branch> -``` +``` ... ### 4. Cherry-Pick Commits -```bash +```bash # For each commit hash extracted from the robot PR ... /git:cherry-pick-by-patch <commit-hash> -``` +``` ... ### 5. Apply Necessary Fixes Based on Errors -**For JSON validation errors:** -```bash +**For JSON validation errors:** +```bash # Add files to exclusion list in hack/verify-jsonformat.sh # Edit the excluded_files array to include the problematic JSON file -``` +``` -**For missing test annotations:** -```bash +**For missing test annotations:** +```bash # Regenerate annotations ... git commit -m "Update generated test annotations" -``` +``` ### 6. Push and Create Replacement PR -```bash +```bash # Use the discovered fork remote (from step 3) ... )" -``` +``` ### Example 1: With Error Messages Pasted Directly -``` +```text /git:fix-robot-pr https://github.com/openshift/origin/pull/30524 ... + "[sig-api-machinery][Feature:APIServer] TestTLSMinimumVersions": " [Suite:openshift/conformance/parallel]", -``` +``` ### Example 2: With Error Log File Reference -``` +```text /git:fix-robot-pr https://github.com/openshift/origin/pull/30524 ... Error log file: /path/to/ci-errors.log -``` +``` ### Example 3: With CI Failure Page Link -``` +```text /git:fix-robot-pr https://github.com/openshift/origin/pull/30524 ... CI failure: https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/30524/... -``` +``` ### Example 4: No Error Messages (Auto-detect) -``` +```text /git:fix-robot-pr https://github.com/openshift/origin/pull/30524 -``` +```Also applies to: 39-41, 65-81, 89-95, 102-105, 108-113, 128-162, 187-199, 222-226, 232-237, 242-244
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
plugins/git/commands/fix-robot-pr.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
plugins/git/commands/fix-robot-pr.md
10-10: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
187-187: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
222-222: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
232-232: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
242-242: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (2)
plugins/git/commands/fix-robot-pr.md (2)
157-160: Verify co-authorship attribution is appropriate for the project.The PR body template embeds "Claude Code" and "Co-Authored-By: Claude" attribution. Confirm whether this aligns with project conventions for auto-generated content. Some OSS projects may prefer different attribution or removal of tooling references.
If this attribution should be removed or modified, apply this diff:
## References - Original PR: #<robot-pr-number> - JIRA: <bug-id> -🤖 Generated with [Claude Code](https://claude.com/claude-code) - -Co-Authored-By: Claude <[email protected]> +Co-Authored-By: <your-github-username>
1-281: Comprehensive documentation for a well-structured manual workflow.The documentation is thorough and well-organized. The implementation steps are clear and actionable, the examples cover multiple input scenarios (direct errors, file paths, CI URLs, auto-detection), and error-handling guidance is sound. The frontmatter and structure follow the required format for plugin command documentation.
✅ Verification: No Hardcoded Git Remote NamesAll git operations in Git Commands Using Remotes (3 total):
Remote Discovery Implementation:Section 3 (Lines 66-74): Initial discovery UPSTREAM_REMOTE=$(git remote -v | grep "openshift.*fetch" | grep -v "$(git config user.name)" | awk '{print $1}' | head -1)
FORK_REMOTE=$(git remote -v | grep "$(git config user.name).*push" | awk '{print $1}' | head -1)
UPSTREAM_REMOTE=${UPSTREAM_REMOTE:-upstream} # Fallback only
FORK_REMOTE=${FORK_REMOTE:-origin} # Fallback onlySection 6 (Lines 131-132): Rediscovery for standalone execution FORK_REMOTE=$(git remote -v | grep "$(git config user.name).*push" | awk '{print $1}' | head -1)
FORK_REMOTE=${FORK_REMOTE:-origin} # Fallback onlyVerification:
Result: All git operations correctly use discovered remotes. Pre-merge check should pass. |
|
/test lint |
|
@wangke19: Cannot trigger testing until a trusted user reviews the PR and leaves an DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/cc: @dgoodwin |
|
/assign |
|
/ok-to-test |
|
@bryan-cox @theobarberbany Could you help to review this PR? |
|
/uncc @dgoodwin |
docs/data.json
Outdated
| { | ||
| "argument_hint": "<pr-url> [error-messages]", | ||
| "description": "Fix a cherrypick-robot PR that needs manual intervention", | ||
| "name": "fix-robot-pr", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we rename this to be more specific on what this command is doing? There are many robot type PRs - dependabot, konflux, CI backport, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good suggestion!
Updated.
plugins/git/commands/fix-robot-pr.md
Outdated
| 3. CI failure URL if provided | ||
| 4. Automatically fetch from PR status checks | ||
|
|
||
| Common error patterns: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this section is helpful 🤔
For example, in HyperShift, many of these shell scripts are actually Makefile commands. I assume this sort of thing could vary wildly within OpenShift.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed specific.
plugins/git/commands/fix-robot-pr.md
Outdated
| **For missing test annotations:** | ||
| ```bash | ||
| # Regenerate annotations | ||
| hack/update-generated.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems repo specific.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed specific.
- Rename command to be more specific about cherrypick-robot PRs (vs other robot types like dependabot, konflux, CI backport) - Remove repo-specific error patterns section that assumed OpenShift/origin structure (hack/verify-*.sh scripts) - Generalize fix implementation to work across different repositories - Update examples to use generic repo URLs instead of openshift/origin - Add guidance to consult repository documentation for specific commands - Update PLUGINS.md and docs/data.json with new command name Addresses feedback from PR review about command specificity and repository assumptions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugins/git/commands/fix-cherrypick-robot-pr.md (1)
137-161: Customize PR body template for your use case.The example PR body (lines 137-161) includes hardcoded references to "Claude Code" and "Claude" co-author attribution. While useful as a template example, users should customize these lines to match their team's conventions and attribution practices before using the generated PR body in production.
Consider adding a note after the PR body template example clarifying that users should customize the footer lines to remove or adjust the Claude references as appropriate for their repository.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (3)
PLUGINS.md(1 hunks)docs/data.json(1 hunks)plugins/git/commands/fix-cherrypick-robot-pr.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- PLUGINS.md
- docs/data.json
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
plugins/git/commands/fix-cherrypick-robot-pr.md
10-10: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
186-186: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
203-203: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
213-213: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
223-223: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (1)
plugins/git/commands/fix-cherrypick-robot-pr.md (1)
1-265: Comprehensive and well-structured documentation for the cherry-pick robot PR fix command.The documentation effectively guides users through a 7-step workflow to replace failed cherrypick-robot PRs with manually crafted alternatives. The design choices align well with PR objectives: repo-agnostic operation, dynamic remote discovery with fallbacks, strategy-based guidance over hardcoded fixes, and flexible error input handling (pasted errors, log files, CI URLs, or auto-detection). The Examples section effectively demonstrates each input scenario, and the Notes section clearly explains fallback behavior and limitations.
| git:fix-cherrypick-robot-pr | ||
|
|
||
| ## Synopsis | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add language specifiers to fenced code blocks for proper rendering.
The following code blocks are missing language specifiers (lines 10, 186, 203, 213, 223), which triggers markdown linting warnings (MD040). Add appropriate language tags (e.g., bash, shell, or text) to improve rendering and pass linting.
Apply this diff to fix the code block language specifiers:
## Synopsis+bash
/git:fix-cherrypick-robot-pr [error-messages]
Then apply the same fix pattern to lines 186, 203, 213, and 223 by replacing the opening ``` with ```bash or another appropriate language identifier.
Also applies to: 186-186, 203-203, 213-213, 223-223
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
10-10: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In plugins/git/commands/fix-cherrypick-robot-pr.md around lines 10, 186, 203,
213, and 223, several fenced code blocks are missing language specifiers causing
MD040 lint warnings; update each opening triple-backtick to include an
appropriate language tag (e.g., replace ``` with ```bash or ```shell or ```text
as fits the block) so the markdown code fences render correctly and satisfy the
linter.
|
/approve |
|
Test it in local:
● I'll help you fix this cherrypick-robot PR. Let me start by extracting information from the PR and analyzing the error. ● Bash(gh pr view openshift/origin#30524 --json baseRefName,title,commits,number,statusCheckRollup) Todos ────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── gh pr view openshift/origin#30524 --json baseRefName,title,commits,number,statusCheckRollup Do you want to proceed? |
|
@rvanderp3 can I get a |
|
/lgtm |
|
@gangwgr: changing LGTM is restricted to collaborators DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/lgtm |
|
@rvanderp3: changing LGTM is restricted to collaborators DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/lgtm |
|
/test lint |
|
@wangke19: The specified target(s) for Use DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/test all |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bryan-cox, gangwgr, LiangquanLi930, rvanderp3, wangke19, xingxingxia The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/assign @bentito |
Summary
This PR adds a new
/git:fix-cherrypick-robot-prcommand to the git plugin that automates the process of fixing cherrypick-robot PRs that need manual intervention.Problem
The cherrypick-robot creates automated PRs but cannot:
Previously, developers had to manually perform all these steps, which is time-consuming and error-prone.
Solution
The new
/git:fix-cherrypick-robot-prcommand automates the entire workflow:cherry-pick-<issue>-to-<release>Usage
Key Features
Changes
plugins/git/commands/fix-cherrypick-robot-pr.mdDesign Decisions
Based on PR feedback:
fix-robot-prtofix-cherrypick-robot-prto clearly distinguish from other robot types (dependabot, konflux, CI backport, etc.)Testing
Summary by CodeRabbit
/git:fix-cherrypick-robot-prcommand for the Git plugin, enabling users to manually fix cherry-pick robot PRs that encounter errors. The command automates error analysis, cherry-picks commits, applies necessary fixes, creates a replacement PR, and closes the original with a reference. Supports multiple error detection methods.✏️ Tip: You can customize this high-level summary in your review settings.