feat(user-sync): exclude system accounts and complete idempotent sync

Author: raederan

src/osw/tools/user_sync/config.py

@@ -13,8 +13,9 @@
 # Public ORCID API used to enrich ORCID users.
 ORCID_API_BASE_DEFAULT = "https://pub.orcid.org/v3.0"
 
-# MediaWiki group whose members are excluded from the sync (bots).
-BOT_GROUP = "bot"
+# MediaWiki groups whose members are treated as bot/system accounts and skipped.
+# All are standard MediaWiki groups, so this is not a per-instance skip-list.
+SYSTEM_GROUPS = ["bot", "sysop", "bureaucrat", "interface-admin"]
 
 
 @dataclass
@@ -28,10 +29,11 @@ class SyncConfig:
     limit: Optional[int] = None
     include_non_orcid: bool = True
     exclude_bot_group: bool = True
+    exclude_system_usernames: bool = True
     create_redirects: bool = True
     link_organizations: bool = True
     orcid_api_base: str = ORCID_API_BASE_DEFAULT
-    excluded_groups: List[str] = field(default_factory=lambda: [BOT_GROUP])
+    excluded_groups: List[str] = field(default_factory=lambda: list(SYSTEM_GROUPS))
 
 
 def build_arg_parser() -> argparse.ArgumentParser:
@@ -86,6 +88,12 @@ def build_arg_parser() -> argparse.ArgumentParser:
         action="store_false",
         help="Sync only accounts whose username is an ORCID iD.",
     )
+    parser.add_argument(
+        "--include-system",
+        dest="exclude_system_usernames",
+        action="store_false",
+        help="Do not skip MediaWiki reserved system accounts (Maintenance script etc.).",
+    )
     return parser
 
 
@@ -99,6 +107,7 @@ def config_from_args(argv: Optional[List[str]] = None) -> SyncConfig:
         assume_yes=args.assume_yes,
         limit=args.limit,
         include_non_orcid=args.include_non_orcid,
+        exclude_system_usernames=args.exclude_system_usernames,
         create_redirects=args.create_redirects,
         link_organizations=args.link_organizations,
     )

src/osw/tools/user_sync/reconcile.py

@@ -58,6 +58,24 @@ def _existing_label(entity: Any) -> Optional[str]:
     return None
 
 
+def _existing_orgs(entity: Any) -> Any:
+    """Read organization page-title refs without resolving the relation.
+
+    Loaded entities keep relation targets as IRIs in ``__iris__``; reading the
+    attribute itself would make oold resolve (and fail on) the linked items.
+    """
+    iris = getattr(entity, "__iris__", None)
+    if isinstance(iris, dict):
+        value = iris.get("organization")
+        if value is None:
+            return None
+        return value if isinstance(value, (list, tuple, set)) else [value]
+    try:
+        return getattr(entity, "organization", None)
+    except Exception:  # pragma: no cover - defensive
+        return None
+
+
 def existing_fields(entity: Any) -> Dict[str, Any]:
     """Normalized comparable field view of a loaded User entity."""
     return {
@@ -67,9 +85,7 @@ def existing_fields(entity: Any) -> Dict[str, Any]:
         "orcid": _norm_scalar("orcid", getattr(entity, "orcid", None)),
         "emails": _norm_set("emails", getattr(entity, "email", None)),
         "websites": _norm_set("websites", getattr(entity, "website", None)),
-        "organizations": _norm_set(
-            "organizations", getattr(entity, "organization", None)
-        ),
+        "organizations": _norm_set("organizations", _existing_orgs(entity)),
     }
 
 

src/osw/tools/user_sync/redirects.py

@@ -22,14 +22,18 @@ def is_redirect(content: str) -> bool:
 def ensure_redirect(osw: Any, username: str, item_title: str) -> Optional[str]:
     """Create or refresh ``User:<username>`` -> item redirect.
 
-    Returns the page title if written, or None if skipped (real page exists).
+    Returns the page title if written, or None if skipped (a real page already
+    exists, or the redirect is already correct).
     """
     title = f"User:{username}"
+    target = redirect_wikitext(item_title)
     page = WtPage(wtSite=osw.site, title=title)
     if getattr(page, "exists", False):
-        content = page.get_content() or ""
+        content = page.get_slot_content("main") or ""
         if content.strip() and not is_redirect(content):
-            return None
-    page.set_content(redirect_wikitext(item_title))
+            return None  # do not clobber a real user page
+        if content.strip() == target:
+            return None  # already correct
+    page.set_slot_content("main", target)
     page.edit(comment="user-sync: redirect to user item")
     return title

