Skip to content

fix: handle-pr-review summary SHA interpolation (NES-1519)#8942

Merged
jaco-brink merged 3 commits intomainfrom
jacobusbrink/nes-1519-fix-handle-pr-review-summary-comment-sha-interpolation-and
Apr 1, 2026
Merged

fix: handle-pr-review summary SHA interpolation (NES-1519)#8942
jaco-brink merged 3 commits intomainfrom
jacobusbrink/nes-1519-fix-handle-pr-review-summary-comment-sha-interpolation-and

Conversation

@jaco-brink
Copy link
Copy Markdown
Collaborator

@jaco-brink jaco-brink commented Apr 1, 2026

Summary

  • Fix the handle-pr-review Cursor skill step 9 summary comment to resolve git rev-parse --short HEAD before posting, replacing the single-quoted heredoc (<<'EOF') that prevented shell variable expansion of $COMMIT_SHA
  • Create Claude Code command at .claude/commands/handle-pr-review.md, ported from the workspace version which already has the fix
  • Add cross-reference sync comments to both files so future editors know to keep them aligned

Closes NES-1519

Test plan

  • Run /handle-pr-review on any core PR and verify the summary comment shows the actual commit SHA (e.g. abc1234) instead of the literal $COMMIT_SHA string
  • Verify the Claude Code command is discoverable via /handle-pr-review
  • Confirm cross-reference comments exist in both files

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • Added a comprehensive end-to-end workflow for fetching, triaging (Fix / Adjusted Fix / Challenge / Skip), and resolving PR review threads, including confirmation and auto modes.
    • Expanded guidance for composing the PR summary comment to include commit SHA automatically, clearer fixed/challenged/skipped formats with file/line anchors, and shell quoting rules so embedded metadata interpolates correctly.
    • Added recovery and CI/check guidance for common auth, rate-limit, and push/merge issues.

- Fix step 9 in Cursor skill to resolve `git rev-parse --short HEAD`
  before posting the summary comment, avoiding single-quoted heredocs
  (`<<'EOF'`) that prevent shell variable expansion
- Create Claude Code command at `.claude/commands/handle-pr-review.md`,
  ported from the workspace version which already has the fix
- Add cross-reference sync comments to both files

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@linear
Copy link
Copy Markdown

linear Bot commented Apr 1, 2026

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 1, 2026

Walkthrough

Adds a detailed PR review workflow command specification and synchronizes the corresponding skill documentation to adjust PR summary generation (including dynamic commit SHA insertion and updated summary formatting).

Changes

Cohort / File(s) Summary
PR Review Workflow Command
.claude/commands/handle-pr-review.md
New comprehensive workflow doc: PR identification and state checks, GraphQL retrieval and pagination of unresolved review threads, triage rules (Fix / Fix (adjusted) / Challenge / Skip), grouping/pagination handling, resolution and reply strategies (GraphQL resolveReviewThread, REST reply fallback), CI/pre-push guidance, and recovery notes.
Skill Implementation Sync
.cursor/skills/handle-pr-review/SKILL.md
Sync comments added and summary posting example converted from static markdown to an executable bash snippet that computes COMMIT_SHA=$(git rev-parse --short HEAD) and embeds it in the gh pr comment body. Summary item formats updated to include inline file/line anchors and revised "Skipped" formatting; advice added to avoid single-quoted heredocs so variables interpolate.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main fix: correcting SHA interpolation in the handle-pr-review summary comment generation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jacobusbrink/nes-1519-fix-handle-pr-review-summary-comment-sha-interpolation-and

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented Apr 1, 2026

View your CI Pipeline Execution ↗ for commit b75159b

Command Status Duration Result
nx affected --target=subgraph-check --base=1ebd... ✅ Succeeded <1s View ↗
nx affected --target=extract-translations --bas... ✅ Succeeded <1s View ↗
nx affected --target=lint --base=1ebd921a318ec7... ✅ Succeeded <1s View ↗
nx affected --target=type-check --base=1ebd921a... ✅ Succeeded <1s View ↗
nx run-many --target=codegen --all --parallel=3 ✅ Succeeded 2s View ↗
nx run-many --target=prisma-generate --all --pa... ✅ Succeeded 3s View ↗

