Skip to content

TESTING - DO NOT MERGE - feat(agents): add ticket, review, and fix-comment workflow skills#1749

Draft
mirooon wants to merge 2 commits into
mainfrom
feat/linear-github-workflow-skills
Draft

TESTING - DO NOT MERGE - feat(agents): add ticket, review, and fix-comment workflow skills#1749
mirooon wants to merge 2 commits into
mainfrom
feat/linear-github-workflow-skills

Conversation

@mirooon
Copy link
Copy Markdown
Contributor

@mirooon mirooon commented Apr 28, 2026

Summary

  • Adds three Claude Code / Cursor skills covering the full Linear → code → PR review loop
  • /ticket <ID> — end-to-end Linear ticket implementation with 5 phases: understand AC → plan → implement → verify → update Linear status + post summary
  • /review <PR> — inventory all open review threads (BLOCKER/CHANGE/QUESTION/NITPICK), fix/reply in priority order, push, reply per thread with Linear ticket context pulled via MCP
  • /fix-comment <PR> <comment-id> — surgical single-thread fix + reply

What's included

Karpathy principles baked in — think before coding, simplicity first, surgical changes, goal-driven execution throughout all three skills.

Rule cross-referencesticket.md includes a rule activation map (which of the 25 .agents/rules/*.mdc activates for each file type) and critical guards for whitelist branching (502), event emission locations ([CONV:EVENTS]), storage/selector shifts (002), and gas surfacing (106).

Skill escalation — all three skills know when to hand off:

  • Failing tx with hash → /analyze-tx
  • Add a chain → /add-network
  • Remove a chain → /deprecate-network
  • Reviewer mentions tx hash → /analyze-tx before fixing

Linear context in reviewreview.md and fix-comment.md extract the Linear ticket ID from the branch name / PR title / body and fetch full AC + comments via Linear MCP, so QUESTION thread replies are grounded in original intent.

Settings — adds gh *, git checkout/commit/add/push/remote to the project allow list.

Test plan

  • /ticket ENG-<id> — verify it fetches ticket, creates branch, implements, posts Linear comment
  • /review <pr> — verify it inventories threads, classifies correctly, fixes and replies
  • /fix-comment <pr> <id> — verify single-thread surgical fix + reply
  • Confirm symlinks: ls -l .claude/skills/*/SKILL.md all point into .agents/commands/

🤖 Generated with Claude Code

Three new Claude Code / Cursor skills for the full Linear → code → PR loop:

- /ticket <ID>        — read Linear AC, plan, implement, verify, update status, post summary
- /review <PR>        — inventory all review threads (BLOCKER/CHANGE/QUESTION/NITPICK),
                        fix/reply in priority order, push, reply per thread
- /fix-comment <PR> <ID> — surgical single-thread fix + reply

All three embed Karpathy's four principles (think before coding, simplicity first,
surgical changes, goal-driven execution), pull Linear ticket context via MCP,
use gh CLI for GitHub ops, and include escalation paths to existing skills
(/analyze-tx for failing tx hashes, /add-network, /deprecate-network).

Also adds a rule activation map and critical guards (whitelist branching, event
locations, storage/selector impact, gas surfacing) to ticket.md, and rule-anchored
response guidance (gas/security/architecture/events) to review.md.

.claude/settings.json: add gh *, git checkout/commit/add/push/remote to allow list.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lifi-action-bot lifi-action-bot marked this pull request as draft April 28, 2026 13:28
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 28, 2026

Walkthrough

Adds three new agent command specs (/review, /fix-comment, /ticket), corresponding Claude and Cursor skill/alias pointer files, and expands allowed shell/Git/GitHub CLI permissions in .claude/settings.json. Documentation-only additions define multi-phase workflows and strict abort/verification rules.

Changes

