mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-10-20 05:15:31 +00:00
feat(attribution): conflict-free contributor mappings via contributors/emails/ directory
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 <login>@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.
This commit is contained in:
parent
07f07c7b51
commit
06adcfabf9
7 changed files with 333 additions and 13 deletions
19
.github/workflows/contributor-check.yml
vendored
19
.github/workflows/contributor-check.yml
vendored
|
|
@ -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}\": \"<github-username>\","
|
||||
echo " python3 scripts/add_contributor.py ${email} <github-username>"
|
||||
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
|
||||
|
|
|
|||
38
contributors/README.md
Normal file
38
contributors/README.md
Normal file
|
|
@ -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 <email> <github-login>
|
||||
# or by hand:
|
||||
echo "<github-login>" > contributors/emails/<email>
|
||||
```
|
||||
|
||||
- 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 (`<id>+<login>@users.noreply.github.com` and
|
||||
`<login>@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.
|
||||
2
contributors/emails/.gitkeep
Normal file
2
contributors/emails/.gitkeep
Normal file
|
|
@ -0,0 +1,2 @@
|
|||
_placeholder
|
||||
# keeps the directory present in git; not a real mapping
|
||||
99
scripts/add_contributor.py
Normal file
99
scripts/add_contributor.py
Normal file
|
|
@ -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 <email> <github-login> [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())
|
||||
|
|
@ -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} <github-username> # {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}": "<github-username>",')
|
||||
print(f" python3 scripts/add_contributor.py {email} <github-username> # {name}")
|
||||
print()
|
||||
print("To find the GitHub username:")
|
||||
print(" gh api 'search/users?q=EMAIL+in:email' --jq '.items[0].login'")
|
||||
|
|
|
|||
|
|
@ -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/<email> → 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(
|
||||
|
|
|
|||
137
tests/scripts/test_contributor_map.py
Normal file
137
tests/scripts/test_contributor_map.py
Normal file
|
|
@ -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"
|
||||
Loading…
Add table
Add a link
Reference in a new issue