From 06adcfabf92d38234b032c5de9508fde19ab74f0 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Fri, 17 Jul 2026 06:12:27 -0700 Subject: [PATCH] feat(attribution): conflict-free contributor mappings via contributors/emails/ directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The AUTHOR_MAP dict in scripts/release.py was a merge-conflict magnet: every concurrent salvage PR appended entries to the same lines of the same file, so parallel PRs re-conflicted on every merge to main. New system: one file per email under contributors/emails/ — filename is the commit-author email, first non-comment line is the GitHub login. File additions never conflict, so any number of PRs can add mappings concurrently. - scripts/release.py: AUTHOR_MAP is now LEGACY_AUTHOR_MAP (frozen) merged with the directory at import time (directory wins). All existing consumers (resolve_author, contributor_audit.py) unchanged. - scripts/add_contributor.py: idempotent CLI to add a mapping; refuses conflicting reassignments (incl. against the legacy map), validates email/login shapes. - contributor-check.yml: attribution gate now accepts a mapping file OR a legacy entry; failure message prints the exact add_contributor command. Also auto-resolves bare @users.noreply.github.com emails is intentionally NOT added (kept id+login form only, matching previous behavior). - contributor_audit.py: guidance now points at add_contributor.py. - tests/scripts/test_contributor_map.py: 12 tests covering loader, merge precedence, CLI idempotency/conflict/validation, subprocess E2E. --- .github/workflows/contributor-check.yml | 19 ++-- contributors/README.md | 38 +++++++ contributors/emails/.gitkeep | 2 + scripts/add_contributor.py | 99 +++++++++++++++++ scripts/contributor_audit.py | 8 +- scripts/release.py | 43 +++++++- tests/scripts/test_contributor_map.py | 137 ++++++++++++++++++++++++ 7 files changed, 333 insertions(+), 13 deletions(-) create mode 100644 contributors/README.md create mode 100644 contributors/emails/.gitkeep create mode 100644 scripts/add_contributor.py create mode 100644 tests/scripts/test_contributor_map.py diff --git a/.github/workflows/contributor-check.yml b/.github/workflows/contributor-check.yml index b7c3db7f827..fd9e76752af 100644 --- a/.github/workflows/contributor-check.yml +++ b/.github/workflows/contributor-check.yml @@ -27,7 +27,9 @@ jobs: exit 0 fi - # Check each email against AUTHOR_MAP in release.py + # An email is mapped if it has a file in contributors/emails/ + # (one file per email — conflict-free) or an entry in the frozen + # legacy AUTHOR_MAP in scripts/release.py. MISSING="" while IFS= read -r email; do # Skip teknium and bot emails @@ -36,9 +38,12 @@ jobs: continue ;; esac - # Check if email is in AUTHOR_MAP (either as a key or matches noreply pattern) if echo "$email" | grep -qP '\+.*@users\.noreply\.github\.com'; then - continue # GitHub noreply emails auto-resolve + continue # GitHub id+login noreply emails auto-resolve + fi + + if [ -f "contributors/emails/${email}" ]; then + continue # mapped via the contributors directory fi if ! grep -qF "\"${email}\"" scripts/release.py 2>/dev/null; then @@ -49,19 +54,19 @@ jobs: if [ -n "$MISSING" ]; then echo "" - echo "⚠️ New contributor email(s) not in AUTHOR_MAP:" + echo "⚠️ New contributor email(s) without a mapping:" echo -e "$MISSING" echo "" - echo "Please add mappings to scripts/release.py AUTHOR_MAP:" + echo "Add a mapping file (do NOT edit AUTHOR_MAP in release.py):" echo -e "$MISSING" | while read -r line; do email=$(echo "$line" | sed 's/^ *//' | cut -d' ' -f1) [ -z "$email" ] && continue - echo " \"${email}\": \"\"," + echo " python3 scripts/add_contributor.py ${email} " done echo "" echo "To find the GitHub username for an email:" echo " gh api 'search/users?q=EMAIL+in:email' --jq '.items[0].login'" exit 1 else - echo "✅ All contributor emails are mapped in AUTHOR_MAP." + echo "✅ All contributor emails are mapped." fi diff --git a/contributors/README.md b/contributors/README.md new file mode 100644 index 00000000000..32ac5eaf425 --- /dev/null +++ b/contributors/README.md @@ -0,0 +1,38 @@ +# Contributor email → GitHub login mappings + +This directory replaces appending entries to `AUTHOR_MAP` in +`scripts/release.py`. The old dict caused constant merge conflicts when +several salvage PRs landed at once — every PR edited the same lines of the +same file. Here, **each mapping is its own file**, and file additions never +conflict. + +## Adding a mapping + +One file per commit-author email, under `emails/`: + +```bash +python3 scripts/add_contributor.py +# or by hand: +echo "" > contributors/emails/ +``` + +- File **name** = the exact commit-author email (as shown by `git log --format='%ae'`). +- File **content** = the GitHub login on the first non-comment line. + Lines starting with `#` are comments (use them for the PR reference). + +Example — `contributors/emails/jane.doe@example.com`: + +``` +janedoe +# PR #12345 salvage (gateway: fix session key routing) +``` + +## Rules + +- Do NOT add new entries to `AUTHOR_MAP` in `scripts/release.py`. That dict + is frozen legacy data; the release tooling merges it with this directory + (directory entries win on duplicates). +- GitHub noreply emails (`+@users.noreply.github.com` and + `@users.noreply.github.com`) auto-resolve — no file needed. +- The `Contributor Attribution Check` CI job fails a PR whose commits carry + an unmapped email; the failure message prints the exact command to run. diff --git a/contributors/emails/.gitkeep b/contributors/emails/.gitkeep new file mode 100644 index 00000000000..74f551fbce8 --- /dev/null +++ b/contributors/emails/.gitkeep @@ -0,0 +1,2 @@ +_placeholder +# keeps the directory present in git; not a real mapping diff --git a/scripts/add_contributor.py b/scripts/add_contributor.py new file mode 100644 index 00000000000..8192fe4f55c --- /dev/null +++ b/scripts/add_contributor.py @@ -0,0 +1,99 @@ +#!/usr/bin/env python3 +"""Add a contributor email → GitHub login mapping. + +Writes one file per email under contributors/emails/ (filename = email, +content = login). File additions never merge-conflict, unlike the legacy +AUTHOR_MAP dict in scripts/release.py, which is frozen — do not append to it. + +Usage (from the repo root): + python3 scripts/add_contributor.py [comment...] + + # e.g. + python3 scripts/add_contributor.py jane@example.com janedoe "PR #12345 salvage" + +Idempotent: if the mapping already exists with the same login, prints +"present" and exits 0. If the email maps to a DIFFERENT login (here or in the +legacy AUTHOR_MAP), refuses with exit 1 so a typo can't silently reassign +someone's commits. +""" + +import re +import sys +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent +EMAILS_DIR = REPO_ROOT / "contributors" / "emails" + +_EMAIL_RE = re.compile(r"^[^/\\\s]+@[^/\\\s]+$") +_LOGIN_RE = re.compile(r"^[A-Za-z0-9](?:[A-Za-z0-9]|-(?=[A-Za-z0-9])){0,38}$") + + +def read_mapping_file(path: Path) -> str | None: + """Return the login from a mapping file (first non-comment line).""" + try: + for line in path.read_text(encoding="utf-8").splitlines(): + line = line.strip() + if line and not line.startswith("#"): + return line + except OSError: + pass + return None + + +def _legacy_login(email: str) -> str | None: + """Look the email up in the frozen legacy AUTHOR_MAP in release.py.""" + try: + sys.path.insert(0, str(REPO_ROOT / "scripts")) + from release import LEGACY_AUTHOR_MAP # noqa: PLC0415 + + return LEGACY_AUTHOR_MAP.get(email) + except Exception: + return None + + +def add_contributor(email: str, login: str, comment: str = "") -> int: + email = email.strip() + login = login.strip().lstrip("@") + + if not _EMAIL_RE.match(email): + print(f"error: {email!r} does not look like a commit-author email", file=sys.stderr) + return 2 + if not _LOGIN_RE.match(login): + print(f"error: {login!r} is not a valid GitHub login", file=sys.stderr) + return 2 + + path = EMAILS_DIR / email + existing = read_mapping_file(path) if path.is_file() else None + if existing is None: + existing = _legacy_login(email) + if existing is not None: + if existing == login: + print("present") + return 0 + print( + f"error: {email} already maps to {existing!r} (asked for {login!r}) — " + "resolve manually", + file=sys.stderr, + ) + return 1 + + EMAILS_DIR.mkdir(parents=True, exist_ok=True) + body = login + "\n" + if comment: + body += f"# {comment}\n" + path.write_text(body, encoding="utf-8") + print(f"added: contributors/emails/{email} -> {login}") + return 0 + + +def main() -> int: + if len(sys.argv) < 3: + print(__doc__, file=sys.stderr) + return 2 + email, login = sys.argv[1], sys.argv[2] + comment = " ".join(sys.argv[3:]) + return add_contributor(email, login, comment) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/contributor_audit.py b/scripts/contributor_audit.py index c4216cfa909..9c371e4ae16 100644 --- a/scripts/contributor_audit.py +++ b/scripts/contributor_audit.py @@ -411,10 +411,10 @@ def main(): if all_unknowns: print() print(f"=== Unknown Emails ({len(all_unknowns)}) ===") - print("These emails are not in AUTHOR_MAP and should be added:") + print("These emails have no mapping and should be added via:") print() for email, name in sorted(all_unknowns.items()): - print(f' "{email}": "{name}",') + print(f" python3 scripts/add_contributor.py {email} # {name}") # ---- Strict mode: fail CI if new unmapped emails are introduced ---- if args.strict and all_unknowns: @@ -439,10 +439,10 @@ def main(): if new_unknowns: print() print(f"=== STRICT MODE FAILURE: {len(new_unknowns)} new unmapped email(s) ===") - print("Add these to AUTHOR_MAP in scripts/release.py before merging:") + print("Add mapping files before merging (do NOT edit AUTHOR_MAP):") print() for email, name in sorted(new_unknowns.items()): - print(f' "{email}": "",') + print(f" python3 scripts/add_contributor.py {email} # {name}") print() print("To find the GitHub username:") print(" gh api 'search/users?q=EMAIL+in:email' --jq '.items[0].login'") diff --git a/scripts/release.py b/scripts/release.py index 0b8cfddddff..4b704bd2b7e 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -43,8 +43,12 @@ ACP_REGISTRY_MANIFEST = REPO_ROOT / "acp_registry" / "agent.json" # Git email → GitHub username mapping # ────────────────────────────────────────────────────────────────────── -# Auto-extracted from noreply emails + manual overrides -AUTHOR_MAP = { +# FROZEN legacy mappings — do NOT add new entries here. New contributor +# mappings live as one-file-per-email entries under contributors/emails/ +# (see contributors/README.md), which merge-conflict-free by construction. +# This dict is kept only so existing history keeps resolving; the effective +# AUTHOR_MAP below merges it with the directory (directory wins). +LEGACY_AUTHOR_MAP = { "122438640+ragingbulld@users.noreply.github.com": "ragingbulld", # PR #65606 salvage (non-finite API wait deadlines; #65746) "zzpigpinggai@users.noreply.github.com": "zzpigpinggai", # PR #66017 salvage of #63617 (OpenRouter explicit-provider picker visibility) "sam7894604@gmail.com": "sam7894604", # PR #55803 salvage (discord: /reasoning slash choices) @@ -2045,6 +2049,41 @@ AUTHOR_MAP = { } +# ────────────────────────────────────────────────────────────────────── +# Directory-based mappings: contributors/emails/ → login +# ────────────────────────────────────────────────────────────────────── +CONTRIBUTORS_EMAILS_DIR = REPO_ROOT / "contributors" / "emails" + + +def _load_contributor_dir(directory: "Path | None" = None) -> dict: + """Load one-file-per-email mappings from contributors/emails/. + + Filename = commit-author email, first non-comment line = GitHub login. + Additions never merge-conflict (each mapping is a distinct file), which + is why new entries go here instead of the frozen LEGACY_AUTHOR_MAP. + """ + directory = directory or CONTRIBUTORS_EMAILS_DIR + mapping = {} + if not directory.is_dir(): + return mapping + for path in sorted(directory.iterdir()): + if not path.is_file() or path.name.startswith("."): + continue + try: + for line in path.read_text(encoding="utf-8").splitlines(): + line = line.strip() + if line and not line.startswith("#"): + mapping[path.name] = line.lstrip("@") + break + except OSError: + continue + return mapping + + +# Effective map: frozen legacy dict + directory entries (directory wins). +AUTHOR_MAP = {**LEGACY_AUTHOR_MAP, **_load_contributor_dir()} + + def git(*args, cwd=None): """Run a git command and return stdout.""" result = subprocess.run( diff --git a/tests/scripts/test_contributor_map.py b/tests/scripts/test_contributor_map.py new file mode 100644 index 00000000000..3109a5e3143 --- /dev/null +++ b/tests/scripts/test_contributor_map.py @@ -0,0 +1,137 @@ +"""Tests for the conflict-free contributor mapping system. + +New contributor email → GitHub login mappings live as one file per email +under contributors/emails/ (additions never merge-conflict). The legacy +AUTHOR_MAP dict in scripts/release.py is frozen; release.py merges both at +import time with the directory winning on duplicates. +""" + +import subprocess +import sys +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[2] +SCRIPTS_DIR = REPO_ROOT / "scripts" + +sys.path.insert(0, str(SCRIPTS_DIR)) + +import release # noqa: E402 +from add_contributor import add_contributor, read_mapping_file # noqa: E402 + + +# ── directory loader behavior ───────────────────────────────────────── + + +def test_loader_reads_login_from_first_noncomment_line(tmp_path): + d = tmp_path / "emails" + d.mkdir() + (d / "jane@example.com").write_text("# salvage PR #1\njanedoe\n# trailing note\n") + mapping = release._load_contributor_dir(d) + assert mapping == {"jane@example.com": "janedoe"} + + +def test_loader_strips_at_prefix_and_skips_dotfiles(tmp_path): + d = tmp_path / "emails" + d.mkdir() + (d / "a@b.com").write_text("@somelogin\n") + (d / ".gitkeep").write_text("_placeholder\n") + mapping = release._load_contributor_dir(d) + assert mapping == {"a@b.com": "somelogin"} + + +def test_loader_missing_directory_returns_empty(tmp_path): + assert release._load_contributor_dir(tmp_path / "nope") == {} + + +def test_effective_map_merges_legacy_and_directory(): + # Invariant: every legacy entry survives into the effective map unless + # shadowed by a directory entry, and the directory contributes on top. + assert set(release.LEGACY_AUTHOR_MAP) <= ( + set(release.AUTHOR_MAP) | set(release._load_contributor_dir()) + ) + for email, login in release._load_contributor_dir().items(): + assert release.AUTHOR_MAP[email] == login + + +def test_resolve_author_uses_directory_entry(tmp_path, monkeypatch): + d = tmp_path / "emails" + d.mkdir() + (d / "dirwin@example.com").write_text("dirwinner\n") + merged = {**release.LEGACY_AUTHOR_MAP, **release._load_contributor_dir(d)} + monkeypatch.setattr(release, "AUTHOR_MAP", merged) + assert release.resolve_author("Dir Winner", "dirwin@example.com") == "@dirwinner" + + +# ── add_contributor.py CLI behavior ─────────────────────────────────── + + +@pytest.fixture() +def emails_dir(tmp_path, monkeypatch): + import add_contributor + + d = tmp_path / "contributors" / "emails" + monkeypatch.setattr(add_contributor, "EMAILS_DIR", d) + return d + + +def test_add_creates_mapping_file(emails_dir): + rc = add_contributor("new@example.com", "newperson", "PR #999 salvage") + assert rc == 0 + path = emails_dir / "new@example.com" + assert path.is_file() + assert read_mapping_file(path) == "newperson" + assert "# PR #999 salvage" in path.read_text() + + +def test_add_is_idempotent(emails_dir): + assert add_contributor("x@y.com", "xperson") == 0 + assert add_contributor("x@y.com", "xperson") == 0 + assert read_mapping_file(emails_dir / "x@y.com") == "xperson" + + +def test_add_refuses_conflicting_login(emails_dir): + assert add_contributor("x@y.com", "xperson") == 0 + assert add_contributor("x@y.com", "someoneelse") == 1 + # original mapping untouched + assert read_mapping_file(emails_dir / "x@y.com") == "xperson" + + +def test_add_refuses_login_conflicting_with_legacy_map(emails_dir): + email, login = next(iter(release.LEGACY_AUTHOR_MAP.items())) + assert add_contributor(email, login + "x") == 1 + assert not (emails_dir / email).exists() + + +def test_add_rejects_invalid_email_and_login(emails_dir): + assert add_contributor("not-an-email", "ok") == 2 + assert add_contributor("has space@x.com", "ok") == 2 + assert add_contributor("a/b@x.com", "ok") == 2 # path separator + assert add_contributor("a@b.com", "-bad-") == 2 + assert not emails_dir.exists() or not any( + p for p in emails_dir.iterdir() if not p.name.startswith(".") + ) + + +def test_add_strips_at_prefix(emails_dir): + assert add_contributor("z@z.com", "@zeta") == 0 + assert read_mapping_file(emails_dir / "z@z.com") == "zeta" + + +def test_cli_entrypoint_end_to_end(tmp_path): + # Run the real script in a subprocess against a temp repo layout. + scripts = tmp_path / "scripts" + scripts.mkdir() + for name in ("add_contributor.py",): + (scripts / name).write_text((SCRIPTS_DIR / name).read_text()) + # Minimal stub release.py so the legacy lookup import works + (scripts / "release.py").write_text("LEGACY_AUTHOR_MAP = {}\n") + proc = subprocess.run( + [sys.executable, str(scripts / "add_contributor.py"), + "cli@example.com", "cliperson", "via subprocess"], + cwd=tmp_path, capture_output=True, text=True, + ) + assert proc.returncode == 0, proc.stderr + out = (tmp_path / "contributors" / "emails" / "cli@example.com").read_text() + assert out.splitlines()[0] == "cliperson"