feat(github): add async hooks for PR state tracking

[nsheaps]

Summary

• Add PostToolUse, SessionStart, and Stop async hooks to the github plugin that silently monitor PR state (comments, reviews, CI status, merge readiness, body content) across all projects in multi-repo sessions
• State is cached locally in ~/.claude/plugin-cache/github/<project>/pr-state/ and compared on each invocation — when changes are detected, the agent is informed with details about old vs new values
• Multi-project discovery scans sibling git repos to find all active PRs in sessions launched with multiple repositories (e.g., ai-mktpl + github-actions + claude-utils)
• Cache directory is configurable via prStateCacheDir in plugin settings with 3-tier resolution
• PostToolUse checks are throttled (default 60s, configurable via prStateCheckInterval) to avoid excessive API calls
• Documents future integration with Claude Code channels feature for autonomous session wake on PR state changes

Changes

• hooks/hooks.json — Register PostToolUse, SessionStart, Stop hooks
• hooks/scripts/pr-state-check.sh — Main hook entry point with early throttle exit, uses hook-logging.sh for output
• hooks/scripts/lib/pr-state.sh — Core library: fetch PR state via gh API, cache atomically, diff via consolidated jq calls, report changes with human-readable labels
• hooks/scripts/lib/pr-discover.sh — Multi-project PR discovery across sibling git repos with input validation
• hooks/scripts/test-pr-state.sh — 35 automated unit tests for URL parsing, validation, and state diffing
• github.settings.yaml — New prStateTracking, prStateCheckInterval, and prStateCacheDir config keys
• plugin.json — Updated description and keywords
• README.md — Full documentation of PR state tracking, change detection table, channels roadmap
• skills/pr-state-tracking/SKILL.md — Skill documentation for the feature

Iteration history

1. Initial implementation
2. Address 7 PR review comments (operator precedence, validation, atomic writes, etc.)
3. Consolidated ~20 jq calls into single invocations, fixed O(N²) check diff, added security hardening
4. Converted to hook-logging.sh pattern, added tests, human-readable labels, cache pruning, jq 1.7 compat fix

Test plan

• Verify URL parsing handles HTTPS, SSH, and web proxy formats (unit tests)
• Verify owner/repo validation rejects path traversal attempts (unit tests)
• Verify state diff detects title, body, draft, state, labels, reviews, comments, CI changes (unit tests)
• Verify merged PR detection (unit tests)
• Verify multiple simultaneous changes are all reported (unit tests)
• Verify human-readable label output (unit tests)
• Verify jq 1.7 check diff compatibility (unit tests)
• Verify SessionStart hook establishes baseline state for discovered PRs
• Verify PostToolUse hook detects and reports changes
• Verify PostToolUse throttle skips checks within cooldown interval
• Verify Stop hook performs final state check
• Verify multi-project discovery works with sibling git repos
• Verify cache directory is created with secure permissions (700)
• Verify graceful degradation when gh or jq is not available
• Verify main/master branches are correctly skipped in discovery

https://claude.ai/code/session_01HJDTfa1KwAnxW1oFVgHqvc

[github-actions]

Plugin Version Status

Versions are auto-bumped in PRs. Manual bumps to higher versions are preserved.

Plugin	Base	Current	Action
github	0.1.15	0.1.16	Already bumped

[henry-nsheaps]

❌ Two bugs and a performance concern need to be addressed before merge.

