A label filter is checked on every row: housekeeping listed all 59 open issues as parked (#138)

Co-authored-by: teahubxyz <boring+teahub@agentmail.to>

.github/time/housekeeping.py

@@ -68,6 +68,12 @@ def age_days(ts: str | None, now: dt.datetime) -> float:
     return (now - d).total_seconds() / 86400 if d else 0.0
 
 
+def labelled(name: str, query: str = "state=open") -> list[dict]:
+    """Issues (and PRs) that carry `name`. Each row is checked too: Gitea ignores a filter on a label the repo does not
+    have and returns everything, which on the first live run listed every open issue as a parked card."""
+    return [i for i in paged(f"{repo()}/issues?{query}&labels={urllib.parse.quote(name)}") if tm.has_label(i, name)]
+
+
 def same_repo(pr: dict) -> bool:
     return ((pr.get("head") or {}).get("repo") or {}).get("full_name", tm.REPO) == tm.REPO  # a fork's branch is not ours
 
@@ -121,7 +127,7 @@ def person(user: dict | None) -> str | None:
 def signup_people() -> dict[str, set[str]]:
     """venture slug -> distinct people who opened an early-access issue for it."""
     out: dict[str, set[str]] = {}
-    for i in paged(f"{repo()}/issues?state=all&labels=signup&type=issues"):
+    for i in labelled("signup", "state=all&type=issues"):
         m = re.search(r"^###\s*Venture\s*$\n+(.*?)(?=^###\s|\Z)", i.get("body") or "", re.M | re.S)
         who = person(i.get("user"))
         if m and who and m.group(1).strip():
@@ -131,7 +137,7 @@ def signup_people() -> dict[str, set[str]]:
 
 def step_signals(live: bool, now: dt.datetime) -> int:
     days = int(float(os.environ.get("TIME_VENTURE_MEASURE_DAYS") or 7))
-    due = [i for i in paged(f"{repo()}/issues?state=open&labels=signal&type=issues")
+    due = [i for i in labelled("signal", "state=open&type=issues")
            if not i.get("pull_request") and age_days(i.get("created_at"), now) >= days]
     if not due:
         return 0
@@ -183,7 +189,7 @@ def stale(now: dt.datetime) -> dict[str, list[tuple[int, float, str]]]:
 
     def add(cat: str, number: int, age: float, title: str) -> None:
         out.setdefault(cat, []).append((number, age, title))
-    for i in paged(f"{repo()}/issues?state=open&labels=needs-human"):   # issues and PRs alike
+    for i in labelled("needs-human"):   # issues and PRs alike
         age = age_days(i.get("updated_at") or i.get("created_at"), now)
         if age > NEEDS_HUMAN_DAYS:
             add(f"`needs-human`, untouched for over {NEEDS_HUMAN_DAYS} days", i["number"], age, i.get("title", ""))
@@ -191,9 +197,9 @@ def stale(now: dt.datetime) -> dict[str, list[tuple[int, float, str]]]:
         age = age_days(p.get("updated_at") or p.get("created_at"), now)
         if age >= PR_IDLE_DAYS:
             add(f"open PRs idle {PR_IDLE_DAYS} days or more", p["number"], age, p.get("title", ""))
-    for i in paged(f"{repo()}/issues?state=open&labels=parked&type=issues"):
+    for i in labelled("parked", "state=open&type=issues"):
         add("parked cards", i["number"], age_days(i.get("updated_at") or i.get("created_at"), now), i.get("title", ""))
-    for card in paged(f"{repo()}/issues?state=open&labels=ready&type=issues"):   # last beat: error for a while, or parked
+    for card in labelled("ready", "state=open&type=issues"):   # last beat: error for a while, or parked
         beats = [b for b in tm.beats_of(paged(f"{repo()}/issues/{card['number']}/comments")) if b["beat"] not in ("blocked", "capped")]
         last = beats[-1] if beats else None
         if not last:
@@ -216,7 +222,7 @@ def stale_text(found: dict[str, list[tuple[int, float, str]]]) -> str:
 
 
 def log_issue() -> int:
-    for i in paged(f"{repo()}/issues?state=open&labels=astra&type=issues"):
+    for i in labelled("astra", "state=open&type=issues"):
         if i.get("title") == LOG_TITLE:
             return i["number"]
     return LOG_FALLBACK

.github/time/test_housekeeping.py

@@ -23,13 +23,15 @@ LOG = 57
 
 
 class Forge:
-    def __init__(self, branches=(), prs=(), issues=(), milestones=(), reactions=None, comments=None):
+    def __init__(self, branches=(), prs=(), issues=(), milestones=(), reactions=None, comments=None, repo_labels=None):
         self.branches = [{"name": b} if isinstance(b, str) else b for b in branches]
         self.prs = list(prs)
         self.issues = {i["number"]: {"state": "open", "labels": [], "created_at": iso(0), "updated_at": iso(0), "body": "", **i} for i in issues}
         self.milestones = [dict(m) for m in milestones]
         self.reactions = reactions or {}
         self.cmts = comments or {}
+        # the repo's labels; by default the ones its issues carry. Gitea ignores a filter on any other label.
+        self.repo_labels = set(repo_labels) if repo_labels is not None else {l for i in self.issues.values() for l in i["labels"]}
         self.writes = []   # every non-GET call
         self.posted = []   # (issue, body)
 
@@ -72,7 +74,7 @@ class Forge:
         if route == "/repos/o/r/issues" and method == "GET":
             rows = [i for i in self.issues.values()
                     if (q.get("state", ["open"])[0] in ("all", i["state"]))
-                    and all(l in [x for x in i["labels"]] for l in q.get("labels", [""])[0].split(",") if l)]
+                    and all(l in i["labels"] for l in q.get("labels", [""])[0].split(",") if l and l in self.repo_labels)]
             return self.page([{**i, "labels": [{"name": l} for l in i["labels"]]} for i in rows], path)
         if route.startswith("/repos/o/r/issues/") and method == "PATCH":
             self.issues[int(route.split("/")[-1])].update(body)
@@ -161,12 +163,26 @@ def test_stale():
     t = texts[0]
     assert "#30" in t and "#40" in t and "#32" in t, t
     assert "#31" not in t and "#41" not in t and "#33" not in t, t
-    assert any("3 stale" in b for _, b in f.posted) or any("4 stale" in b for _, b in f.posted), f.posted
+    assert any("3 stale" in b for _, b in f.posted), f.posted   # #30, #32, #40: exactly three
     # an empty list posts no stale comment, only the one summary line
     f = Forge(); f.run()
     assert [b for _, b in f.posted if "stale list" in b] == [] and len(f.posted) == 1 and "0 stale" in f.posted[0][1]
 
 
+def test_unknown_label():
+    """No `parked` label in the repo: Gitea answers `labels=parked` with every open issue. None of them is stale, and
+    a `signal` filter answered with everything must not close an old issue that only looks like a signal."""
+    f = Forge(issues=[{"number": 57, "title": "Astra log", "labels": ["astra"], "updated_at": iso(9)},
+                      {"number": 72, "title": "a parent", "labels": ["planned"], "updated_at": iso(9)},
+                      {"number": 96, "title": "queued card", "labels": ["queued"], "updated_at": iso(9)},
+                      {"number": 99, "title": "old note", "labels": [], "created_at": iso(30), "body": "see ?venture=spent"}],
+              repo_labels={"astra", "planned", "queued"})
+    f.run()
+    assert [b for _, b in f.posted if "stale list" in b] == [], f.posted
+    assert any("0 stale" in b for _, b in f.posted) and f.issues[99]["state"] == "open", f.posted
+    assert [n for n, _ in f.posted] == [LOG], "the summary goes to the Astra log, found by its own label"
+
+
 def test_dry_run():
     f = Forge(branches=["main", "time/card-53"], prs=[pr(1, "time/card-53", "closed", True)],
               issues=[sig(10, "spent", 8), {"number": 30, "labels": ["needs-human"], "updated_at": iso(9)}],
@@ -178,7 +194,7 @@ def test_dry_run():
 
 
 if __name__ == "__main__":
-    tests = [test_branches, test_signals, test_milestones, test_stale, test_dry_run]
+    tests = [test_branches, test_signals, test_milestones, test_stale, test_unknown_label, test_dry_run]
     for t in tests:
         t()
         print("ok", t.__name__)

.github/time/test_time.py

@@ -810,6 +810,20 @@ if _gt is None: os.environ.pop("GITHUB_TOKEN", None)
 else: os.environ["GITHUB_TOKEN"] = _gt
 print("ALL OK 54")
 
+# 54b Gitea ignores a filter on a label the repo does not have and returns every issue: issues() keeps only the rows that
+# carry every label the query names (a row with no labels field, as in the fakes here, is taken as filtered)
+_h54 = tm.http
+tm.http = lambda *a, **k: [{"number": 1, "labels": [{"name": "spec"}]}, {"number": 2, "labels": []},
+                           {"number": 3, "labels": [{"name": "queued"}]}, {"number": 4},
+                           {"number": 5, "pull_request": {"merged": False}, "labels": [{"name": "spec"}]},
+                           {"number": 6, "labels": [{"name": "spec"}, {"name": "ready"}]}]
+assert [i["number"] for i in tm.issues("state=open&labels=spec")] == [1, 4, 6], "only spec cards, never the rest"
+assert [i["number"] for i in tm.issues("state=open&labels=spec,ready")] == [4, 6], "every named label"
+assert [i["number"] for i in tm.issues("state=open")] == [1, 2, 3, 4, 6], "no label named: no filter"
+assert tm.has_label({"labels": ["ready"]}, "ready") and not tm.has_label({}, "ready")
+tm.http = _h54
+print("ALL OK 54b")
+
 # the worker itself (worker.js), against a fake D1, Workers AI and Pages
 import shutil as _sh, subprocess as _sp  # noqa: E402
 if _sh.which("node"):

.github/time/time.py

@@ -77,9 +77,19 @@ def gh(method: str, path: str, body=None, *, as_time: bool | None = None):
             time.sleep(0.5 * (attempt + 1))
 
 
+def has_label(item: dict, name: str) -> bool:
+    return any((l.get("name") if isinstance(l, dict) else l) == name for l in item.get("labels") or [])
+
+
 def issues(query: str) -> list[dict]:
-    """Open/closed issues (never PRs) of this repo; `query` is the Gitea query string without type/limit."""
-    return [i for i in gh("GET", f"/repos/{REPO}/issues?type=issues&limit=50&{query}") if not i.get("pull_request")]
+    """Open/closed issues (never PRs) of this repo; `query` is the Gitea query string without type/limit.
+
+    Each row is checked against the labels the query names: Gitea ignores a filter on a label the repo does not have
+    and returns every issue, so a deleted `spec` label would make every open issue a spec to split. A row with no
+    `labels` field at all (older payloads, test fakes) is taken as the server filtered it."""
+    want = [l for l in (urllib.parse.parse_qs(query).get("labels") or [""])[0].split(",") if l]
+    rows = [i for i in gh("GET", f"/repos/{REPO}/issues?type=issues&limit=50&{query}") if not i.get("pull_request")]
+    return [i for i in rows if "labels" not in i or all(has_label(i, l) for l in want)]
 
 
 _label_ids: dict[str, int] | None = None

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## [Unreleased]
 
 ### Security
+- A label filter is checked on every row. Gitea ignores `labels=<name>` for a label the repo does not have and returns every issue: housekeeping's first live run listed all 59 open issues as parked cards (there is no `parked` label), and the same answer to `labels=spec` would have had Time split every open issue as a spec. `issues()` in time.py and the sweep's queries now keep only rows that carry the labels they asked for.
 - A commit status from one of Time's own workflows (`heartbeat /`, `mention /`, `venture /`, `astra /`, `doctor /`, `update /`, `steward /`) is not a check: it neither blocks nor earns a merge. A seat's review is a `pull_request_review` event that wakes `heartbeat.yml` during a heartbeat; the concurrency group cancels the run and Gitea records it as a failure on the PR head, which had stopped approved, CI-green PRs from landing.
 - The worker's own commit status (`time/worker-run`) can fail or delay a merge but never earns one: with no other check on the PR head, Time dispatches the project's CI workflow on the PR branch (`TIME_CI_WORKFLOW`, default `ci.yml`, which must accept `workflow_dispatch`; GitHub skips `pull_request` runs for pushes made with the Actions token) and merges on that. `off` keeps the old "no checks" stop.
 - `.github/` is protected from auto-merge whatever `TIME_PROTECTED` says.