☁️ Nx Cloud last updated this comment at 2026-04-01 05:29:26 UTC

jaco-brink and others added 2 commits April 1, 2026 18:24
Address review concerns:
- Make sync comments directional: command is the canonical version,
  Cursor skill is a simplified subset
- Reference section names instead of step numbers in cross-references
  to avoid brittleness when steps are renumbered

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jaco-brink jaco-brink self-assigned this Apr 1, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
.claude/commands/handle-pr-review.md (1)

118-135: Optional: Add language identifier to fenced code block.

The triage example could specify markdown as the language for better syntax highlighting and to satisfy linters.

📝 Optional improvement
-```
+```markdown
 ## Triage — PR `#123`
 
 **Will fix:**
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.claude/commands/handle-pr-review.md around lines 118 - 135, The fenced code
block in the triage example lacks a language identifier; update the opening
backticks for the block that contains "## Triage — PR `#123`" to include
"markdown" (i.e., change ``` to ```markdown) so linters and syntax highlighters
correctly treat the content as Markdown; locate the block in
.claude/commands/handle-pr-review.md around the example snippet and apply this
single-line edit to the fenced code opener.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In @.claude/commands/handle-pr-review.md:
- Around line 118-135: The fenced code block in the triage example lacks a
language identifier; update the opening backticks for the block that contains
"## Triage — PR `#123`" to include "markdown" (i.e., change ``` to ```markdown) so
linters and syntax highlighters correctly treat the content as Markdown; locate
the block in .claude/commands/handle-pr-review.md around the example snippet and
apply this single-line edit to the fenced code opener.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 1683d5f8-81d6-4e2d-8d35-afa87e87513e

📥 Commits

Reviewing files that changed from the base of the PR and between 1ebd921 and 9b4f09e.

📒 Files selected for processing (2)
  • .claude/commands/handle-pr-review.md
  • .cursor/skills/handle-pr-review/SKILL.md

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
.claude/commands/handle-pr-review.md (1)

118-134: Add language identifier to fenced code block.

The code block showing the triage output example should specify a language identifier for proper rendering. Consider using markdown or text.

📝 Proposed fix
 Present the triage to the user for confirmation before proceeding:
 
-```
+```text
 ## Triage — PR `#123`
 
 **Will fix:**

As per coding guidelines: markdownlint-cli2 requires fenced code blocks to have a language specified (MD040).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.claude/commands/handle-pr-review.md around lines 118 - 134, The fenced code
block in the triage example lacks a language identifier (MD040); update the
opening triple-backticks in .claude/commands/handle-pr-review.md for the triage
output block to include a language (e.g., change ``` to ```text or ```markdown)
so the block is lint-compliant; ensure only the opening fence is modified for
the block that begins with "## Triage — PR `#123`" and leave the block contents
unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In @.claude/commands/handle-pr-review.md:
- Around line 118-134: The fenced code block in the triage example lacks a
language identifier (MD040); update the opening triple-backticks in
.claude/commands/handle-pr-review.md for the triage output block to include a
language (e.g., change ``` to ```text or ```markdown) so the block is
lint-compliant; ensure only the opening fence is modified for the block that
begins with "## Triage — PR `#123`" and leave the block contents unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e390acad-b632-4ed5-b014-4edeb06d1a15

📥 Commits

Reviewing files that changed from the base of the PR and between 9b4f09e and b75159b.

📒 Files selected for processing (2)
  • .claude/commands/handle-pr-review.md
  • .cursor/skills/handle-pr-review/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • .cursor/skills/handle-pr-review/SKILL.md

@jaco-brink jaco-brink merged commit 326d155 into main Apr 1, 2026
20 of 22 checks passed
@jaco-brink jaco-brink deleted the jacobusbrink/nes-1519-fix-handle-pr-review-summary-comment-sha-interpolation-and branch April 1, 2026 05:52
tanflem pushed a commit that referenced this pull request Apr 3, 2026
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant