-
Notifications
You must be signed in to change notification settings - Fork 316
Description
Summary
The mark_pull_request_as_ready_for_review safe output handler reports success but does not actually convert draft PRs to ready-for-review.
Root Cause
The handler at setup/js/mark_pull_request_as_ready_for_review.cjs uses the REST API pulls.update endpoint with draft: false:
async function markPullRequestAsReadyForReview(github, owner, repo, prNumber) {
const { data: pr } = await github.rest.pulls.update({
owner, repo, pull_number: prNumber, draft: false,
});
return pr;
}The GitHub REST API PATCH /repos/{owner}/{repo}/pulls/{pull_number} does not support setting draft: false to un-draft a PR. The parameter is silently ignored. To convert a draft PR to ready-for-review, the GraphQL mutation markPullRequestAsReadyForReview must be used instead.
Because the REST call does not error (it just ignores the draft field), the handler proceeds to add a comment and logs Marked PR #N as ready for review, giving the appearance of success while the PR remains in draft.
Expected Fix
Replace the REST API call with the GraphQL mutation:
mutation {
markPullRequestAsReadyForReview(input: { pullRequestId: $pullRequestNodeId }) {
pullRequest { number isDraft url title }
}
}This requires fetching the PR's node_id first (already available from the getPullRequestDetails call).
Reproduction
- Create a workflow with
mark-pull-request-as-ready-for-reviewsafe output - Have the agent use it on a draft PR
- The workflow logs show success but the PR remains in draft
Evidence
In a workflow run that uses the mark-pull-request-as-ready-for-review safe output, the safe_outputs job processed all 4 messages (push, mark-ready, remove-labels, add-comment) with zero errors. The log showed:
✓ Marked PR #N as ready for review and added comment: https://github.com/...
After the run completed, the PR was still draft: true. All other safe outputs in the same run (push-to-pull-request-branch, remove-labels, add-comment) executed correctly.
The safe_outputs job in the compiled lock file has pull-requests: write permission, so a GraphQL mutation should have sufficient access.
Affected Component
- File:
setup/js/mark_pull_request_as_ready_for_review.cjs, line ~80 - The handler also does not validate the response to confirm
draftactually changed, so there is no error even when the operation has no effect