update-review-checklist

DLC: Update Review Checklist

Cluster recurring reviewer themes across recent merged PRs and open a PR that proposes new entries for the target repo's docs/code-review-checklist.md. The skill never auto-merges — every proposal is human-reviewed.

Standalone: /dlc:update-review-checklist [<owner>/<repo>] [--lookback 30d] [--threshold 2] [--dry-run] [--unattended]

Scheduled (recommended monthly cadence):

/schedule '0 0 1 * *' /dlc:update-review-checklist <owner>/<repo>

Why This Matters

A docs/code-review-checklist.md doc decays without a forcing function. Manual curation lapses after the first few months and the checklist drifts away from what reviewers are actually catching in PRs. Reviewer attention is the most expensive resource on the team — when the same class of finding shows up in two or more PRs, the checklist deserves an entry so the next author catches it before review.

The recurring nature is the entire signal. A one-off finding is noise; a finding that recurs across multiple PRs is a pattern. Monthly cadence gives enough signal to cluster without flooding the checklist with low-recurrence entries. A run that finds zero promotable clusters is a healthy result — it means recent reviews have surfaced novel issues, not the same ones again.

Notification Rules

PushNotification fires only for terminal states that benefit the user knowing about:

• pr_opened — a checklist update PR was opened (include the PR URL)
• pending_human — under --unattended, clusters that need a human call were skipped
• Errors that block the run (missing prerequisites, GitHub API failures)

Routine outcomes (no clusters found, missing checklist file, dry-run with nothing to propose) are silent — printed to stdout but not pushed. This keeps the monthly scheduled cadence quiet by default and noisy only when there is something to act on.

Step 0: Parse Arguments

Capture the original calling directory once, before any later step changes cwd into a clone:

ORIG_CWD="$(pwd)"

Read $ARGUMENTS and extract:

• REPO — optional positional owner/name. If absent, default to gh repo view --json owner,name for the current working directory.
• LOOKBACK — --lookback <Nd> (default 30d). Format is a positive integer followed by d.
• THRESHOLD — --threshold <N> (default 2). Minimum number of distinct PRs a cluster must span to be promoted. Must be a positive integer ≥1.
• DRY_RUN — --dry-run flag. When present, set DRY_RUN=true.
• UNATTENDED — --unattended flag. When present, suppress AskUserQuestion calls; ambiguous clusters and ambiguous dedup matches become Pending-Human items reported in the final summary instead of prompting the user.

Reject unknown flags, non-Nd LOOKBACK values, and non-positive-integer THRESHOLD values with a one-line error and exit non-zero. THRESHOLD validation matters because it is emitted unquoted into the state-file JSON in Step 8 — a malformed value corrupts the file.

Initialise two counters that other steps mutate. PENDING_HUMAN is a JSON array because Step 8 pipes it to jq (jq 'length // 0') and Steps 4/5 mutate it via jq appends — an unset or non-JSON value breaks the pipeline:

PENDING_HUMAN='[]'   # JSON array; append records via `jq '. + [$new]'`
ENTRIES_ADDED=0

• PENDING_HUMAN — JSON array of {theme, source_prs, reason} records (clusters held back from the PR because they needed a human call). Used in Step 9 to emit the Pending-Human: line.
• ENTRIES_ADDED — count of clusters actually written to the checklist file (distinct from "clusters that survived clustering" — Pending-Human clusters are not counted here).

Step 1: Precondition Check

Verify the target repo has docs/code-review-checklist.md. Capture stderr so a 404 (missing file — expected) can be distinguished from real failures (auth, rate limit, network).

First confirm the repo itself is accessible, because gh api returns the same HTTP 404 for "repo not found / no access" as it does for "file not found". Without this disambiguation, an auth or typo failure would be silently treated as "missing checklist" and exit 0 in a scheduled run — masking real problems:

if ! repo_err=$(gh api "repos/$REPO" --silent 2>&1 >/dev/null); then
  printf 'Cannot access repo %s: %s\n' "$REPO" "$repo_err" >&2
  # Caller should fire PushNotification with $repo_err and exit non-zero.
  exit 1