Cohort / File(s) Summary
Agent Command Specifications
.agents/commands/fix-comment.md, .agents/commands/review.md, .agents/commands/ticket.md
Adds three detailed command specifications: /fix-comment (single PR comment repair flow), /review (end-to-end PR review orchestration with inventory/triage/implement/verify/publish phases), and /ticket (Linear ticket planning/implementation/publish workflow).
Claude settings & Skills
.claude/settings.json, .claude/skills/fix-comment/SKILL.md, .claude/skills/review/SKILL.md, .claude/skills/ticket/SKILL.md
Expands permissions.allow to permit Git and gh CLI patterns and adds three small skill pointer files referencing the new command specs.
Cursor command aliases
.cursor/commands/fix-comment.md, .cursor/commands/review.md, .cursor/commands/ticket.md
Adds three one-line alias/pointer files that forward to the canonical .agents/commands/* docs.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is comprehensive and well-structured, covering summary, what's included, key features, and a test plan. However, it does not follow the repository's required template which asks for Linear task link, implementation rationale, and standard checklists. Fill in the required template sections: specify which Linear task belongs to this PR, explain implementation rationale, and complete the pre-review and reviewer checklists.
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The PR title clearly describes the main change: adding three workflow skills (/ticket, /review, /fix-comment) for agent-based Linear→GitHub integration.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/linear-github-workflow-skills

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.

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.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
.claude/skills/fix-comment/SKILL.md (2)

1-5: ⚠️ Potential issue | 🟡 Minor

Fix Phase 0 step numbering: steps 3 and 4 are both numbered 3.

Lines 41–48 contain a duplicate step number. The steps should be:

  1. Fetch the comment (line 31)
  2. Resolve the Linear ticket (line 37)
  3. Read the diff context (line 41)
  4. State exactly what the reviewer is asking (line 48) — currently labeled 3.
  5. Classify the request (line 53) — currently labeled 4.

Renumber steps 48 and 53 to fix the sequence.

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

In @.claude/skills/fix-comment/SKILL.md around lines 1 - 5, The ordered list in
the Fix Phase 0 section misnumbers the steps: both "Read the diff context" and
"State exactly what the reviewer is asking" are labeled "3."; update the
numbering so "Read the diff context" is 3, "State exactly what the reviewer is
asking" is 4, and "Classify the request" is 5 (adjust the subsequent step number
accordingly) to restore the correct sequence in the steps block.

12-121: ⚠️ Potential issue | 🟠 Major

Add explicit GraphQL mutation to mark review threads resolved.

The spec promises "marks resolved" (line 13) but provides no API implementation in Phase 2. Resolving pull request review threads requires the GraphQL resolveReviewThread mutation, not the REST API. Add a step after the reply:

gh api graphql --input - <<EOF
mutation {
  resolveReviewThread(input: {threadId: "$THREAD_ID"}) {
    thread {
      id
      isResolved
    }
  }
}
EOF

First query threads to get their IDs (format PRRT_...), then resolve. This gap affects both /fix-comment and /review skills. Update both specs to show the complete flow or clarify that resolution is intentionally deferred to manual marking.

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

In @.claude/skills/fix-comment/SKILL.md around lines 12 - 121, The spec must
explicitly implement resolving PR review threads using the GraphQL mutation
resolveReviewThread rather than relying on REST; update Phase 2 in both the
/fix-comment and /review skill docs to first query review threads to obtain
thread IDs (format PRRT_...) and then call the GraphQL resolveReviewThread
mutation with THREAD_ID before or after posting the reply, and include the exact
mutation name resolveReviewThread and the variable THREAD_ID so consumers know
where to plug the ID; keep the rest of the Phase 2 steps (commit/push/reply)
unchanged.
.cursor/commands/fix-comment.md (1)

12-121: ⚠️ Potential issue | 🟠 Major

The proposed resolution API call is incorrect; GitHub only supports resolving review threads via GraphQL, not REST API.

The concern is valid—line 12 promises "marks resolved" but Phase 2 omits this step. However, the proposed gh api ... -X PATCH -f resolved=true will not work. The GitHub REST API endpoint for PR comments (PATCH /repos/{owner}/{repo}/pulls/comments/{comment_id}) does not support a resolved field.

Resolving review threads requires the GraphQL API:

mutation {
  resolveReviewThread(input: { threadId: "PRRT_<thread_id>" }) {
    thread { id isResolved }
  }
}

The workflow must first query the pull request's reviewThreads to fetch thread IDs (format: PRRT_...), then invoke this mutation. Alternatively, clarify whether "marks resolved" in the intro is aspirational or a requirement—if implementation is out of scope for this command, remove the phrase to avoid misleading users.

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

In @.cursor/commands/fix-comment.md around lines 12 - 121, The doc incorrectly
instructs using the REST "gh api ... -X PATCH -f resolved=true" to mark threads
resolved; GitHub requires the GraphQL mutation resolveReviewThread and thread
IDs from a pull request's reviewThreads. Update Phase 2 — REPLY to (a) remove
the REST resolve example and instead call out the GraphQL flow: query the PR's
reviewThreads to get the PRRT_... threadId(s) and call the resolveReviewThread
mutation, referencing the mutation name resolveReviewThread and the field
reviewThreads; or if resolving threads is out of scope for this command, remove
the "marks resolved" phrase from the intro to avoid promising unsupported
behavior.
🧹 Nitpick comments (6)
.claude/skills/fix-comment/SKILL.md (3)

19-23: Clarify stripping the optional r prefix for $COMMENT_ID.

Document comment IDs as r1234567 or numeric (Line 21-22), but API usage uses $COMMENT_ID directly. Add an explicit “strip leading r” rule before constructing API URLs.

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

In @.claude/skills/fix-comment/SKILL.md around lines 19 - 23, The docs and
argument parsing must explicitly strip a leading "r" from $COMMENT_ID before
using it in API URLs: update the parsing logic that consumes
$ARGUMENTS/$COMMENT_ID (the code that parses PR number and comment id) to accept
values like "r1234567" or "1234567", remove a leading lowercase or uppercase "r"
from the comment id token, validate that both PR number and numeric comment id
remain, and abort with a clear error message if either is missing or
non-numeric; ensure the sanitized numeric COMMENT_ID is then the value used
wherever API URLs are constructed.

48-52: Clarifying-question reply format missing.

Abort condition says post a clarifying question; stop (Line 49-52), but reply format options don’t include a clarifying-question category (Line 105-110). Add one bullet for consistent behavior.

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

In @.claude/skills/fix-comment/SKILL.md around lines 48 - 52, The reply-format
list in SKILL.md is missing a "clarifying-question" entry required by the abort
condition; update the reply format section (the list of response categories
shown near the reply-format options) to add a bullet labeled
"Clarifying-question" (or similar) explaining the exact format to use when the
code asks a focused clarifying question, and ensure it mirrors the other
bullets' style so the abort flow (the rule referencing "post a reply asking one
focused clarifying question") has a matching documented response type.

41-52: Fix duplicate Phase 0 step numbering.

Duplicate 3. labels: “Read the diff context” (Line 41-46) and “State exactly what the reviewer is asking” (Line 48-52). Renumber for clarity.

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

In @.claude/skills/fix-comment/SKILL.md around lines 41 - 52, Two Phase 0 steps
share the label "3." — update the duplicate numbering: change the step titled
"State exactly what the reviewer is asking — write it out in one sentence." to
"4." (and increment any following step numbers accordingly) so the sequence is
unique and sequential; ensure the headings "Read the diff context" and "State
exactly what the reviewer is asking" are numbered 3 and 4 respectively.
.cursor/commands/fix-comment.md (3)

19-23: Clarify stripping the optional r prefix for $COMMENT_ID.

The input section states the comment ID can be r1234567 or numeric (Line 21-22), but later GitHub API commands use $COMMENT_ID directly in URL paths (Line 33-34, Line 102-104, and the proposed resolve call above). The doc should explicitly say to strip a leading r before using it as the numeric comment id in API paths.

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

In @.cursor/commands/fix-comment.md around lines 19 - 23, The docs say
$COMMENT_ID may have an optional leading "r" but later examples use $COMMENT_ID
directly in API path variables; update the text and parsing guidance so you
strip a leading "r" from $COMMENT_ID before using it in API URLs and show the
normalized numeric value; explicitly state that $ARGUMENTS should parse PR
number and COMMENT_ID, remove a leading "r" from COMMENT_ID if present, and
abort with a clear error if either value is missing or becomes empty after
stripping.

48-52: Clarifying-question reply format is not specified.

Abort condition says “Intent is ambiguous → post a clarifying question; stop” (Line 49-52), but the reply format list (Line 105-110) doesn’t include a dedicated “Clarifying question” format. This can lead to inconsistent replies across runs.

Suggested fix: add one bullet under “Reply format” for clarifying questions (e.g., “Clarifying question: <question>”).

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

In @.cursor/commands/fix-comment.md around lines 48 - 52, The Reply format
section lacks a defined clarifying-question template, causing inconsistency;
update the document by adding a new bullet under the "Reply format" list that
explicitly defines the clarifying-question reply format (e.g., add "**Clarifying
question**: `<question>`") and ensure the abort-condition text referencing "post
a clarifying question; stop" points to this template; modify the section
containing "Reply format" and the abort-condition clause that mentions
clarifying questions so they reference the new "**Clarifying question**" bullet
and example format.

41-52: Fix duplicate Phase 0 step numbering.

In Phase 0, the doc has two separate items labeled 3.—one for “Read the diff context” (Line 41-46) and another for “State exactly what the reviewer is asking” (Line 48-52). This is minor but harms readability and can cause automation/tooling confusion if any parser relies on numbering.

Suggested fix: renumber the second 3. to 4. (and shift subsequent numbers if needed).

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

In @.cursor/commands/fix-comment.md around lines 41 - 52, The Phase 0 list
contains duplicate step numbers: the item starting "State exactly what the
reviewer is asking" is labeled "3." but should be "4." to avoid duplicate
numbering; update the markdown in .cursor/commands/fix-comment.md by renumbering
that second "3." to "4." (and increment any following numbered items
accordingly) so the sequence in the Phase 0 ordered list is consecutive and
unique.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.agents/commands/fix-comment.md:
- Around line 21-34: Normalize and validate COMMENT_ID before any GitHub API
calls: strip a leading "r" or non-digit characters from the COMMENT_ID input and
replace usages of $COMMENT_ID with the normalized numeric id; abort with a clear
error if the result is empty or not numeric. Apply this normalization once
before the first gh api call that fetches the comment (gh api
repos/{owner}/{repo}/pulls/comments/$COMMENT_ID) and reuse the normalized
variable for the later gh api invocation so both calls use the numeric ID.

In @.agents/commands/review.md:
- Around line 110-113: The instructions under the "**For NITPICK threads**"
bullet are contradictory with the guidance in the "Phase 2" vs "Phase 4"
sections (notably the Phase 2 rule allowing silent fixes and the Phase 4 rule
requiring replies); pick one consistent behavior (either "silent fix no reply"
or "always reply when acted") and update all occurrences to match: change the
"**For NITPICK threads**" bullet and both the Phase 2 and Phase 4 descriptions
so they state the same policy, and adjust any example wording in the nearby
paragraphs (including the block referenced around lines 162-173) to use the
chosen phrasing for consistency.
- Around line 48-52: The current review commands use REST endpoints
(repos/{owner}/{repo}/pulls/$PR/comments and /reviews) which don’t expose
thread-level fields or support resolving threads; replace those REST calls with
GraphQL queries against pullRequest.reviewThreads (requesting fields like id,
isResolved, isOutdated, comments { body, author, createdAt }) and add usage
examples for the resolveReviewThread mutation to mark threads resolved; update
every occurrence noted (the blocks around the existing fetch commands and the
sections at 54-63 and 147-173) to show the GraphQL query and the
resolveReviewThread mutation examples and to reference pullRequest.reviewThreads
and resolveReviewThread by name so maintainers can locate and use them.

In @.agents/commands/ticket.md:
- Around line 83-99: The branch name examples are inconsistent: change the
uppercase example "Branch:   feat/ENG-423-<slug>" to use the lowercase
convention to match the git command example and Phase 2 mandate; update the
branch example line and any other occurrences of "feat/ENG-423-..." or "ENG-423"
in the same snippet to "feat/eng-423-<slug>" so both the displayed Branch line
and the git checkout example (git checkout -b
feat/<TICKET-ID-lowercase>-<short-slug>) uniformly use lowercase casing.

In @.claude/settings.json:
- Around line 16-25: The shell permission entries are too broad—specifically the
patterns "Bash(gh *)" and "Bash(git remote *)"—and must be narrowed to
least-privilege subcommands actually used by the skills; replace the wildcard
patterns with explicit allowed subcommands (e.g. only the gh and git remote
subcommands your code invokes such as gh pr create, gh pr view, gh repo clone,
git remote get-url) and remove the catch-all patterns, updating the settings so
only those exact Bash(...) entries remain; ensure the unique symbols "Bash(gh
*)" and "Bash(git remote *)" are replaced with the precise commands your
workflow needs.

---

Outside diff comments:
In @.claude/skills/fix-comment/SKILL.md:
- Around line 1-5: The ordered list in the Fix Phase 0 section misnumbers the
steps: both "Read the diff context" and "State exactly what the reviewer is
asking" are labeled "3."; update the numbering so "Read the diff context" is 3,
"State exactly what the reviewer is asking" is 4, and "Classify the request" is
5 (adjust the subsequent step number accordingly) to restore the correct
sequence in the steps block.
- Around line 12-121: The spec must explicitly implement resolving PR review
threads using the GraphQL mutation resolveReviewThread rather than relying on
REST; update Phase 2 in both the /fix-comment and /review skill docs to first
query review threads to obtain thread IDs (format PRRT_...) and then call the
GraphQL resolveReviewThread mutation with THREAD_ID before or after posting the
reply, and include the exact mutation name resolveReviewThread and the variable
THREAD_ID so consumers know where to plug the ID; keep the rest of the Phase 2
steps (commit/push/reply) unchanged.

In @.cursor/commands/fix-comment.md:
- Around line 12-121: The doc incorrectly instructs using the REST "gh api ...
-X PATCH -f resolved=true" to mark threads resolved; GitHub requires the GraphQL
mutation resolveReviewThread and thread IDs from a pull request's reviewThreads.
Update Phase 2 — REPLY to (a) remove the REST resolve example and instead call
out the GraphQL flow: query the PR's reviewThreads to get the PRRT_...
threadId(s) and call the resolveReviewThread mutation, referencing the mutation
name resolveReviewThread and the field reviewThreads; or if resolving threads is
out of scope for this command, remove the "marks resolved" phrase from the intro
to avoid promising unsupported behavior.

---

Nitpick comments:
In @.claude/skills/fix-comment/SKILL.md:
- Around line 19-23: The docs and argument parsing must explicitly strip a
leading "r" from $COMMENT_ID before using it in API URLs: update the parsing
logic that consumes $ARGUMENTS/$COMMENT_ID (the code that parses PR number and
comment id) to accept values like "r1234567" or "1234567", remove a leading
lowercase or uppercase "r" from the comment id token, validate that both PR
number and numeric comment id remain, and abort with a clear error message if
either is missing or non-numeric; ensure the sanitized numeric COMMENT_ID is
then the value used wherever API URLs are constructed.
- Around line 48-52: The reply-format list in SKILL.md is missing a
"clarifying-question" entry required by the abort condition; update the reply
format section (the list of response categories shown near the reply-format
options) to add a bullet labeled "Clarifying-question" (or similar) explaining
the exact format to use when the code asks a focused clarifying question, and
ensure it mirrors the other bullets' style so the abort flow (the rule
referencing "post a reply asking one focused clarifying question") has a
matching documented response type.
- Around line 41-52: Two Phase 0 steps share the label "3." — update the
duplicate numbering: change the step titled "State exactly what the reviewer is
asking — write it out in one sentence." to "4." (and increment any following
step numbers accordingly) so the sequence is unique and sequential; ensure the
headings "Read the diff context" and "State exactly what the reviewer is asking"
are numbered 3 and 4 respectively.

In @.cursor/commands/fix-comment.md:
- Around line 19-23: The docs say $COMMENT_ID may have an optional leading "r"
but later examples use $COMMENT_ID directly in API path variables; update the
text and parsing guidance so you strip a leading "r" from $COMMENT_ID before
using it in API URLs and show the normalized numeric value; explicitly state
that $ARGUMENTS should parse PR number and COMMENT_ID, remove a leading "r" from
COMMENT_ID if present, and abort with a clear error if either value is missing
or becomes empty after stripping.
- Around line 48-52: The Reply format section lacks a defined
clarifying-question template, causing inconsistency; update the document by
adding a new bullet under the "Reply format" list that explicitly defines the
clarifying-question reply format (e.g., add "**Clarifying question**:
`<question>`") and ensure the abort-condition text referencing "post a
clarifying question; stop" points to this template; modify the section
containing "Reply format" and the abort-condition clause that mentions
clarifying questions so they reference the new "**Clarifying question**" bullet
and example format.
- Around line 41-52: The Phase 0 list contains duplicate step numbers: the item
starting "State exactly what the reviewer is asking" is labeled "3." but should
be "4." to avoid duplicate numbering; update the markdown in
.cursor/commands/fix-comment.md by renumbering that second "3." to "4." (and
increment any following numbered items accordingly) so the sequence in the Phase
0 ordered list is consecutive and unique.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 120d0b79-16a3-413e-ad94-712f35d3f755

📥 Commits

Reviewing files that changed from the base of the PR and between 7789eb2 and 695a866.

📒 Files selected for processing (10)
  • .agents/commands/fix-comment.md
  • .agents/commands/review.md
  • .agents/commands/ticket.md
  • .claude/settings.json
  • .claude/skills/fix-comment/SKILL.md
  • .claude/skills/review/SKILL.md
  • .claude/skills/ticket/SKILL.md
  • .cursor/commands/fix-comment.md
  • .cursor/commands/review.md
  • .cursor/commands/ticket.md

Comment on lines +21 to +34
- Comment ID: `r1234567` (the `r` prefix is optional; numeric ID also accepted)

Parse both values; abort with a clear message if either is missing.

---

## Phase 0 — UNDERSTAND (before touching any file)

> *"Don't assume. Don't hide confusion. Surface tradeoffs."* — Karpathy

1. **Fetch the comment**:
```bash
gh api repos/{owner}/{repo}/pulls/comments/$COMMENT_ID
```
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

GitHub REST API for pull request review comments: does POST /repos/{owner}/{repo}/pulls/{pull_number}/comments/{comment_id}/replies require numeric comment_id, and are values like r1234567 valid?

💡 Result:

Yes, the POST /repos/{owner}/{repo}/pulls/{pull_number}/comments/{comment_id}/replies endpoint requires a numeric comment_id. The official GitHub documentation specifies that comment_id is an integer (format: int64), described as "The unique identifier of the comment." Examples use numeric placeholders like COMMENT_ID, and response schemas confirm the id field is an integer. Values like r1234567 are not valid for this endpoint, as they do not match the required integer type. Such formats (e.g., r prefixed with numbers) may appear in GitHub's web UI for referencing comments (like in permalinks), but the REST API strictly uses numeric IDs obtained from listing comments or comment objects.

Citations:


🏁 Script executed:

cat -n .agents/commands/fix-comment.md | head -120

Repository: lifinance/contracts

Length of output: 4821


Normalize COMMENT_ID before GitHub API calls.

Documentation at line 21 accepts r1234567, but the API calls at lines 33 and 102 pass $COMMENT_ID directly. GitHub REST API requires numeric comment IDs only; the r prefix must be stripped first, or the documented input format will fail with an API error.

Suggested fix
- gh api repos/{owner}/{repo}/pulls/comments/$COMMENT_ID
+ COMMENT_ID="${COMMENT_ID#r}"
+ gh api repos/{owner}/{repo}/pulls/comments/$COMMENT_ID

Apply the same normalization before line 102's API call.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- Comment ID: `r1234567` (the `r` prefix is optional; numeric ID also accepted)
Parse both values; abort with a clear message if either is missing.
---
## Phase 0 — UNDERSTAND (before touching any file)
> *"Don't assume. Don't hide confusion. Surface tradeoffs."* — Karpathy
1. **Fetch the comment**:
```bash
gh api repos/{owner}/{repo}/pulls/comments/$COMMENT_ID
```
- Comment ID: `r1234567` (the `r` prefix is optional; numeric ID also accepted)
Parse both values; abort with a clear message if either is missing.
---
## Phase 0 — UNDERSTAND (before touching any file)
> *"Don't assume. Don't hide confusion. Surface tradeoffs."* — Karpathy
1. **Fetch the comment**:
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.agents/commands/fix-comment.md around lines 21 - 34, Normalize and validate
COMMENT_ID before any GitHub API calls: strip a leading "r" or non-digit
characters from the COMMENT_ID input and replace usages of $COMMENT_ID with the
normalized numeric id; abort with a clear error if the result is empty or not
numeric. Apply this normalization once before the first gh api call that fetches
the comment (gh api repos/{owner}/{repo}/pulls/comments/$COMMENT_ID) and reuse
the normalized variable for the later gh api invocation so both calls use the
numeric ID.

Comment on lines +48 to +52
3. **Fetch all review threads** (open + resolved, to understand context):
```bash
gh api repos/{owner}/{repo}/pulls/$PR/comments --paginate
gh api repos/{owner}/{repo}/pulls/$PR/reviews --paginate
```
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

For GitHub pull request reviews, which API exposes thread-level fields like isResolved/isOutdated and supports resolving a thread: REST pulls comments endpoints or GraphQL pullRequest.reviewThreads + resolveReviewThread?

💡 Result:

GraphQL pullRequest.reviewThreads + resolveReviewThread exposes thread-level fields like isResolved/isOutdated and supports resolving a thread. REST pulls comments endpoints do not support thread-level fields or resolving threads.

Citations:


Switch to GraphQL pullRequest.reviewThreads API to expose thread status and resolution.

The current REST endpoints for fetching comments and reviews do not expose thread-level fields (isResolved, isOutdated) or provide mutation support for resolving threads. Use the GraphQL API to fetch review threads with status information and to resolve them via the resolveReviewThread mutation.

Suggested fix
- gh api repos/{owner}/{repo}/pulls/$PR/comments --paginate
- gh api repos/{owner}/{repo}/pulls/$PR/reviews --paginate
+ gh api graphql -f query='
+ query($owner:String!,$repo:String!,$pr:Int!) {
+   repository(owner:$owner,name:$repo) {
+     pullRequest(number:$pr) {
+       reviewThreads(first:100) {
+         nodes { id isResolved isOutdated comments(first:20){ nodes { id body path line author { login } } } }
+       }
+     }
+   }
+ }' -F owner='{owner}' -F repo='{repo}' -F pr=$PR

Also applies to: 54-63, 147-173

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

In @.agents/commands/review.md around lines 48 - 52, The current review commands
use REST endpoints (repos/{owner}/{repo}/pulls/$PR/comments and /reviews) which
don’t expose thread-level fields or support resolving threads; replace those
REST calls with GraphQL queries against pullRequest.reviewThreads (requesting
fields like id, isResolved, isOutdated, comments { body, author, createdAt })
and add usage examples for the resolveReviewThread mutation to mark threads
resolved; update every occurrence noted (the blocks around the existing fetch
commands and the sections at 54-63 and 147-173) to show the GraphQL query and
the resolveReviewThread mutation examples and to reference
pullRequest.reviewThreads and resolveReviewThread by name so maintainers can
locate and use them.

Comment on lines +110 to +113
**For NITPICK threads**:
- Fix silently (no reply needed unless the fix is non-obvious)
- If the nitpick is controversial or conflicts with a rule, note in the PR summary comment and leave it

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Align NITPICK reply behavior across phases.

Phase 2 says NITPICK can be fixed without reply, but Phase 4 requires replying to each acted thread. Keep one behavior to avoid contradictory agent output.

Also applies to: 162-173

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

In @.agents/commands/review.md around lines 110 - 113, The instructions under
the "**For NITPICK threads**" bullet are contradictory with the guidance in the
"Phase 2" vs "Phase 4" sections (notably the Phase 2 rule allowing silent fixes
and the Phase 4 rule requiring replies); pick one consistent behavior (either
"silent fix no reply" or "always reply when acted") and update all occurrences
to match: change the "**For NITPICK threads**" bullet and both the Phase 2 and
Phase 4 descriptions so they state the same policy, and adjust any example
wording in the nearby paragraphs (including the block referenced around lines
162-173) to use the chosen phrasing for consistency.

Comment on lines +83 to +99
```
Branch: feat/ENG-423-<slug>
Files: src/Facets/FooFacet.sol, script/deploy/facets/DeployFoo.s.sol
Tests: forge test --match-contract FooFacetTest
Lint: bunx solhint src/Facets/FooFacet.sol
```

---

## Phase 2 — IMPLEMENT

> *"Touch only what you must. Clean up only your own mess."* — Karpathy

1. **Branch** — always create a feature branch; never commit directly to `main`/`develop`:
```bash
git checkout -b feat/<TICKET-ID-lowercase>-<short-slug>
# e.g. feat/eng-423-add-foo-facet
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Unify branch-name casing across phases.

Phase 1 shows feat/ENG-423-... while Phase 2 mandates lowercase feat/eng-423-.... Use one format to avoid parser drift across /ticket, /review, and /fix-comment.

Suggested fix
-   Branch:   feat/ENG-423-<slug>
+   Branch:   feat/eng-423-<slug>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
```
Branch: feat/ENG-423-<slug>
Files: src/Facets/FooFacet.sol, script/deploy/facets/DeployFoo.s.sol
Tests: forge test --match-contract FooFacetTest
Lint: bunx solhint src/Facets/FooFacet.sol
```
---
## Phase 2 — IMPLEMENT
> *"Touch only what you must. Clean up only your own mess."* — Karpathy
1. **Branch** — always create a feature branch; never commit directly to `main`/`develop`:
```bash
git checkout -b feat/<TICKET-ID-lowercase>-<short-slug>
# e.g. feat/eng-423-add-foo-facet
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.agents/commands/ticket.md around lines 83 - 99, The branch name examples
are inconsistent: change the uppercase example "Branch:   feat/ENG-423-<slug>"
to use the lowercase convention to match the git command example and Phase 2
mandate; update the branch example line and any other occurrences of
"feat/ENG-423-..." or "ENG-423" in the same snippet to "feat/eng-423-<slug>" so
both the displayed Branch line and the git checkout example (git checkout -b
feat/<TICKET-ID-lowercase>-<short-slug>) uniformly use lowercase casing.

Comment thread .claude/settings.json
Comment on lines +16 to +25
"Bash(git checkout *)",
"Bash(git commit *)",
"Bash(git add *)",
"Bash(git push *)",
"Bash(git remote *)",
"Bash(bash -n *)",
"Bash(jq *)",
"Bash(wc *)",
"Bash(ls *)",
"Bash(gh *)",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Constrain shell permissions to least-privilege subcommands.

The new Bash(gh *) and Bash(git remote *) patterns are much broader than the documented workflow and allow unrelated high-impact operations. Please narrow these to explicit subcommands used by the skills.

Suggested tightening
-      "Bash(git remote *)",
+      "Bash(git remote get-url *)",
-      "Bash(gh *)",
+      "Bash(gh pr view *)",
+      "Bash(gh pr comment *)",
+      "Bash(gh api repos/*/pulls/*)",
+      "Bash(gh api repos/*/pulls/*/comments/*/replies*)"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"Bash(git checkout *)",
"Bash(git commit *)",
"Bash(git add *)",
"Bash(git push *)",
"Bash(git remote *)",
"Bash(bash -n *)",
"Bash(jq *)",
"Bash(wc *)",
"Bash(ls *)",
"Bash(gh *)",
"Bash(git checkout *)",
"Bash(git commit *)",
"Bash(git add *)",
"Bash(git push *)",
"Bash(git remote get-url *)",
"Bash(bash -n *)",
"Bash(jq *)",
"Bash(wc *)",
"Bash(ls *)",
"Bash(gh pr view *)",
"Bash(gh pr comment *)",
"Bash(gh api repos/*/pulls/*)",
"Bash(gh api repos/*/pulls/*/comments/*/replies*)"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.claude/settings.json around lines 16 - 25, The shell permission entries are
too broad—specifically the patterns "Bash(gh *)" and "Bash(git remote *)"—and
must be narrowed to least-privilege subcommands actually used by the skills;
replace the wildcard patterns with explicit allowed subcommands (e.g. only the
gh and git remote subcommands your code invokes such as gh pr create, gh pr
view, gh repo clone, git remote get-url) and remove the catch-all patterns,
updating the settings so only those exact Bash(...) entries remain; ensure the
unique symbols "Bash(gh *)" and "Bash(git remote *)" are replaced with the
precise commands your workflow needs.

