ci: annotate a red pytest job with the tests that failed, not a warning

[chenmingwei23]

Problem / Motivation

A red pytest job is read from its check run's ANNOTATIONS -- that is what the PR
page renders, what a fork contributor who cannot re-run a job has, and what a
triage report copies. Nothing in this repository wrote one, so the annotations
came from the python problem matcher that actions/setup-python registers by
default (add-problem-matchers defaults to true). Its pattern is a traceback
frame (File "...", line N, in f) followed by raise SomeError('msg'), applied
to the whole log -- including the part where pytest prints its warnings summary.

Measured on the six Backend Tests jobs cited in #7296, that pattern matched a
warning traceback every time and a pytest failure not once:

job	annotations	what actually failed
98966209341 Windows (3)	1x Event loop is closed	test_mcp_gatewayd_coverage.py::TestZombieDiagnostic::test_dead_accept_loop_is_dumped_and_stops_the_daemon (missing diag.jsonl)
98986152965 Windows (3)	1x Event loop is closed	test_md_notebook.py::test_sync_refuses_rather_than_pushing_a_subset (git add timed out after 120s)
99498254362 Windows (3)	2x Event loop is closed	17 failed, test_project_bundle_git / test_project_capabilities
99498254412 (3.10, 3)	4x Event loop is closed	7 failed, same family
99498254667 (3.12, 3)	4x Event loop is closed	7 failed, same family
99525858843 Windows (2)	10x Event loop is closed	test_data_home_not_relocatable.py, 2 alias cases

The annotated line number is the same on every platform because it is not a line
in this repository: 545 is asyncio/base_events.py, inside _check_closed,
reached from a PytestUnraisableExceptionWarning about a garbage-collected
coroutine. So the reported symptom is real and its stated cause is not -- in all
six jobs Event loop is closed was a WARNING, the shard exit code came from
ordinary named failures, and not one of those tests was named in an annotation.

Why it matters

