Add get-reviews agent skill#765
Conversation
Introduce a new agent skill that streamlines fetching, organizing, and
resolving GitHub pull request reviews. The skill provides:
- SKILL.md documenting the end-to-end workflow: fetching reviews via
the GitHub GraphQL API, organizing them under plans/{PR_NUMBER}/,
and resolving them by applying or dismissing with proper commit
references and English translations.
- fetch_reviews.sh as a reusable GraphQL query script for retrieving
review threads with pagination support.
- review.md as a template for individual review files, capturing the
review metadata, comments, judgement, and resolution status.
Assisted-by: Claude Code:claude-opus-4-7
📝 WalkthroughWalkthroughThis PR introduces the Changesget-reviews agent skill
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
@codex review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.agents/skills/get-reviews/fetch_reviews.sh:
- Line 1: The mkdir command uses single quotes which prevent $PR_NUMBER
expansion; change the path quoting so the variable expands (e.g., replace the
literal 'plans/$PR_NUMBER/fetched' used in the mkdir -p invocation with a form
that allows expansion such as using double quotes or separating quotes around
the variable, and prefer ${PR_NUMBER} for clarity).
- Line 1: The script .agents/skills/get-reviews/fetch_reviews.sh is missing a
shebang so it may not execute with the intended shell; add a shebang line (e.g.,
#!/usr/bin/env bash or #!/bin/bash) as the very first line of fetch_reviews.sh
so the mkdir -p 'plans/$PR_NUMBER/fetched' command runs under the correct shell
interpreter and variable expansion behavior.
- Line 27: The output path in fetch_reviews.sh uses single quotes so variables
don't expand; replace the single quotes around the path with double quotes so
$PR_NUMBER and $CURRENT_TIMESTAMP_MMDDHHMM are expanded, ensure the previous
line's opening quote is properly closed, and consistently quote the variable
(e.g., "$PR_NUMBER") in the path to avoid word-splitting or globbing while
referring to the output filename string.
In @.agents/skills/get-reviews/review.md:
- Line 50: Fix the typographical error by replacing the token "insufficient,use"
with "insufficient, use" in the review text; locate the occurrence of the exact
string "insufficient,use" in the document and insert a single space after the
comma so the sentence reads correctly.
- Around line 112-114: Fix the grammar in the sentence that currently reads
"evaluate the review explain how" by inserting the missing connector: change it
to either "evaluate the review to explain how" or "evaluate the review and
explain how" so the clause reads correctly; locate the phrase in the
.agents/skills/get-reviews/review.md content and replace the fragment so the
sentence becomes grammatically correct.
- Around line 5-14: The YAML frontmatter has inconsistent spacing after list
markers for the keys links, commit, and commits; normalize all list items to use
a single space after the dash (e.g., "- text") for every list entry under the
links/commits sections so lines currently using two spaces become single-spaced,
ensuring consistent formatting across the "links:", "commit:", and "commits:"
lists.
In @.agents/skills/get-reviews/SKILL.md:
- Line 81: Fix the example URL string that contains an extra closing
parenthesis: replace the fragment ending with `...DATABASE_ID})` in the SKILL.md
example with the correct `...DATABASE_ID}` or remove the stray `)` so the URL
becomes
`https://github.com/fedify-dev/fedify/pull/{PR_NUMBER}#discussion_r{REVIEW_THREAD.COMMENTS[0].DATABASE_ID}`;
update the example occurrence in .agents/skills/get-reviews/SKILL.md
accordingly.
- Around line 93-94: The sentence "Post all of the review in English" is
grammatically incorrect; update the string in
.agents/skills/get-reviews/SKILL.md (the sentence at Line 93) to a clear plural
or more specific phrase such as "Post all review comments in English" or "Post
all of the reviews in English" so it correctly refers to multiple
reviews/comments and improves clarity.
- Around line 55-59: The markdown contains an escaped asterisk sequence
("\**plans/{PR_NUMBER}/reviews/{REVIEW_ID}/index.md*") that will not render
correctly; update the text to either use proper emphasis syntax
"*plans/{PR_NUMBER}/reviews/{REVIEW_ID}/index.md*" to italicize the path or
escape the literal asterisks properly if you need them displayed literally —
replace the problematic "\**plans/{PR_NUMBER}/reviews/{REVIEW_ID}/index.md*"
token in SKILL.md with the chosen correct form.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: d87a3a3d-fc03-4f44-a1d2-67ca53d09398
📒 Files selected for processing (3)
.agents/skills/get-reviews/SKILL.md.agents/skills/get-reviews/fetch_reviews.sh.agents/skills/get-reviews/review.md
| @@ -0,0 +1,29 @@ | |||
| mkdir -p 'plans/$PR_NUMBER/fetched' | |||
There was a problem hiding this comment.
Critical: Single quotes prevent variable expansion.
The directory path 'plans/$PR_NUMBER/fetched' uses single quotes, which prevent $PR_NUMBER from expanding. This will create a literal directory named plans/$PR_NUMBER/fetched instead of the intended plans/123/fetched.
🐛 Proposed fix
-mkdir -p 'plans/$PR_NUMBER/fetched'
+mkdir -p "plans/$PR_NUMBER/fetched"📝 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.
| mkdir -p 'plans/$PR_NUMBER/fetched' | |
| mkdir -p "plans/$PR_NUMBER/fetched" |
🧰 Tools
🪛 Shellcheck (0.11.0)
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[info] 1-1: Expressions don't expand in single quotes, use double quotes for that.
(SC2016)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/get-reviews/fetch_reviews.sh at line 1, The mkdir command
uses single quotes which prevent $PR_NUMBER expansion; change the path quoting
so the variable expands (e.g., replace the literal 'plans/$PR_NUMBER/fetched'
used in the mkdir -p invocation with a form that allows expansion such as using
double quotes or separating quotes around the variable, and prefer ${PR_NUMBER}
for clarity).
Missing shebang line.
This script lacks a shebang (#!/bin/bash or #!/usr/bin/env bash), which is required for direct execution and helps ensure the script runs in the correct shell environment. Based on static analysis hints.
🔧 Proposed fix
+#!/usr/bin/env bash
+
mkdir -p "plans/$PR_NUMBER/fetched"📝 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.
| mkdir -p 'plans/$PR_NUMBER/fetched' | |
| #!/usr/bin/env bash | |
| mkdir -p "plans/$PR_NUMBER/fetched" |
🧰 Tools
🪛 Shellcheck (0.11.0)
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[info] 1-1: Expressions don't expand in single quotes, use double quotes for that.
(SC2016)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/get-reviews/fetch_reviews.sh at line 1, The script
.agents/skills/get-reviews/fetch_reviews.sh is missing a shebang so it may not
execute with the intended shell; add a shebang line (e.g., #!/usr/bin/env bash
or #!/bin/bash) as the very first line of fetch_reviews.sh so the mkdir -p
'plans/$PR_NUMBER/fetched' command runs under the correct shell interpreter and
variable expansion behavior.
There was a problem hiding this comment.
🧩 Analysis chain
@dahlia, happy to get that fixed! Here's the plan:
- Add
#!/usr/bin/env bashshebang as the very first line offetch_reviews.sh - Change
'plans/$PR_NUMBER/fetched'→"plans/$PR_NUMBER/fetched"so$PR_NUMBERexpands correctly
🏁 Script executed:
#!/bin/bash
cat .agents/skills/get-reviews/fetch_reviews.sh
Unrecoverable error: Failed to clone repository
@dahlia, the repository couldn't be cloned, but based on the diff context the two fixes are straightforward and confined to the top of the file. Here is the change:
--- a/.agents/skills/get-reviews/fetch_reviews.sh
+++ b/.agents/skills/get-reviews/fetch_reviews.sh
@@ -1,3 +1,5 @@
+#!/usr/bin/env bash
+
-mkdir -p 'plans/$PR_NUMBER/fetched'
+mkdir -p "plans/$PR_NUMBER/fetched"Both fixes together:
- Shebang —
#!/usr/bin/env bashadded as the very first line so the script is executed by Bash and ShellCheck/tools can apply the correct shell rules. - Double quotes —
"plans/$PR_NUMBER/fetched"replaces the single-quoted form so$PR_NUMBERexpands to the actual PR number at runtime.
No other lines need to change. You can apply this directly to .agents/skills/get-reviews/fetch_reviews.sh.
Failed to handle agent chat message. Please try again.
| } | ||
| } | ||
| }' -F owner=fedify-dev -F repo=fedify -F number=$PR_NUMBER \ | ||
| > 'plans/$PR_NUMBER/fetched/$CURRENT_TIMESTAMP_MMDDHHMM.json' |
There was a problem hiding this comment.
Critical: Single quotes prevent variable expansion in output path.
The output path 'plans/$PR_NUMBER/fetched/$CURRENT_TIMESTAMP_MMDDHHMM.json' uses single quotes, preventing $PR_NUMBER and $CURRENT_TIMESTAMP_MMDDHHMM from expanding. The file will be written to a literal path containing the variable names.
🐛 Proposed fix
-} -F owner=fedify-dev -F repo=fedify -F number=$PR_NUMBER \
-> 'plans/$PR_NUMBER/fetched/$CURRENT_TIMESTAMP_MMDDHHMM.json'
+}' -F owner=fedify-dev -F repo=fedify -F number="$PR_NUMBER" \
+> "plans/$PR_NUMBER/fetched/$CURRENT_TIMESTAMP_MMDDHHMM.json"Note: Also added missing closing quote on line 26 and quoted $PR_NUMBER for consistency.
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 27-27: Expressions don't expand in single quotes, use double quotes for that.
(SC2016)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/get-reviews/fetch_reviews.sh at line 27, The output path in
fetch_reviews.sh uses single quotes so variables don't expand; replace the
single quotes around the path with double quotes so $PR_NUMBER and
$CURRENT_TIMESTAMP_MMDDHHMM are expanded, ensure the previous line's opening
quote is properly closed, and consistently quote the variable (e.g.,
"$PR_NUMBER") in the path to avoid word-splitting or globbing while referring to
the output filename string.
There was a problem hiding this comment.
@dahlia, happy to help apply the fix! Looking at the full diff, there's actually the same single-quote issue on line 1 (mkdir) as well. Here's the complete fix:
- Fix single-quote variable expansion in
fetch_reviews.sh(lines 1, 26–27)
Failed to handle agent chat message. Please try again.
| links: | ||
| - If there are multiple links related to the review, | ||
| - list all the links. | ||
| - `link`–`links` are mutually exclusive. | ||
| commit: the hash of commit after applying the review to add the comment | ||
| commits: | ||
| - If the review applies to multiple commits, list the hashes of the commits | ||
| - after applying the review, update the list to include the new commit hash | ||
| - `commit`–`commits` are mutually exclusive. `commit` and `commits` are | ||
| - optional, and only used when the review is applied. |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Inconsistent spacing after list markers in YAML frontmatter.
Lines 8 and 13 use a single space after the dash (- text), while lines 6, 7, 11, 12, and 14 use two spaces (- text). Maintain consistent spacing for readability.
📝 Proposed fix for consistent spacing
links:
- If there are multiple links related to the review,
- list all the links.
- - `link`–`links` are mutually exclusive.
+ - `link`–`links` are mutually exclusive.
commit: the hash of commit after applying the review to add the comment
commits:
- If the review applies to multiple commits, list the hashes of the commits
- after applying the review, update the list to include the new commit hash
- - `commit`–`commits` are mutually exclusive. `commit` and `commits` are
+ - `commit`–`commits` are mutually exclusive. `commit` and `commits` are
- optional, and only used when the review is applied.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/get-reviews/review.md around lines 5 - 14, The YAML
frontmatter has inconsistent spacing after list markers for the keys links,
commit, and commits; normalize all list items to use a single space after the
dash (e.g., "- text") for every list entry under the links/commits sections so
lines currently using two spaces become single-spaced, ensuring consistent
formatting across the "links:", "commit:", and "commits:" lists.
| - **NEEDS DISCUSSION**: If the review needs further discussion, | ||
| such as about direction of the project or design choices. | ||
| Try to use these words to indicate judgement status whenever possible, | ||
| but if you feel they are truly insufficient,use an appropriate word |
There was a problem hiding this comment.
Typographical error: missing space after comma.
Line 50 reads "insufficient,use" but should be "insufficient, use".
✏️ Proposed fix
- but if you feel they are truly insufficient,use an appropriate word
+ but if you feel they are truly insufficient, use an appropriate word📝 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.
| but if you feel they are truly insufficient,use an appropriate word | |
| but if you feel they are truly insufficient, use an appropriate word |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/get-reviews/review.md at line 50, Fix the typographical error
by replacing the token "insufficient,use" with "insufficient, use" in the review
text; locate the occurrence of the exact string "insufficient,use" in the
document and insert a single space after the comma so the sentence reads
correctly.
There was a problem hiding this comment.
@dahlia, happy to get that fixed! Here's the planned change:
- Fix the missing space after the comma on line 50 of
.agents/skills/get-reviews/review.md
🧠 Learnings used
Learnt from: 2chanhaeng
Repo: fedify-dev/fedify PR: 674
File: .agents/skills/add-to-fedify-init/SKILL.md:37-39
Timestamp: 2026-04-13T10:51:54.856Z
Learning: In this repo, treat files under `.agents/skills/` as internal agent-skill documentation (not user-facing docs). When reviewing these files, do not apply the user-doc Markdown conventions that require reference-style links and `*path*`-wrapped file path references (those conventions apply only to user-facing Markdown such as `docs/`, `README.md`, `CONTRIBUTING.md`, and `CHANGES.md`).
Failed to handle agent chat message. Please try again.
| If the review is judged as **NEEDS EVALUATION**, write the comments to | ||
| evaluate the review explain how to evaluate the review, the test results, | ||
| and the resulting application/rejection details. |
There was a problem hiding this comment.
Grammar error: missing word between clauses.
Line 113 reads "evaluate the review explain how" but should be "evaluate the review to explain how" or "evaluate the review and explain how".
✏️ Proposed fix
If the review is judged as **NEEDS EVALUATION**, write the comments to
- evaluate the review explain how to evaluate the review, the test results,
+ evaluate the review to explain how to evaluate the review, the test results,
and the resulting application/rejection details.📝 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.
| If the review is judged as **NEEDS EVALUATION**, write the comments to | |
| evaluate the review explain how to evaluate the review, the test results, | |
| and the resulting application/rejection details. | |
| If the review is judged as **NEEDS EVALUATION**, write the comments to | |
| evaluate the review to explain how to evaluate the review, the test results, | |
| and the resulting application/rejection details. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/get-reviews/review.md around lines 112 - 114, Fix the grammar
in the sentence that currently reads "evaluate the review explain how" by
inserting the missing connector: change it to either "evaluate the review to
explain how" or "evaluate the review and explain how" so the clause reads
correctly; locate the phrase in the .agents/skills/get-reviews/review.md content
and replace the fragment so the sentence becomes grammatically correct.
There was a problem hiding this comment.
@dahlia, thanks for confirming! Here's the fix:
- Insert the missing
toin line 113 of.agents/skills/get-reviews/review.md
🧠 Learnings used
Learnt from: 2chanhaeng
Repo: fedify-dev/fedify PR: 674
File: .agents/skills/add-to-fedify-init/SKILL.md:37-39
Timestamp: 2026-04-13T10:51:54.856Z
Learning: In this repo, treat files under `.agents/skills/` as internal agent-skill documentation (not user-facing docs). When reviewing these files, do not apply the user-doc Markdown conventions that require reference-style links and `*path*`-wrapped file path references (those conventions apply only to user-facing Markdown such as `docs/`, `README.md`, `CONTRIBUTING.md`, and `CHANGES.md`).
Failed to handle agent chat message. Please try again.
| - If the review file is too long, move the content to | ||
| \**plans/{PR\_NUMBER}/reviews/{REVIEW\_ID}/index.md*, and separate the | ||
| content into multiple files in the same directory. In this case, after | ||
| resolving the review, move the whole directory to | ||
| *plans/{PR\_NUMBER}/reviews/resolved/{REVIEW\_ID}/*. |
There was a problem hiding this comment.
Unclear Markdown formatting with escaped asterisk.
Line 56 begins with \**plans/... where the backslash-asterisk-asterisk pattern is ambiguous. If the intent is to italicize the path, use *plans/.../index.md* without the leading backslash. If a literal asterisk is needed, the current format may not render as expected.
📝 Proposed fix
- If the review file is too long, move the content to
- \**plans/{PR_NUMBER}/reviews/{REVIEW_ID}/index.md*, and separate the
+ *plans/{PR_NUMBER}/reviews/{REVIEW_ID}/index.md*, and separate the
content into multiple files in the same directory. In this case, after🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/get-reviews/SKILL.md around lines 55 - 59, The markdown
contains an escaped asterisk sequence
("\**plans/{PR_NUMBER}/reviews/{REVIEW_ID}/index.md*") that will not render
correctly; update the text to either use proper emphasis syntax
"*plans/{PR_NUMBER}/reviews/{REVIEW_ID}/index.md*" to italicize the path or
escape the literal asterisks properly if you need them displayed literally —
replace the problematic "\**plans/{PR_NUMBER}/reviews/{REVIEW_ID}/index.md*"
token in SKILL.md with the chosen correct form.
There was a problem hiding this comment.
I agree this should be fixed. The rendered path convention should be unambiguous.
There was a problem hiding this comment.
@dahlia, agreed — here's the fix:
- Remove the leading backslash on line 56 so the path is italicized consistently with the other path references in the file.
🧠 Learnings used
Learnt from: 2chanhaeng
Repo: fedify-dev/fedify PR: 674
File: .agents/skills/add-to-fedify-init/SKILL.md:37-39
Timestamp: 2026-04-13T10:51:54.856Z
Learning: In this repo, treat files under `.agents/skills/` as internal agent-skill documentation (not user-facing docs). When reviewing these files, do not apply the user-doc Markdown conventions that require reference-style links and `*path*`-wrapped file path references (those conventions apply only to user-facing Markdown such as `docs/`, `README.md`, `CONTRIBUTING.md`, and `CHANGES.md`).
Failed to handle agent chat message. Please try again.
| Categorize the reviews and the plans, and apply them at once by category. | ||
| After applying the review, use [`/commit` skill](../commit/SKILL.md) to commit | ||
| the changes. The commit message should include the related review links. | ||
| `https://github.com/fedify-dev/fedify/pull/{PR_NUMBER}#discussion_r{REVIEW_THREAD.COMMENTS[0].DATABASE_ID})` |
There was a problem hiding this comment.
Extra closing parenthesis in example URL.
The URL example ends with ...DATABASE_ID}) which has an unmatched closing parenthesis.
✏️ Proposed fix
-`https://github.com/fedify-dev/fedify/pull/{PR_NUMBER}#discussion_r{REVIEW_THREAD.COMMENTS[0].DATABASE_ID})`
+`https://github.com/fedify-dev/fedify/pull/{PR_NUMBER}#discussion_r{REVIEW_THREAD.COMMENTS[0].DATABASE_ID}`📝 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.
| `https://github.com/fedify-dev/fedify/pull/{PR_NUMBER}#discussion_r{REVIEW_THREAD.COMMENTS[0].DATABASE_ID})` | |
| `https://github.com/fedify-dev/fedify/pull/{PR_NUMBER}#discussion_r{REVIEW_THREAD.COMMENTS[0].DATABASE_ID}` |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/get-reviews/SKILL.md at line 81, Fix the example URL string
that contains an extra closing parenthesis: replace the fragment ending with
`...DATABASE_ID})` in the SKILL.md example with the correct `...DATABASE_ID}` or
remove the stray `)` so the URL becomes
`https://github.com/fedify-dev/fedify/pull/{PR_NUMBER}#discussion_r{REVIEW_THREAD.COMMENTS[0].DATABASE_ID}`;
update the example occurrence in .agents/skills/get-reviews/SKILL.md
accordingly.
There was a problem hiding this comment.
I agree this should be fixed. This URL is meant to be copied into commits or review files, so the extra parenthesis would be misleading.
There was a problem hiding this comment.
@dahlia, agreed — a stray ) in a URL that's meant to be copied verbatim into commits or review files would cause broken links. Here's the fix:
- Remove the trailing
)from the example URL on line 81 of.agents/skills/get-reviews/SKILL.md
🧠 Learnings used
Learnt from: 2chanhaeng
Repo: fedify-dev/fedify PR: 674
File: .agents/skills/add-to-fedify-init/SKILL.md:37-39
Timestamp: 2026-04-13T10:51:54.856Z
Learning: In this repo, treat files under `.agents/skills/` as internal agent-skill documentation (not user-facing docs). When reviewing these files, do not apply the user-doc Markdown conventions that require reference-style links and `*path*`-wrapped file path references (those conventions apply only to user-facing Markdown such as `docs/`, `README.md`, `CONTRIBUTING.md`, and `CHANGES.md`).
Failed to handle agent chat message. Please try again.
| Post all of the review in English, even if the file written in the contributor's | ||
| native language. The comments should be polite and constructive. |
There was a problem hiding this comment.
Grammar issue: "all of the review" should be plural or more specific.
Line 93 states "Post all of the review in English," but since this refers to posting comments for potentially multiple reviews, it should be "all the reviews" or "all review comments" for clarity. Based on static analysis hints.
📝 Proposed fix
-Post all of the review in English, even if the file written in the contributor's
+Post all review comments in English, even if the file written in the contributor's
native language. The comments should be polite and constructive.Or alternatively:
-Post all of the review in English, even if the file written in the contributor's
+Post all the reviews in English, even if the file written in the contributor's
native language. The comments should be polite and constructive.📝 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.
| Post all of the review in English, even if the file written in the contributor's | |
| native language. The comments should be polite and constructive. | |
| Post all review comments in English, even if the file written in the contributor's | |
| native language. The comments should be polite and constructive. |
🧰 Tools
🪛 LanguageTool
[style] ~93-~93: Consider removing “of” to be more concise
Context: ...nt in the contributor's language. Post all of the review in English, even if the file wri...
(ALL_OF_THE)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/get-reviews/SKILL.md around lines 93 - 94, The sentence "Post
all of the review in English" is grammatically incorrect; update the string in
.agents/skills/get-reviews/SKILL.md (the sentence at Line 93) to a clear plural
or more specific phrase such as "Post all review comments in English" or "Post
all of the reviews in English" so it correctly refers to multiple
reviews/comments and improves clarity.
There was a problem hiding this comment.
I agree this should be fixed. The sentence should refer to review comments clearly.
There was a problem hiding this comment.
@dahlia, agreed! Here's the planned fix:
- Fix the grammar issue at Line 93 of
.agents/skills/get-reviews/SKILL.md - Replace "Post all of the review in English" with "Post all review comments in English"
🧠 Learnings used
Learnt from: 2chanhaeng
Repo: fedify-dev/fedify PR: 674
File: .agents/skills/add-to-fedify-init/SKILL.md:37-39
Timestamp: 2026-04-13T10:51:54.856Z
Learning: In this repo, treat files under `.agents/skills/` as internal agent-skill documentation (not user-facing docs). When reviewing these files, do not apply the user-doc Markdown conventions that require reference-style links and `*path*`-wrapped file path references (those conventions apply only to user-facing Markdown such as `docs/`, `README.md`, `CONTRIBUTING.md`, and `CHANGES.md`).
Failed to handle agent chat message. Please try again.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new 'get-reviews' skill for AI agents, consisting of a documentation guide, a shell script for fetching GitHub reviews via GraphQL, and a review template. Feedback focuses on fixing critical bugs in the shell script related to variable expansion and GraphQL variable declarations. Additionally, several documentation refinements are required, such as correcting typos, adhering to the repository's style guide for em dashes, and replacing placeholders with concrete examples to enhance readability.
| mkdir -p 'plans/$PR_NUMBER/fetched' | ||
| gh api graphql -f query='query($owner: String!, $repo: String!, $number: Int!) { | ||
| repository(owner: $owner, name: $repo) { | ||
| pullRequest(number: $number) { | ||
| reviewThreads(first: $NUMBER_OF_THREADS) { | ||
| nodes { | ||
| id | ||
| isResolved | ||
| isOutdated | ||
| path | ||
| line | ||
| comments(first: $NUMBER_OF_COMMENTS_PER_THREAD) { | ||
| nodes { | ||
| id | ||
| databaseId | ||
| author { login } | ||
| body | ||
| url | ||
| createdAt | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }' -F owner=fedify-dev -F repo=fedify -F number=$PR_NUMBER \ | ||
| > 'plans/$PR_NUMBER/fetched/$CURRENT_TIMESTAMP_MMDDHHMM.json' |
There was a problem hiding this comment.
The script contains two significant issues that will prevent it from functioning correctly:
- Shell Variable Expansion: Using single quotes around paths like 'plans/$PR_NUMBER/fetched' prevents the shell from expanding the variables. This will result in literal directory and file names containing the characters $PR_NUMBER. Use double quotes instead.
- GraphQL Variable Declaration: The variables $NUMBER_OF_THREADS and $NUMBER_OF_COMMENTS_PER_THREAD are used inside the query string but are not declared in the GraphQL query header, nor are they expanded by the shell (due to single quotes). This will cause the GitHub API to return a validation error. They should be declared as GraphQL variables and passed using the -F flag.
mkdir -p "plans/$PR_NUMBER/fetched"
gh api graphql -f query='query($owner: String!, $repo: String!, $number: Int!, $threads: Int!, $comments: Int!) {
repository(owner: $owner, name: $repo) {
pullRequest(number: $number) {
reviewThreads(first: $threads) {
nodes {
id
isResolved
isOutdated
path
line
comments(first: $comments) {
nodes {
id
databaseId
author { login }
body
url
createdAt
}
}
}
}
}
}
}' -F owner=fedify-dev -F repo=fedify -F number=$PR_NUMBER \
-F threads=$NUMBER_OF_THREADS -F comments=$NUMBER_OF_COMMENTS_PER_THREAD \
> "plans/$PR_NUMBER/fetched/$CURRENT_TIMESTAMP_MMDDHHMM.json"There was a problem hiding this comment.
I agree this should be fixed. The undeclared GraphQL variables are a real runtime issue, not just style.
| - After applying or dismissing the review, move the file to | ||
| *plans/{PR\_NUMBER}/reviews/resolved/{REVIEW\_ID}.md*. | ||
| - If the review file is too long, move the content to | ||
| \**plans/{PR\_NUMBER}/reviews/{REVIEW\_ID}/index.md*, and separate the |
There was a problem hiding this comment.
File paths in Markdown should be formatted using italics. Additionally, avoid using placeholders like {PR_NUMBER} in documentation to ensure snippets are complete and easily understood.
| \**plans/{PR\_NUMBER}/reviews/{REVIEW\_ID}/index.md*, and separate the | |
| _plans/123/reviews/456/index.md_, and separate the |
References
- In Markdown documentation, format file paths using italics.
- In documentation, code snippets should be complete and avoid placeholders like { ... } to be easily copy-pasted and understood.
| Categorize the reviews and the plans, and apply them at once by category. | ||
| After applying the review, use [`/commit` skill](../commit/SKILL.md) to commit | ||
| the changes. The commit message should include the related review links. | ||
| `https://github.com/fedify-dev/fedify/pull/{PR_NUMBER}#discussion_r{REVIEW_THREAD.COMMENTS[0].DATABASE_ID})` |
There was a problem hiding this comment.
There is a trailing parenthesis typo in the URL. Additionally, avoid using placeholders like {PR_NUMBER} in documentation snippets to ensure they are complete and easily understood.
| `https://github.com/fedify-dev/fedify/pull/{PR_NUMBER}#discussion_r{REVIEW_THREAD.COMMENTS[0].DATABASE_ID})` | |
| url: "https://github.com/fedify-dev/fedify/pull/123#discussion_r456" |
References
- In documentation, code snippets should be complete and avoid placeholders like { ... } to be easily copy-pasted and understood.
| links: | ||
| - If there are multiple links related to the review, | ||
| - list all the links. | ||
| - `link`–`links` are mutually exclusive. |
There was a problem hiding this comment.
The repository style guide (line 225) specifies that em dashes (—) should be used in narrative text without surrounding spaces. Currently, an en dash (–) is used.
References
- Em dash (—): In narrative text (within sentences), use em dashes without surrounding spaces (e.g., word—word). (link)
| commits: | ||
| - If the review applies to multiple commits, list the hashes of the commits | ||
| - after applying the review, update the list to include the new commit hash | ||
| - `commit`–`commits` are mutually exclusive. `commit` and `commits` are |
There was a problem hiding this comment.
The repository style guide (line 225) specifies that em dashes (—) should be used in narrative text without surrounding spaces. Currently, an en dash (–) is used.
References
- Em dash (—): In narrative text (within sentences), use em dashes without surrounding spaces (e.g., word—word). (link)
| - **NEEDS DISCUSSION**: If the review needs further discussion, | ||
| such as about direction of the project or design choices. | ||
| Try to use these words to indicate judgement status whenever possible, | ||
| but if you feel they are truly insufficient,use an appropriate word |
There was a problem hiding this comment.
Missing space after the comma in insufficient,use.
| but if you feel they are truly insufficient,use an appropriate word | |
| but if you feel they are truly insufficient, use an appropriate word |
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
dahlia
left a comment
dahlia
left a comment
There was a problem hiding this comment.
I think many of bot reviews are valid. Also left my few comments.
| @@ -0,0 +1,29 @@ | |||
| mkdir -p 'plans/$PR_NUMBER/fetched' | |||
| } | ||
| } | ||
| }' -F owner=fedify-dev -F repo=fedify -F number=$PR_NUMBER \ | ||
| > 'plans/$PR_NUMBER/fetched/$CURRENT_TIMESTAMP_MMDDHHMM.json' |
| mkdir -p 'plans/$PR_NUMBER/fetched' | ||
| gh api graphql -f query='query($owner: String!, $repo: String!, $number: Int!) { | ||
| repository(owner: $owner, name: $repo) { | ||
| pullRequest(number: $number) { | ||
| reviewThreads(first: $NUMBER_OF_THREADS) { | ||
| nodes { | ||
| id | ||
| isResolved | ||
| isOutdated | ||
| path | ||
| line | ||
| comments(first: $NUMBER_OF_COMMENTS_PER_THREAD) { | ||
| nodes { | ||
| id | ||
| databaseId | ||
| author { login } | ||
| body | ||
| url | ||
| createdAt | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }' -F owner=fedify-dev -F repo=fedify -F number=$PR_NUMBER \ | ||
| > 'plans/$PR_NUMBER/fetched/$CURRENT_TIMESTAMP_MMDDHHMM.json' |
There was a problem hiding this comment.
I agree this should be fixed. The undeclared GraphQL variables are a real runtime issue, not just style.
| - If the review file is too long, move the content to | ||
| \**plans/{PR\_NUMBER}/reviews/{REVIEW\_ID}/index.md*, and separate the | ||
| content into multiple files in the same directory. In this case, after | ||
| resolving the review, move the whole directory to | ||
| *plans/{PR\_NUMBER}/reviews/resolved/{REVIEW\_ID}/*. |
There was a problem hiding this comment.
I agree this should be fixed. The rendered path convention should be unambiguous.
| Categorize the reviews and the plans, and apply them at once by category. | ||
| After applying the review, use [`/commit` skill](../commit/SKILL.md) to commit | ||
| the changes. The commit message should include the related review links. | ||
| `https://github.com/fedify-dev/fedify/pull/{PR_NUMBER}#discussion_r{REVIEW_THREAD.COMMENTS[0].DATABASE_ID})` |
There was a problem hiding this comment.
I agree this should be fixed. This URL is meant to be copied into commits or review files, so the extra parenthesis would be misleading.
| If the review is judged as **NEEDS EVALUATION**, write the comments to | ||
| evaluate the review explain how to evaluate the review, the test results, | ||
| and the resulting application/rejection details. |
| Post all of the review in English, even if the file written in the contributor's | ||
| native language. The comments should be polite and constructive. |
There was a problem hiding this comment.
I agree this should be fixed. The sentence should refer to review comments clearly.
| -------------------- | ||
|
|
||
| After fetching the PR and its reviews, organize the reviews. | ||
| [*plans* directory in the root](../../../plans/) is a good place to store them. |
There was a problem hiding this comment.
Since this workflow intentionally creates local review state under plans/, I think this directory should be ignored by Git.
| and save them in a JSON file: | ||
|
|
||
| - Replace `$VARIABLES` with the actual values or variables in the command. | ||
| - If already saved reviews existed, use `after: $LAST_REVIEW_ID` instead of |
There was a problem hiding this comment.
after expects a GraphQL cursor, not a review thread ID. If incremental fetching is part of the skill, the query should fetch pageInfo { hasNextPage endCursor } and the docs should refer to the saved cursor rather than $LAST_REVIEW_ID.
| id: REVIEW_ID | ||
| description: summary of the review | ||
| link: the link to the review on GitHub | ||
| links: |
There was a problem hiding this comment.
The template currently includes both link and links, and both commit and commits, even though the comments say those fields are mutually exclusive. It would be clearer to make the alternatives commented examples so newly created review files do not start in an inconsistent shape.
2 participants
This PR adds a new agent skill at .agents/skills/get-reviews/ that gives AI agents an end-to-end workflow for handling GitHub PR reviews on this repository, with explicit checkpoints that keep authoritative judgement with the contributor.
The motivation, design principles, and out-of-scope items are described in detail in #764. In short:
CORRECT/WRONG/PARTIAL/NEEDS EVALUATION/NEEDS DISCUSSION) and any non-English review's translated response are gated behind a mandatory contributor checkpoint, so the agent proposes but does not unilaterally decide or post.Files
Closes #764.
Assisted-by: Claude Code:claude-opus-4-7