-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Post changeset review issues as inline file comments #13026
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| --- | ||
| "wrangler": minor | ||
| --- | ||
|
|
||
| # fix bug | ||
|
|
||
| update stuff | ||
|
Comment on lines
+1
to
+7
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Test changeset files would trigger unintended wrangler releases if merged Both Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| --- | ||
| "wrangler": patch | ||
| --- | ||
|
|
||
| Fix `wrangler dev` failing when configuration file contains trailing commas | ||
|
|
||
| Previously, `wrangler dev` would crash with a JSON parse error if `wrangler.json` contained trailing commas in arrays or objects. Since JSONC supports trailing commas, this is now handled correctly by using a JSONC-aware parser. |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -44,6 +44,38 @@ jobs: | |||||||||||||||||
| # Recover deleted files so Claude can read them (needed for Version Packages PRs) | ||||||||||||||||||
| recover_deleted_files: ${{ github.event.pull_request.title == 'Version Packages' }} | ||||||||||||||||||
|
|
||||||||||||||||||
| - name: Clean up prior changeset reviews | ||||||||||||||||||
| if: github.event.pull_request.title == 'Version Packages' || steps.changed-changesets.outputs.added_files_count > 0 | ||||||||||||||||||
| env: | ||||||||||||||||||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||||||||||||||||||
| PR_NUMBER: ${{ github.event.pull_request.number }} | ||||||||||||||||||
| REPO: ${{ github.repository }} | ||||||||||||||||||
| run: | | ||||||||||||||||||
| COMMENTS=$(gh api "repos/${REPO}/issues/${PR_NUMBER}/comments" --paginate | jq -s 'add') | ||||||||||||||||||
|
|
||||||||||||||||||
| # Delete sticky OK comments (identified by embedded HTML marker) | ||||||||||||||||||
| echo "$COMMENTS" | jq -r '.[] | select(.body | contains("<!-- changeset-review -->")) | .id' | while read -r COMMENT_ID; do | ||||||||||||||||||
| echo "Deleting sticky OK comment ${COMMENT_ID}" | ||||||||||||||||||
| gh api "repos/${REPO}/issues/comments/${COMMENT_ID}" -X DELETE || true | ||||||||||||||||||
| done | ||||||||||||||||||
|
|
||||||||||||||||||
| # Find marker comments from prior issue-review runs, delete the associated review comments and the marker | ||||||||||||||||||
| echo "$COMMENTS" | jq -c '.[] | select(.body | contains("<!-- changeset-review-marker"))' | while read -r ITEM; do | ||||||||||||||||||
| COMMENT_ID=$(echo "$ITEM" | jq -r '.id') | ||||||||||||||||||
| BODY=$(echo "$ITEM" | jq -r '.body') | ||||||||||||||||||
| REVIEW_ID=$(echo "$BODY" | grep -oP '(?<=review-id:)\d+') | ||||||||||||||||||
| if [ -n "$REVIEW_ID" ]; then | ||||||||||||||||||
| echo "Deleting comments on prior review ${REVIEW_ID}" | ||||||||||||||||||
| # Submitted reviews cannot be deleted via the API; delete each review comment individually instead | ||||||||||||||||||
| gh api "repos/${REPO}/pulls/${PR_NUMBER}/reviews/${REVIEW_ID}/comments" \ | ||||||||||||||||||
| | jq -r '.[].id' | while read -r RC_ID; do | ||||||||||||||||||
| gh api "repos/${REPO}/pulls/comments/${RC_ID}" -X DELETE || true | ||||||||||||||||||
| done | ||||||||||||||||||
|
Comment on lines
+70
to
+73
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Missing error handling on review-comments listing API call can permanently block the workflow At Problematic codegh api "repos/${REPO}/pulls/${PR_NUMBER}/reviews/${REVIEW_ID}/comments" \
| jq -r '.[].id' | while read -r RC_ID; do
gh api "repos/${REPO}/pulls/comments/${RC_ID}" -X DELETE || true
done
Suggested change
Was this helpful? React with 👍 or 👎 to provide feedback. |
||||||||||||||||||
| fi | ||||||||||||||||||
| echo "Deleting marker comment ${COMMENT_ID}" | ||||||||||||||||||
| gh api "repos/${REPO}/issues/comments/${COMMENT_ID}" -X DELETE || true | ||||||||||||||||||
| done | ||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Submitted reviews cannot be deleted via the REST API. The GitHub API To actually remove old reviews, you have two options:
Option 2 is probably what you want since the goal is to remove stale resolvable threads.
Comment on lines
+47
to
+77
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Cleanup step deletes old review comments before new review is generated, causing review loss on OpenCode failure The "Clean up prior changeset reviews" step (Step 3) runs before the "Review Changesets with OpenCode" step (Step 4). If OpenCode fails (e.g. API timeout, bad response, rate limit), Step 4 fails, the job aborts, and "Post review results" (Step 5) never runs. At this point, all prior review comments and marker comments have already been deleted by Step 3, leaving the PR with no changeset review feedback at all. The cleanup should occur only after the new review JSON is successfully generated, or at least only after verifying the new review was posted. Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback. |
||||||||||||||||||
|
|
||||||||||||||||||
| - name: Review Changesets with OpenCode | ||||||||||||||||||
| id: opencode-review | ||||||||||||||||||
| # Run for Version Packages PRs (which delete changesets) or regular PRs with new changesets | ||||||||||||||||||
|
|
@@ -69,20 +101,66 @@ jobs: | |||||||||||||||||
| 5. **Dependabot**: Do not validate dependency update changesets for create-cloudflare | ||||||||||||||||||
| 6. **Experimental features**: Changesets for experimental features should include note on how users can opt in. | ||||||||||||||||||
|
|
||||||||||||||||||
| If all changesets pass, just output \"✅ All changesets look good\" - no need for a detailed checklist. | ||||||||||||||||||
| Write your review as JSON to \`changeset-review.json\` in this exact format (no other output): | ||||||||||||||||||
| { | ||||||||||||||||||
| \"status\": \"ok\", | ||||||||||||||||||
| \"summary\": \"✅ All changesets look good.\", | ||||||||||||||||||
| \"files\": [] | ||||||||||||||||||
| } | ||||||||||||||||||
| or when there are issues: | ||||||||||||||||||
| { | ||||||||||||||||||
| \"status\": \"issues\", | ||||||||||||||||||
| \"summary\": \"⚠️ Issues found in N of M changesets.\", | ||||||||||||||||||
| \"files\": [ | ||||||||||||||||||
| { | ||||||||||||||||||
| \"path\": \".changeset/foo.md\", | ||||||||||||||||||
| \"status\": \"issues\", | ||||||||||||||||||
| \"comment\": \"<specific problems with this file>\" | ||||||||||||||||||
| } | ||||||||||||||||||
| ] | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| Do not review other files, only the changesets. This is specifically a changeset review action. | ||||||||||||||||||
| Only include files in the array when they have status \"issues\". | ||||||||||||||||||
| The top-level status must be \"issues\" if ANY file has issues, otherwise \"ok\". | ||||||||||||||||||
| Do not review other files, only the changesets. This is specifically a changeset review action." | ||||||||||||||||||
|
|
||||||||||||||||||
| If there are issues, output \"⚠️ Issues found\" followed by the specific problems. | ||||||||||||||||||
| - name: Post review results | ||||||||||||||||||
| if: steps.opencode-review.outcome == 'success' | ||||||||||||||||||
| env: | ||||||||||||||||||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||||||||||||||||||
| PR_NUMBER: ${{ github.event.pull_request.number }} | ||||||||||||||||||
| REPO: ${{ github.repository }} | ||||||||||||||||||
| HEAD_SHA: ${{ github.event.pull_request.head.sha }} | ||||||||||||||||||
| run: | | ||||||||||||||||||
| STATUS=$(jq -r '.status' changeset-review.json) | ||||||||||||||||||
| SUMMARY=$(jq -r '.summary' changeset-review.json) | ||||||||||||||||||
|
|
||||||||||||||||||
| Write your review to changeset-review.md." | ||||||||||||||||||
| if [ "$STATUS" = "issues" ]; then | ||||||||||||||||||
| # Build the request body: file-level comments (subject_type: "file") for each problematic changeset | ||||||||||||||||||
| REQUEST_BODY=$(jq -n \ | ||||||||||||||||||
| --arg commit_id "$HEAD_SHA" \ | ||||||||||||||||||
| --argjson comments "$(jq '[.files[] | select(.status == "issues") | {path: .path, subject_type: "file", body: .comment}]' changeset-review.json)" \ | ||||||||||||||||||
| '{commit_id: $commit_id, event: "COMMENT", body: "", comments: $comments}') | ||||||||||||||||||
|
|
||||||||||||||||||
| - name: Post review comment | ||||||||||||||||||
| if: steps.opencode-review.outcome == 'success' | ||||||||||||||||||
| uses: marocchino/sticky-pull-request-comment@773744901bac0e8cbb5a0dc842800d45e9b2b405 # v2 | ||||||||||||||||||
| with: | ||||||||||||||||||
| header: changeset-review | ||||||||||||||||||
| path: changeset-review.md | ||||||||||||||||||
| # Post a pull request review with inline file-level comments | ||||||||||||||||||
| REVIEW_RESPONSE=$(echo "$REQUEST_BODY" | gh api "repos/${REPO}/pulls/${PR_NUMBER}/reviews" \ | ||||||||||||||||||
| -X POST \ | ||||||||||||||||||
| --input -) | ||||||||||||||||||
|
|
||||||||||||||||||
| REVIEW_ID=$(echo "$REVIEW_RESPONSE" | jq -r '.id') | ||||||||||||||||||
| echo "Posted review ${REVIEW_ID}" | ||||||||||||||||||
|
|
||||||||||||||||||
| # Post a hidden marker comment so the next run can find and delete this review | ||||||||||||||||||
| gh api "repos/${REPO}/issues/${PR_NUMBER}/comments" \ | ||||||||||||||||||
| -X POST \ | ||||||||||||||||||
| -f "body=<!-- changeset-review-marker review-id:${REVIEW_ID} -->" | ||||||||||||||||||
| else | ||||||||||||||||||
| # Post a normal (non-resolvable) PR comment with a hidden marker for cleanup | ||||||||||||||||||
| gh api "repos/${REPO}/issues/${PR_NUMBER}/comments" \ | ||||||||||||||||||
| -X POST \ | ||||||||||||||||||
| -f "body=<!-- changeset-review --> | ||||||||||||||||||
| ${SUMMARY}" | ||||||||||||||||||
| fi | ||||||||||||||||||
|
|
||||||||||||||||||
| - name: Skip notice | ||||||||||||||||||
| if: github.event.pull_request.title != 'Version Packages' && steps.changed-changesets.outputs.added_files_count == 0 | ||||||||||||||||||
|
|
||||||||||||||||||
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.
🔴 Changeset uses h1 header, violating mandatory changeset formatting rule
.changeset/test-bad-changeset.md:5uses# fix bug— an h1 (#) header — in the changeset description. AGENTS.md explicitly states under Changeset Rules: "No h1/h2/h3 headers in changeset descriptions (changelog uses h3)". This is also documented in.changeset/README.md:84: "Changeset descriptions must NOT use h1 (#), h2 (##), or h3 (###) headers." If merged, this h1 would break the generated changelog formatting since the changelog already uses h3 for section headers.Was this helpful? React with 👍 or 👎 to provide feedback.