From abfb85e178b6cc68ec439f6112e7e5fc814f8d77 Mon Sep 17 00:00:00 2001 From: topcoder1 Date: Fri, 3 Jul 2026 11:46:35 -0700 Subject: [PATCH 1/2] fix(reusables): survive large PRs and symlinked prettier targets Two mechanical failure modes observed on topcoder1/dotclaude#121 (23,448 insertions / 161 files): 1. `gh pr diff --name-only` HTTP-406s past 20k diff lines, killing every workflow that used it as its changed-file source. Switch claude-author-automerge, safe-paths-automerge, pr-classify, and codex-review to the paginated files API (the idiom lint.yml / prettier-autofix.yml already used). The gating workflows fail closed at the API's 3000-file listing cap instead of silently under-classifying; codex-review takes its line count from the PR's additions+deletions instead of fetching the diff body. 2. prettier hard-errors on explicitly specified symlinks (dotclaude tracks symlinked SKILL.md files by convention). lint.yml and prettier-autofix.yml now drop symlinks from the explicit target list; the guarded empty-array assignment keeps the block bash-3.2 safe so the selftest can execute it on macOS. Selftests: test_pr_files_listing.sh bans executable `gh pr diff` in the reusables and pins the files-API idiom; test_prettier_symlink_filter.sh extracts the shipped filter block, runs it against a fixture tree, and guards lint/prettier-autofix drift; test_workflow_guards.py wires the host-independent .sh selftests into the pytest self-test path so CI enforces them. Failing runs: dotclaude runs 28676286316 (automerge) and 28676286301 (prettier-autofix). Co-Authored-By: Claude Fable 5 --- .github/workflows/claude-author-automerge.yml | 35 ++++++---- .github/workflows/codex-review.yml | 18 ++--- .github/workflows/lint.yml | 27 +++++++ .github/workflows/pr-classify.yml | 12 +++- .github/workflows/prettier-autofix.yml | 27 +++++++ .github/workflows/safe-paths-automerge.yml | 18 +++-- selftest/README.md | 12 ++++ selftest/test_pr_files_listing.sh | 44 ++++++++++++ selftest/test_prettier_symlink_filter.sh | 70 +++++++++++++++++++ selftest/test_workflow_guards.py | 32 +++++++++ 10 files changed, 269 insertions(+), 26 deletions(-) create mode 100644 selftest/test_pr_files_listing.sh create mode 100644 selftest/test_prettier_symlink_filter.sh create mode 100644 selftest/test_workflow_guards.py diff --git a/.github/workflows/claude-author-automerge.yml b/.github/workflows/claude-author-automerge.yml index a4f1f01..1e72018 100644 --- a/.github/workflows/claude-author-automerge.yml +++ b/.github/workflows/claude-author-automerge.yml @@ -399,21 +399,21 @@ jobs: fi # Option A: explicit human approval via label. - # Run BEFORE the risk-tier path-scan so an oversized PR (>20k diff - # lines, GitHub's `gh pr diff` HTTP 406 cap) can still bypass-merge. - # Without this ordering, the path-scan dies on the diff fetch BEFORE - # ever consulting the bypass label, leaving the workflow exit-1 - # with no way for the human to override. Verified against - # topcoder1/attaxion_dev#71 (2026-05-04, 261k-line data drop) where - # `auto-merge-approved` was applied but the workflow died at the - # diff-fetch step regardless. + # Run BEFORE the risk-tier path-scan so a PR the scan can't + # classify can still bypass-merge. Historically that meant any + # >20k-line diff: the scan fetched names via `gh pr diff`, which + # HTTP-406s past 20k lines (topcoder1/attaxion_dev#71, 2026-05-04, + # 261k-line data drop, died before consulting this label — hence + # the ordering). The scan now lists names via the paginated files + # API (dotclaude#121, 2026-07-02), so diff size no longer kills it; + # the remaining can't-run case is the REST 3000-file listing cap, + # where the scan fails closed. # # When a Claude PR matches the risk-tier classifier (or the # path-scan can't run), you can apply the `auto-merge-approved` # label from the PR list page (one click, no PR detail navigation) - # to bypass both the risk gate AND the size-cap failure mode. - # Confirms "yes I read it, auto-merge it" without requiring the - # actual click-merge round-trip. + # to bypass the risk gate. Confirms "yes I read it, auto-merge it" + # without requiring the actual click-merge round-trip. - name: Check risk bypass label (Option A) id: bypass_label if: | @@ -446,9 +446,20 @@ jobs: GH_TOKEN: ${{ github.token }} PR: ${{ github.event.pull_request.number }} RISK_MAIN_GO: ${{ inputs.risk_main_go }} + BYPASS_LABEL: ${{ inputs.risk_bypass_label }} run: | set -euo pipefail - changed=$(gh pr diff "$PR" --name-only) + # Changed-file names via the paginated files API — `gh pr diff + # --name-only` HTTP-406s on diffs >20k lines (dotclaude#121). + # The listing endpoint caps at 3000 files; past that the scan + # can't see every path, so fail closed (the bypass label above + # stays the human override). + changed=$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR}/files?per_page=100" \ + --paginate --jq '.[].filename') + if [ "$(printf '%s\n' "$changed" | sed '/^$/d' | wc -l)" -ge 3000 ]; then + echo "::error::PR changes 3000+ files — file listing is truncated and the risk scan cannot classify every path. Apply the '${BYPASS_LABEL:-auto-merge-approved}' label or merge manually." + exit 1 + fi # Risk-tier glob patterns (kept in sync with global CLAUDE.md policy). # If you add a category, also update install-automerge-policy.sh and # the CLAUDE.md policy block. diff --git a/.github/workflows/codex-review.yml b/.github/workflows/codex-review.yml index 6046143..b54f50d 100644 --- a/.github/workflows/codex-review.yml +++ b/.github/workflows/codex-review.yml @@ -61,15 +61,15 @@ jobs: PR: ${{ github.event.pull_request.number }} run: | set -euo pipefail - files=$(gh pr diff "$PR" --name-only) - # Count + and - lines from the unified diff, excluding the file - # markers (+++/---). This is the actual change volume the - # reviewer would read. - lines=$(gh pr diff "$PR" | awk ' - /^(\+\+\+|---)/ { next } - /^[+-]/ { n++ } - END { print n+0 } - ') + # Changed-file names via the paginated files API — `gh pr diff` + # HTTP-406s on diffs >20k lines (dotclaude#121), which are + # exactly the oversized PRs the size gate must still classify. + files=$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR}/files?per_page=100" \ + --paginate --jq '.[].filename') + # additions+deletions equals the +/- line count (sans +++/--- + # file markers) the old `gh pr diff | awk` pass computed, without + # fetching a diff body subject to the same 20k-line cap. + lines=$(gh pr view "$PR" --json additions,deletions --jq '.additions + .deletions') { echo "files<<__FILES_EOF__" echo "$files" diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 0522217..09d2646 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -309,6 +309,33 @@ jobs: exit 0 fi + # Prettier hard-errors on explicitly specified symlinks + # ("Explicitly specified pattern '' is a symbolic link") — + # some repos track symlinked files by convention (dotclaude's + # skills/*/SKILL.md). Drop them: the link target gets formatted + # whenever it appears in the diff itself. Glob mode is unaffected + # (prettier skips symlinks it expands from a glob on its own). + # Mirrors prettier-autofix.yml — keep both filters in sync. + SYMLINKS=() + KEPT=() + for f in "${TARGETS[@]}"; do + if [ -L "$f" ]; then SYMLINKS+=("$f"); else KEPT+=("$f"); fi + done + if [ "${#SYMLINKS[@]}" -gt 0 ]; then + echo "Skipping ${#SYMLINKS[@]} symlink(s) (prettier rejects explicit symlink targets):" + printf ' %s\n' "${SYMLINKS[@]}" + fi + # Guarded assignment: expanding an EMPTY array under `set -u` + # is an unbound-variable error on bash <4.4 (macOS ships 3.2 — + # the selftest runs this block locally). + TARGETS=() + if [ "${#KEPT[@]}" -gt 0 ]; then TARGETS=("${KEPT[@]}"); fi + if [ "${#TARGETS[@]}" -eq 0 ]; then + echo "All PR-changed glob matches are symlinks — nothing to check." + echo "mode=none" >> "$GITHUB_OUTPUT" + exit 0 + fi + echo "Mode: files (${#TARGETS[@]} target(s) after glob ∩ changed-files):" printf ' %s\n' "${TARGETS[@]}" { diff --git a/.github/workflows/pr-classify.yml b/.github/workflows/pr-classify.yml index 38d4413..ff45014 100644 --- a/.github/workflows/pr-classify.yml +++ b/.github/workflows/pr-classify.yml @@ -87,7 +87,17 @@ jobs: run: | set -euo pipefail retry() { local i out; for i in 1 2 3; do if out=$("$@"); then printf '%s' "$out"; return 0; fi; [ "$i" -lt 3 ] && sleep $((i * 5)); done; return 1; } - changed=$(retry gh pr diff "$PR" --name-only) + # Changed-file names via the paginated files API — `gh pr diff + # --name-only` HTTP-406s on diffs >20k lines (dotclaude#121). + changed=$(retry gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR}/files?per_page=100" \ + --paginate --jq '.[].filename') + # The listing endpoint caps at 3000 files. A truncated list can + # compute a falsely benign class, and the risk:* label is + # load-bearing for claude-author-automerge.yml — so stay red. + if [ "$(printf '%s\n' "$changed" | sed '/^$/d' | wc -l)" -ge 3000 ]; then + echo "::error::PR changes 3000+ files — file listing is truncated, cannot classify. Merge manually." + exit 1 + fi class=$(echo "$changed" | node .github/scripts/classify.mjs) echo "class=$class" >> "$GITHUB_OUTPUT" echo "Computed risk class: $class" diff --git a/.github/workflows/prettier-autofix.yml b/.github/workflows/prettier-autofix.yml index 434c4eb..b381c4b 100644 --- a/.github/workflows/prettier-autofix.yml +++ b/.github/workflows/prettier-autofix.yml @@ -212,6 +212,33 @@ jobs: exit 0 fi + # Prettier hard-errors on explicitly specified symlinks + # ("Explicitly specified pattern '' is a symbolic link") — + # some repos track symlinked files by convention (dotclaude's + # skills/*/SKILL.md). Drop them: the link target gets formatted + # whenever it appears in the diff itself. Glob mode is unaffected + # (prettier skips symlinks it expands from a glob on its own). + # Mirrors lint.yml — keep both filters in sync. + SYMLINKS=() + KEPT=() + for f in "${TARGETS[@]}"; do + if [ -L "$f" ]; then SYMLINKS+=("$f"); else KEPT+=("$f"); fi + done + if [ "${#SYMLINKS[@]}" -gt 0 ]; then + echo "Skipping ${#SYMLINKS[@]} symlink(s) (prettier rejects explicit symlink targets):" + printf ' %s\n' "${SYMLINKS[@]}" + fi + # Guarded assignment: expanding an EMPTY array under `set -u` + # is an unbound-variable error on bash <4.4 (macOS ships 3.2 — + # the selftest runs this block locally). + TARGETS=() + if [ "${#KEPT[@]}" -gt 0 ]; then TARGETS=("${KEPT[@]}"); fi + if [ "${#TARGETS[@]}" -eq 0 ]; then + echo "All PR-changed glob matches are symlinks — nothing to fix." + echo "mode=none" >> "$GITHUB_OUTPUT" + exit 0 + fi + echo "Mode: files (${#TARGETS[@]} target(s) after glob ∩ changed-files):" printf ' %s\n' "${TARGETS[@]}" { diff --git a/.github/workflows/safe-paths-automerge.yml b/.github/workflows/safe-paths-automerge.yml index ff272e1..2f603ce 100644 --- a/.github/workflows/safe-paths-automerge.yml +++ b/.github/workflows/safe-paths-automerge.yml @@ -69,10 +69,20 @@ jobs: EXTRA_GLOBS: ${{ inputs.extra_safe_globs }} run: | set -euo pipefail - # `gh pr diff` without a local checkout requires --repo; the - # workflow doesn't check out the caller, so pass it explicitly. - # The bare form fails with "fatal: not a git repository". - changed=$(gh pr diff --repo "$REPO" "$PR" --name-only) + # Changed-file names via the paginated files API — `gh pr diff + # --name-only` HTTP-406s on diffs >20k lines (dotclaude#121). + # (Also sidesteps the old "--repo required without a local + # checkout" footgun — the API form never needs a checkout.) + changed=$(gh api "repos/${REPO}/pulls/${PR}/files?per_page=100" \ + --paginate --jq '.[].filename') + # The listing endpoint caps at 3000 files. Past that we can't + # prove every changed file is safe → defer (same stance as the + # empty-diff branch below). + if [ "$(printf '%s\n' "$changed" | sed '/^$/d' | wc -l)" -ge 3000 ]; then + echo "all_safe=0" >> "$GITHUB_OUTPUT" + echo "reason=file-list-truncated" >> "$GITHUB_OUTPUT" + exit 0 + fi # Built-in safe globs: docs and test files. Patterns are # POSIX-extended regex (used with `grep -E`). diff --git a/selftest/README.md b/selftest/README.md index cd8a707..b7141c3 100644 --- a/selftest/README.md +++ b/selftest/README.md @@ -18,6 +18,18 @@ arbitrary helper scripts; that's a different kind of repo. - `test_smoke.py` — trivial passing test invoked by `tests-runner.yml`'s self-test path. Verifies pytest discovery, pyproject.toml plumbing, and the `uv run pytest` invocation end-to-end. +- `test_automerge_risk_patterns.sh` / `test_bb_automerge_risk_patterns.sh` + — risk-tier regex behavior, driven by the shared corpus in + `risk_patterns_corpus.txt`. +- `test_pr_files_listing.sh` — no reusable may fetch changed files via + `gh pr diff` (HTTP 406 past 20k diff lines); pins the paginated + files-API idiom instead. +- `test_prettier_symlink_filter.sh` — extracts the symlink filter from + `lint.yml` / `prettier-autofix.yml`, runs it against a fixture tree, + and asserts the two copies haven't drifted. +- `test_workflow_guards.py` — pytest wrapper that runs the `.sh` + selftests above, so `tests-runner.yml`'s self-test path enforces them + in CI. - Future: one self-test fixture per reusable (`.coverage-floor` JSON for `coverage-floor.yml`, lessons-section markdown for `regression-convention.yml`, pty fixture for `tty-tests.yml`). diff --git a/selftest/test_pr_files_listing.sh b/selftest/test_pr_files_listing.sh new file mode 100644 index 0000000..dfd57a5 --- /dev/null +++ b/selftest/test_pr_files_listing.sh @@ -0,0 +1,44 @@ +#!/usr/bin/env bash +# Guards against reintroducing `gh pr diff` as the changed-file source in +# the reusable workflows. +# +# `gh pr diff` HTTP-406s once a PR's diff exceeds 20k lines ("Sorry, the +# diff exceeded the maximum number of lines"), which killed the automerge +# risk scan, safe-paths classify, pr-classify, and the codex cost gate on +# large PRs (topcoder1/dotclaude#121, 23k insertions / 161 files; earlier +# topcoder1/attaxion_dev#71, 261k lines). The paginated files API +# (`gh api repos/{o}/{r}/pulls/{n}/files --paginate`) lists names at any +# diff size; past its 3000-file listing cap the gating workflows fail +# closed instead of silently under-classifying. +# +# Run from the repo root: +# bash selftest/test_pr_files_listing.sh +set -euo pipefail + +failed=0 + +# 1. No reusable may EXECUTE `gh pr diff`. Mentions in comments and in +# --allowedTools strings handed to review agents are fine. +viol=$(grep -rn 'gh pr diff' .github/workflows/*.yml \ + | grep -v 'allowedTools' \ + | grep -vE '^[^:]+:[0-9]+:[[:space:]]*#' || true) +if [ -n "$viol" ]; then + echo "✗ executable 'gh pr diff' found in reusables (use the paginated files API):" + echo "$viol" | sed 's/^/ /' + failed=1 +else + echo "✓ no executable 'gh pr diff' in .github/workflows/" +fi + +# 2. Every changed-file consumer pins the paginated files API idiom. +for wf in claude-author-automerge safe-paths-automerge pr-classify codex-review lint prettier-autofix; do + f=".github/workflows/${wf}.yml" + if grep -q -- '--paginate' "$f" && grep -q 'pulls/' "$f" && grep -q '/files' "$f"; then + echo "✓ ${wf}.yml lists changed files via the paginated files API" + else + echo "✗ ${wf}.yml is missing the paginated files API changed-file listing" + failed=1 + fi +done + +exit "$failed" diff --git a/selftest/test_prettier_symlink_filter.sh b/selftest/test_prettier_symlink_filter.sh new file mode 100644 index 0000000..6bf4596 --- /dev/null +++ b/selftest/test_prettier_symlink_filter.sh @@ -0,0 +1,70 @@ +#!/usr/bin/env bash +# Behavioral test for the prettier symlink filter in lint.yml and +# prettier-autofix.yml. +# +# Prettier hard-errors on explicitly specified symlinks ("Explicitly +# specified pattern '' is a symbolic link.") — repos like +# topcoder1/dotclaude track symlinked SKILL.md files by convention, which +# turned every autofix/lint run red on PRs touching them (dotclaude#121). +# Both workflows drop symlinks from the explicit target list before +# invoking prettier. +# +# The filter block is EXTRACTED from the workflow YAML and executed, so +# this exercises the shipped bash, not a mirrored copy. A drift check +# asserts lint.yml and prettier-autofix.yml carry an identical filter. +# +# Run from the repo root: +# bash selftest/test_prettier_symlink_filter.sh +set -euo pipefail + +extract_filter() { + awk '/^[[:space:]]*SYMLINKS=\(\)/{grab=1} grab{print} grab && /TARGETS=\("\$[{]KEPT\[@\][}]"\)/{exit}' "$1" +} + +failed=0 + +# 1. Drift check: the filter block must be identical in both workflows. +a=$(extract_filter .github/workflows/lint.yml) +b=$(extract_filter .github/workflows/prettier-autofix.yml) +if [ -z "$a" ] || [ -z "$b" ]; then + echo "✗ could not extract the symlink filter block from one of the workflows" + exit 1 +fi +if [ "$a" = "$b" ]; then + echo "✓ lint.yml and prettier-autofix.yml symlink filters are identical" +else + echo "✗ symlink filter drifted between lint.yml and prettier-autofix.yml:" + diff <(echo "$a") <(echo "$b") | sed 's/^/ /' || true + failed=1 +fi + +# 2. Behavioral: run the extracted block against a fixture tree. +snippet=$(echo "$a" | sed 's/^[[:space:]]*//') +tmp=$(mktemp -d) +trap 'rm -rf "$tmp"' EXIT +( + cd "$tmp" + echo '# real' > real.md + ln -s real.md link.md + ln -s missing.md dangling.md + + TARGETS=(real.md link.md dangling.md) + eval "$snippet" > /dev/null + if [ "${#TARGETS[@]}" -eq 1 ] && [ "${TARGETS[0]}" = "real.md" ]; then + echo "✓ filter keeps regular files, drops symlinks (incl. dangling)" + else + echo "✗ expected TARGETS=(real.md), got: ${TARGETS[*]:-}" + exit 1 + fi + + TARGETS=(link.md dangling.md) + eval "$snippet" > /dev/null + if [ "${#TARGETS[@]}" -eq 0 ]; then + echo "✓ all-symlink input empties the target list" + else + echo "✗ expected empty TARGETS, got: ${TARGETS[*]}" + exit 1 + fi +) || failed=1 + +exit "$failed" diff --git a/selftest/test_workflow_guards.py b/selftest/test_workflow_guards.py new file mode 100644 index 0000000..f23c7dd --- /dev/null +++ b/selftest/test_workflow_guards.py @@ -0,0 +1,32 @@ +"""Run the shell selftests under pytest. + +tests-runner.yml's self-test path executes `uv run pytest -q` on this +repo's own PRs, so wrapping the .sh selftests here is what makes them +CI-enforced rather than run-manually-only documentation. +""" + +import pathlib +import subprocess + +import pytest + +REPO_ROOT = pathlib.Path(__file__).resolve().parent.parent + + +# test_bb_automerge_risk_patterns.sh is deliberately absent: it resolves +# bb-automerge.py from the local ~/.claude/templates checkout and imports +# `requests`, neither of which exists on this repo's CI runner. Run it +# manually on a workstation. +@pytest.mark.parametrize( + "script", + [ + "selftest/test_automerge_risk_patterns.sh", + "selftest/test_pr_files_listing.sh", + "selftest/test_prettier_symlink_filter.sh", + ], +) +def test_shell_selftest(script): + proc = subprocess.run( + ["bash", script], cwd=REPO_ROOT, capture_output=True, text=True + ) + assert proc.returncode == 0, f"{script} failed:\n{proc.stdout}\n{proc.stderr}" From 904aba714f43e51a02725fb738bb07f276d9969c Mon Sep 17 00:00:00 2001 From: topcoder1 Date: Fri, 3 Jul 2026 11:54:31 -0700 Subject: [PATCH 2/2] fix(codex-review): fail closed at the 3000-file listing cap too Codex round-1 finding: a truncated file list could hide an always_review path from codex-gate.mjs and silently skip a required review. Same guard as the other gating workflows. Co-Authored-By: Claude Fable 5 --- .github/workflows/codex-review.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/codex-review.yml b/.github/workflows/codex-review.yml index b54f50d..4506175 100644 --- a/.github/workflows/codex-review.yml +++ b/.github/workflows/codex-review.yml @@ -66,6 +66,13 @@ jobs: # exactly the oversized PRs the size gate must still classify. files=$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR}/files?per_page=100" \ --paginate --jq '.[].filename') + # The listing endpoint caps at 3000 files. A truncated list could + # hide an always_review path from codex-gate.mjs and silently + # skip a required review — stay red like the other gates do. + if [ "$(printf '%s\n' "$files" | sed '/^$/d' | wc -l)" -ge 3000 ]; then + echo "::error::PR changes 3000+ files — file listing is truncated, cost gate cannot classify. Review manually." + exit 1 + fi # additions+deletions equals the +/- line count (sans +++/--- # file markers) the old `gh pr diff | awk` pass computed, without # fetching a diff body subject to the same 20k-line cap.