Skip to content

Add github-triage plugin#192

Open
ESultanik wants to merge 3 commits into
mainfrom
esultanik/github-triage
Open

Add github-triage plugin#192
ESultanik wants to merge 3 commits into
mainfrom
esultanik/github-triage

Conversation

@ESultanik

Copy link
Copy Markdown

Summary

Adds a new github-triage plugin: a single, explicitly-invoked skill (/github-triage) that triages a repository's open GitHub issues and pull requests via the gh CLI.

Repository selection — uses the sole GitHub remote by default; prompts when PWD is not a git repo, has no GitHub remote, or has several.

Pull requests (optional, run first so merges feed issue resolution):

  • Incrementally merge allowlisted bot PRs (Dependabot/Renovate) and maintainer-approved PRs — one at a time, oldest first, re-checking mergeability/CI before each and confirming it landed.
  • Spawn one read-only review subagent per never-reviewed (human-authored) PR; each review is saved to github-pr-<number>-review.md locally and never posted.

Issues:

  • Close already-resolved issues with a comment citing the resolving PR/commit.
  • Cross-link issues and their pending fix PRs (non-destructive comment; body-edit for auto-close is opt-in).
  • Assign local-only priority (Critical/High/Medium/Low) and change-size (size/XSXXL, Kubernetes/Prow thresholds) to everything outstanding — never posted to GitHub. Large sets (>32) can be saved to Markdown.

All GitHub writes — merges included — are gated behind a single review-and-iterate approval; nothing runs autonomously.

Validation

  • Verified PR-readiness logic against live gh --json field semantics: mergeStateStatus == CLEAN as the merge gate (MERGEABLE alone is insufficient; UNKNOWN is never-merge), per-node statusCheckRollup (CheckRun vs StatusContext), author.is_bot + a normalized allowlist, and latestReviews state + authorAssociation rather than the branch-protection-driven reviewDecision.
  • Ran an end-to-end dry run against trailofbits/graphtage (19 issues, 7 PRs, no writes), which caught and fixed four real bugs: gh repo view is positional (not -R); NEUTRAL/SKIPPED checks must not block merge; not-ready bot PRs belong in "Needs work", not the review bucket; and the bot allowlist must match gh's app/dependabot login form.
  • All repo CI gates pass: check_claude_loadability.py, check_codex_loadability.py, validate_plugin_metadata.py. SKILL.md is 410 lines (<500). Registered in the marketplace, README, and CODEOWNERS.

🤖 Generated with Claude Code

ESultanik and others added 3 commits June 25, 2026 10:44
Triages open GitHub issues for the current repository via the gh CLI:
closes already-resolved issues with comments citing the resolving PR or
commit, cross-links issues with pending fix PRs, and assigns local-only
priority and change-size (size/XS–XXL) estimates for everything else.

All GitHub writes are gated behind a single review-and-iterate approval;
priority/effort are never posted. Registers the plugin in the marketplace,
README, and CODEOWNERS.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Before issue triage, optionally clear open PRs (so merges feed the
"already resolved" issue check):
- incrementally merge allowlisted bot PRs and maintainer-approved PRs,
  one at a time, re-verifying mergeability/CI before each and confirming
  each landed;
- spawn one read-only review subagent per never-reviewed PR, saving each
  review to github-pr-<number>-review.md locally (never posted).

PR readiness uses verified gh --json semantics: mergeStateStatus==CLEAN
(MERGEABLE alone is insufficient; UNKNOWN is never-merge), per-node
statusCheckRollup (CheckRun status+conclusion vs StatusContext state),
author.is_bot + trusted allowlist, and latestReviews state+authorAssociation
rather than the branch-protection-driven reviewDecision. All merges gated;
never --auto/--admin/force.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Validated the skill against trailofbits/graphtage (real issues + PRs),
which surfaced four bugs:

- gh repo view takes the repo positionally, not -R; fixed the
  default-branch and merge-method lookups.
- CI readiness treated NEUTRAL/SKIPPED checks as failures, wrongly
  blocking mergeable Dependabot PRs (CLEAN with a NEUTRAL CodeQL run).
  Reworked to "CI not blocking" (hard failures + pending only), with
  mergeStateStatus==CLEAN as the authority.
- Not-ready bot PRs were routed to the review-subagent bucket; bots are
  now excluded so they fall to Needs work.
- Bot allowlist now normalizes gh's author.login renderings
  (app/dependabot and dependabot[bot]) so bot detection actually matches.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@ESultanik ESultanik requested a review from dguido as a code owner June 25, 2026 15:36

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

@dguido

dguido commented Jun 29, 2026

Copy link
Copy Markdown
Member

@claude review once

