diff --git a/agents/repo-finder.md b/agents/repo-finder.md index abde717..cc2f9fe 100644 --- a/agents/repo-finder.md +++ b/agents/repo-finder.md @@ -601,6 +601,48 @@ fi ```bash +# De facto maintainers, for --defacto. Association reports PUBLIC org membership +# only, so a private member or a lead never added to the org comments as +# CONTRIBUTOR and their confirmation is discarded. These are the people who +# demonstrably lead the repo: >=5 of the last 20 merged PRs, bots excluded. +# +# 5, not 3: measured across 18 Go and AI repos, the non-bot distribution has a +# clean gap there. 6 authors sit at 5-15 of 20 and every one is an insider whose +# association under-reports; below the gap, 13 authors sit at exactly 3 and are +# ordinary contributors. A cutoff of 3 admits 23 authors and re-creates the +# commit-rank error PR #25 fixed. +# +# $BOT_NAMES is a first pass only. It misses handles like `...-cherrypick-robot`, +# where `bot` is not a whole word. That is safe: the filter's own bot regex is a +# superset (`bot$` catches it) and runs before grading, so a bot that survives +# here is dropped there and can never be graded. +# +# Both filters run inside jq. `grep -ivE "$BOT_NAMES"` exits 1 when it matches +# nothing, and matching nothing is the ordinary case here — most repos have no +# bot above the threshold — so the pipeline could not tell an empty-but-correct +# result from a jq that died, and `|| :` turned both into "no de facto +# maintainers". Empty is a valid answer; a failure to compute one is not, and +# only jq's own exit code separates them. +# +# `// empty` drops ghost authors: a deleted account serializes as `author: null`, +# which `.author.login` yields as null and five of which would group into a +# literal `null` maintainer. Same guard as 3c, same reason. +# +# An unset $BOT_NAMES would make `test("")` match every login and empty the set +# in silence — the failure mode this tier exists to remove. Assert it instead. +# +# Reuses $MERGED_PRS from 3a. No extra API calls. +: "${BOT_NAMES:?BOT_NAMES is unset — define it (3d) before deriving de facto maintainers}" +DEFACTO="$SCRATCH/defacto.txt" +if ! jq -r --arg bots "$BOT_NAMES" ' + [ .[].author.login // empty ] + | group_by(.) | map(select(length >= 5) | .[0]) + | .[] | select(test($bots; "i") | not) + ' "$MERGED_PRS" > "$DEFACTO"; then + echo "FATAL: could not derive de facto maintainers from $MERGED_PRS" >&2 + exit 10 +fi + # Pure function of $ISSUES and $MAINTAINERS — no API calls. Issues failing the # age or triage-signal gate produce no output; every later rejection carries a # reason. See scripts/orchestrator/triage_filter.sh. @@ -610,7 +652,7 @@ fi # leaves a TRUNCATED file that reads as a clean, shorter candidate set — the exact # failure this file exists to prevent. Abort instead of trusting it. if ! "${CLAUDE_PLUGIN_ROOT}/scripts/orchestrator/triage_filter.sh" \ - --issues "$ISSUES" --maintainers "$MAINTAINERS" > "$SCRATCH/stage_a_all.jsonl"; then + --issues "$ISSUES" --maintainers "$MAINTAINERS" --defacto "$DEFACTO" > "$SCRATCH/stage_a_all.jsonl"; then echo "FATAL: triage_filter.sh failed — Stage A output is unreliable, aborting." >&2 exit 10 fi @@ -751,7 +793,7 @@ visibly drop one. Apply the prose here, then say in `notes` that it did. - **Is a bug** (not a feature request): +3 - **Has reproduction steps**: +2 - **Labeled good-first-issue or help-wanted**: +2 -- **Maintainer signal**, graded from `maintainer_signal`: `invites_pr` **+5** · `confirms` **+3** · `neutral` **+1** · `none` **0**. Association is the prerequisite, never the grade — a union-only commenter earns nothing here. +- **Maintainer signal**, graded from `maintainer_signal`: `invites_pr` **+5** · `confirms` **+3** · `neutral` **+1** · `none` **0**. Gradeable means association **or** de facto membership (>=5 of the last 20 merged PRs, bots excluded) — never the grade itself, which the comment text sets. The de facto tier exists because association reports public org membership only and under-reports exactly those leads. The 3c union stays ungraded: a union-only commenter earns nothing here, because commit rank is not authority. - **Maintainer engaged within the last 28 days**: +2 - **Issue age**, graded: 2–30d **+1** · 30–90d **0** · 90–365d **−1** · >365d **−2**. Scored, never hard-skipped — old is riskier, not worthless. - **Scope is small** (likely < 100 lines, single-file fix): +2 diff --git a/scripts/orchestrator/triage_filter.sh b/scripts/orchestrator/triage_filter.sh index cb5fc14..70deb37 100755 --- a/scripts/orchestrator/triage_filter.sh +++ b/scripts/orchestrator/triage_filter.sh @@ -25,11 +25,12 @@ set -euo pipefail die() { echo "FATAL: $*" >&2; exit 10; } -ISSUES="" MAINTAINERS="" NOW="" +ISSUES="" MAINTAINERS="" DEFACTO="" NOW="" while [ $# -gt 0 ]; do case "$1" in --issues) [ $# -ge 2 ] || die "--issues needs a value"; ISSUES="$2"; shift 2 ;; --maintainers) [ $# -ge 2 ] || die "--maintainers needs a value"; MAINTAINERS="$2"; shift 2 ;; + --defacto) [ $# -ge 2 ] || die "--defacto needs a value"; DEFACTO="$2"; shift 2 ;; --now) [ $# -ge 2 ] || die "--now needs a value"; NOW="$2"; shift 2 ;; *) die "unknown argument: $1" ;; esac @@ -39,6 +40,10 @@ done [ -n "$MAINTAINERS" ] || die "--maintainers is required" [ -f "$ISSUES" ] || die "issues file not found: $ISSUES" [ -f "$MAINTAINERS" ] || die "maintainers file not found: $MAINTAINERS" +# Optional: callers predating the de facto tier pass no list, and an absent list +# reads as empty, which restores the association-only behaviour exactly. +[ -n "$DEFACTO" ] && { [ -f "$DEFACTO" ] || die "defacto file not found: $DEFACTO"; } +[ -n "$DEFACTO" ] || DEFACTO=/dev/null command -v jq >/dev/null 2>&1 || die "jq is required" # Injectable clock so the 24h rule is testable against a fixed fixture. @@ -47,8 +52,17 @@ case "$NOW" in (*[!0-9]*) die "--now must be a unix timestamp";; esac # Regexes use `.` where an apostrophe belongs (don.t, I.ll): this jq program is a # single-quoted shell string and cannot contain one. -jq -c --rawfile m "$MAINTAINERS" --argjson now "$NOW" ' +jq -c --rawfile m "$MAINTAINERS" --rawfile d "$DEFACTO" --argjson now "$NOW" ' ($m | rtrimstr("\n") | split("\n") | map(select(. != ""))) as $maint + # De facto maintainers: narrower than $maint by intent, admitted to grading + # because they demonstrably lead the repo. See the caller for how it is built. + # Narrower by intent is not narrower by construction: $maint ranks contributors + # by ALL-TIME commits, so a lead who joined recently can clear 5 of the last 20 + # merged PRs and still sit outside the top 25 — and the association legs of + # $maint cannot catch them either, since under-reported association is the whole + # reason they are here. So $defacto is unioned into $mc below rather than + # assumed to be inside it. + | ($d | rtrimstr("\n") | split("\n") | map(select(. != ""))) as $defacto # Bots are not maintainers. Superset of the 3c bot regex: a repo whose CI bot # files, labels and comments its own issues yields a perfect triage signal on @@ -129,20 +143,39 @@ jq -c --rawfile m "$MAINTAINERS" --argjson now "$NOW" ' # `// ""` guards ghost authors: a deleted account serializes as `author: null`, # and `null | test(...)` throws, aborting the whole batch. | ($i.comments | map(select((.author.login // "") | test($bots) | not))) as $human + # $defacto belongs here, not only in $mgrade. This set drives the triage gate, + # `maintainer_commented` and `last_maintainer_comment`. Grading a de facto lead + # who is outside $maint while leaving them out of $mc drops the issue at the + # gate unless a label rescues it — the exact silent discard this tier exists to + # fix — and any issue that a label does rescue emits the contradiction + # `maintainer_commented: false` beside `maintainer_signal: confirms`. | ($human | map(select( (.authorAssociation | IN("OWNER","MEMBER","COLLABORATOR")) or (.author.login as $a | $maint | index($a)) + or (.author.login as $a | $defacto | index($a)) ))) as $mc | ($human | map(select( .authorAssociation | IN("OWNER","MEMBER","COLLABORATOR") ))) as $mca + + # Grading set, deliberately distinct from both sets above. Association is the + # primary key, but it reports *public* org membership only, so a private member + # or a lead never added to the org arrives as CONTRIBUTOR and their confirmation + # is discarded. $defacto restores exactly those. It is not the $maint union: + # commit rank is not authority, and grading off the union would score any + # prolific committer as a maintainer, which is the distinction PR #25 drew. + # $mca stays association-only because maintainer_comment_assoc reports it. + | ($human | map(select( + (.authorAssociation | IN("OWNER","MEMBER","COLLABORATOR")) + or (.author.login as $a | $defacto | index($a)) + ))) as $mgrade | ($i.labels | map(.name | ascii_downcase)) as $L | ($mc | map(.body // "") | join("\n") | ascii_downcase) as $mbody # Grade only what is left after stripping links and bare @mentions: a comment # that is only a pointer somewhere else states no position on this issue, and no # sentiment tier can read one. - | ($mca | map((.body // "") + | ($mgrade | map((.body // "") | ascii_downcase | gsub("https?://\\S+"; " ") | gsub("@[a-z0-9_-]+"; " ") diff --git a/tests/scripts/test_triage_filter.sh b/tests/scripts/test_triage_filter.sh index c51901b..2c70eb0 100755 --- a/tests/scripts/test_triage_filter.sh +++ b/tests/scripts/test_triage_filter.sh @@ -16,7 +16,18 @@ FRESH="2026-07-11T18:00:00Z" # 6 hours old — must be dropped RECENT="2026-07-10T00:00:00Z" # 2 days old — a claim this fresh is still live STALE="2026-05-20T00:00:00Z" # 53 days old — a claim this old is abandoned -printf 'unionlead\n' > "$tmpdir/maintainers.txt" +printf 'unionlead\ndefactolead\n' > "$tmpdir/maintainers.txt" + +# De facto maintainers: authored a large share of the recent merged PRs, but +# GitHub reports them as CONTRIBUTOR (private org membership, or a lead who was +# never added to the org). +# +# Two of them, because containment in the 3c union is an intent and not a +# guarantee: `defactolead` is in the union, `outsidelead` is not. The union ranks +# contributors by ALL-TIME commits, so a lead who joined recently clears 5 of the +# last 20 merged PRs while sitting outside the top 25, and association cannot +# catch them either — under-reported association is why they are in this file. +printf 'defactolead\noutsidelead\n' > "$tmpdir/defacto.txt" cat > "$tmpdir/issues.json" < "$tmpdir/issues.json" < "$tmpdir/issues.json" < "$OUT" +"$FILTER" --issues "$tmpdir/issues.json" --maintainers "$tmpdir/maintainers.txt" \ + --defacto "$tmpdir/defacto.txt" --now "$NOW" > "$OUT" verdict() { jq -r --argjson n "$1" 'select(.number == $n) | .verdict' "$OUT"; } reason() { jq -r --argjson n "$1" 'select(.number == $n) | .reason' "$OUT"; } @@ -356,9 +378,34 @@ want 916 KEEP "stale claim is abandoned, not competing" want 917 KEEP "bare link is not a defect signal" [ "$(field 917 maintainer_signal)" = "none" ] || { echo "FAIL #917 signal: got '$(field 917 maintainer_signal)'"; exit 1; } -# A union-only lead still earns no signal, whatever they wrote. +# A union-only lead still earns no signal, whatever they wrote. Commit rank is +# not authority: the 3c union is deliberately wide so the fortress check errs +# toward skipping, and reading endorsement off it would grade any prolific +# committer as a maintainer. [ "$(field 904 maintainer_signal)" = "none" ] || { echo "FAIL #904 signal: got '$(field 904 maintainer_signal)'"; exit 1; } +# A de facto lead is graded. They are in the union too, so the association-only +# rule above would silently discard the strongest signal the repo has, which is +# how a repo led by a long-term contributor outside the org scores `none` on an +# issue its lead confirmed. +want 927 KEEP "de facto lead confirmed" +[ "$(field 927 maintainer_signal)" = "confirms" ] || { echo "FAIL #927 signal: got '$(field 927 maintainer_signal)'"; exit 1; } +[ "$(field 927 maintainer_comment_assoc)" = "false" ] || { echo "FAIL #927 assoc must stay false"; exit 1; } + +# The same lead, outside the union. Grading alone is not enough: the triage gate, +# `maintainer_commented` and `last_maintainer_comment` all read the $mc set, so a +# de facto lead admitted to grading but not to $mc leaves this issue with no +# maintainer comment on record — it fails the gate and never reaches the ranker, +# which is the discard this tier exists to fix. `bug` is deliberately the only +# label: a good-first-issue label would rescue the issue at the gate and hide the +# defect behind a KEEP that reads `maintainer_commented: false` next to +# `maintainer_signal: confirms`. +want 928 KEEP "de facto lead outside the union still counts as a maintainer comment" +[ "$(field 928 maintainer_signal)" = "confirms" ] || { echo "FAIL #928 signal: got '$(field 928 maintainer_signal)'"; exit 1; } +[ "$(field 928 maintainer_commented)" = "true" ] || { echo "FAIL #928 must record a maintainer comment"; exit 1; } +[ "$(field 928 maintainer_comment_assoc)" = "false" ] || { echo "FAIL #928 assoc must stay false"; exit 1; } +[ "$(field 928 last_maintainer_comment)" = "$OLD" ] || { echo "FAIL #928 last_maintainer_comment: got '$(field 928 last_maintainer_comment)'"; exit 1; } + # The strongest signal is also the shortest one anyone writes. A prose floor # applied before the match would grade this as no signal at all. want 920 KEEP "terse invite" @@ -391,4 +438,24 @@ want 925 KEEP "asking about an issue is not claiming it" want 926 SKIP "anchored outsider claim still skips" want_reason 926 claimed +# --defacto is optional, and an absent list must restore association-only +# behaviour exactly — callers predating this tier pass no list at all. Asserted, +# not diffed by hand: the whole tier is opt-in, and an /dev/null default that +# quietly changed a verdict would be a regression nothing else here can see. +OUT_LEGACY="$tmpdir/out_legacy.jsonl" +"$FILTER" --issues "$tmpdir/issues.json" --maintainers "$tmpdir/maintainers.txt" \ + --now "$NOW" > "$OUT_LEGACY" +legacy() { jq -r --argjson n "$1" "select(.number == \$n) | .$2" "$OUT_LEGACY"; } + +# 927's author is in the union, so the issue still clears the gate — but with no +# list to lift them past association, the signal they earned above is gone. +[ "$(legacy 927 maintainer_signal)" = "none" ] || { echo "FAIL legacy #927 signal: got '$(legacy 927 maintainer_signal)'"; exit 1; } +# 928's author is in neither set without the list, so the issue is not a +# candidate at all. This is the pre-fix behaviour, reproduced exactly. +[ -z "$(legacy 928 verdict)" ] || { echo "FAIL legacy #928: must fail the gate, got '$(legacy 928 verdict)'"; exit 1; } +# Every issue that does not turn on the de facto tier must be byte-identical. +diff <(grep -Ev '"number":(927|928)[,}]' "$OUT") \ + <(grep -Ev '"number":(927|928)[,}]' "$OUT_LEGACY") \ + || { echo "FAIL: --defacto changed a verdict it must not touch"; exit 1; } + echo "OK test_triage_filter.sh"