Collapse spam triage into the agentic issue-triage workflow - #14027
Conversation
d57b3ab to
4e44195
Compare
4e44195 to
da8d4cb
Compare
There was a problem hiding this comment.
🟡 Not ready to approve
The updated eval harness has verified runtime issues (missing PyYAML check and brittle verdict parsing) that can break the “evals are kept” requirement until corrected.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR consolidates issue spam triage into the existing agentic issue-triage.md workflow (using gh aw/Copilot) to replace the retired gh models-based automation and eliminate competing issues:opened workflows that were racing and clobbering labels.
Changes:
- Removes the legacy
gh modelsspam detection workflow + scripts and drops theclose-single-wordjob to avoid racey “multiple closers / label replacement” behavior. - Introduces a shared prompt component (
shared/spam-criteria.md) imported intoissue-triage.md, enabling direct application ofsuspected-spamto trigger the shared close workflow. - Rebuilds the eval harness to run the existing corpus via
copilot -p, producing comparable per-case results and disagreement-set diffs.
File summaries
| File | Description |
|---|---|
| .github/workflows/triage-issues.yml | Removes close-single-word job to reduce competing issues:opened responders. |
| .github/workflows/shared/spam-criteria.md | New shared spam criteria content imported into agentic triage and reused by eval harness. |
| .github/workflows/scripts/spam-detection/process-issue.sh | Deleted legacy issue processing script that commented/labeled/closed via gh models. |
| .github/workflows/scripts/spam-detection/generate-sys-prompt.sh | Deleted legacy dynamic prompt generator (included gh --help + templates at runtime). |
| .github/workflows/scripts/spam-detection/eval.sh | Replaces gh models eval runner with a local harness using copilot -p + JSON output/diffing. |
| .github/workflows/scripts/spam-detection/eval-prompts.yml | Extends corpus with the #13783 false-positive regression case. |
| .github/workflows/scripts/spam-detection/eval-instructions.md | New eval-only PASS/FAIL output contract (kept separate from shared criteria). |
| .github/workflows/scripts/spam-detection/check-issue.sh | Deleted legacy gh models run inference wrapper. |
| .github/workflows/scripts/spam-detection/check-issue-prompts.yml | Deleted legacy gh models prompt file template. |
| .github/workflows/issue-triage.md | Imports shared spam criteria and adds a “direct apply suspected-spam” carve-out before suggestion flow. |
| .github/workflows/issue-triage.lock.yml | Regenerated compiled workflow to include runtime import of spam criteria and updated strict compilation metadata. |
| .github/workflows/detect-spam.yml | Deletes broken gh models-based “Spam Issue Detection” workflow. |
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 3
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
da8d4cb to
fc9299e
Compare
There was a problem hiding this comment.
🟡 Not ready to approve
The updated eval harness can silently turn copilot invocation failures into “unparseable” results without failing fast or surfacing diagnostics, which undermines the reliability of the preserved evals.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (3)
.github/workflows/scripts/spam-detection/eval.sh:85
- The eval runner invokes the
copilotCLI but doesn't verify it's installed. Ifcopilotis missing, the model call fails and the script currently proceeds with empty verdicts, producing a misleading run rather than failing fast.
for tool in jq python3; do
command -v "$tool" >/dev/null || { echo "error: $tool is required" >&2; exit 1; }
done
.github/workflows/scripts/spam-detection/eval.sh:29
- The usage examples in the header comment use
./...eval.sh, which looks like a placeholder path and isn't runnable as written. Using the full script path here avoids copy/paste errors.
# ./...eval.sh -c before.md -o before.json
# ./...eval.sh -c after.md -o after.json
# ./...eval.sh -d before.json,after.json
.github/workflows/scripts/spam-detection/eval.sh:198
- The copilot invocation currently discards stderr and converts any non-zero exit into an empty response (
|| raw=""). This can turn authentication/rate-limit/command-not-found errors into "unparseable" cases with no diagnostics, and can make the run look like a valid result set when it isn't.
raw=$(HOME="$workdir" copilot -p "$(cat "$system")
${input}" \
--model "$model" --effort "$effort" --allow-all-tools --no-color \
--log-level none --disable-builtin-mcps --no-custom-instructions 2>/dev/null) || raw=""
- Files reviewed: 12/12 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
`gh models` was retired on 2026-07-30, so Spam Issue Detection failed on
every new issue. Rather than port it as-is, this collapses three workflows
that raced on `issues: opened` into one path.
The race was actively destructive: `close-single-word` called
`issues.update({labels:['invalid']})`, which replaces rather than appends,
so #14010 lost `suspected-spam` and #14023 lost `needs-triage` depending on
which workflow finished first. Issues also got two closure comments.
`issue-triage.md` now owns spam. It already loaded the `issue-classifier`
skill and had `suspected-spam` in its allowlist; what was missing was the
cli/cli criteria and permission to apply that one label directly. Under
issue-intents, omitting `suggest` applies a label, so the carve-out is a
prompt change rather than a config change. The agent stays silent on spam
and the shared `close-suspected-spam` job posts the comment, removes
`needs-triage`, and closes.
The criteria move from a bash-assembled runtime string to
`shared/spam-criteria.md`, imported via `imports:`. The file is
role-neutral because it has two consumers that must act on it differently:
the workflow applies a label, the eval emits a verdict.
Evals are preserved. `eval.sh` is now a wrapper over a Go runner using
`copilot -p`, since `gh models eval` is gone. It reports a per-case
disagreement set rather than only a pass rate, because a same-prompt re-run
was measured moving the aggregate 0.7 points, which would mask a small real
regression.
The `gh --help` injection is dropped on evidence, not taste. No spam or
legitimacy indicator referenced command knowledge, and ablating it left the
corpus at 256/273, identical to baseline, while an unchanged-prompt control
run moved further. That removes 37% of the prompt and the only piece that
was not reproducible: it rendered the invoking user's extensions and
aliases into the prompt.
Also added #13783 as a PASS case, a genuine report the old classifier
wrongly closed as spam, and a CI workflow for the harness, which lives
under `.github/` and so is invisible to `go test ./...` and `make lint`.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 28d6b772-7c8a-4c71-907a-c89469731e36
fc9299e to
5583cde
Compare
|
Addressed the three suppressed comments from the latest review. Assessment of each:
Discarded stderr — correct, and the most useful of the three. I measured what
I was discarding both. A typo in Errored cases are also counted separately from unparseable ones, because they mean different things: unparseable is the model answering something unexpected, errored is it never answering, and only the first says anything about the criteria. Before/after on a deliberately broken run: The run now also exits non-zero if any case errored, so a run degraded by a bad flag, expired auth or rate limiting cannot be mistaken for a measurement. Confirmed the success path is unaffected: exit 0, no error fields, no footer leakage. On the "not ready to approve" framing: none of these affect the workflow being shipped, which had no findings. They affect the eval harness. That still matters here, because this PR argues from its eval numbers, so a harness that can quietly report garbage undermines the case for the change. |
…esolved comments `skills/dev-loop-pr-comments/read-comments.mjs` returns a pull request's unresolved comments as one list: thread-anchored review comments, review bodies and issue comments, in a single entry shape a consumer switches on by `origin`. Exclusions are signals, never inferences. A resolved thread goes on GraphQL's own `isResolved`, because REST cannot express it and a guess returns every comment the pull request ever carried; a minimised comment goes because hiding one is a human saying it is dealt with; a PENDING review is unsubmitted and a bodyless review is the envelope around inline comments already collected. Absence is carried, not filled in. An outdated comment keeps `line: null` while its stale `originalLine` travels separately, so *no line* and *line moved* stay distinguishable. Bodies never reach a shell: `gh` is spawned with an argument array, `shell: false`, and its stdout is parsed. All four connections paginate, the nested one by count because `gh --paginate` follows only the first pageInfo in a response, and any failure exits non-zero with no JSON so that an empty list can only mean no unresolved comments. The folder holds the module alone — no SKILL.md, no manifest entry — so nothing installs or invokes it yet. The normaliser is exported and importing it runs no fetch. Verified live, read-only, no mutation issued: cli/cli#14007 → 5 review-thread + 2 review-body entries against an independent query's 9 threads / 4 resolved / 5 unresolved comments / 3 reviews of which 2 carry a body; bodies byte-identical for all 5; two entries with line=null, originalLine=431, outdated=true. cli/cli#14027 → 3 threads all resolved, 2 review bodies, 1 issue comment. cli/cli#14099 with the page size forced to 1 reproduces its full-page output byte for byte, exercising every cursor including the per-thread comment refetch. Missing gh, an unknown pull request, an unresolvable repository and a bad argument each exit non-zero with empty stdout. Deviation: the changeset is hand-written as .changeset/140-read-unresolved-pr-comments.md rather than generated by `npm run changeset`, which names files after human-id words; every tracked changeset here is named <issue>-<slug>.md.
…checked, not just parsed The check suite proved read-comments.mjs PARSED. Everything that makes it safe to point at a pull request — which comments it drops, which fields it refuses to invent, and whether a body survives — was unobserved. `scripts/pr-comment-read.mjs` observes it, at the same seam the state-machine harness uses: the exported pure function, driven over fixtures, spawning nothing and reaching no network. Seven scenarios over the normaliser's public interface, none touching an internal. A resolved thread is excluded whole while an unresolved one is kept whole; a minimised comment goes at all three origins; a bodyless, whitespace-only or PENDING review is not a comment while a review that said something is. An outdated comment keeps `line: null` with `originalLine` beside it, so the scenario fails the moment anything substitutes the stale anchor. A body carrying backticks, `$(...)`, quotes, a backslash, a pipe and a newline comes back byte for byte, through the JSON round trip the reader actually prints. Every entry carries the same eleven keys whatever its origin, and an empty pull request is an empty list. Each scenario was confirmed to FAIL against a deliberately broken normaliser before being kept: dropping the `isResolved` guard admits the resolved thread's two comments, and substituting `originalLine` into `line` turns null into 431. check.sh gains one stage after the bundled-module syntax stage, streaming its own ok/FAIL line per scenario with only its exit code read — the state-machine stage's shape. Verified live, read-only, no mutation issued (the module sends query documents only): cli/cli#14027 → 3 entries (2 review-body, 1 issue-comment) against an independent query's 3 threads all resolved, 5 reviews of which 2 carry a body and 3 are bodyless, 1 issue comment. cli/cli#14007 → 7 entries (5 review-thread, 2 review-body) against 9 threads / 4 resolved / 5 unresolved carrying 5 comments none minimised, 3 reviews of which 2 carry a body, 0 issue comments; both null-line entries report originalLine=431, outdated=true, matching the independent query exactly. Both runs exited 0 with empty stderr and one parseable JSON document on stdout. `npm run check` green, including `claude plugin validate . --strict` with the manifest-unlisted skills/dev-loop-pr-comments/ folder in place.
Fixes #14024
Description
gh modelswas retired on 2026-07-30. TheSpam Issue Detectionworkflow is built ongh models run, so it has been failing on every new issue since.Porting it as-is would have preserved a worse problem. Three workflows currently fire on
issues: openedand interfere with each other:close-single-wordcallsissues.update({labels: ['invalid']}), which replaces the label set rather than adding to it. x #14010 lostsuspected-spam; probe #14023 lostneeds-triage; Noey #13974 kept it. Which labels survive depends on which workflow finishes first.Issue Triage (skills-driven)takes ~4 minutes; the deterministic closers finish in ~10 seconds. On probe #14023 the agent fetched the issue 7s after it had already been closed and emittednoop.This collapses all of it into one path:
One comment, one closer, one label writer.
issue-triage.mdwas already the right home: it loads theissue-classifierskill, whose decision tree opens with "Spam, gibberish, or AI-generated slop", andsuspected-spamis already in itsadd-labelsallowlist. What it lacked was the cli/cli-specific spam criteria, which lived ingenerate-sys-prompt.sh. Those move toshared/spam-criteria.mdand reach the agent through gh-aw'simports:key, so they land in the prompt on every run with no tool call.invaliddeliberately stays suggestion-only, because it routes to a different shared workflow that closes with no comment at all.How did you test this change?
The evals are kept, as #14024 asks. The 273-case corpus carries over verbatim; only the runner changed, from
gh models evaltocopilot -p.False positives and negatives are reported separately rather than as one accuracy figure, because closing a real report is the costlier error.
Judged by disagreement set, not pass rate. A re-run of an unchanged prompt moved the aggregate by 0.7 points, so a headline number cannot distinguish a small real change from noise.
eval.sh -d before.json,after.jsonlists which specific cases moved and in which direction.Old prompt vs. the new criteria file: 256/273 both, 2 cases flipped - at the same-prompt noise floor. The port does not change behaviour.
Compilation and lint:
Not yet verified, and it needs to be before this is considered done: that a label applied by the workflow's GitHub App token fires an
issues: labeledevent which triggersclose-suspected-spam. App installation tokens do trigger workflows (unlikeGITHUB_TOKEN) and the shared job'sif:condition matches, butissue-triage.mdhas never applied a label, so there is no direct evidence in this repo, and cross-workflow chaining cannot be tested from a branch. If it does not chain, spam gets labelled but not closed - visible and recoverable.Key points
The
gh --helpinjection was removed on evidence, not taste.generate-sys-prompt.shappended the live stdout ofgh --helpto the prompt. None of the 12 spam indicators reference command knowledge, so I tested whether anything consumed it, using a same-prompt re-run as the noise control:Re-running an unchanged prompt moved the aggregate more than deleting the section did. It is inert, so it is gone: 3085 bytes, 37% of the prompt.
That also removes a reproducibility hazard.
gh --helprendersEXTENSION COMMANDSandALIAS COMMANDSfrom the invoking user's setup, so the old eval put whatever a developer had installed into the prompt.A false positive is fixed. #13783 ("missing installation instructions for Amazon Linux 2023", body: a bare PR link) was labelled
suspected-spam+invalidby the old classifier despite being a genuine report with a linked fix. It is now in the corpus as aPASScase, sitting deliberately close to the existingspam, a hyperlinkFAIL case; the difference is whether the link is coherent with the title.Issue templates are inlined into
spam-criteria.mdrather than read at runtime, because a spam indicator refers to them directly. That introduces drift if.github/ISSUE_TEMPLATE/*.mdchange; they are three files totalling 2775 bytes and change rarely, and the file names its source directory.Spam will now be closed in ~4 minutes rather than ~10 seconds, since the agent replaces the deterministic closers. That is the price of removing the label-clobbering race, and it seems worth paying.
spam-criteria.mdis deliberately role-neutral - it describes what spam looks like and says nothing about what to do about it. The workflow acts on it by applying a label and the eval by emitting a verdict, so an output contract in the shared file would force one consumer to contradict it. The eval-only contract lives ineval-instructions.md.Notes for reviewers
Start with
shared/spam-criteria.md(the content being preserved) and the Step 5 carve-out inissue-triage.md(the behaviour change).eval.shis a tool, not production code, and can be skimmed.The one frontmatter subtlety: under
issue-intent, omittingsuggestapplies a label directly whilesuggest: truemakes it a proposal. So the carve-out is a prompt change, not a frontmatter change -suspected-spamis emitted withoutsuggest, everything else keeps it.Related:
gh-*command labels it cannot apply #14026 - filed while working on this:label-taxonomy.mdtells the agent to add a command label, but nogh-*label is in theadd-labelsallowlist, so it cannot. Pre-existing and out of scope here.gh modelsscripts and is equally broken. Different repo, so not fixed here.Authorship and follow-up
Who wrote this:
Who answers review comments: