From: Daan De Meyer Date: Mon, 16 Mar 2026 12:49:31 +0000 (+0100) Subject: ci: Fix several issues in claude-review workflow X-Git-Tag: v260~10 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3a76c0959fde4430434099f20412ea34445ea566;p=thirdparty%2Fsystemd.git ci: Fix several issues in claude-review workflow Address feedback from facebook/bpfilter#472: - Fix setFailed error message counting file-level comments (without line numbers) that are intentionally skipped, use inlineComments.length instead of comments.length - Fix double severity prefix in inline comments: the prompt told Claude to prefix body with **must-fix**/etc but the post job also prepended "Claude: ", producing "Claude: **must-fix**: ...". Now the prompt says not to prefix and the post job adds "Claude **severity**: " using the structured severity field - Move error tracking instructions to a top-level section after all phases so they apply to all runs, not just the first run - Clarify that line is optional: use "should be" instead of "must be" and document that omitting line still surfaces the comment in the tracking comment summary - Distinguish cancelled vs failed in tracking comment message - Add side: "RIGHT" and subject_type: "line" to createReviewComment per GitHub API recommendations - Downgrade partial inline comment posting failures to warnings; only fail the job when no comments at all could be posted Co-developed-by: Claude Opus 4.6 --- diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml index 160bab85c49..293aef3b6a2 100644 --- a/.github/workflows/claude-review.yml +++ b/.github/workflows/claude-review.yml @@ -209,7 +209,7 @@ jobs: HEAD SHA: ${{ needs.setup.outputs.head_sha }} You are a code reviewer for the ${{ github.repository }} project. Review this pull request and - produce a structured JSON result containing your review comments. Do NOT attempt + produce a structured JSON result containing your review. Do NOT attempt to post comments yourself — just return the JSON. You are in the upstream repo without the patch applied. Do not apply it. @@ -251,12 +251,13 @@ jobs: Give each subagent the PR title, description, full patch, and the list of changed files. Each subagent must return a JSON array of issues: - `[{"file": "path", "line": , "severity": "must-fix|suggestion|nit", "body": "..."}]` + `[{"file": "path", "line": (optional), "severity": "must-fix|suggestion|nit", "body": "..."}]` - `line` must be a line number from the NEW side of the diff **that appears inside + `line` should be a line number from the NEW side of the diff **that appears inside a diff hunk** (i.e. a line that is shown in the patch output). GitHub's review comment API rejects lines outside the diff context, so never reference lines - that are not visible in the patch. + that are not visible in the patch. If you cannot determine a valid diff line, + omit `line` — the comment will still appear in the tracking comment summary. Each subagent MUST verify its findings before returning them: - For style/convention claims, check at least 3 existing examples in the codebase to confirm @@ -273,8 +274,9 @@ jobs: comment if one already exists on the same file and line about the same problem. Also check for author replies that dismiss or reject a previous comment — do NOT re-raise an issue the PR author has already responded to disagreeing with. - 4. Prefix ALL comment bodies with a severity tag: `**must-fix**: `, `**suggestion**: `, - or `**nit**: `. + 4. Do NOT prefix `body` with a severity tag — the severity is already + captured in the `severity` field and will be added automatically when + posting inline comments. 5. Write a `summary` field in markdown for a top-level tracking comment. **If no existing tracking comment was found (first run):** @@ -298,13 +300,6 @@ jobs: Omit empty sections. Each checkbox item must correspond to an entry in `comments`. If there are no issues at all, write a short message saying the PR looks good. - Throughout all phases, track any errors that prevented you from doing - your job fully: permission denials (403, "Resource not accessible by - integration"), tools that were not available, rate limits, or any other - failures that degraded the review quality. If there were any, append a - `### Errors` section listing each failed tool/action and the error - message, so maintainers can fix the workflow configuration. - **If an existing tracking comment was found (subsequent run):** Use the existing comment as the starting point. Preserve the order and wording of all existing items. Then apply these updates: @@ -318,6 +313,15 @@ jobs: in the appropriate severity section, after the existing items. - Do NOT reorder, reword, or remove existing items. + ## Error tracking + + Throughout all phases, track any errors that prevented you from doing + your job fully: permission denials (403, "Resource not accessible by + integration"), tools that were not available, rate limits, or any other + failures that degraded the review quality. If there were any, append a + `### Errors` section to the summary listing each failed tool/action and + the error message, so maintainers can fix the workflow configuration. + ## CRITICAL: Return structured JSON output Your FINAL action must be to return a JSON object matching the following @@ -361,11 +365,12 @@ jobs: /* If the review job failed or was cancelled, update the tracking * comment to reflect that and bail out. */ if (process.env.REVIEW_RESULT !== "success") { + const verb = process.env.REVIEW_RESULT === "cancelled" ? "was cancelled" : "failed"; await github.rest.issues.updateComment({ owner, repo, comment_id: commentId, - body: `Claude review failed — see [workflow run](${runUrl}) for details.\n\n${MARKER}`, + body: `Claude review ${verb} — see [workflow run](${runUrl}) for details.\n\n${MARKER}`, }); core.setFailed("Review job did not succeed."); return; @@ -412,7 +417,9 @@ jobs: commit_id: headSha, path: c.file, line: c.line, - body: `Claude: ${c.body}`, + side: "RIGHT", + subject_type: "line", + body: `Claude: **${c.severity}**: ${c.body}`, }); posted++; } catch (e) { @@ -429,8 +436,6 @@ jobs: else console.log("No inline comments to post."); - const failed = inlineComments.length > 0 && posted < inlineComments.length; - /* Update the tracking comment with Claude's summary. */ if (!summary) summary = "Claude review: no issues found :tada:\n\n" + MARKER; @@ -447,5 +452,7 @@ jobs: console.log("Tracking comment updated successfully."); - if (failed) - core.setFailed(`Failed to post ${comments.length - posted}/${comments.length} inline comment(s).`); + if (inlineComments.length > 0 && posted === 0) + core.setFailed(`Could not post any of ${inlineComments.length} inline comment(s) — see warnings above.`); + else if (posted < inlineComments.length) + core.warning(`${inlineComments.length - posted}/${inlineComments.length} inline comment(s) could not be posted.`);