diff --git a/scripts/pr_gate.py b/scripts/pr_gate.py index 81a86ae0af..3859af9f9d 100644 --- a/scripts/pr_gate.py +++ b/scripts/pr_gate.py @@ -900,6 +900,17 @@ def wait_and_decide( sleep(poll_sec) +def _workflow_command_escape(text: str) -> str: + """Escape text for a GitHub workflow-command (`::error::`) annotation. + + A raw LF would end the annotation (truncating the reason), a raw CR + mangles it, and a raw `%` swallows the following bytes as a bogus + escape. Order is load-bearing: `%` must be escaped FIRST -- doing CR/LF + first would re-escape the `%` those substitutions introduce (%250D). + """ + return text.replace("%", "%25").replace("\r", "%0D").replace("\n", "%0A") + + def _optional_int(raw: str) -> "int | None": """`--pr ""` vaut « pas de PR », pas une erreur d'argparse. @@ -1047,10 +1058,12 @@ def main(argv: Iterable[str] | None = None) -> int: advisory_jobs, ) except GateError as exc: - # Rule 1: an unreadable state is a failure, never a pass. - print(f"[pr-gate] FAIL -- cannot establish check state: {exc}", file=sys.stderr) - _maybe_post_check_run(args, 1, f"FAIL -- cannot establish check state: {exc}") - return 1 + # Rule 1: an unreadable state is a failure, never a pass. Fall + # through to the single emission tail instead of returning here: the + # old early return exited BEFORE the ::error:: point, leaving the + # most opaque failure mode (API unreadable at all) mute in the UI + # (#15472). Verdict semantics unchanged: code 1, same message. + code, message = 1, f"FAIL -- cannot establish check state: {exc}" # #13510: render a starvation as CANCELLED, not FAILURE. The PR stays # BLOCKED (a cancelled required check is not success), but the leg no @@ -1127,6 +1140,20 @@ def main(argv: Iterable[str] | None = None) -> int: print(f"[pr-gate] {message}", flush=True) _maybe_post_check_run(args, code, message) + if code != 0: + # Le check-run derive du job ne porte qu'« Process completed with exit + # code 1 » sur un rouge : seul l'annotation donne la RAISON du verdict + # dans l'UI. Legs DWELL rendus muets mesures sur #15472 (2026-09-10) : + # 8 PR gates rouges sans cause organique, tous des DWELL (plancher de + # merge, mandat 07/09) dont la re-agregation ne rend jamais compte. + # Le message peut porter %, CR ou LF (stderr gh multi-ligne) : un LF + # brut tronque l'annotation, un % brut avale les octets suivants -- + # echappement protocolaire workflow-command, % EN PREMIER (#15472). + print( + f"::error::[pr-gate] {_workflow_command_escape(message)}", + file=sys.stderr, + flush=True, + ) return code diff --git a/scripts/tests/test_pr_gate.py b/scripts/tests/test_pr_gate.py index c6369613d6..dec960fec1 100644 --- a/scripts/tests/test_pr_gate.py +++ b/scripts/tests/test_pr_gate.py @@ -70,6 +70,78 @@ def boom(*_args, **_kwargs): assert pr_gate.main(["--repo", "o/r", "--sha", "deadbeef"]) == 1 +def test_unreadable_api_carries_annotation(capsys, monkeypatch): + """#15472 repair 1: the `cannot establish check state` early return is + the most opaque failure mode (the API cannot be read at all); it must + reach the same ::error:: emission point as any other red, not exit + before it.""" + def boom(*_args, **_kwargs): + raise pr_gate.GateError("gh api exploded") + + monkeypatch.setattr(pr_gate, "wait_and_decide", boom) + assert pr_gate.main(["--repo", "o/r", "--sha", "deadbeef"]) == 1 + captured = capsys.readouterr() + assert ( + "::error::[pr-gate] FAIL -- cannot establish check state: gh api exploded" + in captured.err + ) + # One log line, one annotation -- the same verdict never printed twice + # on the same surface. + assert captured.out.count("cannot establish check state") == 1 + assert captured.err.count("cannot establish check state") == 1 + + +def test_annotation_escapes_workflow_command_metacharacters(capsys, monkeypatch): + """#15472 repair 2: an exception message can be multi-line and carry `%` + (raw gh stderr). A raw CR/LF splits or truncates the ::error:: + annotation; a raw `%` eats the following bytes as a bogus escape. + Escape per the workflow-command protocol, `%` FIRST.""" + def boom(*_args, **_kwargs): + raise pr_gate.GateError( + "gh api o/r failed (exit 1): HTTP 502\r\n100% of requests failed\ntrace end" + ) + + monkeypatch.setattr(pr_gate, "wait_and_decide", boom) + assert pr_gate.main(["--repo", "o/r", "--sha", "deadbeef"]) == 1 + annotation = next( + ln for ln in capsys.readouterr().err.splitlines() + if ln.startswith("::error::") + ) + assert "HTTP 502%0D%0A100%25 of requests failed" in annotation + assert "failed%0Atrace end" in annotation + # Order proof: `%` escaped before CR/LF, so %0D/%0A are never re-escaped. + assert "%250D" not in annotation and "%250A" not in annotation + + +def test_workflow_command_escape_percent_first(): + """Unit pin of the helper: the substitution order is load-bearing.""" + assert ( + pr_gate._workflow_command_escape("100% done\r\nnext") + == "100%25 done%0D%0Anext" + ) + + +def test_posted_check_run_message_is_not_escaped(monkeypatch): + """Escaping belongs to the workflow-command surface ONLY; the Checks-API + check-run (the PR's mergeState surface) carries the message verbatim.""" + posted = {} + + def fake_post(repo, sha, name, code, message): + posted["message"] = message + return {} + + def boom(*_args, **_kwargs): + raise pr_gate.GateError("gh api failed (exit 1): 50%\r\nline2") + + monkeypatch.setattr(pr_gate, "post_check_run", fake_post) + monkeypatch.setattr(pr_gate, "wait_and_decide", boom) + code = pr_gate.main(["--repo", "o/r", "--sha", "deadbeef", "--post-check-run"]) + assert code == 1 + assert posted["message"] == ( + "FAIL -- cannot establish check state: gh api failed (exit 1): 50%\r\nline2" + ) + + def test_completed_without_conclusion_is_pending_not_pass(): """`status=completed, conclusion=null` is a transient GitHub state.""" pending, bad, ok, _adv = pr_gate.classify([run("Odd", None)]) @@ -1289,7 +1361,27 @@ def test_dwell_red_is_prefixed_so_the_cause_is_readable(capsys, monkeypatch): ["--repo", "o/r", "--sha", "deadbeef", "--pr", "7", "--dwell-min", "120"] ) assert code == 1 - assert "DWELL -- tete du 2026-09-07T11:55:00Z" in capsys.readouterr().out + captured = capsys.readouterr() + assert "DWELL -- tete du 2026-09-07T11:55:00Z" in captured.out + # #15472 : le check-run derive du job ne montre que « exit code 1 » sans + # annotation — la RAISON du rouge doit etre portee par un ::error:: mot + # pour mot (c'est la surface que lisent humains et balayages). + assert "::error::[pr-gate] DWELL -- tete du 2026-09-07T11:55:00Z" in captured.err + + +def test_red_ci_verdict_carries_annotation(capsys, monkeypatch): + """Un rouge de CI porte aussi l'annotation -- pas seulement le plancher.""" + monkeypatch.setattr( + pr_gate, "wait_and_decide", + lambda *_a, **_kw: (1, "FAIL -- failing checks: Lean CI"), + ) + + code = pr_gate.main( + ["--repo", "o/r", "--sha", "deadbeef", "--no-self-cancel"] + ) + assert code == 1 + err = capsys.readouterr().err + assert "::error::[pr-gate] FAIL -- failing checks: Lean CI" in err def test_empty_pr_argument_means_no_pr(monkeypatch):