From 56655ba8780aabc5d574feab9f1d91017dc39478 Mon Sep 17 00:00:00 2001 From: Chris Burns <29541485+ChrisJBurns@users.noreply.github.com> Date: Thu, 28 May 2026 22:14:01 +0100 Subject: [PATCH] Delete .claude/skills/pr-review directory We have a stacklok wide skill via plugins for this which sometimes clashes and Claude tends to use this one as opposed to the plugin variant. Removing to remove the chances of it using the wrong one. --- .claude/skills/pr-review/EXAMPLES-INLINE.md | 231 ----------- .claude/skills/pr-review/EXAMPLES-REPLY.md | 400 -------------------- .claude/skills/pr-review/SKILL.md | 335 ---------------- 3 files changed, 966 deletions(-) delete mode 100644 .claude/skills/pr-review/EXAMPLES-INLINE.md delete mode 100644 .claude/skills/pr-review/EXAMPLES-REPLY.md delete mode 100644 .claude/skills/pr-review/SKILL.md diff --git a/.claude/skills/pr-review/EXAMPLES-INLINE.md b/.claude/skills/pr-review/EXAMPLES-INLINE.md deleted file mode 100644 index 5198c3651f..0000000000 --- a/.claude/skills/pr-review/EXAMPLES-INLINE.md +++ /dev/null @@ -1,231 +0,0 @@ -# PR Inline Review Examples - -Common use cases and examples for submitting PR reviews with inline comments. - -## Example 1: Simple Inline Review (No Suggestions) - -**Use case**: Pointing out issues that require discussion or complex fixes - -**Command:** -```bash -gh api -X POST repos/stacklok/toolhive/pulls/2165/reviews --input /tmp/pr-review-comments.json -``` - -**JSON:** -```json -{ - "body": "Found several architectural concerns that need discussion", - "event": "COMMENT", - "comments": [ - { - "path": "docs/arch/02-core-concepts.md", - "line": 605, - "body": "This diagram doesn't accurately reflect the actual architecture. The Workload struct only contains metadata, not direct references to Runtime and Transport. These relationships are managed by WorkloadManager and Runner.\n\nWe should discuss how to simplify this while keeping it accurate.\n\nEvidence: pkg/core/workload.go, pkg/workloads/manager.go" - }, - { - "path": "pkg/runner/config.go", - "line": 136, - "body": "The documentation mentions only 8 fields but RunConfig has 39 serializable fields. Should we document all of them or create a categorized reference?\n\nEvidence: pkg/runner/config.go:32-157" - } - ] -} -``` - -**When to use:** -- Issues require discussion or design decisions -- Changes are too complex for inline suggestions -- Multiple files need coordinated changes -- User needs to provide context or make choices - ---- - -## Example 2: Quick Fixes with Suggestions - -**Use case**: Simple corrections that can be committed directly - -**JSON:** -```json -{ - "body": "Documentation corrections with suggested fixes", - "event": "COMMENT", - "comments": [ - { - "path": "docs/arch/02-core-concepts.md", - "line": 238, - "body": "File path reference is incorrect: `pkg/registry/registry.go` does not exist.\n\n```suggestion\n- Registry manager: `pkg/registry/provider.go`\n```\n\nThe registry functionality is split across multiple files in `pkg/registry/`.\n\nEvidence: Verified via codebase exploration" - }, - { - "path": "docs/arch/02-core-concepts.md", - "line": 597, - "body": "File path is incorrect.\n\n```suggestion\n- Health checker: `pkg/healthcheck/healthcheck.go`\n```\n\nEvidence: Verified via codebase exploration" - }, - { - "path": "docs/arch/02-core-concepts.md", - "line": 127, - "body": "Middleware type name is incorrect. The code uses `authorization`, not `authz`.\n\n```suggestion\n7. **Authorization** (`authorization`) - Cedar policy evaluation\n```\n\nEvidence: pkg/authz/middleware.go:211" - } - ] -} -``` - -**When to use:** -- Typos or incorrect file paths -- Simple one-line corrections -- Version numbers or constants -- Formatting fixes - ---- - -## Example 3: Mixed Review (Some with Suggestions, Some Without) - -**Use case**: Combination of quick fixes and items needing discussion - -**JSON:** -```json -{ - "body": "Documentation review: found quick fixes and items for discussion", - "event": "COMMENT", - "comments": [ - { - "path": "docs/arch/02-core-concepts.md", - "line": 329, - "body": "Command examples are incorrect:\n\n```suggestion\n- `thv client list-registered` - List all registered clients\n- `thv client setup` - Interactively setup clients\n- `thv client status` - Show installation status\n- `thv client register ` - Register a specific client\n- `thv client remove ` - Remove a client\n```\n\nEvidence: cmd/thv/app/client.go:36-41" - }, - { - "path": "docs/arch/02-core-concepts.md", - "line": 136, - "body": "The key fields list is incomplete. RunConfig has 39 serializable fields, but only 8 are listed here.\n\nNotable missing fields include: `name`, `cmdArgs`, `secrets`, `oidcConfig`, `authzConfig`, `auditConfig`, `telemetryConfig`, `group`, `toolsFilter`, `toolsOverride`, `isolateNetwork`, `proxyMode`, and many others.\n\nShould we either:\n1. Categorize fields by purpose (Identity, Security, Middleware, etc.), or\n2. Add a reference to the complete list in `05-runconfig-and-permissions.md`?\n\nEvidence: pkg/runner/config.go:32-157" - }, - { - "path": "docs/arch/02-core-concepts.md", - "line": 627, - "body": "The request flow diagram is incomplete. It shows only 4 middleware types but there are 8 middleware types defined in the codebase.\n\nMissing middleware: Token Exchange, Tool Filter, Tool Call Filter, and Telemetry.\n\nComplete flow should include:\n`Auth → [Token Exchange] → [Tool Filter] → [Tool Call Filter] → Parser → [Telemetry] → [Authorization] → [Audit] → Container`\n\n(Brackets indicate conditional middleware that are only present if configured)\n\nEvidence: pkg/runner/middleware.go:16-27" - } - ] -} -``` - -**When to use:** -- Mix of simple and complex issues -- Some items have clear fixes, others need discussion -- Want to provide suggestions where possible but leave complex items open - ---- - -## Example 4: Multi-line Suggestion - -**Use case**: Fixing multiple lines or a larger code block - -**JSON:** -```json -{ - "body": "Correcting middleware list with complete and accurate information", - "event": "COMMENT", - "comments": [ - { - "path": "docs/arch/02-core-concepts.md", - "line": 110, - "body": "The middleware list should include all 8 types with the correct name for Authorization:\n\n```suggestion\n**Eight middleware types:**\n\n1. **Authentication** (`auth`) - JWT token validation\n2. **Token Exchange** (`tokenexchange`) - OAuth token exchange\n3. **MCP Parser** (`mcp-parser`) - JSON-RPC parsing\n4. **Tool Filter** (`tool-filter`) - Filter and override tools in `tools/list` responses\n5. **Tool Call Filter** (`tool-call-filter`) - Validate and map `tools/call` requests\n6. **Telemetry** (`telemetry`) - OpenTelemetry instrumentation\n7. **Authorization** (`authorization`) - Cedar policy evaluation\n8. **Audit** (`audit`) - Request logging\n```\n\nEvidence: pkg/runner/middleware.go:16-27, pkg/authz/middleware.go:211" - } - ] -} -``` - -**When to use:** -- Correcting lists or tables -- Updating code blocks -- Fixing multiple related lines together -- Ensuring consistent formatting across lines - ---- - -## Example 5: Request Changes (Blocking Review) - -**Use case**: Critical issues that must be fixed before merge - -**JSON:** -```json -{ - "body": "Critical inaccuracies found in documentation that must be corrected before merge", - "event": "REQUEST_CHANGES", - "comments": [ - { - "path": "docs/arch/02-core-concepts.md", - "line": 238, - "body": "**CRITICAL**: This file path does not exist and will break documentation links.\n\n```suggestion\n- Registry manager: `pkg/registry/provider.go`\n```\n\nEvidence: Verified via codebase exploration" - }, - { - "path": "docs/arch/02-core-concepts.md", - "line": 329, - "body": "**CRITICAL**: These commands don't exist and users will get errors if they try to use them.\n\n```suggestion\n- `thv client list-registered` - List all registered clients\n- `thv client setup` - Interactively setup clients\n- `thv client status` - Show installation status\n```\n\nEvidence: cmd/thv/app/client.go:36-41" - } - ] -} -``` - -**When to use:** -- Critical bugs or security issues -- Documentation that will mislead users -- Breaking changes without proper migration -- Must be fixed before merge - ---- - -## Example 6: Approval with Minor Suggestions - -**Use case**: Approving PR but offering optional improvements - -**JSON:** -```json -{ - "body": "LGTM! Just a few minor suggestions for improvement.", - "event": "APPROVE", - "comments": [ - { - "path": "docs/arch/02-core-concepts.md", - "line": 597, - "body": "Minor: This file path could be more accurate.\n\n```suggestion\n- Health checker: `pkg/healthcheck/healthcheck.go`\n```\n\n(Not blocking - can be fixed in a follow-up if preferred)\n\nEvidence: Verified via codebase exploration" - } - ] -} -``` - -**When to use:** -- PR is generally good, minor improvements available -- Non-blocking suggestions for quality improvements -- Optional refactoring or cleanup suggestions -- Style or consistency improvements - ---- - -## Tips for Each Scenario - -### For Simple Reviews (No Suggestions) -- Focus on clear problem descriptions -- Ask questions when context is needed -- Provide references to relevant code -- Suggest next steps or alternatives - -### For Reviews with Suggestions -- Always read the current content first -- Match the existing formatting exactly -- Test the suggestion if possible -- Keep suggestions focused and minimal - -### For Mixed Reviews -- Put suggestions first (quick wins) -- Group related comments together -- Use clear markdown formatting -- Distinguish between blocking and non-blocking issues - -### For Blocking Reviews -- Use `REQUEST_CHANGES` event -- Mark critical items clearly (e.g., **CRITICAL**) -- Provide suggestions where possible for faster resolution -- Explain impact of not fixing the issue - -### For Approvals -- Use `APPROVE` event -- Mark suggestions as optional/non-blocking -- Acknowledge good work in the summary -- Keep suggestions truly minor/optional diff --git a/.claude/skills/pr-review/EXAMPLES-REPLY.md b/.claude/skills/pr-review/EXAMPLES-REPLY.md deleted file mode 100644 index d5af8bb4d6..0000000000 --- a/.claude/skills/pr-review/EXAMPLES-REPLY.md +++ /dev/null @@ -1,400 +0,0 @@ -# PR Review Reply Examples - -Common scenarios with actual commands for replying to and resolving GitHub PR review comments. - -## Example 1: Simple "Fixed in Commit" Reply - -**Scenario:** Copilot suggested fixing nolint comment spacing. You fixed it in commit c4bb55d. - -### Step 1: Get the comment ID - -```bash -gh api repos/stacklok/toolhive-registry-server/pulls/20/comments | jq '.[] | {id, path, line, body: .body[0:100], author: .user.login}' -``` - -**Output:** -```json -{ - "id": 2445150488, - "path": "pkg/versions/version.go", - "line": 24, - "body": "Corrected spacing in nolint comment...", - "author": "copilot-pull-request-reviewer" -} -``` - -### Step 2: Reply to the comment - -```bash -gh api -X POST repos/stacklok/toolhive-registry-server/pulls/20/comments/2445150488/replies \ - -f body="Fixed in c4bb55d" -``` - -### Step 3: Get the thread ID - -```bash -gh api graphql -f query=' -query { - repository(owner: "stacklok", name: "toolhive-registry-server") { - pullRequest(number: 20) { - reviewThreads(first: 20) { - nodes { - id - isResolved - comments(first: 5) { - nodes { - id - body - author { login } - } - } - } - } - } - } -}' | jq '.data.repository.pullRequest.reviewThreads.nodes[] | select(.comments.nodes[0].id == 2445150488) | {threadId: .id, isResolved}' -``` - -**Output:** -```json -{ - "threadId": "PRRT_kwDOP_5nS85emMpx", - "isResolved": false -} -``` - -### Step 4: Resolve the thread - -```bash -gh api graphql -f query=' -mutation { - resolveReviewThread(input: {threadId: "PRRT_kwDOP_5nS85emMpx"}) { - thread { - id - isResolved - } - } -}' -``` - -**Output:** -```json -{ - "data": { - "resolveReviewThread": { - "thread": { - "id": "PRRT_kwDOP_5nS85emMpx", - "isResolved": true - } - } - } -} -``` - ---- - -## Example 2: Batch Processing Multiple Fixed Comments - -**Scenario:** Multiple comments fixed in the same commit. Process them all at once. - -### Step 1: Get all unresolved comments - -```bash -gh api graphql -f query=' -query { - repository(owner: "stacklok", name: "toolhive-registry-server") { - pullRequest(number: 20) { - reviewThreads(first: 20) { - nodes { - id - isResolved - comments(first: 10) { - nodes { - id - path - line - body - author { login } - } - } - } - } - } - } -}' | jq '.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved == false)' -``` - -### Step 2: Present to user for approval - -``` -Found 2 unresolved threads fixed in commit c4bb55d: - -1. pkg/versions/version.go:24 - "Fix nolint spacing" -2. cmd/thv-registry-api/app/commands.go:53 - "Handle GetString error" - -Reply "Fixed in c4bb55d" to both and resolve? (y/n) -``` - -### Step 3: Reply to each comment (if user approves) - -```bash -# Reply to first comment -gh api -X POST repos/stacklok/toolhive-registry-server/pulls/20/comments/2445150488/replies \ - -f body="Fixed in c4bb55d" - -# Reply to second comment -gh api -X POST repos/stacklok/toolhive-registry-server/pulls/20/comments/2445150511/replies \ - -f body="Fixed in c4bb55d" -``` - -### Step 4: Resolve both threads - -```bash -# Resolve first thread -gh api graphql -f query=' -mutation { - resolveReviewThread(input: {threadId: "PRRT_kwDOP_5nS85emMpx"}) { - thread { id isResolved } - } -}' - -# Resolve second thread -gh api graphql -f query=' -mutation { - resolveReviewThread(input: {threadId: "PRRT_kwDOP_5nS85emMqG"}) { - thread { id isResolved } - } -}' -``` - ---- - -## Example 3: Answering a Question (Don't Auto-Resolve) - -**Scenario:** Reviewer asks why you chose a specific approach. Answer but wait for acknowledgment. - -### Step 1: Draft response for user approval - -``` -Reviewer asked: "Why use buffered channel here?" - -Draft response: -"Good question! The buffered channel prevents blocking when the producer -is faster than the consumer. In our case, the metrics collector can -generate events faster than the writer can persist them, so the buffer -(size 100) acts as a temporary queue to smooth out the bursts. - -Alternative would be unbuffered channel, but that would slow down the -collector. I've added a comment in the code to explain this trade-off." - -Send this response? (y/n/edit) -``` - -### Step 2: Send reply (after user approval) - -```bash -gh api -X POST repos/stacklok/toolhive-registry-server/pulls/20/comments/2445160000/replies \ - -f body="Good question! The buffered channel prevents blocking when the producer is faster than the consumer. In our case, the metrics collector can generate events faster than the writer can persist them, so the buffer (size 100) acts as a temporary queue to smooth out the bursts. - -Alternative would be unbuffered channel, but that would slow down the collector. I've added a comment in the code to explain this trade-off." -``` - -### Step 3: Don't resolve yet - -**Important:** Leave the thread unresolved until the reviewer acknowledges or approves your answer. - ---- - -## Example 4: Disagree Respectfully - -**Scenario:** Reviewer suggests a change you don't think is needed. Explain your reasoning. - -### Step 1: Draft respectful disagreement - -``` -Reviewer suggested: "Extract this into a separate function" - -Draft response: -"I considered this, but decided against it for a few reasons: - -1. The logic is only used once in this specific context -2. Extracting would require passing 4 parameters -3. The current form keeps the error handling localized - -However, if you feel strongly about it, I'm happy to refactor! Let me -know your thoughts. - -Related discussion: https://github.com/org/repo/discussions/123" - -Send this response? (y/n/edit) -``` - -### Step 2: Send and wait for discussion - -Don't resolve - this is now a discussion thread. Resolve only after reaching agreement. - ---- - -## Example 5: Already Fixed in Earlier Commit - -**Scenario:** Reviewer comments on something already fixed before the review was submitted. - -### Response: - -```bash -gh api -X POST repos/stacklok/toolhive-registry-server/pulls/20/comments/2445170000/replies \ - -f body="Good catch! This was actually already fixed in an earlier commit (ab956b8) before this review. The updated code now handles this case correctly. - -See: https://github.com/stacklok/toolhive-registry-server/commit/ab956b8#diff-abc123" -``` - -Then resolve immediately since it's already addressed. - ---- - -## Example 6: Need More Context - -**Scenario:** Review comment isn't clear. Ask for clarification. - -### Response: - -```bash -gh api -X POST repos/stacklok/toolhive-registry-server/pulls/20/comments/2445180000/replies \ - -f body="Thanks for the feedback! Could you clarify what you mean by 'handle the edge case'? - -Are you referring to: -- When the input is nil? -- When the slice is empty? -- When the index is out of bounds? - -Once I understand which case you're concerned about, I'll make sure it's properly handled." -``` - -Leave unresolved until clarified and fixed. - ---- - -## Example 7: Acknowledge Non-Blocking Suggestion - -**Scenario:** Reviewer made an optional suggestion you won't implement right now. - -### Response: - -```bash -gh api -X POST repos/stacklok/toolhive-registry-server/pulls/20/comments/2445190000/replies \ - -f body="Great suggestion! I agree this would be a nice improvement. - -For this PR, I'd like to keep the scope focused on the immediate fix, but I've created issue #456 to track this enhancement for a future PR. - -Thanks for the idea!" -``` - -Resolve after user approves (since you've addressed it by creating an issue). - ---- - -## Command Reference - -### Get all PR comments with details -```bash -gh api repos/{owner}/{repo}/pulls/{pr}/comments | \ - jq '.[] | {id, path, line, author: .user.login, body: .body[0:100]}' -``` - -### Reply to a specific comment -```bash -gh api -X POST repos/{owner}/{repo}/pulls/{pr}/comments/{comment_id}/replies \ - -f body="Your reply message" -``` - -### Get all review threads (to find thread IDs) -```bash -gh api graphql -f query=' -query { - repository(owner: "{owner}", name: "{repo}") { - pullRequest(number: {pr}) { - reviewThreads(first: 20) { - nodes { - id - isResolved - comments(first: 10) { - nodes { - id - body - author { login } - } - } - } - } - } - } -}' -``` - -### Find thread ID for a specific comment -```bash -gh api graphql -f query='...' | \ - jq '.data.repository.pullRequest.reviewThreads.nodes[] | - select(.comments.nodes[0].id == COMMENT_ID) | - {threadId: .id, isResolved}' -``` - -### Resolve a thread -```bash -gh api graphql -f query=' -mutation { - resolveReviewThread(input: {threadId: "{thread_id}"}) { - thread { - id - isResolved - } - } -}' -``` - -### Unresolve a thread (if needed) -```bash -gh api graphql -f query=' -mutation { - unresolveReviewThread(input: {threadId: "{thread_id}"}) { - thread { - id - isResolved - } - } -}' -``` - ---- - -## Tips for Each Scenario - -### For "Fixed in Commit" Responses -- Include the short SHA (first 7 chars) -- Optionally link to the commit or diff -- Resolve immediately after replying -- Batch process multiple if same commit - -### For Questions -- Draft answer first, get user approval -- Be thorough but concise -- Include links to relevant docs/code -- Don't auto-resolve - wait for acknowledgment - -### For Disagreements -- Be respectful and explain reasoning -- Offer alternatives or compromise -- Link to relevant discussions or standards -- Never resolve - let discussion conclude naturally - -### For Clarifications -- Ask specific questions -- Offer multiple interpretations -- Be open to learning -- Resolve only after understanding and fixing - -### For Optional Suggestions -- Acknowledge the value -- Explain if deferring (create issue) -- Thank the reviewer -- Can resolve if properly acknowledged diff --git a/.claude/skills/pr-review/SKILL.md b/.claude/skills/pr-review/SKILL.md deleted file mode 100644 index b25a56e425..0000000000 --- a/.claude/skills/pr-review/SKILL.md +++ /dev/null @@ -1,335 +0,0 @@ ---- -name: pr-review -description: Submit inline review comments to GitHub PRs and reply to/resolve review threads using the GitHub CLI and GraphQL API. ---- - -# PR Review - -Submit inline review comments to GitHub Pull Requests and reply to/resolve review threads using the GitHub CLI. - -## Prerequisites - -- GitHub CLI (`gh`) must be installed and authenticated -- User must have write access to the repository -- PR must exist and be open - ---- - -## Part 1: Submitting Inline Review Comments - -### Workflow - -1. **Collect findings**: The user will provide you with: - - Repository owner and name (or detect from current directory) - - PR number - - A list of findings, each containing: - - File path (relative to repo root) - - Line number - - Comment body/description - - (Optional) Suggested fix if it's a simple change - -2. **Read current content**: If providing suggestions, use the Read tool to see the exact current content - -3. **Create review JSON**: Build a JSON structure at `/tmp/pr-review-comments.json`: - ```json - { - "body": "Overall review summary", - "event": "COMMENT", - "comments": [ - { - "path": "path/to/file.ext", - "line": 123, - "body": "Comment text with optional suggestion" - } - ] - } - ``` - -4. **Submit review**: Use GitHub CLI: - ```bash - gh api -X POST repos/{owner}/{repo}/pulls/{pr_number}/reviews --input /tmp/pr-review-comments.json - ``` - -5. **Return URL**: Extract and return the review URL from the response - -### JSON Structure - -#### Top-level fields - -- `body` (required): Overall review summary -- `event` (required): `"COMMENT"`, `"APPROVE"`, or `"REQUEST_CHANGES"` -- `comments` (required): Array of comment objects - -#### Comment object fields - -- `path` (required): File path relative to repository root -- `line` (required): Line number (positive integer) -- `body` (required): Comment text (supports markdown) - -### Inline Code Suggestions - -GitHub supports inline code suggestions that users can commit directly from the PR UI. - -#### When to Use Suggestions - -**Good candidates:** -- Fixing typos or incorrect file paths -- Correcting simple syntax errors -- Updating version numbers or constants -- Renaming variables or functions -- Fixing formatting or indentation -- Adding missing content - -**Not suitable:** -- Complex logic changes requiring multiple files -- Changes that need testing or validation -- Architectural changes requiring discussion -- Changes requiring user decision/context - -#### Suggestion Syntax - -**Single-line:** -````markdown -Description of the issue. - -```suggestion -corrected line of code -``` - -Evidence: reference -```` - -**Multi-line:** -````markdown -Description of the issue. - -```suggestion -first corrected line -second corrected line -third corrected line -``` - -Evidence: reference -```` - -### Submitting Best Practices - -- Be specific with line numbers and file paths -- Provide evidence (link to code/documentation) -- Be constructive - suggest fixes, not just problems -- Use markdown formatting for clarity -- Include context explaining why it's an issue - -#### When Including Suggestions -1. Read the current line(s) using Read tool first -2. Provide exact replacement text -3. Match existing formatting and style -4. Verify syntax is correct -5. One suggestion block per comment - -#### Review Strategy -1. Group related findings into a single review -2. Put simple fixes with suggestions first -3. Use appropriate event type -4. Write clear summary in `body` - -### Output Format - -Report after submission: -- Review ID and URL -- Number of comments submitted -- Number with suggestions -- PR title and number - ---- - -## Part 2: Replying to and Resolving Review Comments - -### Workflow - -#### 1. Gather Review Comments - -Fetch all review comments from the PR and present them organized by: -- Status: unresolved vs resolved -- Type: suggestions, questions, nitpicks, critical issues -- Author: group by reviewer - -**For each comment show:** -- Author and timestamp -- File and line number -- Comment body -- Any existing replies -- Resolution status - -#### 2. Analyze and Recommend - -For each unresolved comment, provide a recommendation: - -**If code needs fixing:** -- "Recommendation: Fix the issue, then reply with commit SHA and resolve" - -**If it's a question:** -- "Recommendation: Answer the question, wait for acknowledgment before resolving" - -**If it's a suggestion to consider:** -- "Recommendation: Discuss trade-offs, decide with user whether to implement" - -**If already addressed:** -- "Recommendation: Reply with commit reference and resolve immediately" - -#### 3. Get User Decisions - -**Present summary:** -``` -Found 5 unresolved review comments: - -1. [Critical] pkg/versions/version.go:24 - @Copilot - "Fix nolint spacing" - Status: Fixed in commit c4bb55d - Recommendation: Reply "Fixed in c4bb55d" and resolve - -2. [Question] pkg/server/handler.go:45 - @reviewer - "Why use buffered channel here?" - Status: Needs answer - Recommendation: Draft response for your review - -How would you like to proceed? -- Reply and resolve all fixed items (1) -- Draft responses for questions (2) -- Process individually -- Custom approach -``` - -#### 4. Execute User's Choice - -Based on user decisions: -- Draft reply messages for approval -- Submit replies after user confirms -- Resolve threads only when user approves - -#### 5. Report Results - -After processing, show: -- What was done (replied/resolved) -- What remains (still needs attention) -- Any errors or issues -- Next steps if any - -### Interactive Decision Points - -#### Before Replying -**Ask:** "Here's my draft reply: '{message}'. Send this?" -- User can edit, approve, or skip - -#### Before Resolving -**Ask:** "Mark this thread as resolved?" -- Only if issue is truly addressed -- User may want to wait for reviewer acknowledgment - -#### For Bulk Operations -**Ask:** "I found 5 comments fixed in commit abc123. Reply 'Fixed in abc123' to all and resolve?" -- Show list of affected comments -- Let user review before executing - -### Reply Best Practices - -- **Be specific**: Reference commit SHAs when applicable -- **Be helpful**: Explain reasoning, not just "fixed" -- **Be respectful**: Thank reviewers for feedback -- **Use markdown**: Format code, lists, links - -### When to Resolve -**Resolve when:** -- Issue is fixed and committed -- Question answered and acknowledged -- Discussion concluded with agreement -- User confirms it's complete - -**Don't auto-resolve:** -- Without user confirmation -- When still discussing -- When waiting for reviewer response -- When unsure about the fix - ---- - -## Command Reference - -### Submit a review -```bash -gh api -X POST repos/{owner}/{repo}/pulls/{pr}/reviews --input /tmp/pr-review-comments.json -``` - -### Get all PR comments with details -```bash -gh api repos/{owner}/{repo}/pulls/{pr}/comments | \ - jq '.[] | {id, path, line, author: .user.login, body: .body[0:100]}' -``` - -### Reply to a specific comment -```bash -gh api -X POST repos/{owner}/{repo}/pulls/{pr}/comments/{comment_id}/replies \ - -f body="Your reply message" -``` - -### Get all review threads (to find thread IDs) -```bash -gh api graphql -f query=' -query { - repository(owner: "{owner}", name: "{repo}") { - pullRequest(number: {pr}) { - reviewThreads(first: 20) { - nodes { - id - isResolved - comments(first: 10) { - nodes { - id - body - author { login } - } - } - } - } - } - } -}' -``` - -### Resolve a thread -```bash -gh api graphql -f query=' -mutation { - resolveReviewThread(input: {threadId: "{thread_id}"}) { - thread { - id - isResolved - } - } -}' -``` - -### Unresolve a thread -```bash -gh api graphql -f query=' -mutation { - unresolveReviewThread(input: {threadId: "{thread_id}"}) { - thread { - id - isResolved - } - } -}' -``` - -## Error Handling - -- **401 Unauthorized**: Run `gh auth login` -- **404 Not Found**: Verify PR number and repo access -- **422 Unprocessable Entity**: Check JSON format -- **Invalid line number**: Ensure line exists at PR's commit - -## See Also - -- [Inline Review Examples](EXAMPLES-INLINE.md) - Examples of submitting review comments -- [Reply Examples](EXAMPLES-REPLY.md) - Examples of replying to and resolving review comments