Skip to content

fix(pr-threads): require --pr and --repo on every gh pr-review call#56

Open
schloerke wants to merge 6 commits into
mainfrom
schloerke/pr-thread-resolve-number
Open

fix(pr-threads): require --pr and --repo on every gh pr-review call#56
schloerke wants to merge 6 commits into
mainfrom
schloerke/pr-thread-resolve-number

Conversation

@schloerke
Copy link
Copy Markdown
Contributor

@schloerke schloerke commented May 8, 2026

Summary

  • Both pr-threads-address and pr-threads-resolve previously promised auto-detection of the PR number when omitted, but never told Claude how to do it, and --pr / --repo were only shown in examples (not flagged as required). This led to bulk loops and per-thread replies dropping --pr.
  • Add a "Resolve PR context first" section to both skills that captures PR_NUMBER and REPO once via gh pr view / gh repo view, and instructs reusing them on every subsequent gh pr-review call.
  • Promote the soft "Repository Context" usage note to a hard "Required flags" rule that enumerates every affected subcommand.
  • Rewrite the bulk-resolve example in pr-threads-resolve to use "$PR_NUMBER" / "$REPO" instead of hardcoded 42 / owner/repo.

Test plan

  • Run /pr-threads-resolve with no PR number on a branch with an open PR; confirm Claude resolves the PR number first and passes --pr on every gh pr-review call.
  • Run /pr-threads-address similarly and confirm --pr / --repo are passed on comments reply and threads resolve.
  • Spot-check ./count-skill-tokens.py github/pr-threads-resolve and github/pr-threads-address are still under limits.

Add a "Resolve PR context first" section to both skills that captures
PR_NUMBER and REPO once via gh, and tighten the usage notes to flag
--pr and --repo as required on every gh pr-review subcommand. Closes a
gap where bulk loops and per-thread replies could drop --pr.
Comment thread github/pr-threads-resolve/SKILL.md Outdated
Move the gh pr-review extension check and the "Resolve PR context" step
under a single "Prerequisites" heading at the top of both skills, and
drop the redundant title/usage/description/note block that duplicated
the YAML frontmatter.
Comment thread github/pr-threads-resolve/SKILL.md Outdated
schloerke added 3 commits May 8, 2026 10:40
Per review on PR #56, replace the bash PR_NUMBER / REPO variable
approach with explicit gh lookup commands that the model runs once and
substitutes the resulting literal values into every --pr / --repo flag.
This is more portable across agents and shells.
Both skills always look up the PR number from the current branch, so
the optional positional argument was never necessary. Update the
"Resolve PR context" section to unconditionally fetch the PR number,
and adjust the example invocation to match.
- Drop "Start a Pending Review", "Add Review Comments", and "Submit a
  Review" from pr-threads-address — out of scope for a skill that
  addresses existing threads.
- Drop "View PR Reviews and Comments" from pr-threads-resolve — belongs
  in pr-threads-address only.
- Collapse redundant bash example blocks and example-driven Usage
  Notes; fold the still-useful pointers (thread-ID source, unresolved
  focus) into the relevant subsections inline.
@schloerke
Copy link
Copy Markdown
Contributor Author

Skill Lines ~Tokens Description Tokens
pr-threads-address 85 695 21
pr-threads-resolve 72 545 21

Both under the limits (500 lines / 5000 tokens for SKILL.md; 100 tokens for description).

@schloerke schloerke requested a review from gadenbuie May 8, 2026 14:53
An outdated-but-unresolved thread is still a thread the reviewer chose
not to mark resolved, so the feedback may still apply to the rewritten
code. Hiding those threads risks silently missing real review
feedback.
Copy link
Copy Markdown
Collaborator

@gadenbuie gadenbuie left a comment

Choose a reason for hiding this comment

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

Looks great! Just one more comment about the description based on some text you're removing from the skill body

Comment thread github/pr-threads-address/SKILL.md
@@ -1,3 +1,3 @@
---
name: pr-threads-resolve
description: Bulk resolve unresolved PR review threads. Useful after manually addressing threads or after using /pr-threads-address.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If you're hoping for the model to automatically use this skill when appropriate, it's worth spending a little more time on the description. Initially, the model will only see this description and has to make a decision about when to load the skill from that description alone.

I noticed this because there's some "when to use" language that was inside the skill (where the model wouldn't see it until after it's lodade) that we're removing (good), but maybe we want to move it into the description instead.

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.

2 participants