src/osw/tools/user_sync/sources.py

@@ -20,6 +20,24 @@
 
 _ALLUSERS_PROPS = "registration|editcount|groups|centralids|blockinfo"
 
+# MediaWiki default reserved system usernames. These ship with MediaWiki and are
+# the same across instances, so skipping them is not a per-instance skip-list.
+RESERVED_USERNAMES = frozenset({
+    "MediaWiki default",
+    "Maintenance script",
+    "Conversion script",
+    "Template namespace initialisation script",
+    "ScriptImporter",
+    "Delete page script",
+    "Move page script",
+    "Command line script",
+    "Unknown user",
+    "MediaWiki message delivery",
+    "Flow talk page manager",
+    "Abuse filter",
+    "New user message",
+})
+
 
 def is_orcid_username(name: str) -> bool:
     """True if a MediaWiki username is an ORCID iD (whitelisted ORCID login)."""
@@ -77,6 +95,7 @@ def _iter_allusers(site: Any, batch: Any = "max") -> Iterator[Dict[str, Any]]:
 def enumerate_mw_users(
     site: Any,
     excluded_groups: Sequence[str] = ("bot",),
+    excluded_usernames: Sequence[str] = (),
     include_non_orcid: bool = True,
     limit: Optional[int] = None,
     batch: Any = "max",
@@ -85,15 +104,19 @@ def enumerate_mw_users(
 
     Args:
         site: An object exposing ``api("query", ...)`` (an mwclient Site).
-        excluded_groups: Accounts in any of these groups are skipped (bots).
+        excluded_groups: Accounts in any of these groups are skipped (bots, admins).
+        excluded_usernames: Exact usernames to skip (reserved system accounts).
         include_non_orcid: If False, keep only ORCID-username accounts.
         limit: Keep at most this many accounts after filtering.
         batch: ``aulimit`` value passed to the API.
     """
     excluded = set(excluded_groups)
+    excluded_names = set(excluded_usernames)
     users: List[MwUser] = []
     for raw in _iter_allusers(site, batch=batch):
         user = MwUser.from_api(raw)
+        if user.name in excluded_names:
+            continue
         if excluded.intersection(user.groups):
             continue
         if not include_non_orcid and not user.is_orcid:

src/osw/tools/user_sync/sync.py

@@ -21,6 +21,7 @@
 from .reconcile import reconcile
 from .redirects import ensure_redirect
 from .sources import (
+    RESERVED_USERNAMES,
     OrcidRateLimitError,
     enumerate_mw_users,
     fetch_orcid_record,
@@ -51,9 +52,11 @@ def summary(self) -> str:
 def _build_proposals(config: SyncConfig, osw: Any, session: Any):
     """Enumerate MediaWiki users, enrich ORCID users and map to proposals."""
     excluded = tuple(config.excluded_groups) if config.exclude_bot_group else ()
+    reserved = RESERVED_USERNAMES if config.exclude_system_usernames else ()
     mw_users = enumerate_mw_users(
         osw.mw_site,
         excluded_groups=excluded,
+        excluded_usernames=reserved,
         include_non_orcid=config.include_non_orcid,
         limit=config.limit,
     )
@@ -98,28 +101,36 @@ def _store_users(osw: Any, resolution: Resolution, report: SyncReport) -> None:
     entities = []
     for resolved in resolution.resolved:
         proposed = resolved.change.proposed
-        if resolved.action == "create":
-            entities.append(build_user(proposed))
-            report.created.append(proposed.full_page_title)
-        elif resolved.action == "update":
-            entities.append(
-                apply_update(resolved.change.existing, proposed, resolved.apply_fields)
-            )
-            report.updated.append(proposed.full_page_title)
-        else:
-            report.skipped.append(proposed.full_page_title)
+        try:
+            if resolved.action == "create":
+                entities.append(build_user(proposed))
+                report.created.append(proposed.full_page_title)
+            elif resolved.action == "update":
+                entities.append(
+                    apply_update(
+                        resolved.change.existing, proposed, resolved.apply_fields
+                    )
+                )
+                report.updated.append(proposed.full_page_title)
+            else:
+                report.skipped.append(proposed.full_page_title)
+        except Exception as exc:  # pragma: no cover - build failure path
+            report.failed[f"build:{proposed.username}"] = str(exc)
     if entities:
-        osw.store_entity(
-            OSW.StoreEntityParam(
-                entities=entities, overwrite=True, edit_comment="user-sync"
+        try:
+            osw.store_entity(
+                OSW.StoreEntityParam(
+                    entities=entities, overwrite=True, edit_comment="user-sync"
+                )
             )
-        )
+        except Exception as exc:  # pragma: no cover - network failure path
+            report.failed["store"] = str(exc)
 
 
 def _create_redirects(osw: Any, resolution: Resolution, report: SyncReport) -> None:
+    # Every in-scope user has an item (created or pre-existing), so ensure the
+    # redirect for all of them; a re-run repairs any missing redirect.
     for resolved in resolution.resolved:
-        if resolved.action not in ("create", "update"):
-            continue
         proposed = resolved.change.proposed
         try:
             written = ensure_redirect(osw, proposed.username, proposed.full_page_title)

tests/tools/test_user_sync_config.py

@@ -15,11 +15,18 @@ def test_defaults():
     assert cfg.include_non_orcid is True
     assert cfg.create_redirects is True
     assert cfg.link_organizations is True
-    assert cfg.excluded_groups == ["bot"]
+    assert cfg.exclude_system_usernames is True
+    assert "bot" in cfg.excluded_groups
+    assert "sysop" in cfg.excluded_groups
     assert USER_CATEGORY.startswith("Category:OSW")
     assert ORGANIZATION_CATEGORY.startswith("Category:OSW")
 
 
+def test_include_system_flag():
+    cfg = config_from_args(["--include-system"])
+    assert cfg.exclude_system_usernames is False
+
+
 def test_config_from_args_parses_flags():
     cfg = config_from_args([
         "--domain",

tests/tools/test_user_sync_reconcile.py

@@ -119,6 +119,29 @@ def test_missing_label_is_gap_fill():
     assert change.diffs[0].name == "label"
 
 
+class _OrgTrap:
+    """A loaded-entity stand-in whose organization attribute must never be read."""
+
+    __iris__ = {"organization": ["Item:OSWa", "Item:OSWb"]}
+    label = [SimpleNamespace(text="X Y")]
+    first_name = "X"
+    surname = "Y"
+    orcid = None
+    email: set = set()
+    website: set = set()
+
+    @property
+    def organization(self):
+        raise RuntimeError("relation must not be resolved during reconcile")
+
+
+def test_existing_orgs_read_from_iris_without_resolving():
+    from osw.tools.user_sync.reconcile import existing_fields
+
+    ef = existing_fields(_OrgTrap())
+    assert ef["organizations"] == {"Item:OSWa", "Item:OSWb"}
+
+
 def test_reconcile_plan_counts():
     proposed = [
         _proposed(username="new-user"),

tests/tools/test_user_sync_redirects.py

@@ -29,10 +29,10 @@ def __init__(self, wtSite=None, title=None, do_init=True):
         self.written = None
         self.edited = False
 
-    def get_content(self):
+    def get_slot_content(self, slot_key):
         return self._content
 
-    def set_content(self, content):
+    def set_slot_content(self, slot_key, content):
         self.written = content
 
     def edit(self, comment=None):
@@ -67,3 +67,10 @@ def test_ensure_redirect_skips_real_page(monkeypatch):
     _patch(monkeypatch, {title: True}, {title: "This is my real user page."})
     written = ensure_redirect(FakeOsw(), "bob", "Item:OSW1")
     assert written is None
+
+
+def test_ensure_redirect_skips_when_already_correct(monkeypatch):
+    title = "User:carol"
+    _patch(monkeypatch, {title: True}, {title: "#REDIRECT [[Item:OSW1]]"})
+    written = ensure_redirect(FakeOsw(), "carol", "Item:OSW1")
+    assert written is None

tests/tools/test_user_sync_sources.py

@@ -94,6 +94,22 @@ def test_enumerate_orcid_only_and_limit():
     assert len(limited) == 1
 
 
+def test_enumerate_excludes_privileged_groups_and_reserved_names():
+    site = FakeSite([
+        _page([
+            {"name": "0000-0002-6374-9831", "groups": ["user"]},
+            {"name": "Admin", "groups": ["user", "sysop", "bureaucrat"]},
+            {"name": "Maintenance script", "groups": ["user"]},
+        ])
+    ])
+    users = enumerate_mw_users(
+        site,
+        excluded_groups=("bot", "sysop", "bureaucrat"),
+        excluded_usernames={"Maintenance script"},
+    )
+    assert [u.name for u in users] == ["0000-0002-6374-9831"]
+
+
 def test_partition_by_orcid():
     users = [
         MwUser(name="0000-0002-6374-9831"),