…onfirmation steps

Updated the review and fix-comment commands to require explicit user confirmation before proceeding. Added a new step to present the analysis and inventory to the user, ensuring clarity and agreement before moving to the next phase. This change aims to improve communication and accuracy in the workflow process.
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.

♻️ Duplicate comments (3)
.agents/commands/review.md (2)

48-63: ⚠️ Potential issue | 🟠 Major

Switch to GraphQL API for thread-level operations.

The REST endpoints (/pulls/$PR/comments, /pulls/$PR/reviews) do not expose thread-level fields like isResolved or isOutdated (Line 61). To build the complete inventory with thread status and to support resolving threads in Phase 4, use the GraphQL pullRequest.reviewThreads query and resolveReviewThread mutation.

🔄 Proposed GraphQL replacement

Replace Lines 48-52 with:

-3. **Fetch all review threads** (open + resolved, to understand context):
-   ```bash
-   gh api repos/{owner}/{repo}/pulls/$PR/comments --paginate
-   gh api repos/{owner}/{repo}/pulls/$PR/reviews --paginate
-   ```
+3. **Fetch all review threads** (open + resolved, to understand context):
+   ```bash
+   gh api graphql -f query='
+   query($owner:String!, $repo:String!, $pr:Int!) {
+     repository(owner:$owner, name:$repo) {
+       pullRequest(number:$pr) {
+         reviewThreads(first:100) {
+           nodes {
+             id
+             isResolved
+             isOutdated
+             comments(first:20) {
+               nodes { id body path line author { login } }
+             }
+           }
+         }
+       }
+     }
+   }' -F owner='{owner}' -F repo='{repo}' -F pr=$PR
+   ```

