Skip to content

feat: dual ID support in gh-resolve-review-threads.sh#33

Open
lklimek wants to merge 2 commits into
mainfrom
feat/gh-resolve-dual-id-support
Open

feat: dual ID support in gh-resolve-review-threads.sh#33
lklimek wants to merge 2 commits into
mainfrom
feat/gh-resolve-dual-id-support

Conversation

@lklimek
Copy link
Copy Markdown
Owner

@lklimek lklimek commented May 5, 2026

Summary

  • New repeatable --id <thread_id> flag in enhanced mode accepts three input formats: GraphQL node IDs (PRRT_* / PR_kw*), REST review-comment IDs (discussion_r<n>), and bare numeric databaseId.
  • Non-node-ID inputs auto-resolve to thread node IDs by matching databaseId against the PR's first 100 review threads via GraphQL; mixed formats dedupe into a single batched resolveReviewThread mutation, removing the manual GraphQL mapping step previously required when feeding pull_request_read MCP output back into the resolver.
  • Legacy invocation (PRRT_* PRRT_* ... positional) is preserved bit-for-bit; legacy mode now also emits a clear pointer toward enhanced mode when handed a REST/numeric ID without PR context.

Test plan

  • 14 existing stub scenarios in the script's harness pass (legacy passthrough, enhanced single-/multi-ID, dedup, mixed-format batching, error paths)
  • Live-PR validation: pass a single discussion_r<n> from a real pull_request_read payload and confirm the matching thread is resolved on GitHub
  • Live-PR validation: pass a bare numeric databaseId and confirm equivalent resolution
  • Live-PR validation: pass a mix of PRRT_* + discussion_r<n> + numeric in one invocation and confirm deduped, single-batch mutation
  • Live-PR validation: confirm legacy PRRT_* PRRT_* positional invocation still resolves identically (no regression)
  • Confirm pagination TODO (100-thread cap inherited from sibling scripts) does not bite on a PR with >100 threads (exercise or document explicitly)

🤖 Co-authored by Claudius the Magnificent AI Agent

…via --id

Enhanced mode now takes a repeatable --id <thread_id> flag accepting
GraphQL node IDs (PRRT_*/PR_kw*), REST review-comment IDs (discussion_r<n>),
and bare numeric databaseId. REST/numeric IDs are auto-converted to thread
node IDs by matching databaseId against the PR's reviewThreads, eliminating
the manual GraphQL mapping step previously required when consuming
pull_request_read MCP output. Mixed formats deduplicate into a single
batched mutation. Legacy mode (PRRT_* only) is unchanged; it now emits a
clear pointer to enhanced mode when given a REST/numeric ID without
PR context.

Refs memcan TODO 0ae05ab9-253b-4404-8e97-bf2ce6f40fb7

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@lklimek lklimek marked this pull request as ready for review May 5, 2026 09:03
@lklimek lklimek requested a review from Copilot May 5, 2026 09:03
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR expands gh-resolve-review-threads.sh so PR workflows can resolve review threads from either GraphQL thread IDs or REST-style comment IDs, reducing the manual ID-mapping step in GitHub review tooling.

Changes:

  • Adds repeatable --id <id> support to scripts/gh-resolve-review-threads.sh for GraphQL node IDs, discussion_r..., and numeric comment IDs.
  • Preserves legacy positional GraphQL-ID mode while adding clearer validation/errors for unsupported legacy inputs.
  • Updates skill/reference docs and bumps the plugin/changelog version to 3.14.1.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
skills/git-and-github/references/pr-review.md Documents the new resolver invocation forms in the PR-review reference.
skills/check-pr-comments/SKILL.md Updates bot-thread resolution guidance to show direct GraphQL IDs and enhanced --id usage.
skills/check-pr-comments/references/gh-cli-fallback.md Adds the enhanced CLI fallback syntax for resolving threads from REST/numeric IDs.
scripts/gh-resolve-review-threads.sh Implements --id parsing, REST/numeric-to-thread mapping, deduplication, and stricter legacy-mode validation.
CHANGELOG.md Adds the 3.14.1 release notes for the new resolver behavior.
.claude-plugin/plugin.json Bumps the plugin version to 3.14.1.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/gh-resolve-review-threads.sh Outdated
Comment thread scripts/gh-resolve-review-threads.sh Outdated
Address two Copilot review findings on PR #33:

1. The conversion query for `--id` only fetched the first comment of each
   thread and matched `comments[0].databaseId`, so review-reply IDs (which
   carry their own databaseId) would fail with "could not map". Pull
   `comments(first: 100)` per thread and scan every comment via
   `any(.comments.nodes[]; .databaseId == $db)`.

2. `reviewThreads(first: 100)` was unpaginated. Walk
   `pageInfo { hasNextPage endCursor }` with `after: $cursor` until exhausted,
   capped defensively at 50 pages (5000 threads) to prevent runaway loops.
   The fetch is now extracted into a `fetch_all_threads` helper used by both
   the `--id` conversion path and the filter-based selection path.

Replaces the `TODO: paginate review threads beyond the first 100` comment
with a present-state note describing the cap and rationale.

Bump plugin to 3.14.2 and document both fixes in CHANGELOG.md.

Refs: #33 review comments discussion_r3187249305 / discussion_r3187249359

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@lklimek
Copy link
Copy Markdown
Owner Author

lklimek commented May 5, 2026

Both Copilot findings addressed in a5d5fa7:

  • discussion_r3187249305 — conversion query now pulls comments(first: 100) per thread and the jq selector scans every comment via any(.comments.nodes[]; .databaseId == $db), so review-reply IDs resolve to their parent thread.
  • discussion_r3187249359reviewThreads walks pageInfo { hasNextPage endCursor } until exhausted, capped defensively at 50 pages (5000 threads). The fetch was extracted into a fetch_all_threads helper used by both the --id conversion path and the filter path.

Plugin bumped to 3.14.2; CHANGELOG updated.

Dogfood test — resolved both review threads via the new flag itself:

./scripts/gh-resolve-review-threads.sh lklimek/claudius 33 \
  --id discussion_r3187249305 --id discussion_r3187249359
Resolving 2 thread(s)...
{"data":{"t0":{"thread":{"isResolved":true}},"t1":{"thread":{"isResolved":true}}}}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants