Conversation
… smp-apply-github-pr-feedback skill.
There was a problem hiding this comment.
Pull request overview
This PR updates the smp-apply-github-pr-feedback skill’s reference GraphQL snippets to avoid GitHub GraphQL variable usage and instead rely on inlined placeholder values, reducing the chance of incorrect CLI variable substitution when resolving PR review threads.
Changes:
- Replaced GraphQL variable declarations (
query($owner:...)) with inline placeholder values (e.g.,"OWNER",PR_NUMBER,"THREAD_ID"). - Added explicit pagination guidance (first page vs subsequent pages with
endCursor). - Updated Phase 7 instructions in the skill to align with the new “inline values” approach.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| plugins/smp-github/skills/smp-apply-github-pr-feedback/references/graphql-queries.md | Converts query/mutation templates to inline placeholders and adds pagination guidance + examples. |
| plugins/smp-github/skills/smp-apply-github-pr-feedback/SKILL.md | Updates Phase 7 instructions to use the new inline GraphQL templates and pagination flow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,14 +1,46 @@ | |||
| # GraphQL Queries for PR Review Thread Resolution | |||
|
|
|||
| **Important:** All queries below use inline values (placeholder tokens in UPPERCASE) instead of GraphQL variables. Do NOT use GraphQL variable declarations or `-f`/`-F` flags for variable substitution — replace the placeholder tokens directly in the query string. | |||
There was a problem hiding this comment.
The note says "Do NOT use ... -f/-F flags" but the provided gh api graphql examples still require -f query=.... Consider clarifying that the restriction is specifically about using -f/-F for substituting query variables (e.g., -f owner=... -F pr=...), while still passing the query via -f query=....
| **Important:** All queries below use inline values (placeholder tokens in UPPERCASE) instead of GraphQL variables. Do NOT use GraphQL variable declarations or `-f`/`-F` flags for variable substitution — replace the placeholder tokens directly in the query string. | |
| **Important:** All queries below use inline values (placeholder tokens in UPPERCASE) instead of GraphQL variables. Do NOT use GraphQL variable declarations or `-f`/`-F` flags for substituting query variables (for example, `-f owner=... -F pr=...`) — instead, replace the placeholder tokens directly in the query string itself. It is still required to pass the query to `gh api graphql` via `-f query='...query text...'`. |
| If `pageInfo.hasNextPage` is `true`, re-run using this variant, replacing `END_CURSOR` with the `endCursor` value from the previous response. Repeat until the matching `databaseId` is found or all pages are exhausted. | ||
|
|
||
| ```graphql | ||
| query { | ||
| repository(owner: "OWNER", name: "REPO") { | ||
| pullRequest(number: PR_NUMBER) { | ||
| reviewThreads(first: 100, after: "END_CURSOR") { |
There was a problem hiding this comment.
The pagination guidance tells readers to replace END_CURSOR with the endCursor from the response, but the query already wraps it in quotes (after: "END_CURSOR"). This can lead to accidental double-quoting if someone pastes the JSON string including quotes. Consider explicitly stating to paste the raw cursor value without surrounding quotes (or adjust the template to make the quoting unambiguous).
| ## Phase 7 - Resolve Conversation | ||
|
|
||
| 1. Get the review thread node ID using the GraphQL query in `references/graphql-queries.md`. Match the thread whose first comment has `databaseId` equal to the comment ID (or its parent if the comment is a reply). | ||
| 1. Get the review thread node ID by running the fetch query from `references/graphql-queries.md`. Copy the query template exactly, replacing the placeholder values (OWNER, REPO, PR_NUMBER) with the actual values. Do NOT use GraphQL variables or `-f`/`-F` flags for variable substitution — all values must be inlined directly in the query string. Match the thread whose first comment has `databaseId` equal to the root comment ID from Phase 1. |
There was a problem hiding this comment.
This step says "Do NOT use ... -f/-F flags"; however, the gh api graphql call still needs -f query=... to send the query. To avoid ambiguity for the skill runner, consider rewording to forbid only additional -f/-F flags used for variable substitution (owner/repo/pr/threadId), while still using -f query='...'.
| 1. Get the review thread node ID by running the fetch query from `references/graphql-queries.md`. Copy the query template exactly, replacing the placeholder values (OWNER, REPO, PR_NUMBER) with the actual values. Do NOT use GraphQL variables or `-f`/`-F` flags for variable substitution — all values must be inlined directly in the query string. Match the thread whose first comment has `databaseId` equal to the root comment ID from Phase 1. | |
| 1. Get the review thread node ID by running the fetch query from `references/graphql-queries.md`. Copy the query template exactly, replacing the placeholder values (OWNER, REPO, PR_NUMBER) with the actual values. Do NOT use GraphQL variables or any additional `-f`/`-F` flags for variable substitution (owner, repo, PR number) — all values must be inlined directly in the query string. You may still use `-f query='...'` to pass the query to `gh api graphql`. Match the thread whose first comment has `databaseId` equal to the root comment ID from Phase 1. |
Update the GraphQL queries and mutations to use inline values instead of placeholder tokens for variable substitution, preventing potential errors in the smp-apply-github-pr-feedback skill.