Also add the resolveReviewThread mutation example in Phase 4 after replying to threads.

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

In @.agents/commands/review.md around lines 48 - 63, The docs currently use REST
endpoints (/pulls/$PR/comments and /pulls/$PR/reviews) which do not expose
thread-level fields like isResolved/isOutdated; update the steps to call the
GraphQL pullRequest.reviewThreads query to fetch thread nodes (id, isResolved,
isOutdated, comments { id, body, path, line, author { login } }) and replace the
REST examples with a gh api graphql example, and add usage of the
resolveReviewThread mutation in Phase 4 so threads can be resolved
programmatically.

112-114: ⚠️ Potential issue | 🟡 Minor

Align NITPICK reply behavior across phases.

Phase 2 states "Fix silently (no reply needed unless the fix is non-obvious)," but Phase 4 (Lines 164-174) requires replying to every thread acted on, including NITPICK threads with "Done." This contradiction will confuse the agent.

♻️ Proposed fix for consistency

Update Lines 112-114 to match Phase 4's behavior:

 **For NITPICK threads**:
-- Fix silently (no reply needed unless the fix is non-obvious)
+- Apply the fix and reply with "Done."
 - If the nitpick is controversial or conflicts with a rule, note in the PR summary comment and leave it
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.agents/commands/review.md around lines 112 - 114, Change Phase 2's
instruction from "Fix silently (no reply needed unless the fix is non-obvious)"
to match Phase 4's requirement to reply to every thread acted on (including
NITPICKs) by adding a short confirmation reply such as "Done" when the nitpick
is fixed; update the Phase 2 text so it explicitly states that NITPICK threads
must receive a reply after being fixed, mirroring the behavior described in
Phase 4 (Lines 164-174) and removing the contradiction between Phase 2 and Phase
4.
.agents/commands/fix-comment.md (1)