Four unrelated PRs were filed and triaged as an event-loop teardown flake on that
evidence, and the class had already been "fixed" four times (#4764, #4784, #5491,
#5856) before this recurrence. The cost the issue describes -- "the red carries no
actionable test identity", "there is no test name to bisect from" -- is produced
by the annotation pipeline, not by asyncio. Until the annotations name the real
test, every future red of any cause gets read as this flake again, and on a fork
PR the CI red also skips all five fork-*-review lanes, so a misread costs the
review round as well as the CI round.

What changed (motivation -> approach -> change)

Symptom: the check run names a warning and hides the red. Root cause: the only
thing writing annotations is a text matcher that cannot tell a warning traceback
from a failure and has no pattern for a pytest failure at all.

A better matcher cannot be the fix. --color=yes is in addopts, so the summary
line a matcher would have to scrape reads
\x1b[31mFAILED\x1b[0m test/x.py::\x1b[1mtest_y\x1b[0m - AssertionError: ...,
with escape sequences inside the node id. The report objects already carry the
same facts as data, so:

• conftest.py gains pytest_terminal_summary, which emits one ::error
  annotation per failed/errored report: file and 1-based line from
  report.location, title from the node id, message from the E-marked line
  (reprcrash is the preamble for a fixture error and would spend the whole
  annotation saying file <path>, line 5). Controller-only, so an xdist worker's
  reports are not annotated twice; gated on GITHUB_ACTIONS, so a local red is
  unchanged; capped at 10 with a ::notice:: counting the rest, because GitHub
  drops the excess silently.
• It lives in the ROOTDIR conftest, not test/conftest.py: the in-package app
  suites never load the latter and they red the same jobs.
• The four jobs that run pytest (backend-test, backend-test-windows,
  backend-test-macos, backend-test-sandbox) echo
  ::remove-matcher owner=python:: from inside the step that runs pytest, so the
  matcher that lied cannot put a louder wrong answer next to the right one, and
  the directive cannot be separated from the run it protects by a later
  reordering. add-problem-matchers: false was tried first and is a trap: that
  input does not exist on actions/setup-python v7.0.0 at this pin, so the
  action ignored it and the runner answered with an Unexpected input(s)
  warning -- one more annotation saying nothing about the tests. Caught by this
  PR's own first CI run and now pinned by a test.

Deliberately NOT changed: the Event loop is closed unraisables themselves. They
are real leaked coroutines (turn_dispatch.py:385 _bounded_turn and
gatewayd.py:3291 _drain_inbox_to_stub, both GC-finalized after their loop
closed) but they are warnings, they failed nothing here, and silencing them is a
per-site product change that would also have hidden this diagnosis. Widening
_drain_windows_proactor_finalizers past its win32/sessionfinish scope was
considered and rejected for the same reason: it suppresses the evidence.

Tests

test/test_ci_failure_annotations.py, 20 cases. Mutation-verified: 18 of the 20
fail with both production files reverted to origin/main (the 2 that pass are the
subject-list guard and the not-an-input guard, neither of which depends on the fix).

• the annotation carries node id, file, and the 1-based line (report line 41 ->
  line=42); setup/teardown errors are annotated too; a collection error with no
  line omits line= rather than guessing 1
• a fixture error reports fixture 'x' not found, not the file <path>, line 5
  preamble; an indented E = compute() source line is not mistaken for the verdict
• node-id :: and a comma inside a parametrized id are escaped (%3A%3A, %2C)
  so the runner does not truncate the annotation at the first separator; % and
  carriage-return and newline escaped; a 5000-char reason truncated
• 10-annotation cap emits exactly 10 plus one ::notice::; exactly 10 emits no notice
• nothing written for a green run, outside GitHub Actions, or on an xdist worker
• ci.yml source: the four pytest jobs are found by scanning for a step that runs
  pytest (so a NEW pytest job is covered without editing a list); each must remove
  the matcher at or before its FIRST pytest step (ordering asserted, not assumed --
  a directive placed after the run leaves it fully matched); no workflow may pass
  add-problem-matchers to a step at all; the hook must be in the rootdir conftest
  and not in test/conftest.py

Gates on the touched files: black, isort, flake8 clean; check_black_formatting,
check_testpaths_coverage, check_sync_io_in_async, check_loop_bound_locks,
check_brand_name, check_focus_cue, check_changelog_history all pass.

Manual verification

Live proof on this PR's own CI. The first run's Backend Tests (Windows) (3) and
(3.10, 3) went red on a main-owned ratchet drift, and the check-run annotation
now reads:

test/test_security_posture.py:1046
  title: test/test_security_posture.py::TestGateSideLogRedactorSpelling::test_the_census_holds_no_slack
  message: ... - AssertionError: `_BASELINE_LOG_SITE_CENSUS` is now looser than the code

Exact file, exact line, node id as the title -- against the Event loop is closed
at .github:515 that the old matcher put on the same job. That same run is what
exposed the add-problem-matchers mistake, which is fixed in this revision.

Before that, ran a local pytest invocation with two deliberately broken tests and
GITHUB_ACTIONS=true against the real rootdir conftest. Emitted:

annotation field	first probe test	second probe test
file	test/test_zz_annotation_probe.py	test/test_zz_annotation_probe.py
line	1	5
title	...probe.py%3A%3Atest_probe_fails	...probe.py%3A%3Atest_probe_errors
message	...::test_probe_fails - assert 1 == 2	...::test_probe_errors - fixture 'nonexistent_fixture' not found

(Rendered as a table on purpose. A raw ::error ... line pasted at the start of a
line is a LIVE workflow command to any job that prints this description into its
log -- the first revision of this section made the GPT review job emit a bogus
test/test_zz_annotation_probe.py:1 annotation for a file that does not exist in
the repository. Same defect class this PR is about, so it is not repeated here.)

The probe file was removed afterwards; the tree is clean.

Known remaining case, stated rather than fixed: the rootdir pytest_sessionfinish
residue guard can turn a green run red AFTER pytest_terminal_summary has run, so
a residue-only red still carries no annotation. It prints its own explicit
repository root residue section, and folding an annotation into that path needs
its own test setup, so it is left for a follow-up rather than claimed here.

Related Issues

Closes #7296

Pattern harvest

Rule candidate: review-prompt. Pattern: "a CI diagnosis taken from check-run
annotations when no step in the repository writes them" -- if annotations are
produced by a generic text matcher rather than by the tool that knows what
failed, the annotation is a guess and the shard's own summary is the evidence.
Concretely reusable check: any job that runs a test framework should either emit
its own annotations or leave add-problem-matchers off; a job doing neither will
eventually annotate a warning as its failure. The four predecessors of this issue
are what that class costs when it is not caught.

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 4600c5b4e20e2697823c9c2cabc1443b69f58579 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Root-cause fix at the right layer: annotations come from pytest's own report objects, with the lying matcher removed inline where each run happens.

Suggestions

• The pytest-step-must-remove-matcher scan covers only ci.yml; release.yml and test-durations.yml were patched by hand, so a future pytest step added to a non-ci workflow silently reacquires the wrong-annotation matcher. Extending the scan to all workflow files is a one-line generalization of the existing test.

[DESIGN-REVIEWED] 4600c5b

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 4600c5b4e20e2697823c9c2cabc1443b69f58579 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

First-Principles-Verdict: PASS

Every item traces to one measured defect — six red jobs annotated with a warning and zero failing tests — and each fixes or guards that cause.

What this change ships

Intent: make a red CI job's check-run annotations name the tests that actually failed. This is a FIX.

1. A red pytest job's check run now names each failing test with file, line, and reason — justified (measured on six jobs in CI flake recurrence: 'Event loop is closed' teardown failure back on Backend Tests Windows shards across unrelated PRs #7296)
2. The python problem matcher is turned off in every job that runs pytest (6 jobs, 4 workflows) — justified, cause-level
3. Annotations stop at 10, with a notice counting the rest — justified (GitHub drops the excess silently)
4. Failure reasons are cut to one line, 400 chars — justified, declared
5. Local, green, and non-Actions runs emit nothing — justified
6. Guard tests pin matcher-off ordering, the nonexistent add-problem-matchers input, and rootdir-conftest placement — justified (the input trap was measured on this PR's own first run)

Counts run: add-problem-matchers appears in 0 workflow step inputs (1 hit, a comment the parsed-input test correctly ignores); python -m pytest appears in 0 workflows, so the guard's ^\s*pytest\s regex covers every current invocation; pytest-github-actions-annotate-failures appears 0 times — no in-repo mechanism does this job, and the emitter's cap-plus-notice and E-line reason selection are meaningful deltas over that external plugin, not a second spelling. The change sits at cause level (the lying annotation source is removed, the true one added); the deeper leaked-coroutine warnings are declared out of scope with the level named, which lens 6 accepts.

[FIRST-PRINCIPLES-REVIEWED] 4600c5b

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 4600c5b4e20e2697823c9c2cabc1443b69f58579 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 4600c5b

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 4600c5b4e20e2697823c9c2cabc1443b69f58579: <one-sentence reason>

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 4600c5b4e20e2697823c9c2cabc1443b69f58579 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 4600c5b

Verdict parsed from the review's SHA-scoped output markers for commit 4600c5b4e20e2697823c9c2cabc1443b69f58579.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 4600c5b4e20e2697823c9c2cabc1443b69f58579: <one-sentence reason>

[chenmingwei23]

First Principles CONCERNS -- adopted in full at be9b4dd66.

The finding was right and the inconsistency was mine: test_no_workflow_passes_the_input_that_does_not_exist already globbed every workflow while its sibling read only ci.yml, so a pytest job outside ci.yml inherited the matcher with nothing to catch it -- the exact gap the guard exists to close.

• release.yml:release-candidate-tests and test-durations.yml:refresh now echo ::remove-matcher owner=python:: before their pytest invocation.
• TestTheMatcherThatLiedIsOffWhereverPytestRuns now walks .github/workflows/*.yml, keys jobs as <workflow>:<job>, and names all six in the subject-list guard, so a new pytest job in any workflow is covered without editing a list.

Agreed on the reasoning too: a release-time red is read under pressure, which is the worst place to annotate a warning instead of the failing test.

Design Review PASS, GPT 5.6 no blocking findings, Opus 4.8 no findings -- all on 0775b411d, and this revision only widens the same mechanism.

Remaining reds are main-owned and deliberately not folded in: Frontend Lint & Type Check (main at 660 eslint warnings against a 659 ceiling; PR #7480 is the burn-down) and the Backend Tests shard holding test_security_posture.py::TestGateSideLogRedactorSpelling::test_the_census_holds_no_slack (issue #7490, PR #7492). The second is reproducible on this branch's base commit, which is a pure main commit, and it lands on different shards for different PRs because adding a test file shifts the pytest-split membership.

(The one-word comment just above this was an accidental probe post; this is its intended content.)

[chenmingwei23]

Note on the GPT 5.6 Review red, and a second instance of this PR's own defect class.

The lane's VERDICT is clean -- its comment reads no blocking findings with [GPT-REVIEWED] be9b4dd66..., the current head -- while the check-run concluded failure. Its own check run carries this annotation:

test/test_zz_annotation_probe.py:1 -- "test_probe_fails - assert 1 == 2"

That file does not exist in this repository. It was the throwaway probe from the Manual verification section, and the annotation exists because that section pasted a raw workflow-command line starting at column 0. Any job that prints this description into its log therefore EXECUTES it. Fixed by rendering those examples as a table instead; the diff itself was already clean (no added line starts with a command introducer, checked).

The same job also logged two runner errors from reading actions/setup-python's bundled dist, which contains an unexpanded add-matcher template literal. That one is not mine to fix, but it is the same shape: content printed into a log becomes a command.

Adding to the pattern harvest, because it generalizes past this PR: a workflow command at the start of a line is executable by any job that echoes the text, so a PR description, an issue comment, or a fixture that carries one is a live annotation injection. The mechanical form is a lint over PR/issue bodies and test fixtures for a line matching the command introducers.

No code change was needed for this; editing the description re-triggers the review lane, which is what should clear the red.

[NicholasRBowers]

Tier 1 auto-approve: chore/CI plumbing (5 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: CI failure-annotation plumbing (workflow matcher removal + conftest terminal-summary emitter + its test), no runtime impact.