From 4a0a17d352a7ebd683f7a7268b5567e4b3851210 Mon Sep 17 00:00:00 2001 From: Dong1017 <55723162+Dong1017@users.noreply.github.com> Date: Wed, 4 Nov 2026 13:45:48 +0800 Subject: [PATCH 1/4] fix(labels): exempt outside-app labels from the drift report The nightly safety-net job has failed since 2026-11-02 because the live `bot-reviewed` label is missing from `.github/labels.json`. That label belongs to an outside GitHub App: nothing in this repository references it, no workflow applies it, and renaming or deleting it only makes the app recreate it. Its name also cannot enter `labels.json`, whose entries must be `group:value`. Record such labels in `.github/external-labels.json` with the app that owns each one, and exclude those names from the "in the repository, missing from labels.json" report. A template or `skill_eval_*_config.json` reference to one still fails, because this repository must not depend on a label it does not own. Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com> Agent-assisted-by: Claude Code / claude-opus-5 (author) --- .github/external-labels.json | 6 +++++ docs/agent-collaboration.md | 9 +++++++ tests/test_check_labels.py | 52 +++++++++++++++++++++++++++++++++--- tools/check_labels.py | 30 ++++++++++++++++++--- 4 files changed, 90 insertions(+), 7 deletions(-) create mode 100644 .github/external-labels.json diff --git a/.github/external-labels.json b/.github/external-labels.json new file mode 100644 index 000000000..d079a2813 --- /dev/null +++ b/.github/external-labels.json @@ -0,0 +1,6 @@ +[ + { + "name": "bot-reviewed", + "owner": "agent-ci-infra-bot[bot]" + } +] diff --git a/docs/agent-collaboration.md b/docs/agent-collaboration.md index b0f9864cf..7b1734b96 100644 --- a/docs/agent-collaboration.md +++ b/docs/agent-collaboration.md @@ -223,6 +223,15 @@ repository. The nightly `safety-net` job reports drift between the repository la `labels.json`, and any label that an Issue template or `tools/skill_eval_*_config.json` references but `labels.json` lacks. No workflow writes labels. +[`external-labels.json`](../.github/external-labels.json) names the labels that an outside +GitHub App owns and applies, with the app that owns each one. Those names are exempt from the +`group:value` rule and from the drift report, because this repository neither creates them nor +decides their wording: renaming or deleting one only makes the app recreate it. Label +maintenance leaves them alone, and they carry no coordination meaning here, so the rules above +do not apply to them. A name cannot be in both files. An Issue template or +`tools/skill_eval_*_config.json` still may not reference one, since this repository must not +depend on a label it does not own. + ## Evidence refresh — hypothetical example **Hypothetical illustration only; it records no actual PR, run or result in this diff --git a/tests/test_check_labels.py b/tests/test_check_labels.py index 1f633b507..3e149c30d 100644 --- a/tests/test_check_labels.py +++ b/tests/test_check_labels.py @@ -7,6 +7,8 @@ ROOT = Path(__file__).resolve().parents[1] MANIFEST = json.loads((ROOT / ".github/labels.json").read_text(encoding="utf-8")) +EXTERNAL = json.loads((ROOT / ".github/external-labels.json").read_text(encoding="utf-8")) +EXTERNAL_NAMES = {entry["name"] for entry in EXTERNAL} LIVE = [{"name": entry["name"], "description": entry["description"]} for entry in MANIFEST] @@ -23,12 +25,16 @@ def test_repository_manifest_is_valid(): assert check.manifest_problems(MANIFEST) == [] +def test_repository_external_list_is_valid(): + assert check.external_problems(EXTERNAL, MANIFEST) == [] + + def test_templates_and_configs_reference_only_listed_labels(): references = check.referenced_labels(ROOT) assert references[".github/ISSUE_TEMPLATE/scoped-task.md"] == ["kind:task"] assert references[".github/ISSUE_TEMPLATE/campaign-acceptance.yml"] == ["kind:acceptance"] assert references["tools/skill_eval_auto_config.json"] == ["skill-eval:execute"] - assert check.drift(MANIFEST, LIVE, references) == [] + assert check.drift(MANIFEST, LIVE, references, EXTERNAL_NAMES) == [] @pytest.mark.parametrize("bad, message", [ @@ -49,11 +55,31 @@ def test_duplicate_names_are_reported(): ] +@pytest.mark.parametrize("bad, message", [ + ({"name": "bot-reviewed"}, "exactly name and owner"), + ({"name": "bot-reviewed", "owner": "a", "note": "b"}, "exactly name and owner"), + ({"name": "", "owner": "a"}, "nonempty string"), + ({"name": "bot-reviewed", "owner": " "}, "owner must name the app"), +]) +def test_invalid_external_entries_are_reported(bad, message): + [problem] = check.external_problems([bad], []) + assert message in problem + + +def test_external_duplicates_and_overlap_with_the_manifest_are_reported(): + external = [{"name": "bot-reviewed", "owner": "app"}, {"name": "bot-reviewed", "owner": "app"}, + {"name": "kind:bug", "owner": "app"}] + assert check.external_problems(external, [entry("kind:bug")]) == [ + "bot-reviewed: listed more than once", + "kind:bug: external labels cannot also be in labels.json", + ] + + def test_drift_reports_missing_extra_changed_and_unlisted_references(): manifest = [entry("kind:bug"), entry("status:proposed", description="Waiting")] repository = [{"name": "status:proposed", "description": None}, {"name": "bug", "description": "Something isn't working"}] - assert check.drift(manifest, repository, {"form.yml": ["kind:bug", "acceptance"]}) == [ + assert check.drift(manifest, repository, {"form.yml": ["kind:bug", "acceptance"]}, set()) == [ "kind:bug: in labels.json, missing from the repository", "bug: in the repository, missing from labels.json", "status:proposed: repository description differs from labels.json", @@ -61,6 +87,25 @@ def test_drift_reports_missing_extra_changed_and_unlisted_references(): ] +def test_drift_accepts_an_external_label_in_the_repository(): + manifest = [entry("kind:bug")] + repository = [{"name": "kind:bug", "description": "Meaning"}, + {"name": "bot-reviewed", "description": None}] + assert check.drift(manifest, repository, {}, {"bot-reviewed"}) == [] + # Absence is also accepted: an outside app owns whether the label exists. + assert check.drift(manifest, repository[:1], {}, {"bot-reviewed"}) == [] + + +def test_drift_still_rejects_a_template_reference_to_an_external_label(): + manifest = [entry("kind:bug")] + repository = [{"name": "kind:bug", "description": "Meaning"}, + {"name": "bot-reviewed", "description": None}] + assert check.drift(manifest, repository, {"form.yml": ["bot-reviewed"]}, + {"bot-reviewed"}) == [ + "bot-reviewed: referenced by form.yml, missing from labels.json", + ] + + def test_template_and_config_label_forms(tmp_path): templates = tmp_path / ".github/ISSUE_TEMPLATE" templates.mkdir(parents=True) @@ -89,7 +134,8 @@ def test_markdown_template_without_front_matter_fails(tmp_path): def test_main_passes_silently_when_repository_matches(tmp_path, capsys): - assert check.main([write_lines(tmp_path / "labels.jsonl", LIVE)]) == 0 + live = LIVE + [{"name": name, "description": None} for name in sorted(EXTERNAL_NAMES)] + assert check.main([write_lines(tmp_path / "labels.jsonl", live)]) == 0 assert capsys.readouterr().out == "" diff --git a/tools/check_labels.py b/tools/check_labels.py index 5f118a36c..365fc737c 100644 --- a/tools/check_labels.py +++ b/tools/check_labels.py @@ -38,6 +38,26 @@ def manifest_problems(manifest: list[dict]) -> list[str]: return problems +def external_problems(external: list[dict], manifest: list[dict]) -> list[str]: + problems = [] + for entry in external: + name = entry.get("name") + if set(entry) != {"name", "owner"}: + problems.append(f"{name}: expected exactly name and owner") + continue + if not isinstance(name, str) or not name: + problems.append(f"{name}: name must be a nonempty string") + if not isinstance(entry["owner"], str) or not entry["owner"].strip(): + problems.append(f"{name}: owner must name the app that applies it") + names = [entry.get("name") for entry in external] + problems += [f"{name}: listed more than once" + for name in sorted({n for n in names if names.count(n) > 1}, key=str)] + listed = {e["name"] for e in manifest if isinstance(e.get("name"), str)} + problems += [f"{name}: external labels cannot also be in labels.json" + for name in sorted(set(names) & listed)] + return problems + + def _names(value, source: str) -> list[str]: # GitHub accepts a list of names or one comma-separated string in templates. if value is None: @@ -72,14 +92,14 @@ def referenced_labels(root: Path) -> dict[str, list[str]]: def drift(manifest: list[dict], repository: list[dict], - references: dict[str, list[str]]) -> list[str]: + references: dict[str, list[str]], external: set[str]) -> list[str]: expected = {entry["name"]: entry["description"] for entry in manifest} # The labels API returns null for a label without a description. actual = {label["name"]: label["description"] or "" for label in repository} problems = [f"{name}: in labels.json, missing from the repository" for name in sorted(expected.keys() - actual.keys())] problems += [f"{name}: in the repository, missing from labels.json" - for name in sorted(actual.keys() - expected.keys())] + for name in sorted(actual.keys() - expected.keys() - external)] problems += [f"{name}: repository description differs from labels.json" for name in sorted(expected.keys() & actual.keys()) if expected[name] != actual[name]] @@ -95,11 +115,13 @@ def main(argv: list[str] | None = None) -> int: help="JSON lines holding each repository label's name and description") args = parser.parse_args(argv) manifest = json.loads((ROOT / ".github/labels.json").read_text(encoding="utf-8")) + external = json.loads((ROOT / ".github/external-labels.json").read_text(encoding="utf-8")) repository = [json.loads(line) for line in args.repository_labels.read_text(encoding="utf-8").splitlines() if line.strip()] - problems = manifest_problems(manifest) + problems = manifest_problems(manifest) + external_problems(external, manifest) if not problems: - problems = drift(manifest, repository, referenced_labels(ROOT)) + problems = drift(manifest, repository, referenced_labels(ROOT), + {entry["name"] for entry in external}) # Each printed line starts with "label drift" so a label name cannot form a workflow command. for problem in problems: print(f"label drift: {problem}") From 00b96cd46af22b4e8c7eef4bbee63470a4551d11 Mon Sep 17 00:00:00 2001 From: Dong1017 <55723162+Dong1017@users.noreply.github.com> Date: Wed, 4 Nov 2026 17:34:25 +0800 Subject: [PATCH 2/4] fix(labels): report an unusable external name instead of raising Review found that `external_problems` reported a non-string `name` and then still fed it to the duplicate and overlap sets, so a list or dict name raised `TypeError: unhashable type` instead of printing a drift line. Only a usable name now joins those comparisons, and three negative cases cover it. Also state in the docs that `owner` is required but informational: no check verifies it and none can, because GitHub does not report which app created a label. Readers should confirm from the label's event history before relying on an entry. Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com> Agent-assisted-by: Claude Code / claude-opus-5 (author) --- docs/agent-collaboration.md | 8 ++++++++ tests/test_check_labels.py | 8 ++++++++ tools/check_labels.py | 9 ++++++--- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/docs/agent-collaboration.md b/docs/agent-collaboration.md index 7b1734b96..729d44db1 100644 --- a/docs/agent-collaboration.md +++ b/docs/agent-collaboration.md @@ -232,6 +232,14 @@ do not apply to them. A name cannot be in both files. An Issue template or `tools/skill_eval_*_config.json` still may not reference one, since this repository must not depend on a label it does not own. +Each `owner` is a required, informational attribution: the reviewer's record of which app +applies that label, so a later reader can tell why the name is exempt rather than missing. No +check verifies it, and none can — GitHub does not report which app created a label, and an +app's configuration is outside this repository. Treat an entry as evidence of a past decision, +not as a verified fact: before relying on one, confirm from the label's own event history that +the named app still applies it. Adding an entry is a scope decision, so it goes through review +like any other change to this file. + ## Evidence refresh — hypothetical example **Hypothetical illustration only; it records no actual PR, run or result in this diff --git a/tests/test_check_labels.py b/tests/test_check_labels.py index 3e149c30d..f1ec8aee7 100644 --- a/tests/test_check_labels.py +++ b/tests/test_check_labels.py @@ -59,6 +59,7 @@ def test_duplicate_names_are_reported(): ({"name": "bot-reviewed"}, "exactly name and owner"), ({"name": "bot-reviewed", "owner": "a", "note": "b"}, "exactly name and owner"), ({"name": "", "owner": "a"}, "nonempty string"), + ({"name": None, "owner": "a"}, "nonempty string"), ({"name": "bot-reviewed", "owner": " "}, "owner must name the app"), ]) def test_invalid_external_entries_are_reported(bad, message): @@ -66,6 +67,13 @@ def test_invalid_external_entries_are_reported(bad, message): assert message in problem +@pytest.mark.parametrize("name", [["a", "b"], {"a": 1}, 7]) +def test_an_unhashable_or_nonstring_external_name_is_reported_not_raised(name): + # An unusable name must not reach the duplicate/overlap sets and crash the check. + [problem] = check.external_problems([{"name": name, "owner": "app"}], [entry("kind:bug")]) + assert "name must be a nonempty string" in problem + + def test_external_duplicates_and_overlap_with_the_manifest_are_reported(): external = [{"name": "bot-reviewed", "owner": "app"}, {"name": "bot-reviewed", "owner": "app"}, {"name": "kind:bug", "owner": "app"}] diff --git a/tools/check_labels.py b/tools/check_labels.py index 365fc737c..f40777df0 100644 --- a/tools/check_labels.py +++ b/tools/check_labels.py @@ -40,18 +40,21 @@ def manifest_problems(manifest: list[dict]) -> list[str]: def external_problems(external: list[dict], manifest: list[dict]) -> list[str]: problems = [] + names = [] for entry in external: name = entry.get("name") if set(entry) != {"name", "owner"}: problems.append(f"{name}: expected exactly name and owner") continue - if not isinstance(name, str) or not name: + # Only a usable name joins the duplicate and overlap comparisons below. + if isinstance(name, str) and name: + names.append(name) + else: problems.append(f"{name}: name must be a nonempty string") if not isinstance(entry["owner"], str) or not entry["owner"].strip(): problems.append(f"{name}: owner must name the app that applies it") - names = [entry.get("name") for entry in external] problems += [f"{name}: listed more than once" - for name in sorted({n for n in names if names.count(n) > 1}, key=str)] + for name in sorted({n for n in names if names.count(n) > 1})] listed = {e["name"] for e in manifest if isinstance(e.get("name"), str)} problems += [f"{name}: external labels cannot also be in labels.json" for name in sorted(set(names) & listed)] From 9451f08b9c4f72d664f616e9c6f2aa369fe78495 Mon Sep 17 00:00:00 2001 From: Dong1017 <55723162+Dong1017@users.noreply.github.com> Date: Wed, 4 Nov 2026 21:19:53 +0800 Subject: [PATCH 3/4] fix(labels): reject a whitespace-only external label name Review noted that `name` accepted `" "` while `owner` was already checked with `.strip()`. A whitespace-only name can never match a GitHub label, so it was silently admitted as an exemption that exempts nothing. Check it the same way. Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com> Agent-assisted-by: Claude Code / claude-opus-5 (author) --- tests/test_check_labels.py | 1 + tools/check_labels.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_check_labels.py b/tests/test_check_labels.py index f1ec8aee7..545454943 100644 --- a/tests/test_check_labels.py +++ b/tests/test_check_labels.py @@ -59,6 +59,7 @@ def test_duplicate_names_are_reported(): ({"name": "bot-reviewed"}, "exactly name and owner"), ({"name": "bot-reviewed", "owner": "a", "note": "b"}, "exactly name and owner"), ({"name": "", "owner": "a"}, "nonempty string"), + ({"name": " ", "owner": "a"}, "nonempty string"), ({"name": None, "owner": "a"}, "nonempty string"), ({"name": "bot-reviewed", "owner": " "}, "owner must name the app"), ]) diff --git a/tools/check_labels.py b/tools/check_labels.py index f40777df0..b995ceae2 100644 --- a/tools/check_labels.py +++ b/tools/check_labels.py @@ -47,7 +47,7 @@ def external_problems(external: list[dict], manifest: list[dict]) -> list[str]: problems.append(f"{name}: expected exactly name and owner") continue # Only a usable name joins the duplicate and overlap comparisons below. - if isinstance(name, str) and name: + if isinstance(name, str) and name.strip(): names.append(name) else: problems.append(f"{name}: name must be a nonempty string") From 992eae4210f1a03ff5e134b7a80f2b008a3691d4 Mon Sep 17 00:00:00 2001 From: Dong1017 <55723162+Dong1017@users.noreply.github.com> Date: Thu, 5 Nov 2026 15:55:32 +0900 Subject: [PATCH 4/4] fix(labels): report malformed manifest entries without crashing Validate objects before field access in both label manifests, and keep unusable names out of duplicate and overlap comparisons. Add symmetric validation and CLI regressions. Agent-assisted-by: dot (implementation) Signed-off-by: Codex --- tests/test_check_labels.py | 51 ++++++++++++++++++++++++++++++++++++++ tools/check_labels.py | 16 +++++++++--- 2 files changed, 63 insertions(+), 4 deletions(-) diff --git a/tests/test_check_labels.py b/tests/test_check_labels.py index 545454943..f5605b685 100644 --- a/tests/test_check_labels.py +++ b/tests/test_check_labels.py @@ -55,6 +55,39 @@ def test_duplicate_names_are_reported(): ] +@pytest.mark.parametrize("bad", ["kind:bug", None, [], 7, True]) +def test_nonobject_entries_are_reported_in_both_manifests(bad): + assert check.manifest_problems([entry("kind:bug"), bad]) == [ + "labels.json entry 2: expected an object" + ] + assert check.external_problems([{"name": "bot-reviewed", "owner": "app"}, bad], []) == [ + "external-labels.json entry 2: expected an object" + ] + + +def test_invalid_entries_do_not_hide_duplicates_or_overlap(): + manifest = [None, entry("kind:bug"), entry("kind:bug")] + assert check.manifest_problems(manifest) == [ + "labels.json entry 1: expected an object", + "kind:bug: listed more than once", + ] + external = [None, {"name": "kind:bug", "owner": "app"}, + {"name": "kind:bug", "owner": "app"}] + assert check.external_problems(external, manifest) == [ + "external-labels.json entry 1: expected an object", + "kind:bug: listed more than once", + "kind:bug: external labels cannot also be in labels.json", + ] + + +@pytest.mark.parametrize("name", [["a", "b"], {"a": 1}, 7, None]) +def test_unusable_managed_names_do_not_reach_duplicate_checks(name): + bad = {"name": name, "group": "kind", "description": "Meaning"} + problems = check.manifest_problems([bad, bad]) + assert len(problems) == 2 + assert all("name must have the form" in problem for problem in problems) + + @pytest.mark.parametrize("bad, message", [ ({"name": "bot-reviewed"}, "exactly name and owner"), ({"name": "bot-reviewed", "owner": "a", "note": "b"}, "exactly name and owner"), @@ -157,3 +190,21 @@ def test_main_reports_each_drift_line_and_fails(tmp_path, capsys): "label drift: ::error::x: in the repository, missing from labels.json", ] assert "labels.json" in out[2] and len(out) == 3 + + +@pytest.mark.parametrize("filename", ["labels.json", "external-labels.json"]) +@pytest.mark.parametrize("bad", ["kind:bug", None]) +def test_main_reports_nonobject_entries_and_fails_without_drift( + tmp_path, monkeypatch, capsys, filename, bad): + github = tmp_path / ".github" + github.mkdir() + for name, entries in [("labels.json", MANIFEST), ("external-labels.json", EXTERNAL)]: + (github / name).write_text(json.dumps([bad] if name == filename else entries), + encoding="utf-8") + monkeypatch.setattr(check, "ROOT", tmp_path) + # No ISSUE_TEMPLATE directory exists: drift/reference reads must not run after validation fails. + assert check.main([write_lines(tmp_path / "labels.jsonl", LIVE)]) == 1 + assert capsys.readouterr().out.splitlines() == [ + f"label drift: {filename} entry 1: expected an object", + check.ADVICE, + ] diff --git a/tools/check_labels.py b/tools/check_labels.py index b995ceae2..c2dd10ec5 100644 --- a/tools/check_labels.py +++ b/tools/check_labels.py @@ -21,7 +21,10 @@ def manifest_problems(manifest: list[dict]) -> list[str]: problems = [] - for entry in manifest: + for index, entry in enumerate(manifest, start=1): + if not isinstance(entry, dict): + problems.append(f"labels.json entry {index}: expected an object") + continue name = entry.get("name") if set(entry) != {"name", "group", "description"}: problems.append(f"{name}: expected exactly name, group and description") @@ -32,7 +35,8 @@ def manifest_problems(manifest: list[dict]) -> list[str]: description = entry["description"] if not isinstance(description, str) or not 0 < len(description) <= MAX_DESCRIPTION: problems.append(f"{name}: description must have 1-{MAX_DESCRIPTION} characters") - names = [entry.get("name") for entry in manifest] + names = [entry.get("name") for entry in manifest + if isinstance(entry, dict) and isinstance(entry.get("name"), str)] problems += [f"{name}: listed more than once" for name in sorted({name for name in names if names.count(name) > 1}, key=str)] return problems @@ -41,7 +45,10 @@ def manifest_problems(manifest: list[dict]) -> list[str]: def external_problems(external: list[dict], manifest: list[dict]) -> list[str]: problems = [] names = [] - for entry in external: + for index, entry in enumerate(external, start=1): + if not isinstance(entry, dict): + problems.append(f"external-labels.json entry {index}: expected an object") + continue name = entry.get("name") if set(entry) != {"name", "owner"}: problems.append(f"{name}: expected exactly name and owner") @@ -55,7 +62,8 @@ def external_problems(external: list[dict], manifest: list[dict]) -> list[str]: problems.append(f"{name}: owner must name the app that applies it") problems += [f"{name}: listed more than once" for name in sorted({n for n in names if names.count(n) > 1})] - listed = {e["name"] for e in manifest if isinstance(e.get("name"), str)} + listed = {e["name"] for e in manifest + if isinstance(e, dict) and isinstance(e.get("name"), str)} problems += [f"{name}: external labels cannot also be in labels.json" for name in sorted(set(names) & listed)] return problems