Repository navigation
Report the created or updated pull request when a later step fails - #3784
Open
VISHNU0906 wants to merge 1 commit into
Open
VISHNU0906 wants to merge 1 commit into
VISHNU0906 wants to merge 1 commit into
Conversation
create_pull_request and update_pull_request make several API calls in a row. When the reviewer request failed after the pull request had been created, the error had no number or URL, and a retry only reported that a pull request for the head branch already exists. On update, a failure in the draft, reviewer or final read step read as if nothing had changed, although the earlier edits were applied. Keep returning an error, but name the pull request and say it was created or updated when an earlier step already changed it. Fixes github#3756 Signed-off-by: KVR Vishnu <46535652+VISHNU0906@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Client-acquisition failures can still return bare errors after earlier updates succeed, leaving callers unaware of changes already applied.
1 open issue
What changed in this PR
This PR addresses #3756 by making partial-failure errors identify pull requests that were already created or updated, helping callers avoid misleading retries.
Changes:
- Include the created pull request’s number and URL when requesting reviewers fails.
- Track successful update steps and report earlier changes when a later API step fails, while preserving error status.
- Add regression tests for partial failures and unchanged messages when no update succeeded.
| File | Description |
|---|---|
pkg/github/pullrequests.go |
Adds partial-success context to create and update error messages. |
pkg/github/pullrequests_partial_failure_test.go |
Tests partial-failure reporting and errors before any successful update. |
Review effort: Balanced (auto)
Comment on lines
+1091
to
+1095
| stepErrMsg := func(step string) string { | ||
| if updated { | ||
| return fmt.Sprintf("pull request #%d was updated, but %s%s", pullNumber, strings.ToLower(step[:1]), step[1:]) | ||
| } | ||
| return step |
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
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.
Summary
When
create_pull_requestorupdate_pull_requestfails partway, the error now names the pull request and says it was created or updated, instead of reading as if nothing happened.Why
Fixes #3756
Thanks for the detailed report @rafe-walker. If the reviewer request failed after the pull request was created, the error had no number or URL, and a retry only got "A pull request already exists". On update, a failure in the draft, reviewer or final read step came back as a bare error even though the title or other edits had already been applied.
What changed
create_pull_request: a reviewer failure returnspull request #N was created (<url>), but failed to request reviewers: ....update_pull_request: once a step has changed the pull request (REST edit, draft change, reviewer request), any later failure is prefixed withpull request #N was updated, but .... If nothing was applied yet, the message is unchanged.isErrorbehave the same.MCP impact
Schemas are unchanged. Only the error text of these two tools changes, and only on partial failure.
Prompts tested (tool changes only)
Security / limits
Tool renaming
Lint & tests
./script/lint./script/testDocs