fi

if ! gh_err=$(gh api "repos/$REPO/contents/docs/code-review-checklist.md" --silent 2>&1 >/dev/null); then
  if printf '%s\n' "$gh_err" | grep -q "HTTP 404"; then
    echo "$REPO has no docs/code-review-checklist.md — create one first, then re-run /dlc:update-review-checklist. See issue cc-skills#216 for the shape."
    exit 0
  fi
  # Real failure — auth, rate limit, network. Surface the message and notify.
  printf 'Precondition check failed for %s: %s\n' "$REPO" "$gh_err" >&2
  # Caller should fire PushNotification with $gh_err and exit non-zero.
  exit 1
fi

The HTTP 404 substring match is the documented gh api error format — gh api prints gh: ... (HTTP 404) to stderr on missing resources. The repo-level check above ensures that a 404 reaching the file check is unambiguously "file missing" rather than "repo inaccessible".

Step 1.5: Guard Against an Existing Open Update PR

The --skip-prefix filter in the helper script only excludes merged prior runs. An open prior PR is still in flight; opening a second one stacks duplicates and pollutes the review queue.

OPEN_PRIOR=$(gh pr list --repo "$REPO" --state open --limit 200 \
  --json headRefName,url \
  --jq '[ .[] | select(.headRefName | startswith("chore/update-review-checklist-")) ][0].url // empty')

If OPEN_PRIOR is non-empty:

Print one line to stdout naming the existing PR's URL and exit 0. Do not fire PushNotification — a single waiting PR is a normal between-cycles state, not an error:

$REPO has an open checklist-update PR (<URL>). Merge or close it before re-running.

This rule applies in both attended and unattended modes. Dry-run is the one exception: continue through to Step 6 so the human can preview what would be proposed once the prior PR is resolved.

Step 2: Fetch Comments

Run the helper script that lists merged PRs in window, fetches review-thread + review-body + issue-comment data per PR, applies the resolved-by-commit heuristic, and detects severity labels:

The skill's bash block references the helper script via ${CLAUDE_SKILL_DIR}/../../scripts/fetch-merged-pr-comments.sh so it resolves deterministically regardless of execution cwd.

FETCH_ERR="$(mktemp "${TMPDIR:-/tmp}/update-review-checklist-fetch-err.XXXXXX")"
PR_DATA=$(sh "${CLAUDE_SKILL_DIR}/../../scripts/fetch-merged-pr-comments.sh" "$REPO" --lookback "$LOOKBACK" 2>"$FETCH_ERR") || {
  err_msg=$(jq -r '.error // .' "$FETCH_ERR" 2>/dev/null || cat "$FETCH_ERR")
  rm -f "$FETCH_ERR"
  echo "fetch-merged-pr-comments.sh failed: $err_msg" >&2
  exit 1
}

# Validate response shape — the helper writes errors to stderr (handled above)
# but the success path must still contain the expected JSON keys.
if ! printf '%s' "$PR_DATA" | jq -e '.prs and .summary' >/dev/null; then
  echo "fetch-merged-pr-comments.sh: response missing .prs or .summary" >&2
  rm -f "$FETCH_ERR"
  exit 1
fi

# Replay per-PR warnings (GraphQL skips, jq-transform failures) on stderr when
# the helper succeeded — useful for operators in scheduled runs. The script
# uses stderr for *both* fatal errors (already handled above) and these soft
# warnings; we re-emit them to stderr so they reach the same destination an
# operator would inspect, rather than silently discarding them.
if [ -s "$FETCH_ERR" ]; then
  echo "fetch-merged-pr-comments.sh warnings:" >&2
  cat "$FETCH_ERR" >&2
fi
rm -f "$FETCH_ERR"

# Extract cutoff_date from the helper response — used in the Step 9 summary.
CUTOFF_DATE=$(printf '%s' "$PR_D