Mechanical fixes from audit: quorum, push guards, sync, config, policy, collect, fleet

mimar/analyze/policy.py

@@ -100,7 +100,10 @@ def add(cls, status, detail, fix=None):
     ss = pol.get("secret_scanning")
     if ss and ss != "off":
         if private and ss == "public_only":
-            add("secret_scanning", "blocked_by_plan", "secret scanning on private repos needs a paid plan") if free else None
+            if free:
+                add("secret_scanning", "blocked_by_plan", "secret scanning on private repos needs a paid plan")
+            else:
+                add("secret_scanning", "unknown", "secret scanning status not collected for private repos")
         elif snap.get("fork"):
             add("secret_scanning", "skipped", "fork")
         else:

mimar/approve/bus.py

@@ -79,7 +79,7 @@ def sync_bus(ctx) -> dict:
     seats = set(cfg["bus"]["maintainer_seats"])
     top = since
     for e in envs:
-        top = max(top, int(e.get("seq", 0)))
+        top = max(top, int(e.get("seq") or 0))
     for e in cands:
         res["seen"] += 1
         for m in MSG_RE.finditer(e["body"]):

mimar/approve/core.py

@@ -60,7 +60,8 @@ def evaluate(ctx, p: dict, ignore_ttl: bool = False, at: dt.datetime | None = No
     approvers = {}
     for (src, actor), v in actions.items():
         if v["action"] == "approve" and (src == "divan_bus" or actor.lower() in maint):
-            approvers[actor] = v["event_ts"]
+            # one person, one vote: a bus seat and a GitHub login that differ only by case are the same approver
+            approvers[actor.lower()] = v["event_ts"]
     need = quorum_for(cfg, p["kind"])
     if len(approvers) < need:
         why = f"{len(approvers)}/{need} approvals"

mimar/approve/labels.py

@@ -107,9 +107,12 @@ def mirror_label(ctx, p: dict, action: str) -> None:
     n = p.get("issue_number")
     if not n or ctx.cfg.is_paused():
         return
-    ctx.writes.tracker("add_label", number=int(n), labels=[label])
-    events = ctx.gh.paginate(f"{tracker.repo_path(ctx)}/issues/{n}/timeline")
-    mine = [e for e in events if e.get("event") == "labeled" and (e.get("label") or {}).get("name") == label]
+    path = f"{tracker.repo_path(ctx)}/issues/{n}/timeline"
+    before = {str(e.get("id")) for e in ctx.gh.paginate(path)}   # a maintainer's own label event must never be
+    ctx.writes.tracker("add_label", number=int(n), labels=[label])  # mistaken for the one mimar just added
+    events = ctx.gh.paginate(path)
+    mine = [e for e in events if e.get("event") == "labeled" and (e.get("label") or {}).get("name") == label
+            and str(e.get("id")) not in before]
     if mine:
         last = max(mine, key=lambda e: e.get("created_at") or "")
         ctx.db.x("insert or ignore into mirrored_labels(proposal,label,event_id) values(?,?,?)", (p["id"], label, str(last.get("id"))))

mimar/collect/github.py

@@ -1,8 +1,12 @@
 """Batched GraphQL collection of org repo metadata."""
 from __future__ import annotations
 
+import logging
+
 from ..ghclient import GH
 
+log = logging.getLogger("mimar")
+
 QUERY = """
 query($org: String!, $after: String, $n: Int!, $page: Int!) {
   rateLimit { cost remaining resetAt }
@@ -27,7 +31,6 @@
         vulnerabilityAlerts(first: 1, states: OPEN) { totalCount }
         branchProtectionRules(first: 10) { nodes { pattern requiresApprovingReviews requiredApprovingReviewCount
           allowsForcePushes allowsDeletions isAdminEnforced } }
-        collaborators(first: 20) { nodes { login } }
         mimarYml: object(expression: "HEAD:.github/mimar.yml") { ... on Blob { text } }
         parent { nameWithOwner pushedAt }
       }
@@ -66,8 +69,6 @@ def normalise(node: dict) -> dict:
         "vuln_alerts": ((node.get("vulnerabilityAlerts") or {}).get("totalCount")),
         "vuln_enabled": node.get("hasVulnerabilityAlertsEnabled"),
         "protection": (node.get("branchProtectionRules") or {}).get("nodes") or [],
-        "collaborators": [c["login"] for c in ((node.get("collaborators") or {}).get("nodes") or [])]
-        if node.get("collaborators") else None,
         "override_text": (node.get("mimarYml") or {}).get("text"),
         "parent": node.get("parent"),
         "commits": _commits(node),
@@ -113,7 +114,8 @@ def fetch_security_extras(gh: GH, org: str, snap: dict) -> dict | None:
     """Secret scanning status is not in GraphQL; one REST read per public non-fork repo."""
     try:
         data = gh.rest("GET", f"/repos/{org}/{snap['name']}")
-    except Exception:
+    except Exception as e:
+        log.warning("security extras for %s unreadable: %s", snap["name"], e)
         return None
     sa = (data or {}).get("security_and_analysis")
     if sa is None:

mimar/config.py

@@ -71,7 +71,13 @@
             "backend": {"enum": ["claude_cli", "anthropic_api", "none"]},
             "max_calls_per_run": {"type": "integer", "minimum": 0}}},
         "budget": {"type": "object", "properties": {"graphql_points_per_run": {"type": "integer", "minimum": 1}}},
-        "policy": {"type": "object"},
+        "policy": {"type": "object", "properties": {
+            "default_branch": {"type": "string"},
+            # a typo here used to crash mid-run in lowers_protection; fail fast at load instead
+            "branch_protection": {"type": "object", "properties": {
+                "require_pr": {"type": "boolean"}, "required_approvals": {"type": "integer", "minimum": 0},
+                "allow_force_pushes": {"type": "boolean"}, "allow_deletions": {"type": "boolean"},
+                "enforce_admins": {"type": "boolean"}}}}},
         "auto_apply": {"type": "array", "items": {"enum": ["branch_protection", "dependabot_alerts", "secret_scanning",
                                                            "health_fix"]}},
         "policy_actions": {"type": "object", "additionalProperties": {"enum": ["log", "issue", "fix"]},

mimar/execute/base.py

@@ -194,11 +194,14 @@ def safe_push(self, argv_tail: list[str], cwd, default_branch: str | None, allow
         brand new empty repo."""
         full = ["push", *argv_tail]
         refs = [a for a in argv_tail if not a.startswith("-")]
-        dest = ""
-        for r in refs[1:]:
-            dest = r.split(":")[-1].replace("refs/heads/", "")
+        dests = [r.split(":")[-1].replace("refs/heads/", "") for r in refs[1:]]
+        for dest in dests:
             if default_branch and dest == default_branch and not allow_default:
                 raise SafetyError(f"refusing to push to default branch {default_branch}")
+            if not dest.startswith("mimar/") and not allow_default:
+                # the broker only sees the last refspec, so every destination is checked here
+                raise SafetyError(f"refusing to push to non-mimar branch {dest}")
+        dest = dests[-1] if dests else ""
         req = WriteRequest("push_branch", repo or "unknown", {"branch": dest, "base": default_branch or "main",
                                                                **({"initial": True} if allow_default else {})})
         self.ctx.writes.submit(req, lambda: self.g(full, cwd=cwd))

mimar/fleet.py

@@ -205,11 +205,13 @@ def preview(ctx, fixer: Autofixer, repo_glob: str | None = None) -> dict[str, st
     repos = fleet_repos(ctx, repo_glob)
     root = ctx.work_dir("preview") / fixer.name
     out = {}
-    for name in fixer.find_repos(ctx, repos):
-        d = diff_of(ctx, fixer, name, repos[name], root / name)
-        if d.strip():
-            out[name] = d
-    shutil.rmtree(root, ignore_errors=True)
+    try:
+        for name in fixer.find_repos(ctx, repos):
+            d = diff_of(ctx, fixer, name, repos[name], root / name)
+            if d.strip():
+                out[name] = d
+    finally:
+        shutil.rmtree(root, ignore_errors=True)
     return out
 
 

tests/test_analyze.py

@@ -63,6 +63,16 @@ def test_policy_drift_and_plan_limits(collected):
     assert pol[("alpha-core", "required_file:README.md")] == "ok"
 
 
+def test_policy_secret_scanning_private_repo_on_paid_plan_is_unknown(collected):
+    snap = {"name": "r", "private": True, "fork": False, "archived": False, "default_branch": "main",
+            "protection": [], "vuln_enabled": True}
+    paid = dict(collected.cfg)
+    paid["plan"] = "team"
+    fs = policy.check(snap, {"README.md"}, paid, None)
+    ss = [f for f in fs if f["class"] == "secret_scanning"]
+    assert len(ss) == 1 and ss[0]["status"] == "unknown"
+
+
 def test_policy_per_repo_override_changes_requirements(collected):
     snap = {"name": "r", "private": False, "fork": False, "archived": False, "default_branch": "main", "protection": [],
             "vuln_enabled": True, "secret_scanning": "enabled"}

tests/test_approve.py

@@ -176,3 +176,37 @@ def test_bus_down_is_reported_not_fatal(tmp_path, home, world):
     setup(ctx)
     r = sync_approvals(ctx)
     assert r["errors"] and "bus" in r["errors"][0]
+
+
+def test_quorum_counts_case_variants_as_one_approver(tmp_path, home, world):
+    ctx = make_ctx(tmp_path, world, {"quorum": 2, "maintainers": ["mozi"],
+                                     "approval_sources": ["github_label", "divan_bus"],
+                                     "bus": {"maintainer_seats": ["Mozi"]}})
+    p = setup(ctx)
+    ctx.db.set_state(p["id"], hash_log=[["2026-01-01T00:00:00Z", p["plan_hash"]]])
+    core.record(ctx.db, p["id"], "divan_bus", "Mozi", "approve", FUTURE, "e1", True)
+    core.record(ctx.db, p["id"], "github_label", "mozi", "approve", FUTURE, "e2", True)
+    ev = core.evaluate(ctx, proposals.get(ctx.db, p["id"]))
+    assert ev["state"] == "awaiting" and ev["approvers"] == ["mozi"]
+
+
+def test_bus_envelope_without_seq_does_not_break_sync(tmp_path, home, world):
+    bus = FakeBus()
+    ctx = bus_ctx(tmp_path, world, bus)
+    p = setup(ctx)
+    m = msg(1, "saga-divan", f"mimar approve {p['id']}")
+    m["seq"] = None
+    bus.inbox = [m]
+    r = sync_approvals(ctx)
+    assert status(ctx, p) == "approved" and not r["errors"]
+
+
+def test_mirror_label_does_not_swallow_maintainer_event(tmp_path, home, world):
+    bus = FakeBus()
+    ctx = bus_ctx(tmp_path, world, bus, approval_sources=["github_label", "divan_bus"], quorum=2,
+                  maintainers=["maint"])
+    p = setup(ctx)
+    ctx.gh.add_label_event(p["issue_number"], "mimar:approved", "maint", ts="2099-01-02T00:00:00Z")
+    bus.inbox = [msg(1, "saga-divan", f"mimar approve {p['id']}")]
+    sync_approvals(ctx)
+    assert status(ctx, p) == "approved"

tests/test_collect.py

@@ -30,9 +30,25 @@ def test_budget_stops_cleanly(tmp_path, home, world):
 
 
 def test_normalise_handles_missing_optional_fields():
-    n = github.normalise({"name": "x", "isPrivate": True, "isFork": False, "isArchived": False, "collaborators": None,
+    n = github.normalise({"name": "x", "isPrivate": True, "isFork": False, "isArchived": False,
                           "defaultBranchRef": None})
-    assert n["collaborators"] is None and n["commits"] == [] and n["default_branch"] is None
+    assert "collaborators" not in n and n["commits"] == [] and n["default_branch"] is None
+
+
+def test_collect_query_does_not_request_unused_collaborators():
+    assert "collaborators" not in github.QUERY
+
+
+def test_fetch_security_extras_logs_api_errors(caplog):
+    import logging
+
+    class BoomGH:
+        def rest(self, *a, **k):
+            raise RuntimeError("403 Forbidden")
+
+    with caplog.at_level(logging.WARNING, logger="mimar"):
+        assert github.fetch_security_extras(BoomGH(), "acme", {"name": "x"}) is None
+    assert "security extras for x unreadable" in caplog.text
 
 
 def test_override_file_parsed(tmp_path, home, world):

tests/test_execute.py

@@ -119,6 +119,13 @@ def test_safe_push_refuses_default_branch(ctx, tmp_path):
         ex.safe_push(["origin", "mimar/x:refs/heads/main"], tmp_path, "main")
 
 
+def test_safe_push_checks_every_refspec_not_just_the_last(ctx, tmp_path, monkeypatch):
+    ex = executor_for(ctx, "merge")
+    monkeypatch.setattr(ex, "g", lambda *a, **k: (_ for _ in ()).throw(AssertionError("git must not run")))
+    with pytest.raises(SafetyError, match="non-mimar"):
+        ex.safe_push(["origin", "docs:refs/heads/docs", "mimar/x:refs/heads/mimar/x"], tmp_path, "main")
+
+
 # ---- merge: dry run + end to end ---------------------------------------------------------
 
 def test_merge_dry_run_shows_exact_commands(ctx):

tests/test_fleet_planconfig.py

@@ -240,6 +240,26 @@ def test_readme_skeleton_finds_repos_without_readme(collected, world, tmp_path):
     assert "+# mono" in d["mono"] and "TODO: describe" in d["mono"]
 
 
+def test_preview_cleans_workdir_when_fixer_raises(collected):
+    class Boom(Autofixer):
+        name = "boom-test"
+        description = "x"
+        commit_message = "t"
+        pr_title = "t"
+        pr_body = "t"
+
+        def find_repos(self, ctx, repos):
+            return ["old-tool"]
+
+        def apply(self, workdir, repo, snap):
+            raise RuntimeError("boom")
+
+    root = collected.work_dir("preview") / "boom-test"
+    with pytest.raises(RuntimeError, match="boom"):
+        preview(collected, Boom())
+    assert not root.exists()
+
+
 # ---- feature 9: plan-config ---------------------------------------------------------------------
 
 def analysed(ctx):

tests/test_misc.py

@@ -34,6 +34,10 @@ def test_invalid_config_rejected():
         load_config(None, {"org": "o", "auto_apply": ["delete_repos"]})
     with pytest.raises(ConfigError):
         load_config(None, {"org": "o", "notifiers": [{"type": "pager"}]})
+    with pytest.raises(ConfigError):   # a typo here used to crash mid-run in lowers_protection
+        load_config(None, {"org": "o", "policy": {"branch_protection": {"required_approvals": "many"}}})
+    with pytest.raises(ConfigError):
+        load_config(None, {"org": "o", "policy": {"branch_protection": {"require_pr": "yes"}}})
     with pytest.raises(ConfigError):
         load_config(None, {"org": None}).org