From 2470d14f0f81ac529ad341467c4bcb8d35522fdc Mon Sep 17 00:00:00 2001 From: wanjinhao1 <394959433@qq.com> Date: Thu, 1 Oct 2026 04:08:31 +0800 Subject: [PATCH 1/4] Fix #16193: detect case-only duplicate label names in sync_labels manifest --- scripts/ci/sync_labels.py | 11 ++++++++--- tests/test_triage_rules.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/scripts/ci/sync_labels.py b/scripts/ci/sync_labels.py index d8c1de4a2688..24ed5c3a557b 100644 --- a/scripts/ci/sync_labels.py +++ b/scripts/ci/sync_labels.py @@ -58,9 +58,14 @@ def load_manifest(path: Path) -> list[dict[str, str]]: description = entry.get("description", "") if not name: raise SystemExit(f"{path}: every label needs a name") - if name in seen: - raise SystemExit(f"{path}: duplicate label {name!r}") - seen.add(name) + if name.casefold() in seen: + # GitHub treats label names case-insensitively, so `area: cloud` + # and `Area: Cloud` are the same label: a sync would POST the + # first and then hit a conflict on the second (or, if the label + # already exists, apply whichever entry comes last). Keep the + # manifest spelling for requests; only duplicate detection folds. + raise SystemExit(f"{path}: duplicate label {name!r} (names are case-insensitive)") + seen.add(name.casefold()) if len(color) != 6 or any(character not in "0123456789abcdefABCDEF" for character in color): raise SystemExit(f"{path}: {name!r} needs a six-digit hex color, got {color!r}") if len(description) > 100: diff --git a/tests/test_triage_rules.py b/tests/test_triage_rules.py index 47cd50ff54ee..49739b356ad1 100644 --- a/tests/test_triage_rules.py +++ b/tests/test_triage_rules.py @@ -296,6 +296,36 @@ def test_manifest_passes_its_own_validation(self): sync = load("sync_labels", "scripts/ci/sync_labels.py") self.assertEqual(len(sync.load_manifest(MANIFEST)), len(self.manifest["labels"])) + def test_case_only_duplicate_names_are_refused(self): + sync = load("sync_labels", "scripts/ci/sync_labels.py") + manifest = { + "labels": [ + {"name": "area: cloud", "color": "0e8a16", "description": "Cloud things"}, + {"name": "Area: Cloud", "color": "0e8a16", "description": "Same label"}, + ] + } + with tempfile.TemporaryDirectory() as tmp: + path = pathlib.Path(tmp) / "labels.json" + path.write_text(json.dumps(manifest)) + with self.assertRaises(SystemExit): + sync.load_manifest(path) + + def test_dry_run_refuses_case_only_duplicates_before_any_api_call(self): + # load_manifest runs before the token check in main(), so a case-only + # duplicate fails a --dry-run with no credentials and no network. + sync = load("sync_labels", "scripts/ci/sync_labels.py") + manifest = { + "labels": [ + {"name": "bug", "color": "0e8a16"}, + {"name": "BUG", "color": "0e8a16"}, + ] + } + with tempfile.TemporaryDirectory() as tmp: + path = pathlib.Path(tmp) / "labels.json" + path.write_text(json.dumps(manifest)) + with self.assertRaises(SystemExit): + sync.main(["--manifest", str(path), "--dry-run"]) + class CommentTests(unittest.TestCase): def test_comment_carries_the_marker_and_the_reason(self): From 24262c36221295f90d74a5ae164a37d36f05fd1e Mon Sep 17 00:00:00 2001 From: wanjinhao1 <394959433@qq.com> Date: Fri, 2 Oct 2026 06:02:41 +0800 Subject: [PATCH 2/4] Fix #16193 review: use pytest.raises per ruff PT027 --- tests/test_triage_rules.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_triage_rules.py b/tests/test_triage_rules.py index 49739b356ad1..7820c6ec2925 100644 --- a/tests/test_triage_rules.py +++ b/tests/test_triage_rules.py @@ -7,6 +7,7 @@ import time import unittest +import pytest ROOT = pathlib.Path(__file__).resolve().parents[1] MANIFEST = ROOT / ".github/labels.json" @@ -323,7 +324,7 @@ def test_dry_run_refuses_case_only_duplicates_before_any_api_call(self): with tempfile.TemporaryDirectory() as tmp: path = pathlib.Path(tmp) / "labels.json" path.write_text(json.dumps(manifest)) - with self.assertRaises(SystemExit): + with pytest.raises(SystemExit): sync.main(["--manifest", str(path), "--dry-run"]) @@ -485,9 +486,8 @@ def test_zero_and_its_spellings_are_rejected(self): # `--limit 0` used to mean "walk everything", so "00" typed into the # dispatch form started an unbounded pass over the whole backlog. for value in ("0", "00", "-1", " 0", "nope"): - with self.subTest(value=value): - with self.assertRaises(Exception): - AUTO.positive(value) + with self.subTest(value=value), self.assertRaises(Exception): + AUTO.positive(value) def test_a_real_limit_parses(self): self.assertEqual(AUTO.positive("200"), 200) From 6e50d67de38da3c8a429c09b0117b1eb83adcb44 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Thu, 1 Oct 2026 17:47:56 -0700 Subject: [PATCH 3/4] test: prove duplicate diagnostics and invalid label name validation Co-Authored-By: Claude Opus 5.5 --- tests/test_triage_rules.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/test_triage_rules.py b/tests/test_triage_rules.py index 7820c6ec2925..37ed967cbe08 100644 --- a/tests/test_triage_rules.py +++ b/tests/test_triage_rules.py @@ -7,8 +7,6 @@ import time import unittest -import pytest - ROOT = pathlib.Path(__file__).resolve().parents[1] MANIFEST = ROOT / ".github/labels.json" WORKFLOW = ROOT / ".github/workflows/auto-triage.yml" @@ -308,7 +306,7 @@ def test_case_only_duplicate_names_are_refused(self): with tempfile.TemporaryDirectory() as tmp: path = pathlib.Path(tmp) / "labels.json" path.write_text(json.dumps(manifest)) - with self.assertRaises(SystemExit): + with self.assertRaisesRegex(SystemExit, r"duplicate label .*names are case-insensitive"): sync.load_manifest(path) def test_dry_run_refuses_case_only_duplicates_before_any_api_call(self): @@ -324,9 +322,17 @@ def test_dry_run_refuses_case_only_duplicates_before_any_api_call(self): with tempfile.TemporaryDirectory() as tmp: path = pathlib.Path(tmp) / "labels.json" path.write_text(json.dumps(manifest)) - with pytest.raises(SystemExit): + with self.assertRaisesRegex(SystemExit, r"duplicate label .*names are case-insensitive"): sync.main(["--manifest", str(path), "--dry-run"]) + def test_non_string_label_name_has_validation_error(self): + sync = load("sync_labels", "scripts/ci/sync_labels.py") + with tempfile.TemporaryDirectory() as tmp: + path = pathlib.Path(tmp) / "labels.json" + path.write_text(json.dumps({"labels": [{"name": 5, "color": "0e8a16"}]})) + with self.assertRaisesRegex(SystemExit, "every label needs a non-empty string name"): + sync.load_manifest(path) + class CommentTests(unittest.TestCase): def test_comment_carries_the_marker_and_the_reason(self): From 46780b10fa5e51e6503417f5c78cdcfcaf7581ee Mon Sep 17 00:00:00 2001 From: Leo Li Date: Thu, 1 Oct 2026 17:52:46 -0700 Subject: [PATCH 4/4] fix: validate label names before case folding Co-Authored-By: Claude Opus 5.5 --- scripts/ci/sync_labels.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/ci/sync_labels.py b/scripts/ci/sync_labels.py index 24ed5c3a557b..b2bd84621dd3 100644 --- a/scripts/ci/sync_labels.py +++ b/scripts/ci/sync_labels.py @@ -56,8 +56,8 @@ def load_manifest(path: Path) -> list[dict[str, str]]: name = entry.get("name") color = entry.get("color", "") description = entry.get("description", "") - if not name: - raise SystemExit(f"{path}: every label needs a name") + if not isinstance(name, str) or not name: + raise SystemExit(f"{path}: every label needs a non-empty string name") if name.casefold() in seen: # GitHub treats label names case-insensitively, so `area: cloud` # and `Area: Cloud` are the same label: a sync would POST the