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 [/] [--lookback 30d] [--threshold 2] [--dry-run] [--unattended]

Scheduled (recommended monthly cadence):

/schedule '0 0 1 * *' /dlc:update-review-checklist /

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 (default 30d). Format is a positive integer followed by d.
• THRESHOLD — --threshold (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 (). 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 blocks run with cwd at the skill's base directory (plugins/dlc/skills/update-review-checklist/) — same convention as dlc:pr-check, documented in docs/learnings.md:505. The LLM consumer should cd into the skill directory before executing this block. The ../../scripts/ prefix then resolves to plugins/dlc/scripts/.

FETCH_ERR="$(mktemp "${TMPDIR:-/tmp}/update-review-checklist-fetch-err.XXXXXX")"
PR_DATA=$(sh ../../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_DATA" | jq -r '.cutoff_date')

The helper uses die_json to write {error, code} JSON to stderr and exit non-zero, so validation must check the exit code and parse stderr (not look for .error in $PR_DATA). The jq -e '.prs and .summary' check enforces the documented schema on the success path. The mktemp path mirrors the CURRENT_CHECKLIST pattern below — concurrent scheduled runs against the same $TMPDIR cannot collide on a fixed /tmp/fetch-err.json path.

If .summary.truncated == true: Warn on stdout that the helper hit a per-PR pagination cap and some comments may be missing. Continue with the partial data.

If .summary.list_limit_hit == true: Warn that the merged-PR list cap (200) was reached and recent PRs may be missing from analysis. Continue with what was returned.

Existing checklist read. Also fetch the current checklist content for the dedup step in Step 5. Use mktemp so concurrent scheduled runs against different repos cannot overwrite each other's dedup input:

CURRENT_CHECKLIST="$(mktemp "${TMPDIR:-/tmp}/update-review-checklist-existing.XXXXXX")"
trap 'rm -f "$CURRENT_CHECKLIST"' EXIT  # clean up on any exit path — important for scheduled recurring runs

# Capture gh api output separately so a fetch failure (auth, rate limit, 404)
# is not masked by a successful base64 exit status in a pipeline.
ENCODED=$(gh api "repos/$REPO/contents/docs/code-review-checklist.md" --jq '.content') || {
  echo "Failed to fetch docs/code-review-checklist.md from $REPO" >&2
  exit 1
}

# Portable base64 decode: GNU/macOS Big Sur+ accept --decode; older BSD/macOS
# use -D. Fall back across both so the skill works on every reviewer-supported
# runtime without forcing a coreutils dependency. Fail fast if BOTH fail —
# otherwise Step 5 would dedup against an empty/corrupt checklist and propose
# duplicates of entries that already exist.
if ! printf '%s' "$ENCODED" | base64 --decode > "$CURRENT_CHECKLIST" 2>/dev/null; then
  if ! printf '%s' "$ENCODED" | base64 -D > "$CURRENT_CHECKLIST" 2>/dev/null; then
    echo "Failed to base64-decode docs/code-review-checklist.md from $REPO (neither --decode nor -D succeeded)" >&2
    exit 1
  fi
fi

Step 3: Filter Comments

Apply these filters in order; keep counters so the final summary can report what was dropped and why.

1. Drop unresolved-by-commit (.resolved_by_commit == false). A reviewer comment that did not provoke a code change from the PR author is either (a) a wishlist item ignored or deferred, or (b) something the reviewer themselves resolved as not-needed. Either way, it is not a pattern worth promoting to the checklist.
2. Drop hard-skip patterns — read [references/clustering-rubric.md](references/clustering-rubric.md) now and apply its explicit blocklist (typos, formatting nits, "consider" wishlists with no specific action, automated review-summary comments with zero findings).

The .is_bot field on each comment is retained as metadata for the PR body's "Derived from" table, but is not a filter input. Substantive reviewers in many repos are bot accounts (Copilot, CodeRabbit, Codex, Gemini, Qodo); filtering on author type would discard the primary signal. Noise from automated reviewers is content-defined and handled by the hard-skip patterns instead.

Record the per-step drop counts. They surface in the Step 9 summary.

Step 4: Cluster Semantically

Read [references/clustering-rubric.md](references/clustering-rubric.md) now and apply its semantic rules to group the surviving comments into clusters.

A cluster's PR-count is the number of distinct PR numbers its member comments come from — not the total comment count. Two comments from the same PR count once.

A cluster's weight is the sum of member severity weights from the rubric (high=3, medium=2, low|null=1).

Drop any cluster with pr_count " spans PRs #N, #M, #P. Members suggest . Which? (a) Keep as-is and propose (b) Split into and (c) Drop (too ambiguous to be useful) ``

Apply the user's answer to the active set before continuing.

Non-ambiguous clusters pass through unchanged. The active set after this gate is what Step 5 sees.

Step 5: Dedup Against Existing Checklist

Read [references/checklist-schema.md](references/checklist-schema.md) now. For each surviving cluster, apply its semantic-dedup rubric against the checklist content at $CURRENT_CHECKLIST (the per-run path from Step 2). The rubric's three diagnostic questions classify each cluster as duplicate (drop), distinct (keep), or ambiguous (needs a call).

Ambiguous-dedup gate

A cluster is ambiguous against an existing entry when Q1/Q2/Q3 produce a mix of "maybe" answers — typically because the existing entry is generic and the cluster is specific (Q2 subsumption is unclear), or because the surface partially overlaps (Q3 partial-domain match).

For each ambiguous dedup match:

• If UNATTENDED == true: append {theme, source_prs, reason: "ambiguous_dedup_vs_existing:"} to PENDING_HUMAN and drop the cluster from the active set.
• If UNATTENDED == false: invoke AskUserQuestion:

`` Cluster "" (PRs #N, #M) may overlap with existing entry "". Propose anyway? (a) Yes — distinct enough, add to PR (b) No — covered by "" (c) Replace "" with the new wording [INFORMATIONAL — out of scope for v1; treat as (b) but record] ``

Option (c) is out of scope per issue cc-skills#216 (no rewording of existing entries in v1) — record the suggestion in the PR body for the human reviewer's consideration, but treat behaviour as (b) and drop the cluster.

> Tie-breaker for genuine doubt that isn't ambiguous (attended mode without an AskUserQuestion prompt): prefer keeping the candidate. Duplicate proposals get caught in human review; missed proposals do not. The ambiguous-dedup gate above only fires for the specifically ambiguous cases — most clusters will pass cleanly as either duplicate or distinct.

Step 6: Dry-Run Exit Point

If DRY_RUN=true:

Print each surviving cluster to stdout in this format:

Cluster: 
  PRs:      #N, #M, #P  (n distinct, weight=W)
  Members:   on #N: ""
             on #M: ""
  Proposed entry:
    > 
    > Source PRs: #N, #M, #P

Then print a summary block: total clusters proposed, filter drop counts from Step 3, dedup drop count from Step 5.

Do not create a branch, edit any file, or open a PR. Exit 0.

Step 7: Author the PR

If DRY_RUN=false and at least one cluster survives the dedup gate (i.e. the active set is non-empty after Step 5):

Always operate in a fresh clone

Use a temporary work directory unconditionally — even when the target repo is the current working directory. This avoids two-path branching, prevents accidental edits in the user's working tree, and keeps the calling repo's git state clean for state-file writes in Step 8:

WORKDIR="$(mktemp -d "${TMPDIR:-/tmp}/update-review-checklist.XXXXXX")"
# Compose with the Step 2 cleanup so both temp resources are removed. Bash
# `trap '...' EXIT` REPLACES the previous EXIT trap, so a bare
# `trap 'rm -rf "$WORKDIR"' EXIT` here would silently strand the checklist
# tempfile created in Step 2. Compose both cleanups into one handler.
trap 'rm -rf "$WORKDIR"; rm -f "$CURRENT_CHECKLIST"' EXIT

BRANCH="chore/update-review-checklist-$(date -u +%Y-%m-%d-%H%M%S)"

gh repo clone "$REPO" "$WORKDIR" -- --depth 50 >/dev/null
cd "$WORKDIR"
git checkout -b "$BRANCH"

The trap ... EXIT ensures the work dir is cleaned up whether the step succeeds, fails, or is interrupted.

Re-read the checklist from the clone

The $CURRENT_CHECKLIST cache from Step 2 may be stale relative to the freshly cloned HEAD. Re-read in-place so Step 5's dedup is sound against the actual file you're about to edit:

CHECKLIST_PATH="$W

…

## Source & license

This open-source skill is cataloged on AgentStack and links to its original source — we do not rehost the code.

- **Author:** [rube-de](https://github.com/rube-de)
- **Source:** [rube-de/cc-skills](https://github.com/rube-de/cc-skills)
- **License:** MIT

Install and usage instructions live in the source repository linked above.