❌ Bug: review_decision: .auto_merge maps wrong data (pr-state.sh:92)
❌ Bug: Array expansion ${#dirs[@]+...} uses incorrect syntax (pr-discover.sh:46)
⚠️ PostToolUse on * with no throttle — 5 API calls per PR on every tool use
⚠️ Dead code at end of pr-state-check.sh (exit 2 mentioned but never used)
⚠️ Comment says "parallel" but API calls are sequential (pr-state.sh:77)
⚠️ Tilde ~ in config values won't expand — documented pattern will break
⚠️ hook-output.sh symlink added but unused (YAGNI)
✅ Good library separation (discover/state/check)
✅ Comprehensive change detection across many PR dimensions
✅ Graceful degradation when gh/jq unavailable
✅ Well-documented README and SKILL.md

🖱️ Click to expand for full details

Bugs

review_decision: .auto_merge (pr-state.sh:92)

The jq filter maps .auto_merge (merge automation config) to a field named review_decision (which should be the review outcome like APPROVED/CHANGES_REQUESTED). The REST API doesn't expose review_decision — that's only available via GraphQL. This cached data is misleading. See inline comment for fix.

Array expansion in pr-discover.sh:46

"${#dirs[@]+${dirs[@]}}" — the # prefix changes the semantics from the intended ${param+word} safe-expansion pattern. The correct idiom is "${dirs[@]+"${dirs[@]}}". See inline comment for details.

Performance

The PostToolUse hook with matcher * fires on every tool use, making 5 sequential gh api calls per tracked PR each time. In a session with hundreds of tool calls, this will:

• Add up to 15 seconds of latency per tool interaction
• Rapidly consume the 5000/hour GitHub API rate limit
• Scale linearly with number of tracked PRs

A throttle/debounce mechanism (e.g., 60-second cooldown) is essential for this to be viable in real sessions. See inline comment on hooks.json for a suggested implementation.

Code Quality

• Dead code (pr-state-check.sh:125-133): The if/else block at the end of the file has both branches exiting 0, with a misleading comment about exit 2. The entire conditional is dead code.
• Misleading comment (pr-state.sh:77): Says "Fetch ... in parallel" but all calls are sequential.
• Unused symlink (lib/hook-output.sh): Added but never sourced. YAGNI — add it when a script actually needs it.
• Tilde expansion: README documents prStateCacheDir: "~/.claude/..." but bash won't expand ~ in variable values. This will create a literal ~ directory.

What's Good

The overall architecture is solid — separating discovery, state fetching, and change detection into distinct libraries is clean and testable. The change detection in _pr_state_diff is thorough, covering body, title, draft status, merge status, labels, reviews, comments, review comments, and CI checks. The graceful degradation (checking for gh/jq, consuming stdin on early exit) follows good hook patterns.

Score Rationale

• Quality 60%: Two functional bugs (wrong field mapping, incorrect bash syntax) plus dead code with misleading comments lower confidence in correctness.
• Security N/A: No user input handling, no secrets, cache is local files.
• Simplicity 70%: Good separation of concerns but the PostToolUse-on-every-call pattern adds unnecessary complexity and overhead. Sequential API calls could be simplified or parallelized.
• Confidence 85%: Bugs are clear-cut. Performance concern is based on well-understood GitHub API rate limits. The array expansion issue could work in some bash versions but is not portable/correct.

Recommended follow-ups (non-blocking):

• Consider parallelizing the 5 gh api calls in _pr_state_fetch using background processes for better latency
• The _pr_state_diff function parses the same JSON blobs ~20 times via echo | jq — consider extracting all fields in a single jq call for efficiency
• Add a configurable throttle interval setting (e.g., prStateCheckInterval: 60) alongside the hardcoded cooldown

Notes:¹²

Footnotes

1. Workflow Run: https://github.com/nsheaps/ai-mktpl/actions/runs/23509257949/attempts/1 ↩

2. PR: nsheaps/ai-mktpl#306 ↩

[henry-nsheaps]

❌ Bug: Wrong field mapping — review_decision maps to .auto_merge

review_decision: .auto_merge stores the auto-merge configuration object in a field named review_decision. These are completely different concepts:

• auto_merge — whether auto-merge is enabled and its configuration (method, commit title, etc.)
• review_decision — the review outcome (APPROVED, CHANGES_REQUESTED, REVIEW_REQUIRED) — only available via GraphQL, not the REST API

This field is cached but never compared in _pr_state_diff, so it's currently inert. But the cached data is misleading and will cause confusion if this field is used later.

Suggested change

	review_decision: .auto_merge,
	review_decision: null,

Or remove the field entirely since the REST API doesn't provide review_decision. If you need it, use the GraphQL API's reviewDecision field on the PullRequest type.

[henry-nsheaps]

❌ Bug: Incorrect array expansion syntax

"${#dirs[@]+${dirs[@]}}" uses # (length operator) as the parameter for the ${param+word} substitution. This is non-standard and will behave unexpectedly:

• ${#dirs[@]} always evaluates to a number (the array length), which is always "set"
• So this always expands to ${dirs[@]}, making the + guard pointless
• With an empty array under set -u, this may still fail depending on bash version

The correct idiom for safe empty-array iteration is:

Suggested change

	for dir in "${#dirs[@]+${dirs[@]}}"; do
	for dir in "${dirs[@]+"${dirs[@]}}"; do

Or since the caller (pr-state-check.sh) uses bash 4.4+ (implied by set -euo pipefail usage), simply "${dirs[@]}" works — bash 4.4+ treats empty "${array[@]}" as zero words even with set -u.

[henry-nsheaps]

⚠️ Performance: PostToolUse with * matcher fires on every single tool use

This hook makes 5 sequential GitHub API calls per tracked PR on every tool invocation. In a typical session with hundreds of tool calls per hour, this will:

1. Add latency to every agent interaction (up to 15s timeout)
2. Burn through GitHub API rate limits quickly (5000/hour for authenticated requests)
3. For multi-PR sessions, multiply the cost (5 calls × N PRs × every tool use)

Consider adding a throttle/debounce mechanism — e.g., only re-check if N seconds have elapsed since the last check. A simple approach:

# In pr-state-check.sh, after resolving cache_dir
last_check_file="${cache_dir}/.last-check"
now="$(date +%s)"
if [ -f "$last_check_file" ]; then
  last_check="$(cat "$last_check_file")"
  elapsed=$((now - last_check))
  if [ "$elapsed" -lt 60 ]; then  # 60-second cooldown
    exit 0
  fi
fi
echo "$now" > "$last_check_file"

Alternatively, use a more targeted matcher instead of * (e.g., only trigger on Bash or mcp__github__* tool calls that are likely to affect PR state).

[henry-nsheaps]

⚠️ Dead code: Both branches exit 0 — misleading comment about exit 2

The comment on line 125 says "exit 2 signals feedback" but neither branch actually uses exit 2. Both the if and else branches exit 0, making this entire conditional dead code.

If the intent was to exit 2 for PostToolUse/Stop to signal feedback to Claude, the code should do that. If exit 0 is actually correct for all cases, simplify:

Suggested change

	# For PostToolUse and Stop hooks, exit 2 to provide feedback to Claude
	# For SessionStart, exit 0 with the message in stdout
	if [ "$hook_event" = "SessionStart" ]; then
	exit 0
	else
	# Exit 2 signals "feedback" for Stop hooks (shown to Claude)
	# For PostToolUse, stdout is advisory (additionalContext)
	exit 0
	fi
	exit 0

[henry-nsheaps]

⚠️ Misleading comment: API calls are sequential, not parallel

The comment says "Fetch PR details, reviews, comments, and check runs in parallel" but all 5 gh api calls run sequentially. Each blocks until complete before the next starts.

For actual parallelism, you'd need background processes:

pr_json="$(gh api ... &)" 
review_json="$(gh api ... &)"
# ...
wait

Or use a temp-file approach. If sequential is intentional (simpler, avoids race conditions), update the comment to avoid confusion:

Suggested change

	# Fetch PR details, reviews, comments, and check runs in parallel
	local pr_json review_json comment_json checks_json
	# Fetch PR details, reviews, comments, and check runs sequentially
	local pr_json review_json comment_json checks_json

[henry-nsheaps]

⚠️ Unused symlink: hook-output.sh is added but never sourced

None of the new scripts (pr-state-check.sh, pr-state.sh, pr-discover.sh) source hook-output.sh. If this is intended for future use, it should be added in the PR that actually uses it. Per YAGNI, consider removing this until needed.

[henry-nsheaps]

⚠️ Tilde ~ in YAML config values won't expand in bash

When a user sets prStateCacheDir: "~/.claude/plugin-cache/github/my-project" in YAML, the ~ is a literal character — bash only expands tilde at the start of a word in a command, not inside quoted variable expansions.

The script at pr-state-check.sh:52 uses ${HOME} for the default, which is correct. But user-configured values with ~ will create a literal directory named ~.

Either:

1. Document that users should use $HOME instead of ~
2. Add tilde expansion in the script: cache_base="${cache_base/#\~/$HOME}"

[nsheaps]

Review Report — Iteration 2

Category	Pre-Iteration	Post-Iter 1	Post-Iter 2	Status
Simplicity	58	82	85	✅
Flexibility	78	82	85	✅
Usability	81	84	87	✅
Documentation	87	89	89	✅
Security	62	85	85	✅
Repo Patterns	84	84	87	✅
Best Practices	62	82	85	✅
QA & Engineering	47	72	85	✅
Overall	70	82	86	✅

Iteration 2 Changes

• Repo Patterns (+3): Converted pr-state-check.sh output from raw echo to hook-logging.sh pattern (hook_log + hook_respond), matching all other plugin hooks
• QA & Engineering (+13): Added 35 automated unit tests covering URL parsing (HTTPS, SSH, web proxy), identifier validation (path traversal, injection), and full state diffing (title, body, draft, labels, reviews, comments, CI, merge). Fixed jq 1.7 compatibility bug in check diff (from_entries + unique key collision)
• Usability (+3): Label changes now output human-readable comma-separated lists instead of raw JSON arrays. Missing jq/gh now shows a warning on SessionStart instead of silent exit
• Flexibility (+3): Added cache TTL pruning — stale .json files older than 7 days are automatically deleted
• Best Practices (+3): Fixed jq 1.7 compat bug (used $o + $n | keys[] instead of [$o | keys[], $n | keys[]] | unique[] which caused from_entries collision errors)
• Simplicity (+3): Consolidated jq check diff uses cleaner object merge pattern

Remaining non-blocking items (all 🔕)

• 🔕 Simplicity: 3-file library split has no current reuse consumers (acceptable for future channels integration)
• 🔕 Flexibility: No way to restrict discovery to primary project only; per-hook timeout not configurable without editing hooks.json
• 🔕 Usability: PR body change notification provides no content summary (agent must re-fetch)
• 🔕 Security: TOCTOU race in throttle (low severity, concurrent double-check is harmless); unquoted ${gh_hostname_flag} relies on word splitting
• 🔕 Best Practices: API calls 2-5 silently fall back to empty on failure; no pagination on comments/reviews API; no rate-limit awareness
• 🔕 QA: Comment/review count diffing assumes append-only; PostToolUse * matcher retained (throttle mitigates)
• 🔕 Documentation: README doesn't link to SKILL.md for discoverability

Verdict

All 8 categories now score ≥85%. The architecture is solid and extensible for the planned channels integration. Remaining items are reasonable follow-ups for a v1 feature on a draft PR.