Comment on lines +107 to +111
default_branch=$(gh repo view "$REPO" --json defaultBranchRef --jq .defaultBranchRef.name)
# Anchor the issue number so #12 does not match #120, #123, …
git log --oneline "origin/$default_branch" \
| grep -iE "(close|fix|resolve)[sd]? +#<N>([^0-9]|$)"
```

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 The commit-message fallback at SKILL.md lines 107–111 is broken three independent ways that compound, so Bucket A ("already resolved") will silently miss most of the cases the fallback exists to catch: (1) git log --oneline shows only the subject and drops the body, so closing keywords in squash-merge bodies, Conventional Commits trailers, and git trailers like Fixes: #N are invisible; (2) the regex (close|fix|resolve)[sd]? cannot match "fixes" or "fixed" because [sd]? is a single-character class but those inflections add two letters (es/ed); (3) the hardcoded origin/$default_branch directly contradicts the warning two lines above and breaks the fork/upstream, non-origin remote, and non-git-repo / GHE OWNER/REPO-prompt paths that Phase 0 explicitly supports. Suggested fix: drop --oneline (or use git log --grep=… which already searches subject+body), rewrite the alternation as (close[sd]?|fix(es|ed)?|resolve[sd]?), and either look up the remote that maps to $REPO or fetch commits via gh api repos/$REPO/commits.

Extended reasoning...

Three independent defects on the same five lines

The Phase 1 fallback at plugins/github-triage/skills/github-triage/SKILL.md:107-111 reads:

default_branch=$(gh repo view "$REPO" --json defaultBranchRef --jq .defaultBranchRef.name)
# Anchor the issue number so #12 does not match #120, #123, …
git log --oneline "origin/$default_branch" \
  | grep -iE "(close|fix|resolve)[sd]? +#<N>([^0-9]|$)"

The stated purpose (line 102): "For issues it does not cover [closingIssuesReferences], also search commit messages on the default branch." Each of the three defects below independently undermines that purpose, and together they make the fallback nearly inert.

Defect 1 — --oneline drops the commit body

git log --oneline is documented as equivalent to --pretty=oneline --abbrev-commit, and --pretty=oneline is format:%H %s — only the subject, never the body. But closing keywords live in the body in the three most common workflows:

  • GitHub squash-merges (the default in many repos): the squash commit subject is Title (#PR) and the original feature-branch commit messages (where Closes #N typically lives) are concatenated into the body.
  • Conventional Commits: subject is feat(scope): …, Closes #N / Fixes #N is a trailer in the body.
  • Git trailers (Fixes: #N, Closes: #N): by definition placed in the body after a blank line.

So precisely the cases where closingIssuesReferences may miss (a direct push to main, an issue filed after a commit landed and never UI-linked, etc.) are the ones where the keyword may only be present in the body — exactly what this fallback exists to catch.

Defect 2 — the regex cannot match "fixes" or "fixed"

(close|fix|resolve)[sd]? has the verb roots in the alternation and a single optional [sd] for the suffix. That works for close/closes/closed (single-letter suffix) and resolve/resolves/resolved (likewise), but fails on fixes and fixed because those add two letters (es, ed), not one. Step-by-step against Fixes #123:

  1. Alternation matches fix (case-insensitive).
  2. [sd]? sees e — not in the class, so it matches the empty string.
  3. Cursor is at e; next required token is + (one or more spaces). e is not a space → match fails.
  4. No backtracking saves it; the alternation has no other branch that matches "fix" followed by an arbitrary suffix.

Verified at the shell:

$ echo 'Fixes #1'  | grep -iE '(close|fix|resolve)[sd]? +#1([^0-9]|$)'   # no match
$ echo 'Fixed #1'  | grep -iE '(close|fix|resolve)[sd]? +#1([^0-9]|$)'   # no match
$ echo 'Closes #1' | grep -iE '(close|fix|resolve)[sd]? +#1([^0-9]|$)'   # match
$ echo 'Fix #1'    | grep -iE '(close|fix|resolve)[sd]? +#1([^0-9]|$)'   # match

This is meaningful because the SKILL.md itself, four lines earlier at line 100, explicitly lists fixes / fixed as closing keywords the fallback is supposed to recognize — the implementation contradicts the spec on the same page. "Fixes #N" is also the canonical form in GitHub's own docs and arguably the most common closing-keyword phrasing in the wild.

Defect 3 — hardcoded origin/ contradicts the warning above it

Line 104 explicitly warns: "Resolve the default branch authoritatively from the selected repo (not from local origin/HEAD, which may be unset or point at the wrong remote)." Line 109 then immediately re-introduces that exact problem by hardcoding the literal string origin/ in the git log target. The skill resolves $default_branch correctly via gh, but then reads commits from origin/$default_branch regardless of which remote actually points at $REPO. This breaks scenarios Phase 0 explicitly supports:

  • Fork workflow — Phase 0 (lines 62–64, 71) explicitly handles origin and upstream pointing at different GitHub repos and asks the user to pick. If the user picks upstream, origin/<branch> is the fork's branch, which may have entirely different commits than the selected repo. The grep then either misses real closing-keyword commits or matches commits not on the chosen repo.
  • Not a git repo / zero GitHub remotes — Phase 0 explicitly: "Not a git repo, or zero GitHub remotes → Ask the user for the OWNER/REPO to triage." In that path no origin remote exists at all; git log "origin/main" fails with fatal: ambiguous argument 'origin/main': unknown revision or path not in the working tree. The fallback isn't merely incomplete — it errors out.
  • GHE / remote not named origin — also documented in Phase 0; same failure modes.

Step-by-step proof against a realistic scenario

Setup: triage acme/widget. The user is on a fork — origin = alice/widget, upstream = acme/widget — and picks acme/widget at the Phase 0 prompt. Upstream had a direct push to main: subject feat: add CSV export, body containing Fixes #42. closingIssuesReferences doesn't cover #42 (no PR was merged, no UI link). What the fallback does today:

  1. $default_branch = main (correct, from gh repo view acme/widget).
  2. git log --oneline origin/main — but origin is alice/widget, not acme/widget. Either the commit isn't there (silent under-detection — Defect 3) or even if the fork happens to be up to date, --oneline strips the body and Fixes #42 isn't visible (Defect 1).
  3. Even if the body were visible and the right remote were used, the grep would still miss Fixes #42 because the regex can't match "Fixes" (Defect 2).

Issue #42 is silently classified as Bucket C (outstanding) instead of Bucket A (already resolved) — the exact failure the fallback exists to prevent.

Fix

Three small, independent changes on the same five lines:

  1. Use --pretty=format:'%H %s%n%b%n--END--' (or simply git log --grep=<regex> … which by default searches subject and body) so the body is searched.
  2. Rewrite the alternation as (close[sd]?|fix(es|ed)?|resolve[sd]?) so each verb gets its correct suffix group.
  3. Map $REPO to its local remote (parse git remote -v), fall back to gh api repos/$REPO/commits when no local clone of $REPO exists, or skip the fallback gracefully in that case.

Comment on lines +246 to +247
body=$(gh pr view <PR> -R "$REPO" --json body --jq .body)
printf '%s\n\nCloses #%s\n' "$body" "<N>" | gh pr edit <PR> -R "$REPO" --body-file -

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 When a PR has no description, GitHub returns body: null, and gh pr view --jq .body emits the literal 4-character string null. The auto-close-on-merge opt-in path (lines 246-247, repeated verbatim at 342-343) then writes a PR body starting with the literal word null over the user's PR — visible, embarrassing corruption that directly violates the "never clobber the description" rule stated two lines above. Fix in both places: --jq '.body // ""' so a null body becomes an empty string.

Extended reasoning...

What the bug is

In Phase 3 (Bucket B) and again in the Phase 5 example, the skill instructs the model to re-fetch a PR body and write back a version with Closes #<N> appended:

body=$(gh pr view <PR> -R "$REPO" --json body --jq .body)
printf '%s\n\nCloses #%s\n' "$body" "<N>" | gh pr edit <PR> -R "$REPO" --body-file -

GitHub's REST/GraphQL API returns body: null (not "") for any PR opened without a description. jq renders the JSON null as the literal 4-character string null, and gh's --jq runs in the same raw mode — -r only strips quotes from JSON strings; it does not turn null into the empty string.

Step-by-step proof

  1. Verified jq behavior directly:

    $ echo '{"body":null}' | jq -r .body
    null
    

    So body=$(... --jq .body) assigns the 4 ASCII characters n, u, l, l to the shell variable.

  2. A maintainer opens a Dependabot/Renovate PR (these commonly have empty bodies under default configuration) or any human PR created via the GitHub API / gh pr create without --body. Say it is PR gh-cli shim: PATH walk fails inside Homebrew superenv #145, fixing issue init mutation-testing skill #140.

  3. The user opts into the explicit auto-close-on-merge cross-link. The skill runs:

    body=$(gh pr view 145 -R acme/widget --json body --jq .body)   # body="null"
    printf '%s\n\nCloses #%s\n' "$body" "140" | gh pr edit 145 -R acme/widget --body-file -
  4. The PR's description on GitHub becomes:

    null
    
    Closes #140
    

Why existing code doesn't prevent it

The skill explicitly warns immediately above the snippet: "never clobber the description: re-fetch the body immediately before editing." The re-fetch is exactly what introduces the bug — .body is null, and the value is interpolated as-is. The gated approval shown to the user only says "edit PR body to add closing ref" — the literal null payload is never displayed, so the user cannot catch it at gate time. Once written, the only rollback target is empty (the original body was empty), so the visible null cannot be cleanly undone.

Impact

User-visible corruption of a real PR description on GitHub. Low-frequency (requires the auto-close opt-in plus a bodyless PR) but irreversible-without-trace and embarrassing. The pattern is duplicated verbatim at lines 342-343 in the Phase 5 execution example, so any user copy-pasting from either spot inherits the bug.

Fix

Use jq's null-coalescing operator so a null body becomes the empty string before reaching the shell:

body=$(gh pr view <PR> -R "$REPO" --json body --jq '.body // ""')

Apply at both line 247 and line 342.

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