19-34: ⚠️ Potential issue | 🔴 Critical

Normalize COMMENT_ID before GitHub API calls.

The documentation accepts r1234567 format (Line 21), but the GitHub API call at Line 33 uses $COMMENT_ID directly without stripping the r prefix. GitHub's REST API requires numeric comment IDs; passing r1234567 will result in a 404 error.

🔧 Proposed fix

Add normalization and validation immediately after parsing the arguments:

 Parse both values; abort with a clear message if either is missing.
+
+Normalize the comment ID:
+```bash
+# Strip leading 'r' if present and validate numeric
+COMMENT_ID="${COMMENT_ID#r}"
+if ! [[ "$COMMENT_ID" =~ ^[0-9]+$ ]]; then
+  echo "Error: COMMENT_ID must be numeric (optionally prefixed with 'r')"
+  exit 1
+fi
+```

Apply the same normalized $COMMENT_ID at Line 106.

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

In @.agents/commands/fix-comment.md around lines 19 - 34, The script must
normalize and validate COMMENT_ID before any GitHub API calls: after parsing
arguments (where $ARGUMENTS and variables PR number and COMMENT_ID are
extracted) strip a leading 'r' from COMMENT_ID and ensure it contains only
digits, aborting with a clear error if not; then use this normalized COMMENT_ID
variable for the gh api call(s) (the usage at the fetch step and the later call
around Line 106 referenced in the comment) so the REST API receives a numeric
ID.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In @.agents/commands/fix-comment.md:
- Around line 19-34: The script must normalize and validate COMMENT_ID before
any GitHub API calls: after parsing arguments (where $ARGUMENTS and variables PR
number and COMMENT_ID are extracted) strip a leading 'r' from COMMENT_ID and
ensure it contains only digits, aborting with a clear error if not; then use
this normalized COMMENT_ID variable for the gh api call(s) (the usage at the
fetch step and the later call around Line 106 referenced in the comment) so the
REST API receives a numeric ID.

