fix(ci): a PR-authored test id cannot speak in the nightly bot's voice (#2585)

[dolho]

Summary

Both nightlies splice a downloaded diff artifact into a sticky comment authored by github-actions[bot]. The artifact is produced by pytest over the merged PR tree, so a @pytest.mark.parametrize value — arbitrary text chosen by whoever opened the PR, on a public repo that evaluates fork PRs — reaches the comment unfenced.

The impact is not RCE on the runner. It is that the bot's comment can be made to say something its author never wrote, and the bot's voice is the only reason anyone trusts it.

Where the fix went, and why not where the issue said

The issue asks for a fence around the spliced text in each workflow. Reading the producer first changed the answer twice over:

1. The artifact is structured markdown — headings, a per-XML totals table, bullet lists. Fencing it would turn that table into monospace text on every ordinary nightly, forever, destroying the thing that makes the comment useful.
2. The PR-authored surface is narrower than reported. There is no assertion text in this document (_render_summary emits ids and counts, never a failure message), and the table's other column is s.path.name — the junit-base-pr<N>-<seed>.xml name the workflow itself templates. Only the node id is untrusted.

So the neutralisation is at the node id, where the untrusted input actually enters. Both nightlies inherit it because both run this one producer — integration-nightly.yml:432 needed no separate patch.

The two properties, because the backtick rule alone is not enough

• Open the span with one more backtick than the longest run inside. CommonMark closes a span on the first run of equal length, so a hard-coded delimiter is escapable by any input containing one. (The rule ci: pre-merge Alembic head check is stale-by-construction when dev advances #2533 applied to fenced blocks, at the inline level.)
• Collapse newlines first. A code span cannot contain a blank line, and an id carrying \n## breaks out of the bullet however the backticks are counted.

Plus: edge backticks are space-padded (CommonMark strips them back off), and the id is capped at 300 chars so a megabyte-long parametrize value cannot bloat the comment past GitHub's body limit.

AC item 3 — confirmed, not assumed

--randomly-seed=${regressed[0].seed} is a command a human is invited to paste. The seeds come from the workflow constant NIGHTLY_SEEDS: '12345 67890 99999' through the build matrix — never PR-derived, so there is no untrusted token in that line. Pinned by a test, so a change making seeds dynamic fails here.

Tests, and a hole mutation testing found in my own guard

The CommonMark property stated directly, the newline property, bounds, padding, and the control the AC asks for: an ordinary id still renders as a plain single-backtick span, and the document still renders as a table, not a code block.

Mutation-testing then showed the security tests were bypassable: deleting the neutralising call from _format_test_id left every CommonMark test green, because they exercise the helper directly. A neutraliser nothing calls is not a fix, so the property is now also asserted on the rendered document, driven end to end from JUnit XML. With the fix reverted, 4 tests fail.

scripts/ci/diff-pytest-failures.py --self-test → 13 cases pass (was 11).
Nightly-related suites: 69 passed.

Note on the doc anchor

The issue cites HEADW-011; that anchor arrives with the unmerged #2533 PR (#2590), so the durable note went to learnings.md rather than inventing a requirement id.

Fixes #2585

🤖 Generated with Claude Code

https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ

[github-actions]

⚠️ Nightly unit-suite check skipped — merge conflict against dev.

Resolve by running git merge dev locally and pushing the result. The next nightly run will re-test once the conflict is gone.

[github-actions]

⚠️ Live-instance suite skipped — merge conflict against dev.

Resolve by merging dev locally and pushing the result; the next nightly re-tests.

[vybe]

merge-train: ejected at the batch gate — own check. The lint (sys.modules pollution check) job fails on the train and on this branch alone, and passes on dev: tests/unit/test_2585_nightly_comment_fencing.py:61 assigns sys.modules[...] directly and :66 calls sys.modules.pop(...) — the lint wants monkeypatch.setitem/monkeypatch.delitem(..., raising=False) (or the _STUBBED_MODULE_NAMES + autouse restore pattern in test_telegram_webhook_backfill.py). This was the branch's first CI run — the learnings.md conflict had suppressed checks until it was resolved today (that resolution and the Fixes #2585 body patch are already on the branch). Fix the two lines and it rides the next train.

[vybe]

merge-train (2026-09-09): pushed afd860fa to this branch — a merge of origin/dev (105be06) resolving today's second append collision on docs/memory/learnings.md (dev's #2610 entry kept first, this PR's #2585 entry after it). No code changed; your e5a6a3a5 lint fix is intact. Verified locally on the merged tree: python tests/lint_sys_modules.py → no new violations; pytest tests/unit/test_2585_nightly_comment_fencing.py tests/unit/test_lint_sys_modules.py → 51 passed; diff-pytest-failures.py --self-test → 13/13. Riding this train.

[dolho]

Fixed in e5a6a3a5, on top of the dev merge already on the branch.

tests/unit/test_2585_nightly_comment_fencing.py's dpf fixture is module-scoped, so the function-scoped monkeypatch fixture is unavailable to it — which is how the bare sys.modules[...] = mod + pop in a finally got there. pytest.MonkeyPatch.context() is the same mechanism at a scope that fixture cannot reach, and it restores rather than deletes: the old pop would have unbound the name even if something else had legitimately bound it first.

python tests/lint_sys_modules.py → no new violations; the file's 26 tests pass.

[vybe]

merge-train: batch validated on train/20260909-0822 (#2639) — full suite green on the combined batch.