diff --git a/.github/scripts/ci-flaky-rerun.mjs b/.github/scripts/ci-flaky-rerun.mjs index a2fbc64b727..df719cd5f03 100644 --- a/.github/scripts/ci-flaky-rerun.mjs +++ b/.github/scripts/ci-flaky-rerun.mjs @@ -428,12 +428,27 @@ export function eligibleAttemptJob(job, target, runAttempt, options = {}) { } function safeReason(reason) { - return reason - .trim() - .replaceAll('&', '&') - .replaceAll('<', '<') - .replaceAll('>', '>') - .replaceAll('@', '@\u200b'); + return ( + reason + .trim() + // A newline lets a reason open a block-level Markdown construct: a + // line-start fence swallows the folded Chinese block and renders the + // tracking marker as visible code. Keep the reason a single line. + .replace(/\s*\n\s*/g, ' ') + // Bidi overrides can visually rewrite a notice maintainers trust when + // deciding whether to update a branch; the reason is classifier output + // derived from untrusted logs, so drop them like the markup below. + // The class is the full Unicode Bidi_Control set. + .replace(/[\u061C\u200E\u200F\u202A-\u202E\u2066-\u2069]/g, '') + .replaceAll('&', '&') + .replaceAll('<', '<') + .replaceAll('>', '>') + .replaceAll('@', '@\u200b') + ); +} + +function bilingualComment(en, zh, marker) { + return `${en}\n\n
\n中文说明\n\n${zh}\n\n
\n\n${marker}`; } export async function actOnDecision(client, target, decision) { @@ -474,7 +489,17 @@ export async function actOnDecision(client, target, decision) { const marker = markerFor(target, decision.action, count); if (decision.action === 'rerun') { await client.rerunFailedJobs(target.runId); - await client.comment(target.prNumber, marker); + // The notice must be visible: a marker-only comment left maintainers + // unable to tell a patrolled flake from a deterministic failure, so they + // clicked Update Branch — advancing the head and dismissing approvals. + await client.comment( + target.prNumber, + bilingualComment( + `CI Failure Patrol: this failure looks transient, not a PR regression: ${safeReason(decision.reason_en)}\n\nFailed jobs were re-run on the same commit. No Update Branch is needed — if the rerun passes, the failure was a flake.`, + `CI 失败巡逻：此失败疑似瞬时问题，而非 PR 引入的回归：${safeReason(decision.reason_zh)}\n\n已在同一 commit 上重跑失败的 job，无需 Update Branch；重跑通过即为 flake。`, + marker, + ), + ); // A flaky TEST (not infra) also gets a deflake issue so the autofix loop // can stabilize it. Best-effort: the rerun + marker already succeeded, so a // transient issue-creation failure must not propagate (it would surface as @@ -490,10 +515,21 @@ export async function actOnDecision(client, target, decision) { } else if (decision.action === 'comment') { await client.comment( target.prNumber, - `CI is failing because: ${safeReason(decision.reason_en)}\n\n
\n中文说明\n\n${safeReason(decision.reason_zh)}\n\n
\n\n${marker}`, + bilingualComment( + `CI is failing because: ${safeReason(decision.reason_en)}`, + safeReason(decision.reason_zh), + marker, + ), ); } else { - await client.comment(target.prNumber, marker); + await client.comment( + target.prNumber, + bilingualComment( + `CI Failure Patrol: this failure could not be confidently classified: ${safeReason(decision.reason_en)}\n\nNo automatic action was taken. Inspect the failed job before rerunning or updating the branch.`, + `CI 失败巡逻：无法确信归类此失败：${safeReason(decision.reason_zh)}\n\n未采取自动动作。请先人工查看失败 job，再决定重跑或更新分支。`, + marker, + ), + ); } } diff --git a/.qwen/skills/ci-flaky-patrol/SKILL.md b/.qwen/skills/ci-flaky-patrol/SKILL.md index 07d3f18fefc..6c6f6879eae 100644 --- a/.qwen/skills/ci-flaky-patrol/SKILL.md +++ b/.qwen/skills/ci-flaky-patrol/SKILL.md @@ -11,7 +11,7 @@ For every candidate, choose exactly one action: - `rerun`: concrete transient evidence such as a runner/network timeout, interrupted infrastructure, transient install/download failure, or explicit flaky-test evidence. - `comment`: the failure is clearly caused by the PR. Compare the failure with `changedFiles`; the reason must state the causal evidence, not merely that the failure is deterministic. -- `no_action`: evidence is ambiguous, unsafe, incomplete, or does not justify another action. This still records an internal tracking marker on the PR. +- `no_action`: evidence is ambiguous, unsafe, incomplete, or does not justify another action. This still records a tracking marker and a short manual-review notice on the PR. When (and ONLY when) the `rerun` cause is a nondeterministic **TEST** — a specific named test that timed out, is order-dependent, or depends on wall-clock/randomness — also identify it so the loop can open a deflake fix. Add a `flakyTest` object with the exact failing `file` (repo-relative path) and `name` (the full test title, e.g. `describe › it`) taken verbatim from the log. Emit `flakyTest` ONLY for genuine test nondeterminism, NEVER for infra flakiness (ENOSPC, network, runner death, dependency download) — those get a plain `rerun` with no `flakyTest`. If the log does not name a specific test, omit `flakyTest`. Keep `file` and `name` each at most 200 characters (take the test title verbatim; if a nested `describe › it` chain is longer, keep the most specific tail). A malformed or over-length `flakyTest` is simply ignored — the rerun still happens — so never drop a valid rerun over it. diff --git a/scripts/tests/ci-flaky-rerun.test.js b/scripts/tests/ci-flaky-rerun.test.js index e6697b733ea..27765f326f1 100644 --- a/scripts/tests/ci-flaky-rerun.test.js +++ b/scripts/tests/ci-flaky-rerun.test.js @@ -545,13 +545,35 @@ describe('ci flaky rerun patrol', () => { expect(kinds).toContain('comment'); }); - it('reruns before recording the hidden action marker', async () => { + it('reruns before posting the visible rerun notice', async () => { const c = client(); await actOnDecision(c, target(), decision()); - expect(c.calls).toEqual([ - ['rerunFailedJobs', 123], - ['comment', 42, expect.stringMatching(/^$/)], + expect(c.calls.map((call) => call[0])).toEqual([ + 'rerunFailedJobs', + 'comment', ]); + const body = c.calls[1][2]; + expect(body).toContain('runner timeout'); + expect(body).toContain('re-run on the same commit'); + expect(body).toContain('No Update Branch'); + expect(body).toContain('中文说明'); + expect(body).toContain('runner 超时'); + expect(body.split('').length - 1).toBe(1); + expect(body).toMatch(/$/); + }); + + it('keeps markers parseable inside the visible notices', async () => { + // The visible text must not break dedup or the per-head action count: + // both read the marker back out of the posted comment body. + const c = client(); + await actOnDecision(c, target(), decision()); + const posted = { + body: c.calls.find((call) => call[0] === 'comment')[2], + createdAt: '2026-07-12T07:25:00.000Z', + author: { login: 'patrol-bot' }, + }; + expect(alreadyHandled([posted], target(), 'patrol-bot')).toBe(true); + expect(currentActionCount(pr(), [posted], 'patrol-bot')).toBe(1); }); it('posts deterministic failures in English with folded Chinese', async () => { @@ -566,22 +588,36 @@ describe('ci flaky rerun patrol', () => { }), ); expect(c.calls).toHaveLength(1); - expect(c.calls[0][2]).toContain('TypeScript cannot resolve'); - expect(c.calls[0][2]).toContain('
'); - expect(c.calls[0][2]).toContain('TypeScript 无法解析'); - expect(c.calls[0][2]).toContain('action=comment'); + const body = c.calls[0][2]; + expect(body).toContain('TypeScript cannot resolve'); + expect(body).toContain('
'); + expect(body).toContain('TypeScript 无法解析'); + expect(body).toContain('action=comment'); + // Pin the bilingual placement, not just the presence of both strings: + // English before the fold, Chinese inside it. + const enIndex = body.indexOf('TypeScript cannot resolve'); + const foldIndex = body.indexOf('中文说明'); + const zhIndex = body.indexOf('TypeScript 无法解析'); + expect(enIndex).toBeLessThan(foldIndex); + expect(foldIndex).toBeLessThan(zhIndex); + expect(body.split('
').length - 1).toBe(1); }); - it('records no_action without a visible comment', async () => { + it('records no_action with a visible manual-review notice', async () => { const c = client(); await actOnDecision( c, target(), decision({ action: 'no_action', confidence: 'low' }), ); - expect(c.calls).toEqual([ - ['comment', 42, expect.stringMatching(/^$/)], - ]); + expect(c.calls).toHaveLength(1); + const body = c.calls[0][2]; + expect(body).toContain('could not be confidently classified'); + expect(body).toContain('runner timeout'); + expect(body).toContain('中文说明'); + expect(body).toContain('未采取自动动作'); + expect(body.split('
').length - 1).toBe(1); + expect(body).toMatch(/$/); }); it('rejects malformed, unsafe, or mismatched decisions', async () => { @@ -604,19 +640,110 @@ describe('ci flaky rerun patrol', () => { }); it('neutralizes mentions and markup copied into visible reasons', async () => { - const c = client(); - await actOnDecision( - c, - target(), - decision({ - action: 'comment', - reason_en: '@qwen-maintainers ', - reason_zh: '@全体成员 ', - }), - ); - expect(c.calls[0][2]).toContain('@\u200bqwen-maintainers'); - expect(c.calls[0][2]).toContain('<script>'); - expect(c.calls[0][2]).toContain('@\u200b全体成员 </details>'); + // All three actions now surface the reason in a visible comment, so the + // escaping must hold for every action, not just `comment`. + for (const action of ['rerun', 'comment', 'no_action']) { + const c = client(); + await actOnDecision( + c, + target(), + decision({ + action, + confidence: action === 'no_action' ? 'low' : 'high', + reason_en: '@qwen-maintainers ', + reason_zh: '@全体成员 ', + }), + ); + const body = c.calls.find((call) => call[0] === 'comment')[2]; + expect(body).toContain('@\u200bqwen-maintainers'); + expect(body).toContain('<script>'); + expect(body).toContain('@\u200b全体成员 </details>'); + } + }); + + it('strips bidi overrides that could visually rewrite a notice', async () => { + // The reason is classifier output derived from untrusted logs; a bidi + // override could make "no Update Branch needed" read as its opposite. + // Every member of the Unicode Bidi_Control set is also fed individually + // so narrowing the strip class cannot survive this pin. + const bidiControls = [ + '\u061C', + '\u200E', + '\u200F', + '\u202A', + '\u202B', + '\u202C', + '\u202D', + '\u202E', + '\u2066', + '\u2067', + '\u2068', + '\u2069', + ]; + for (const action of ['rerun', 'comment', 'no_action']) { + const confidence = action === 'no_action' ? 'low' : 'high'; + const c = client(); + await actOnDecision( + c, + target(), + decision({ + action, + confidence, + reason_en: 'safe \u202Ereversed\u202C text', + reason_zh: '安全 \u2066隔离\u2069 \u061C文本', + }), + ); + const body = c.calls.find((call) => call[0] === 'comment')[2]; + expect(body).not.toMatch( + /[\u061C\u200E\u200F\u202A-\u202E\u2066-\u2069]/, + ); + expect(body).toContain('safe reversed text'); + expect(body).toContain('安全 隔离 文本'); + for (const ch of bidiControls) { + const cc = client(); + await actOnDecision( + cc, + target(), + decision({ + action, + confidence, + reason_en: `a${ch}b`, + reason_zh: `a${ch}b`, + }), + ); + const single = cc.calls.find((call) => call[0] === 'comment')[2]; + expect(single).not.toContain(ch); + expect(single).toContain('ab'); + } + } + }); + + it('collapses newlines so a reason cannot open a block-level fence', async () => { + // A line-start fence in a reason swallows the folded Chinese block and + // renders the tracking marker as visible code; keep reasons single-line. + for (const action of ['rerun', 'comment', 'no_action']) { + const c = client(); + await actOnDecision( + c, + target(), + decision({ + action, + confidence: action === 'no_action' ? 'low' : 'high', + reason_en: 'npm error:\n```\nnpm error code ETIMEDOUT', + reason_zh: 'npm 错误：\n```\nnpm error code ETIMEDOUT', + }), + ); + const body = c.calls.find((call) => call[0] === 'comment')[2]; + expect(body).not.toMatch(/^```/m); + expect(body).toContain('npm error: ``` npm error code ETIMEDOUT'); + expect(body.split('').length - 1).toBe(1); + const posted = { + body, + createdAt: '2026-07-12T07:25:00.000Z', + author: { login: 'patrol-bot' }, + }; + expect(alreadyHandled([posted], target(), 'patrol-bot')).toBe(true); + } }); it('stops after three actions on the same head', async () => {