In @.agents/commands/review.md:
- Around line 48-63: The docs currently use REST endpoints (/pulls/$PR/comments
and /pulls/$PR/reviews) which do not expose thread-level fields like
isResolved/isOutdated; update the steps to call the GraphQL
pullRequest.reviewThreads query to fetch thread nodes (id, isResolved,
isOutdated, comments { id, body, path, line, author { login } }) and replace the
REST examples with a gh api graphql example, and add usage of the
resolveReviewThread mutation in Phase 4 so threads can be resolved
programmatically.
- Around line 112-114: Change Phase 2's instruction from "Fix silently (no reply
needed unless the fix is non-obvious)" to match Phase 4's requirement to reply
to every thread acted on (including NITPICKs) by adding a short confirmation
reply such as "Done" when the nitpick is fixed; update the Phase 2 text so it
explicitly states that NITPICK threads must receive a reply after being fixed,
mirroring the behavior described in Phase 4 (Lines 164-174) and removing the
contradiction between Phase 2 and Phase 4.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 12140018-2252-47e2-b0fc-3624ede91906

📥 Commits

Reviewing files that changed from the base of the PR and between 695a866 and 45e531b.

📒 Files selected for processing (2)
  • .agents/commands/fix-comment.md
  • .agents/commands/review.md

@mirooon mirooon changed the title feat(agents): add ticket, review, and fix-comment workflow skills TESTING - DO NOT MERGE - feat(agents): add ticket, review, and fix-comment workflow skills Apr 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants