From 5a9fcece8cd6ee7dca0deb234ba42a9f862915df Mon Sep 17 00:00:00 2001 From: gavinbee <29419542+gavinbee@users.noreply.github.com> Date: Thu, 3 Sep 2026 23:08:45 -0400 Subject: [PATCH 1/2] Reconcile labels from settings.yml MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The labels block landed in #47 as a documented-but-inert key: settings.yml named the canonical set, and apply-settings.py read only repository, branches and rulesets, so `no-issue` still had to be created by hand in each repo. This wires it up, which is the rest of #46. Reconciliation is additive on purpose. A label named in settings.yml is created when missing and corrected when its colour or description drifted; labels the YAML doesn't mention are left alone. Repos carry GitHub's defaults plus the ones Dependabot creates, and deleting a label strips it from every issue and PR that uses it — not something a scheduled job should do unprompted. settings.yml is the minimum set, not the whole set. Verification re-reads the labels and compares them, rather than assuming the write took. That is deliberately not the shape of verify_ruleset, which only checks that a ruleset name exists and is the subject of #45 — no reason to add a second instance of the same blind spot. Colour comparison normalises: GitHub stores six lowercase hex digits with no '#', so "#EDEDED" in YAML and "ededed" from the API are not drift. A null description from the API compares equal to an absent one in YAML, and an entry with no colour does not blank the colour already on the repo. Dry-run against real repos: deck-eval-gen's hand-created `no-issue` reads as in sync, and this repo's missing one reads as a create. Closes #46 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CgLE8cg9huy2EZnRoXhmwp --- .github/settings.yml | 9 +-- scripts/apply-settings.py | 129 ++++++++++++++++++++++++++++++++++- tests/test_apply_settings.py | 58 ++++++++++++++++ 3 files changed, 190 insertions(+), 6 deletions(-) diff --git a/.github/settings.yml b/.github/settings.yml index d223212..140355f 100644 --- a/.github/settings.yml +++ b/.github/settings.yml @@ -26,10 +26,11 @@ repository: # Labels that should exist in every repo. # -# NOT YET APPLIED. `apply-settings.py` acts on `repository`, `branches` and `rulesets` -# only, so these labels are still created per repo by hand with `gh label create`. This -# block is the canonical set; teaching the reconciler to apply it is the remaining work on -# https://github.com/swimblocks/.github/issues/46, which stays open until it lands. +# Applied by `apply-settings.py`, which creates a missing label and corrects a drifted +# colour or description. Reconciliation is **additive**: labels not listed here are left +# alone, because repos also carry GitHub's defaults and Dependabot's (`dependencies`, +# `python`), and deleting a label strips it from every issue and PR that uses it. This is +# the minimum set, not the whole set. labels: - name: no-issue color: ededed diff --git a/scripts/apply-settings.py b/scripts/apply-settings.py index 00a87b7..f560441 100644 --- a/scripts/apply-settings.py +++ b/scripts/apply-settings.py @@ -26,8 +26,8 @@ # Only fields under `repository:` that we know how to PATCH via # `PATCH /repos/{owner}/{repo}`. Anything else in the YAML is left for a -# future iteration (branches, labels, collaborators — Probot Settings' -# other top-level sections). +# future iteration (collaborators and Probot Settings' other top-level +# sections); `branches`, `rulesets` and `labels` have their own handling below. PATCHABLE = { "allow_squash_merge", "allow_merge_commit", @@ -89,6 +89,124 @@ def verify(repo: str, repo_block: dict) -> bool: return ok +# --------------------------------------------------------------------------- +# Labels +# +# Reconciliation here is **additive**: a label in `settings.yml` is created if +# missing and corrected if its colour or description drifted, but labels the +# YAML doesn't mention are left alone. Repos carry GitHub's defaults plus ones +# Dependabot creates (`dependencies`, `python`), and deleting a label removes it +# from every issue and PR that uses it — not something a scheduled job should do +# behind your back. `settings.yml` is the minimum set, not the whole set. +# --------------------------------------------------------------------------- + +def normalize_color(value: str | None) -> str: + """GitHub stores label colours as six lowercase hex digits with no '#'.""" + return str(value or "").lstrip("#").lower() + + +def label_updates(desired: dict, actual: dict) -> dict: + """Fields of *desired* that differ from *actual*. Empty dict means in sync.""" + updates: dict[str, str] = {} + want_color = normalize_color(desired.get("color")) + if want_color and want_color != normalize_color(actual.get("color")): + updates["color"] = want_color + want_desc = desired.get("description") or "" + if want_desc != (actual.get("description") or ""): + updates["description"] = want_desc + return updates + + +def list_labels(repo: str) -> dict[str, dict]: + """Every label on *repo*, keyed by lowercased name. + + GitHub treats label names case-insensitively, so the key is lowercased to + stop a `No-Issue` on the repo reading as missing against a `no-issue` here. + """ + # `--jq .[]` emits one compact object per line, which pages cleanly; + # `--paginate` alone would concatenate raw JSON arrays. + out = subprocess.run( + ["gh", "api", f"repos/{repo}/labels", "--paginate", "--jq", ".[]"], + check=True, capture_output=True, text=True, + ).stdout + labels = [json.loads(line) for line in out.splitlines() if line.strip()] + return {label["name"].lower(): label for label in labels} + + +def apply_labels(repo: str, labels_block: list[dict]) -> bool: + """Create or correct each label in *labels_block*. Returns False on any failure.""" + if not labels_block: + return True + try: + actual = list_labels(repo) + except subprocess.CalledProcessError as e: + print(f" FAIL labels: could not list (exit {e.returncode})") + return False + + ok = True + for desired in labels_block: + name = desired.get("name") + if not name: + continue + existing = actual.get(name.lower()) + try: + if existing is None: + subprocess.run( + ["gh", "api", "-X", "POST", f"repos/{repo}/labels", + "-f", f"name={name}", + "-f", f"color={normalize_color(desired.get('color'))}", + "-f", f"description={desired.get('description') or ''}"], + check=True, stdout=subprocess.DEVNULL, + ) + print(f" OK label '{name}': created") + continue + updates = label_updates(desired, existing) + if not updates: + continue + args: list[str] = [] + for key, value in updates.items(): + args.extend(["-f", f"{key}={value}"]) + subprocess.run( + ["gh", "api", "-X", "PATCH", f"repos/{repo}/labels/{name}", *args], + check=True, stdout=subprocess.DEVNULL, + ) + print(f" OK label '{name}': updated {', '.join(sorted(updates))}") + except subprocess.CalledProcessError as e: + print(f" FAIL label '{name}': write failed (exit {e.returncode})") + ok = False + return ok + + +def verify_labels(repo: str, labels_block: list[dict]) -> bool: + """Re-read the labels and compare, so a write that didn't take is reported.""" + if not labels_block: + return True + try: + actual = list_labels(repo) + except subprocess.CalledProcessError as e: + print(f" FAIL labels: could not list (exit {e.returncode})") + return False + + ok = True + for desired in labels_block: + name = desired.get("name") + if not name: + continue + existing = actual.get(name.lower()) + if existing is None: + print(f" FAIL label '{name}': missing") + ok = False + continue + diff = label_updates(desired, existing) + if diff: + got = {k: (existing.get(k) or "") for k in diff} + print(f" FAIL label '{name}': {got} (expected {diff})") + ok = False + else: + print(f" OK label '{name}': present") + return ok + + # Top-level keys of the branch protection PUT body. Anything not in this set is # rejected by the API, so we filter `protection:` in the YAML down to these. PROTECTION_KEYS = { @@ -225,6 +343,7 @@ def main(argv: list[str]) -> int: args = patch_args(repo_block) rulesets_block = settings.get("rulesets") or [] + labels_block = settings.get("labels") or [] failures: list[str] = [] for repo in argv[1:]: @@ -233,6 +352,12 @@ def main(argv: list[str]) -> int: if not verify(repo, repo_block): failures.append(repo) + # Labels do not depend on visibility — private repos get them too. + if labels_block: + apply_labels(repo, labels_block) + if not verify_labels(repo, labels_block) and repo not in failures: + failures.append(repo) + is_public = get_repo_visibility(repo) == "public" if is_public and rulesets_block: diff --git a/tests/test_apply_settings.py b/tests/test_apply_settings.py index 5c7b826..ab20ae9 100644 --- a/tests/test_apply_settings.py +++ b/tests/test_apply_settings.py @@ -84,3 +84,61 @@ def test_non_wrapper_dict_passes_through_unchanged(self): def test_output_always_covers_every_protection_key(self): flat = apply_settings._flatten_protection({}) assert set(flat) == apply_settings.PROTECTION_KEYS + + +class TestNormalizeColor: + def test_strips_leading_hash(self): + assert apply_settings.normalize_color("#EDEDED") == "ededed" + + def test_lowercases(self): + assert apply_settings.normalize_color("A2EEEF") == "a2eeef" + + def test_already_normal_is_unchanged(self): + assert apply_settings.normalize_color("ededed") == "ededed" + + def test_none_becomes_empty(self): + assert apply_settings.normalize_color(None) == "" + + +class TestLabelUpdates: + _DESIRED = {"name": "no-issue", "color": "ededed", "description": "Skips the issue"} + + def test_in_sync_returns_nothing(self): + actual = {"color": "ededed", "description": "Skips the issue"} + assert apply_settings.label_updates(self._DESIRED, actual) == {} + + def test_hash_prefix_and_case_do_not_count_as_drift(self): + desired = {"color": "#EDEDED", "description": "Skips the issue"} + actual = {"color": "ededed", "description": "Skips the issue"} + assert apply_settings.label_updates(desired, actual) == {} + + def test_colour_drift_is_reported_normalized(self): + actual = {"color": "FF0000", "description": "Skips the issue"} + assert apply_settings.label_updates(self._DESIRED, actual) == { + "color": "ededed", + } + + def test_description_drift_is_reported(self): + actual = {"color": "ededed", "description": "something else"} + assert apply_settings.label_updates(self._DESIRED, actual) == { + "description": "Skips the issue", + } + + def test_null_description_on_the_repo_counts_as_empty(self): + # The API returns null, not "", for a label with no description. + desired = {"color": "ededed"} + actual = {"color": "ededed", "description": None} + assert apply_settings.label_updates(desired, actual) == {} + + def test_missing_desired_colour_is_not_treated_as_drift(self): + # A YAML entry with no colour shouldn't blank the repo's colour. + desired = {"name": "no-issue", "description": "Skips the issue"} + actual = {"color": "ededed", "description": "Skips the issue"} + assert apply_settings.label_updates(desired, actual) == {} + + def test_both_fields_drifted(self): + actual = {"color": "FF0000", "description": None} + assert apply_settings.label_updates(self._DESIRED, actual) == { + "color": "ededed", + "description": "Skips the issue", + } From 8ff34d2b3971c34f8f62a7107b7c85b3f28de6e4 Mon Sep 17 00:00:00 2001 From: gavinbee <29419542+gavinbee@users.noreply.github.com> Date: Thu, 3 Sep 2026 23:29:24 -0400 Subject: [PATCH 2/2] Only create labels; leave existing ones alone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review raised both of these. Colour drift is not worth reconciling. A repo that recolours its labels to group fifty of them visually is doing something useful, and a weekly job reverting that is churn. Colour carries no policy weight; what does is that the label exists, since a PR cannot be given a label the repo lacks. The same argument mostly covers description, so the simplest defensible rule is create-if-missing and nothing else: settings.yml seeds a new label, the repo owns it afterwards. If the canonical description ever changes, that is rare and deliberate, and better done as a one-off than by carrying update machinery for a case that may not arise. That removes label_updates entirely, along with the false-drift handling it needed for "#EDEDED" versus "ededed" and a null description versus an absent one. verify_labels reduces to a presence check — which is complete here, because presence is the whole assertion. That is what separates it from verify_ruleset, where the rules are the substance and go unchecked (#45); the point is that verification should cover exactly what the tool claims. normalize_color stays for the create path so a '#' in YAML never reaches the API. The settings.yml comment also restated "applied by apply-settings.py", which the file header already says and no other block repeats. Trimmed to the part that is actually non-obvious: labels are seeded, not governed, and unlisted ones are never removed. Net 34 lines lighter, and one fewer test than before for more behaviour covered. Refs #46 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CgLE8cg9huy2EZnRoXhmwp --- .github/settings.yml | 11 ++--- scripts/apply-settings.py | 86 ++++++++++++++---------------------- tests/test_apply_settings.py | 75 ++++++++++++++----------------- 3 files changed, 69 insertions(+), 103 deletions(-) diff --git a/.github/settings.yml b/.github/settings.yml index 140355f..93e402e 100644 --- a/.github/settings.yml +++ b/.github/settings.yml @@ -24,13 +24,10 @@ repository: delete_branch_on_merge: true allow_update_branch: true -# Labels that should exist in every repo. -# -# Applied by `apply-settings.py`, which creates a missing label and corrects a drifted -# colour or description. Reconciliation is **additive**: labels not listed here are left -# alone, because repos also carry GitHub's defaults and Dependabot's (`dependencies`, -# `python`), and deleting a label strips it from every issue and PR that uses it. This is -# the minimum set, not the whole set. +# Labels that should exist in every repo. Created when missing and then left alone — +# the values here seed a new label, they don't govern an existing one. Labels not +# listed are untouched: repos carry GitHub's defaults and Dependabot's too, and +# deleting a label strips it from every issue and PR that used it. labels: - name: no-issue color: ededed diff --git a/scripts/apply-settings.py b/scripts/apply-settings.py index f560441..7e9f073 100644 --- a/scripts/apply-settings.py +++ b/scripts/apply-settings.py @@ -92,12 +92,15 @@ def verify(repo: str, repo_block: dict) -> bool: # --------------------------------------------------------------------------- # Labels # -# Reconciliation here is **additive**: a label in `settings.yml` is created if -# missing and corrected if its colour or description drifted, but labels the -# YAML doesn't mention are left alone. Repos carry GitHub's defaults plus ones -# Dependabot creates (`dependencies`, `python`), and deleting a label removes it -# from every issue and PR that uses it — not something a scheduled job should do -# behind your back. `settings.yml` is the minimum set, not the whole set. +# Create-if-missing, and nothing else. The values here seed a new label; they do +# not govern an existing one. A repo that recolours its labels to group them +# visually is doing something useful, and a weekly job reverting that would be +# churn — colour carries no policy weight. What does carry weight is that the +# label *exists*, since a PR cannot be given a label the repo doesn't have. +# +# Nor are unlisted labels removed: repos carry GitHub's defaults and the ones +# Dependabot creates, and deleting a label strips it from every issue and PR +# that used it. `settings.yml` is the minimum set, not the whole set. # --------------------------------------------------------------------------- def normalize_color(value: str | None) -> str: @@ -105,18 +108,6 @@ def normalize_color(value: str | None) -> str: return str(value or "").lstrip("#").lower() -def label_updates(desired: dict, actual: dict) -> dict: - """Fields of *desired* that differ from *actual*. Empty dict means in sync.""" - updates: dict[str, str] = {} - want_color = normalize_color(desired.get("color")) - if want_color and want_color != normalize_color(actual.get("color")): - updates["color"] = want_color - want_desc = desired.get("description") or "" - if want_desc != (actual.get("description") or ""): - updates["description"] = want_desc - return updates - - def list_labels(repo: str) -> dict[str, dict]: """Every label on *repo*, keyed by lowercased name. @@ -133,8 +124,13 @@ def list_labels(repo: str) -> dict[str, dict]: return {label["name"].lower(): label for label in labels} +def missing_labels(desired: list[dict], actual: dict[str, dict]) -> list[dict]: + """The entries of *desired* that *actual* has no label for.""" + return [d for d in desired if d.get("name") and d["name"].lower() not in actual] + + def apply_labels(repo: str, labels_block: list[dict]) -> bool: - """Create or correct each label in *labels_block*. Returns False on any failure.""" + """Create any label in *labels_block* the repo lacks. Returns False on failure.""" if not labels_block: return True try: @@ -144,41 +140,30 @@ def apply_labels(repo: str, labels_block: list[dict]) -> bool: return False ok = True - for desired in labels_block: - name = desired.get("name") - if not name: - continue - existing = actual.get(name.lower()) + for desired in missing_labels(labels_block, actual): + name = desired["name"] try: - if existing is None: - subprocess.run( - ["gh", "api", "-X", "POST", f"repos/{repo}/labels", - "-f", f"name={name}", - "-f", f"color={normalize_color(desired.get('color'))}", - "-f", f"description={desired.get('description') or ''}"], - check=True, stdout=subprocess.DEVNULL, - ) - print(f" OK label '{name}': created") - continue - updates = label_updates(desired, existing) - if not updates: - continue - args: list[str] = [] - for key, value in updates.items(): - args.extend(["-f", f"{key}={value}"]) subprocess.run( - ["gh", "api", "-X", "PATCH", f"repos/{repo}/labels/{name}", *args], + ["gh", "api", "-X", "POST", f"repos/{repo}/labels", + "-f", f"name={name}", + "-f", f"color={normalize_color(desired.get('color'))}", + "-f", f"description={desired.get('description') or ''}"], check=True, stdout=subprocess.DEVNULL, ) - print(f" OK label '{name}': updated {', '.join(sorted(updates))}") + print(f" OK label '{name}': created") except subprocess.CalledProcessError as e: - print(f" FAIL label '{name}': write failed (exit {e.returncode})") + print(f" FAIL label '{name}': create failed (exit {e.returncode})") ok = False return ok def verify_labels(repo: str, labels_block: list[dict]) -> bool: - """Re-read the labels and compare, so a write that didn't take is reported.""" + """Re-read the labels and confirm each one exists. + + Presence is the whole assertion — this only ever creates — so checking + presence checks everything claimed. (Contrast `verify_ruleset`, where the + rules are the substance and go unchecked: https://github.com/swimblocks/.github/issues/45) + """ if not labels_block: return True try: @@ -192,18 +177,11 @@ def verify_labels(repo: str, labels_block: list[dict]) -> bool: name = desired.get("name") if not name: continue - existing = actual.get(name.lower()) - if existing is None: + if name.lower() in actual: + print(f" OK label '{name}': present") + else: print(f" FAIL label '{name}': missing") ok = False - continue - diff = label_updates(desired, existing) - if diff: - got = {k: (existing.get(k) or "") for k in diff} - print(f" FAIL label '{name}': {got} (expected {diff})") - ok = False - else: - print(f" OK label '{name}': present") return ok diff --git a/tests/test_apply_settings.py b/tests/test_apply_settings.py index ab20ae9..96ea6a3 100644 --- a/tests/test_apply_settings.py +++ b/tests/test_apply_settings.py @@ -100,45 +100,36 @@ def test_none_becomes_empty(self): assert apply_settings.normalize_color(None) == "" -class TestLabelUpdates: - _DESIRED = {"name": "no-issue", "color": "ededed", "description": "Skips the issue"} - - def test_in_sync_returns_nothing(self): - actual = {"color": "ededed", "description": "Skips the issue"} - assert apply_settings.label_updates(self._DESIRED, actual) == {} - - def test_hash_prefix_and_case_do_not_count_as_drift(self): - desired = {"color": "#EDEDED", "description": "Skips the issue"} - actual = {"color": "ededed", "description": "Skips the issue"} - assert apply_settings.label_updates(desired, actual) == {} - - def test_colour_drift_is_reported_normalized(self): - actual = {"color": "FF0000", "description": "Skips the issue"} - assert apply_settings.label_updates(self._DESIRED, actual) == { - "color": "ededed", - } - - def test_description_drift_is_reported(self): - actual = {"color": "ededed", "description": "something else"} - assert apply_settings.label_updates(self._DESIRED, actual) == { - "description": "Skips the issue", - } - - def test_null_description_on_the_repo_counts_as_empty(self): - # The API returns null, not "", for a label with no description. - desired = {"color": "ededed"} - actual = {"color": "ededed", "description": None} - assert apply_settings.label_updates(desired, actual) == {} - - def test_missing_desired_colour_is_not_treated_as_drift(self): - # A YAML entry with no colour shouldn't blank the repo's colour. - desired = {"name": "no-issue", "description": "Skips the issue"} - actual = {"color": "ededed", "description": "Skips the issue"} - assert apply_settings.label_updates(desired, actual) == {} - - def test_both_fields_drifted(self): - actual = {"color": "FF0000", "description": None} - assert apply_settings.label_updates(self._DESIRED, actual) == { - "color": "ededed", - "description": "Skips the issue", - } +class TestMissingLabels: + _DESIRED = [ + {"name": "no-issue", "color": "ededed", "description": "Skips the issue"}, + ] + + def test_absent_label_is_reported(self): + assert apply_settings.missing_labels(self._DESIRED, {}) == self._DESIRED + + def test_present_label_is_not_reported(self): + actual = {"no-issue": {"color": "ededed", "description": "Skips the issue"}} + assert apply_settings.missing_labels(self._DESIRED, actual) == [] + + def test_name_match_is_case_insensitive(self): + # GitHub label names are case-insensitive, so a differently-cased + # label on the repo must not read as missing. + actual = {"no-issue": {"color": "ededed"}} + desired = [{"name": "No-Issue", "color": "ededed"}] + assert apply_settings.missing_labels(desired, actual) == [] + + def test_drifted_colour_is_not_treated_as_missing(self): + # Existing labels are left alone; only absence is acted on. + actual = {"no-issue": {"color": "ff0000", "description": "something else"}} + assert apply_settings.missing_labels(self._DESIRED, actual) == [] + + def test_entry_without_a_name_is_skipped(self): + assert apply_settings.missing_labels([{"color": "ededed"}], {}) == [] + + def test_only_the_absent_entries_are_returned(self): + desired = [{"name": "no-issue"}, {"name": "needs-decision"}] + actual = {"no-issue": {"color": "ededed"}} + assert apply_settings.missing_labels(desired, actual) == [ + {"name": "needs-decision"}, + ]