TESTING - DO NOT MERGE - feat(agents): add ticket, review, and fix-comment workflow skills #1749
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,124 @@ | ||
| --- | ||
| name: fix-comment | ||
| description: Fix a single specific PR review comment/thread — understand, fix surgically, reply | ||
| usage: /fix-comment <PR-NUMBER> <COMMENT-ID> | ||
| --- | ||
|
|
||
| # Fix Single PR Comment (`/fix-comment`) | ||
|
|
||
| > **Usage**: `/fix-comment 312 r1234567` | ||
| > | ||
| > Focused single-thread version of `/review`. Reads exactly one comment, understands | ||
| > the full diff context around it, fixes or replies, pushes, marks resolved. | ||
| > Does not touch any other thread. | ||
|
|
||
| --- | ||
|
|
||
| ## Inputs | ||
|
|
||
| `$ARGUMENTS` — PR number and comment ID, space-separated. | ||
| - PR number: `312` | ||
| - 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 | ||
| ``` | ||
| Extract: body, file path, diff hunk (original_line, line), commit_id. | ||
|
|
||
| 2. **Resolve the Linear ticket** — extract ticket ID from branch name, PR title, or PR body | ||
| (e.g. `feat/eng-423-*` → `ENG-423`). If found, call `get_issue($ID)` and `list_comments($ID)` | ||
| to understand original intent and AC. Use when replying to QUESTION threads. | ||
|
|
||
| 3. **Read the diff context** — expand ±20 lines around the referenced line: | ||
| ```bash | ||
| gh api repos/{owner}/{repo}/pulls/$PR/files \ | ||
| --jq '.[] | select(.filename == "<file>") | .patch' | ||
| ``` | ||
| Also read the current file state from disk. | ||
|
|
||
| 3. **State exactly what the reviewer is asking** — write it out in one sentence. | ||
| If the intent is ambiguous: | ||
| - Post a reply asking one focused clarifying question | ||
| - Stop — do not guess at the fix | ||
|
|
||
| 4. **Classify the request**: | ||
| - **BLOCKER** — correctness/security issue; must fix | ||
| - **CHANGE** — explicit edit; clear what to do | ||
| - **QUESTION** — asking why; answer in reply, code change only if "this is wrong" | ||
| - **NITPICK** — optional polish; fix only if unambiguous | ||
|
|
||
| 5. **Present the analysis and stop**. Show: what the reviewer is asking, the classification, and your proposed fix (or reply text). Ask: | ||
| > "Does this look right? Say yes to proceed, or give me corrections." | ||
| Wait for explicit confirmation before Phase 1. | ||
|
|
||
| --- | ||
|
|
||
| ## Phase 1 — FIX | ||
|
|
||
| > *"Touch only what you must. Clean up only your own mess."* — Karpathy | ||
|
|
||
| **For BLOCKER / CHANGE**: | ||
| - Edit exactly the lines referenced by the comment — no more, no less | ||
| - No adjacent cleanup; no unasked renames; match existing style | ||
| - If fixing this line requires touching a dependency, note that explicitly before editing | ||
|
|
||
| **For QUESTION**: | ||
| - Draft the reply text; only edit code if the reply would be "this is wrong" | ||
|
|
||
| **For NITPICK**: | ||
| - Apply the polish if clear; skip if controversial (note in reply) | ||
|
|
||
| Run the relevant verify checks immediately after the change: | ||
|
|
||
| | Changed files | Command | | ||
| |---|---| | ||
| | `src/**/*.sol` | `bunx solhint <file>` + targeted `forge test` | | ||
| | `script/**/*.ts` | `bunx tsc-files --noEmit <file>` + `bunx eslint <file>` | | ||
| | Formatting | `bun format:fix` | | ||
|
|
||
| Do not proceed to Phase 2 if any check fails. | ||
|
|
||
| --- | ||
|
|
||
| ## Phase 2 — REPLY | ||
|
|
||
| 1. **Commit**: | ||
| ```bash | ||
| git commit -m "fix: address comment $COMMENT_ID on PR #$PR" | ||
| ``` | ||
|
|
||
| 2. **Push**: | ||
| ```bash | ||
| git push | ||
| ``` | ||
|
|
||
| 3. **Reply to the thread**: | ||
| ```bash | ||
| gh api repos/{owner}/{repo}/pulls/$PR/comments/$COMMENT_ID/replies \ | ||
| -f body="<reply>" | ||
| ``` | ||
| Reply format: | ||
| - **Fixed**: "Fixed in `<short-sha>`. <one line: what changed and why>." | ||
| - **Question answered**: "<explanation>. Let me know if you'd like more context." | ||
| - **Nitpick fixed**: "Done." | ||
| - **Skipped**: "Left as-is — <one-sentence reason>. Happy to revisit if you feel strongly." | ||
|
|
||
| --- | ||
|
|
||
| ## Abort Conditions | ||
|
|
||
| - Comment ID not found → report the exact API error; do not guess another ID | ||
| - Intent is ambiguous → post a clarifying question; stop | ||
| - Comment references a specific on-chain tx hash → run `/analyze-tx <network> <hash>` first; | ||
| use the trace as source of truth before attempting any fix | ||
| - Fix requires Diamond storage layout change → stop, flag to user | ||
| - Tests fail after 1 fix attempt → report exact failure; stop | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,215 @@ | ||
| --- | ||
| name: review | ||
| description: Address all open PR review threads — categorize, triage, fix/reply, verify, push, mark resolved | ||
| usage: /review <PR-NUMBER> | ||
| --- | ||
|
|
||
| # PR Review — Address Comments (`/review`) | ||
|
|
||
| > **Usage**: `/review 312` | ||
| > | ||
| > Reads every open review thread on the PR, classifies each one, implements fixes | ||
| > in priority order, replies to each thread, and pushes — without touching anything | ||
| > beyond what reviewers explicitly asked for. | ||
|
|
||
| --- | ||
|
|
||
| ## Inputs | ||
|
|
||
| `$ARGUMENTS` — a GitHub PR number (e.g. `312`). | ||
|
|
||
| Determine the repo from `git remote get-url origin`. Abort with a clear message if the | ||
| remote is not a GitHub URL. | ||
|
|
||
| --- | ||
|
|
||
| ## Phase 0 — INVENTORY (before touching any file) | ||
|
|
||
| > *"Don't assume. Don't hide confusion. Surface tradeoffs."* — Karpathy | ||
|
|
||
| 1. **Fetch PR metadata**: | ||
| ```bash | ||
| gh pr view $PR --json title,headRefName,baseRefName,body,author | ||
| ``` | ||
|
|
||
| 2. **Resolve the Linear ticket** — extract the ticket ID from the PR (in priority order): | ||
| - Branch name (e.g. `feat/eng-423-add-foo` → `ENG-423`) | ||
| - PR title (e.g. `ENG-423: add FooFacet`) | ||
| - PR body (look for `Closes ENG-423`, `Fixes ENG-423`, or a bare `ENG-423` reference) | ||
|
|
||
| If a ticket ID is found: | ||
| - `get_issue($ID)` — title, description, acceptance criteria, status | ||
| - `list_comments($ID)` — prior discussion, design decisions, clarifications | ||
|
|
||
| Use this context throughout: when a reviewer questions *why* something was done, | ||
| the ticket AC and discussion is the authoritative source for intent. | ||
| If no ticket ID is found, proceed without — note it in the Phase 1 triage output. | ||
|
|
||
| 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 | ||
| ``` | ||
|
Comment on lines
+48
to
+52
Contributor
There was a problem hiding this comment. 🧩 Analysis chain🌐 Web query:
💡 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 The current REST endpoints for fetching comments and reviews do not expose thread-level fields ( 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=$PRAlso applies to: 54-63, 147-173 🤖 Prompt for AI Agents |
||
|
|
||
| 3. **Build the thread inventory** — for every comment thread, record: | ||
| | Field | Value | | ||
| |---|---| | ||
| | Thread ID | `r<id>` | | ||
| | File + line | `src/Facets/Foo.sol:42` | | ||
| | Reviewer | `@handle` | | ||
| | Body | (first 2 lines) | | ||
| | Status | open / resolved | | ||
| | Type | BLOCKER / CHANGE / QUESTION / NITPICK | | ||
|
|
||
| 4. **Type classification rules**: | ||
| - **BLOCKER** — breaks correctness, safety, or security; "this will X"; "bug:"; "wrong"; direct security concern; must be fixed before merge | ||
| - **CHANGE** — explicit edit request: naming, structure, docs, style; typically clear what to do | ||
| - **QUESTION** — asking why; asking for explanation; "why does this..."; "what is..." | ||
| - **NITPICK** — prefixed with "nit:", "minor:", "optional:", "feel free to ignore"; polish only | ||
|
|
||
| When type is unclear, prefer the higher-severity classification. | ||
|
|
||
| 5. **Scan for tx-hash escalation triggers** — before classifying threads, check if any | ||
| thread references a specific transaction hash or says "this tx reverts / fails on-chain". | ||
| If found: run `/analyze-tx <network> <hash>` first. The trace output is the source of | ||
| truth for understanding the root cause; use it to inform your fix and your reply. | ||
|
|
||
| 6. **Print the inventory** to the user and **stop**. Ask: | ||
| > "Does this inventory look correct? Any threads to add, remove, or reclassify before I start fixing?" | ||
| Wait for explicit confirmation before proceeding to Phase 1. | ||
|
|
||
| --- | ||
|
|
||
| ## Phase 1 — TRIAGE | ||
|
|
||
| 1. **Surface threads requiring a decision** — any BLOCKER where the right fix is | ||
| ambiguous. Ask **one focused question** per ambiguous BLOCKER. Wait for answer | ||
| before implementing that thread. | ||
|
|
||
| 2. **Fix order**: `BLOCKER` → `CHANGE` → `QUESTION replies` → `NITPICK` | ||
|
|
||
| 3. **Skip conditions** (document in final summary, do not silently omit): | ||
| - Thread already resolved (race condition) | ||
| - Reviewer explicitly said "optional, up to you" — note as skipped, explain why | ||
| - Thread conflicts with another — flag to user before picking a resolution | ||
|
|
||
| --- | ||
|
|
||
| ## Phase 2 — IMPLEMENT | ||
|
|
||
| > *"Touch only what you must. Clean up only your own mess."* — Karpathy | ||
|
|
||
| **For BLOCKER / CHANGE threads**: | ||
| - Edit exactly the lines the reviewer references — no more, no less | ||
| - Do not refactor adjacent code that works correctly | ||
| - Do not rename things beyond the reviewer's explicit ask | ||
| - Match existing style; do not introduce new patterns unless asked | ||
|
|
||
| **For QUESTION threads**: | ||
| - Draft a reply that explains the reasoning (reference file/line if helpful) | ||
| - Only make a code change if the explanation would be "this is wrong" — in which case reclassify as BLOCKER/CHANGE | ||
|
|
||
| **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 | ||
|
|
||
|
Comment on lines
+112
to
+115
Contributor
There was a problem hiding this comment. 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 |
||
| **No new abstractions** — do not extract helpers, introduce shared utilities, or add | ||
| indirection unless at least two independent reviewer comments independently request it. | ||
|
|
||
| **Rule-anchored responses** — when a reviewer raises a domain concern, ground your reply | ||
| in the relevant rule rather than improvising: | ||
|
|
||
| | Reviewer raises | Use this anchor | What to say / check | | ||
| |---|---|---| | ||
| | Gas / efficiency concern | `106-gas` `[CONV:GAS]` | Prefer existing Solady/Solmate patterns; surface tradeoffs, don't micro-optimize silently | | ||
| | Security / access control | `105-security` | Check `Validatable`, `LibAsset`, `LibSwap`, `LibAllowList` for prior art; state governance impact | | ||
| | Architecture / Diamond concern | `002-architecture` `[CONV:ARCH-DIAMOND]` | Storage slots, selector layout, facet separation | | ||
| | Wrong event location | `002-architecture` `[CONV:EVENTS]` | `LiFiTransferStarted` only in `_startBridge`; `LiFiTransferCompleted` only in Executor | | ||
| | Whitelist file in wrong PR | `502-whitelist-branching` | PR must target `main`; split if targeting feature branch | | ||
| | Failing on-chain tx | `600-transaction-analysis` | Run `/analyze-tx <network> <hash>` first; reply with trace findings | | ||
|
|
||
| --- | ||
|
|
||
| ## Phase 3 — VERIFY | ||
|
|
||
| Run all checks relevant to files you changed. **State exact output; do not claim clean without running.** | ||
|
|
||
| | Changed files | Command | | ||
| |---|---| | ||
| | `src/**/*.sol` | `forge test` (full suite or targeted) | | ||
| | `script/**/*.ts`, `tasks/**/*.ts` | `bun test:ts` + `bunx tsc-files --noEmit <files>` | | ||
| | Any `.sol` | `bunx solhint <file>` | | ||
| | Any `.ts`/`.js` | `bunx eslint <file>` | | ||
| | Formatting | `bun format:fix` | | ||
|
|
||
| If any check fails, fix it before Phase 4. Do not push a red build. | ||
|
|
||
| --- | ||
|
|
||
| ## Phase 4 — PUBLISH | ||
|
|
||
| > *"Define success criteria. Loop until verified."* — Karpathy | ||
|
|
||
| 1. **Commit**: | ||
| ```bash | ||
| git commit -m "address PR #$PR review comments" | ||
| ``` | ||
| If fixes naturally split into logical units, use multiple commits with descriptive messages. | ||
|
|
||
| 2. **Push**: | ||
| ```bash | ||
| git push | ||
| ``` | ||
|
|
||
| 3. **Reply to each thread** — for every thread you acted on: | ||
| ```bash | ||
| # Reply to a comment thread | ||
| gh api repos/{owner}/{repo}/pulls/$PR/comments/$COMMENT_ID/replies \ | ||
| -f body="<reply text>" | ||
| ``` | ||
| Reply format by type: | ||
| - **BLOCKER/CHANGE fixed**: "Fixed in <commit sha short>. <one-line description of what changed>." | ||
| - **QUESTION answered**: "<explanation>. Happy to clarify further if needed." | ||
| - **NITPICK fixed**: "Done." (brief is fine for nitpicks) | ||
| - **NITPICK skipped**: "Left as-is — <one-sentence reason>." | ||
|
|
||
| 4. **Post a PR summary comment**: | ||
| ```bash | ||
| gh pr comment $PR --body "..." | ||
| ``` | ||
| ```markdown | ||
| ## Review comments addressed | ||
|
|
||
| | Thread | File | Type | Action | | ||
| |---|---|---|---| | ||
| | r12345 | `src/Foo.sol:42` | CHANGE | Fixed — renamed to `fooBar` | | ||
| | r12346 | `src/Bar.sol:10` | QUESTION | Replied — explained reentrancy guard intent | | ||
| | r12347 | `src/Baz.sol:5` | NITPICK | Fixed silently | | ||
|
|
||
| **Tests**: forge test — 42 passed, 0 failed | ||
| **Lint**: solhint — no issues | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
| ## LI.FI Conventions (apply to any files you touch) | ||
|
|
||
| - SPDX-License-Identifier: LGPL-3.0-only on all `.sol` files | ||
| - NatSpec on all public/external functions you modify | ||
| - `viem` only in TypeScript (no `ethers`) | ||
| - No Diamond storage layout changes without explicit ticket | ||
| - No governance bypass (Safe/timelock) | ||
|
|
||
| --- | ||
|
|
||
| ## Abort Conditions | ||
|
|
||
| - A BLOCKER's required fix is unclear and reviewer is unresponsive → post a comment | ||
| asking for clarification; do not guess; do not push | ||
| - A fix requires changing the Diamond storage layout → stop, flag to user | ||
| - Tests fail after 2 fix attempts → stop, report exact failure to user | ||
|
|
||
| ## Hard Rules | ||
|
|
||
| - **Never approve the PR** (`gh pr review --approve` is forbidden). Post a summary comment only. | ||
| - **Never push or reply without user confirmation** — the Phase 0 inventory gate must be passed first. | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 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 -120Repository: lifinance/contracts
Length of output: 4821
Normalize
COMMENT_IDbefore GitHub API calls.Documentation at line 21 accepts
r1234567, but the API calls at lines 33 and 102 pass$COMMENT_IDdirectly. GitHub REST API requires numeric comment IDs only; therprefix must be stripped first, or the documented input format will fail with an API error.Suggested fix
Apply the same normalization before line 102's API call.
📝 Committable suggestion
🤖 Prompt for AI Agents