nisse-review: repository patterns, and the wall checked every round

At bring-up, nisse-review read no repositories: Gitea's /user/repos leaves
out a public repository's Read collaborators, so dev/nisse never showed up.
`repositories` now holds patterns (dev/nisse, dev/*, */*) matched against
every repository the account can read, public ones included (an
organization's or user's list, or the instance's search); unset, it is
/user/repos as before. A configured repository it can't read is left out
with an oplog line.

The read-only wall (R3) is checked every round, not only at startup: an
account granted push or admin while running stops (exit 3) rather than
review on.

nisse-core's Gitea client gains GiteaNotFound for 404s, owner_repositories
and visible_repositories.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J5Utyfzw7gF7UhjRDEtjTk

nisse-review/src/nisse_review/access.py

@@ -0,0 +1,79 @@
+"""What nisse-review reviews, and the wall around it (R1, R3).
+
+The repositories are worked out again every round: ``repositories`` in the
+configuration is a list of patterns ("dev/nisse", "dev/*", "*/*"), matched
+against what the account can read, public repositories included; unset, it
+is every repository the account was given (``/user/repos``). Access is
+managed in Gitea for private repositories and in the configuration for
+public ones.
+
+The wall is checked as often: an account that can push to, or administer,
+any repository it reviews is refused, at startup and in every round, so a
+grant made while it runs stops it.
+"""
+
+from collections.abc import Iterable
+from fnmatch import fnmatchcase
+
+from nisse_core.gitea import GiteaClient, GiteaNotFound
+from nisse_core.oplog import OpLog
+
+_GLOB = frozenset("*?[")
+
+
+class AccessRefused(RuntimeError):
+    pass
+
+
+def _glob(text: str) -> bool:
+    return any(c in _GLOB for c in text)
+
+
+def reviewed(
+    gitea: GiteaClient, patterns: Iterable[str] | None, oplog: OpLog
+) -> list[str]:
+    """The repositories to review, sorted."""
+    if patterns is None:
+        return sorted(gitea.readable_repositories())
+    found: set[str] = set()
+    for pattern in patterns:
+        owner, _, name = pattern.partition("/")
+        if not _glob(owner) and not _glob(name):
+            found.add(pattern)  # checked by ``check``: it must be readable
+            continue
+        try:
+            candidates = (
+                gitea.visible_repositories()
+                if _glob(owner)
+                else gitea.owner_repositories(owner)
+            )
+        except GiteaNotFound:
+            oplog.emit("access.no_owner", cause="no such owner", pattern=pattern)
+            continue
+        found |= {repo for repo in candidates if fnmatchcase(repo, pattern)}
+    return sorted(found)
+
+
+def check(gitea: GiteaClient, repos: list[str], oplog: OpLog) -> list[str]:
+    """The repositories it can read, after the wall: refused outright if it
+    could write to any of them. A configured one it can't read is left out."""
+    readable: list[str] = []
+    too_much: list[str] = []
+    for repo in repos:
+        try:
+            permissions = gitea.repository(repo).get("permissions") or {}
+        except GiteaNotFound:
+            oplog.emit(
+                "access.unreadable", cause="not found or not readable", repo=repo
+            )
+            continue
+        if permissions.get("push") or permissions.get("admin"):
+            too_much.append(repo)
+        elif permissions.get("pull", True):
+            readable.append(repo)
+    if too_much:
+        raise AccessRefused(
+            f"{gitea.whoami()} can write to {', '.join(too_much)}: nisse-review's "
+            "account must have read access only (Part R, R3)"
+        )
+    return readable

nisse-review/src/nisse_review/build.py

@@ -15,6 +15,7 @@ from nisse_core.models.context import BudgetedClient
 from nisse_core.models.llm import LiteLLMClient
 from nisse_core.oplog import OpLog
 from nisse_core.state.cancel import Cancel
+from nisse_review.access import AccessRefused, check, reviewed
 from nisse_review.config import ReviewConfig
 from nisse_review.review import Reviewer
 from nisse_review.scheduler import Scheduler
@@ -41,23 +42,18 @@ class Built:
         self.store.close()
 
 
-def check_access(gitea: GiteaClient) -> tuple[str, list[str]]:
-    """Who it is and what it reads (R3): read only. An account that can push
-    to, or administer, a repository it reviews is refused: its hard wall is
-    that it can't change what it reviews."""
+def check_access(
+    gitea: GiteaClient, patterns: frozenset[str] | None, oplog: OpLog
+) -> tuple[str, list[str]]:
+    """Who it is and what it reviews (R1, R3): read only. An account that
+    can push to, or administer, a repository it reviews is refused: its hard
+    wall is that it can't change what it reviews. The scheduler checks again
+    every round (``access``)."""
     account = gitea.whoami()
-    repos = gitea.readable_repositories()
-    too_much = [
-        repo
-        for repo in repos
-        if (perms := gitea.repository(repo).get("permissions") or {}).get("push")
-        or perms.get("admin")
-    ]
-    if too_much:
-        raise StartupError(
-            f"{account} can write to {', '.join(too_much)}: nisse-review's "
-            "account must have read access only (Part R, R3)"
-        )
+    try:
+        repos = check(gitea, reviewed(gitea, patterns, oplog), oplog)
+    except AccessRefused as exc:
+        raise StartupError(str(exc)) from exc
     return account, repos
 
 
@@ -100,7 +96,7 @@ def build(
 ) -> Built:
     oplog = oplog or OpLog(AGENT)
     gitea = gitea or GiteaClient(config.gitea_url, config.gitea_token)
-    account, repos = check_access(gitea)
+    account, repos = check_access(gitea, config.repositories, oplog)
     client = client if client is not None else llm_client(config)
     config.state_db.parent.mkdir(parents=True, exist_ok=True)
     store = StateStore(config.state_db, oplog=oplog)
@@ -146,6 +142,6 @@ def build(
         scratch=Path(scratch),
         poll_s=tuning.review.poll_s,
         attempts=tuning.review.attempts,
-        only=config.repositories,
+        patterns=config.repositories,
     )
     return Built(scheduler, account, repos, store, gitea)

nisse-review/src/nisse_review/config.py

@@ -43,8 +43,8 @@ REQUIRED_KEYS = (
 OPTIONAL_KEYS = (
     "llm.max_output_tokens",
     "llm.request_timeout_s",
-    # Only these of the repositories it can read; unset, every one (R1).
-    # For tests and a first bring-up.
+    # Patterns of the repositories it reviews ("dev/nisse", "dev/*", "*/*"),
+    # public ones included; unset, every one it was given (R1, ``access``).
     "repositories",
 )
 
@@ -132,6 +132,11 @@ class ReviewConfig:
         if missing:
             errors.append("unset or empty — " + ", ".join(missing))
         max_output, timeout = llm_numbers(values, errors)
+        patterns = people(values["repositories"]) if "repositories" in values else None
+        for pattern in sorted(patterns or ()):
+            owner, slash, name = pattern.partition("/")
+            if not (owner and slash and name) or "/" in name:
+                errors.append(f"repositories: {pattern!r} is not owner/name")
         if errors:
             raise ConfigError("cannot start: " + "; ".join(errors))
         reviewer = str(values["roles.reviewer"])
@@ -149,7 +154,5 @@ class ReviewConfig:
             request_timeout_s=timeout,
             tuning=tuning,
             vault=vault,
-            repositories=(
-                people(values["repositories"]) if "repositories" in values else None
-            ),
+            repositories=patterns,
         )

nisse-review/src/nisse_review/main.py

@@ -9,6 +9,7 @@ from types import FrameType
 
 from nisse_core.config import ConfigError
 from nisse_core.envfile import load_env_file
+from nisse_review.access import AccessRefused
 from nisse_review.build import StartupError, build
 from nisse_review.config import ReviewConfig
 
@@ -35,6 +36,10 @@ def main() -> int:
     )
     try:
         built.scheduler.run_forever()
