Skip to content

Collapse spam triage into the agentic issue-triage workflow - #14027

Merged
williammartin merged 1 commit into
trunkfrom
williammartin-collapse-spam-triage
Jul 31, 2026
Merged

Collapse spam triage into the agentic issue-triage workflow#14027
williammartin merged 1 commit into
trunkfrom
williammartin-collapse-spam-triage

Conversation

@williammartin

@williammartin williammartin commented Jul 31, 2026

Copy link
Copy Markdown
Member

Fixes #14024

Description

gh models was retired on 2026-07-30. The Spam Issue Detection workflow is built on gh 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: opened and interfere with each other:

issues:opened ─┬─ Spam Issue Detection ────── comment + [suspected-spam, invalid] + close   (BROKEN)
               ├─ Issue Triaging ─┬─ label-incoming ──── +needs-triage
               │                  └─ close-single-word ─ comment + labels=[invalid] + close
               └─ Issue Triage (skills-driven) ── suggests labels + comments
  • Two bots comment on the same issue. Noey #13974 and x #14020 each got two closure comments seconds apart, one from each closer.
  • Labels get clobbered. close-single-word calls issues.update({labels: ['invalid']}), which replaces the label set rather than adding to it. x #14010 lost suspected-spam; probe #14023 lost needs-triage; Noey #13974 kept it. Which labels survive depends on which workflow finishes first.
  • The capable classifier never gets to act. 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 emitted noop.

This collapses all of it into one path:

issues:opened ─┬─ Issue Triaging ── label-incoming ── +needs-triage
               └─ Issue Triage (skills-driven) ── applies suspected-spam (no comment)
                                                        │
                                    issues:labeled ─────┘
                                                        ▼
                                     close-suspected-spam (shared)
                                     comment + remove needs-triage + close

One comment, one closer, one label writer.

issue-triage.md was already the right home: it loads the issue-classifier skill, whose decision tree opens with "Spam, gibberish, or AI-generated slop", and suspected-spam is already in its add-labels allowlist. What it lacked was the cli/cli-specific spam criteria, which lived in generate-sys-prompt.sh. Those move to shared/spam-criteria.md and reach the agent through gh-aw's imports: key, so they land in the prompt on every run with no tool call.

invalid deliberately 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 eval to copilot -p.

$ ./.github/workflows/scripts/spam-detection/eval.sh
running 274 cases on gpt-5-mini (effort low, concurrency 8)

cases            274
correct          256 (93.4%)
false positives  1  (legitimate issue judged spam)
false negatives  17  (spam issue judged legitimate)
duration         376s

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.json lists 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:

$ gh aw compile .github/workflows/issue-triage.md
✓ issue-triage.md
$ shellcheck .github/workflows/scripts/spam-detection/eval.sh   # clean
$ ./.github/workflows/scripts/spam-detection/eval.sh -V
FAIL  73
PASS  201
total 274

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: labeled event which triggers close-suspected-spam. App installation tokens do trigger workflows (unlike GITHUB_TOKEN) and the shared job's if: condition matches, but issue-triage.md has 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 --help injection was removed on evidence, not taste. generate-sys-prompt.sh appended the live stdout of gh --help to 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:

Arm Prompt Correct vs baseline
A current, verbatim 256/273 (93.8%) -
A2 identical to A 258/273 (94.5%) +0.7pp, 2 cases flipped
B A minus the docs section 256/273 (93.8%) +0.0pp, 4 cases flipped

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 --help renders EXTENSION COMMANDS and ALIAS COMMANDS from 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 + invalid by the old classifier despite being a genuine report with a linked fix. It is now in the corpus as a PASS case, sitting deliberately close to the existing spam, a hyperlink FAIL case; the difference is whether the link is coherent with the title.

Issue templates are inlined into spam-criteria.md rather than read at runtime, because a spam indicator refers to them directly. That introduces drift if .github/ISSUE_TEMPLATE/*.md change; 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.md is 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 in eval-instructions.md.

Notes for reviewers

Start with shared/spam-criteria.md (the content being preserved) and the Step 5 carve-out in issue-triage.md (the behaviour change). eval.sh is a tool, not production code, and can be skimmed.

The one frontmatter subtlety: under issue-intent, omitting suggest applies a label directly while suggest: true makes it a proposal. So the carve-out is a prompt change, not a frontmatter change - suspected-spam is emitted without suggest, everything else keeps it.

Related:

Authorship and follow-up

Who wrote this:

  • A human wrote it.
  • An agent wrote it under close human direction.
  • An agent wrote it independently, and no human has guided the implementation beyond the initial prompt.

Who answers review comments:

  • @williammartin will read and reply directly.
  • An agent will draft replies and @username will read them before they are posted.
  • Nobody has explicitly committed to replying.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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 models spam detection workflow + scripts and drops the close-single-word job to avoid racey “multiple closers / label replacement” behavior.
  • Introduces a shared prompt component (shared/spam-criteria.md) imported into issue-triage.md, enabling direct application of suspected-spam to 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.

Comment thread .github/workflows/scripts/spam-detection/eval.sh Outdated
Comment thread .github/workflows/scripts/spam-detection/eval.sh Outdated
Comment thread .github/workflows/issue-triage.lock.yml Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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 copilot CLI but doesn't verify it's installed. If copilot is 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
@williammartin
williammartin force-pushed the williammartin-collapse-spam-triage branch from fc9299e to 5583cde Compare July 31, 2026 14:02
@williammartin

Copy link
Copy Markdown
Member Author

Addressed the three suppressed comments from the latest review. Assessment of each:

copilot presence unchecked — correct, and inconsistent of me. I checked jq, python3 and PyYAML but not the one binary the whole script exists to call. Added; it now fails with error: copilot is required instead of producing a full corpus of empty verdicts. Verified against a PATH with copilot removed.

./...eval.sh placeholder paths — correct. They were shorthand I never expanded. Now full paths, since that block is meant to be copy-pasted.

Discarded stderr — correct, and the most useful of the three. I measured what copilot actually does:

  • success: exit 0, stderr is a 137-byte stats footer (noise)
  • failure: exit 1, stderr is Error: Model "..." is not available (the only diagnostic)

I was discarding both. A typo in --model therefore produced 274 "unparseable" cases, exit 0, and no clue why. Now stderr is captured and surfaced only on non-zero exit, so the footer still stays out of the results.

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:

# before: unparseable      3          (exit 0)

# after:
errored          3  (no verdict returned)
first error:
  exit 1: Error: Model "definitely-not-a-model" from --model flag is not available.
incorrect cases:
  [want FAIL got error] spam, a hyperlink

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.

@williammartin
williammartin marked this pull request as ready for review July 31, 2026 14:58
@williammartin
williammartin requested a review from a team as a code owner July 31, 2026 14:58
@williammartin
williammartin requested a review from tidy-dev July 31, 2026 14:58
@williammartin
williammartin merged commit 5131aaf into trunk Jul 31, 2026
16 checks passed
@williammartin
williammartin deleted the williammartin-collapse-spam-triage branch July 31, 2026 15:18
ieuanign added a commit to ieuanign/skills that referenced this pull request Aug 9, 2026
…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.
ieuanign added a commit to ieuanign/skills that referenced this pull request Aug 9, 2026
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update spam detection workflow to use "gh aw"

2 participants