+    except AccessRefused as exc:
+        # R3: a grant made while it ran. It stops rather than review on.
+        print(f"nisse-review: stopping: {exc}", file=sys.stderr)
+        return 3
     finally:
         built.close()
     return 0

nisse-review/src/nisse_review/scheduler.py

@@ -1,7 +1,7 @@
 """nisse-review's scheduler (R1): its one duty, and its items one at a time.
 
-Every round, it asks Gitea for the open PRs in every repository its account
-can read. A PR that is ready (not a draft, not marked work in progress) gets
+Every round, it works out the repositories it reviews and checks it still
+can't write to any (``access``), then asks Gitea for their open PRs. A PR that is ready (not a draft, not marked work in progress) gets
 a work item; the oldest ready item is reviewed, and its review posted. An
 item closes when its PR merges or closes. Later reviews, replies and rounds
 come with M7.5.
@@ -19,6 +19,7 @@ from pathlib import Path
 from nisse_core.gitea import GiteaClient, GiteaError, PullRequest
 from nisse_core.oplog import OpLog
 from nisse_core.state.store import WorkItem
+from nisse_review.access import check, reviewed
 from nisse_review.reader import Repository
 from nisse_review.review import Reviewer
 from nisse_review.state import (
@@ -54,7 +55,7 @@ class Scheduler:
         scratch: Path,
         poll_s: float = 60.0,
         attempts: int = 2,
-        only: frozenset[str] | None = None,
+        patterns: frozenset[str] | None = None,
     ) -> None:
         self._store = store
         self._gitea = gitea
@@ -63,7 +64,8 @@ class Scheduler:
         self._scratch = scratch
         self._poll_s = poll_s
         self._attempts = attempts
-        self._only = only  # the repositories reviewed; None: every readable one
+        # The repositories reviewed, as patterns; None: every one it was given.
+        self._patterns = patterns
         self._stop = threading.Event()
 
     # -- the loop ---------------------------------------------------------------
@@ -77,10 +79,17 @@ class Scheduler:
         self._stop.set()
 
     def round(self) -> bool:
-        """Intake, then one review if one is ready; True if one ran."""
+        """The wall, intake, then one review if one is ready; True if one
+        ran. An account that can write to a repository it reviews raises
+        ``AccessRefused``: it stops, never reviews on (R3)."""
         try:
+            repos = check(
+                self._gitea,
+                reviewed(self._gitea, self._patterns, self._oplog),
+                self._oplog,
+            )
             self.recover()
-            self.intake()
+            self.intake(repos)
         except GiteaError as exc:
             self._oplog.emit("intake.failed", cause=str(exc)[:300])
             return False
@@ -92,11 +101,11 @@ class Scheduler:
 
     # -- intake (R1) ------------------------------------------------------------
 
-    def intake(self) -> None:
+    def intake(self, repos: list[str] | None = None) -> None:
+        if repos is None:
+            repos = reviewed(self._gitea, self._patterns, self._oplog)
         open_items = {i.id: i for i in self._store.work_items("open")}
-        for repo in self._gitea.readable_repositories():
-            if self._only is not None and repo not in self._only:
-                continue
+        for repo in repos:
             seen: set[str] = set()
             for pr in self._gitea.open_pull_requests(repo):
                 key = item_id(repo, pr.number)

nisse-review/tests/review_fakes.py

@@ -8,6 +8,7 @@ from typing import Any
 from nisse_core.gitea import (
     Comment,
     GiteaError,
+    GiteaNotFound,
     Issue,
     LineComment,
     PullRequest,
@@ -69,6 +70,9 @@ class FakeGitea:
     comments: list[tuple[str, int, str]] = field(default_factory=list)
     refuse_off_diff: bool = False
     down: bool = False
+    # Readable but not given (public): missing from /user/repos, as Gitea
+    # leaves a public repository's Read collaborators out of it.
+    public: set[str] = field(default_factory=set)
 
     def _up(self) -> None:
         if self.down:
@@ -78,10 +82,23 @@ class FakeGitea:
         return self.account
 
     def readable_repositories(self) -> list[str]:
+        self._up()
+        return [r for r in self.repos if r not in self.public]
+
+    def visible_repositories(self) -> list[str]:
         self._up()
         return list(self.repos)
 
+    def owner_repositories(self, owner: str) -> list[str]:
+        self._up()
+        found = [r for r in self.repos if r.startswith(f"{owner}/")]
+        if not found:
+            raise GiteaNotFound(f"no owner {owner}")
+        return found
+
     def repository(self, repo: str) -> dict[str, Any]:
+        if repo not in self.repos:
+            raise GiteaNotFound(f"no repository {repo}")
         return {
             "permissions": self.permissions.get(
                 repo, {"pull": True, "push": False, "admin": False}

nisse-review/tests/test_review.py

@@ -15,6 +15,7 @@ from nisse_core.knowledge.vault import Vault
 from nisse_core.log.canonical import AssistantText
 from nisse_core.loop.driver import Driver
 from nisse_core.oplog import OpLog
+from nisse_review.access import AccessRefused, check, reviewed
 from nisse_review.build import StartupError, check_access
 from nisse_review.review import Reviewer, _diff_lines, marker
 from nisse_review.scheduler import Scheduler, ready
@@ -102,7 +103,7 @@ def test_intake_opens_an_item_per_pr_and_closes_it_with_the_pr(
     assert (item(w, 7).position, item(w, 8).position) == (READY, WAITING)
     assert item(w, 7).scope == REPO
     gitea.repos["o/elsewhere"] = [pr(9)]
-    w["scheduler"]._only = frozenset({REPO})
+    w["scheduler"]._patterns = frozenset({REPO})
     w["scheduler"].intake()
     assert w["store"].work_item("o/elsewhere#9") is None
     gitea.repos[REPO] = [pr(8, draft=False)]  # 7 merged; 8 is ready now
@@ -300,10 +301,69 @@ def test_with_gitea_down_a_crashed_review_waits(tmp_path: Path) -> None:
 
 def test_an_account_that_can_write_is_refused(tmp_path: Path) -> None:
     gitea = FakeGitea(repos={REPO: [], "o/other": []})
-    assert check_access(gitea) == ("nisse-review", [REPO, "o/other"])  # type: ignore[arg-type]
+    log = OpLog("t", io.StringIO())
+    assert check_access(gitea, None, log) == ("nisse-review", [REPO, "o/other"])  # type: ignore[arg-type]
     gitea.permissions["o/other"] = {"pull": True, "push": True}
     with pytest.raises(StartupError, match="read access only"):
-        check_access(gitea)  # type: ignore[arg-type]
+        check_access(gitea, None, log)  # type: ignore[arg-type]
+
+
+def test_patterns_reach_public_repositories_the_account_was_not_given(
+    tmp_path: Path,
+) -> None:
+    # Gitea leaves a public repository's Read collaborators out of
+    # /user/repos: a pattern finds it all the same (option a: the
+    # configuration decides).
+    gitea = FakeGitea(
+        repos={"dev/nisse": [pr(7)], "dev/wordle": [], "infra/tofu": [pr(3)]},
+        public={"dev/nisse", "dev/wordle", "infra/tofu"},
+    )
+    log = OpLog("t", io.StringIO())
+    assert reviewed(gitea, None, log) == []  # type: ignore[arg-type]
+    assert reviewed(gitea, {"dev/*"}, log) == ["dev/nisse", "dev/wordle"]  # type: ignore[arg-type]
+    assert reviewed(gitea, {"*/*"}, log) == ["dev/nisse", "dev/wordle", "infra/tofu"]  # type: ignore[arg-type]
+    assert reviewed(gitea, {"dev/n*", "infra/tofu"}, log) == ["dev/nisse", "infra/tofu"]  # type: ignore[arg-type]
+    w = world(tmp_path, Scripted(), gitea, patterns=frozenset({"dev/*"}))
+    w["scheduler"].round()
+    assert w["store"].work_item("dev/nisse#7") is not None
+    assert w["store"].work_item("infra/tofu#3") is None
+
+
+def test_a_configured_repository_it_cannot_read_is_left_out() -> None:
+    gitea = FakeGitea(repos={REPO: []})
+    out = io.StringIO()
+    log = OpLog("t", out)
+    patterns = {REPO, "o/gone", "nobody/*"}
+    assert check(gitea, reviewed(gitea, patterns, log), log) == [REPO]  # type: ignore[arg-type]
+    assert '"access.unreadable"' in out.getvalue() and "o/gone" in out.getvalue()
+    assert '"access.no_owner"' in out.getvalue()
+
+
+def test_write_access_granted_while_running_stops_it(tmp_path: Path) -> None:
+    gitea = FakeGitea(repos={REPO: [pr()]})
+    w = world(tmp_path, Scripted(), gitea)
+    w["scheduler"].intake()
+    gitea.permissions[REPO] = {"pull": True, "push": True}
+    with pytest.raises(AccessRefused, match="read access only"):
+        w["scheduler"].round()
+    assert item(w).position == READY  # nothing reviewed
+
+
+def test_a_repository_pattern_must_be_owner_slash_name(tmp_path: Path) -> None:
+    from nisse_core.config import ConfigError
+    from nisse_review.config import ReviewConfig
+
+    tree = {
+        "state_db": str(tmp_path / "s.db"),
+        "llm": {"base_url": "http://llm", "token": "k"},
+        "roles": {"reviewer": "local", "expert": "frontier"},
+        "gitea": {"url": "https://g", "token": "t", "operators": ["don"]},
+    }
+    ok = ReviewConfig.from_tree({**tree, "repositories": ["dev/*", "*/*"]}, {})
+    assert ok.repositories == {"dev/*", "*/*"}
+    for bad in ("dev", "dev/", "/nisse", "a/b/c"):
+        with pytest.raises(ConfigError, match="is not owner/name"):
+            ReviewConfig.from_tree({**tree, "repositories": [bad]}, {})
 
 
 def test_diff_lines_are_the_heads_lines_a_diff_shows() -> None: