From dd2ac0e814a20a5d9b609ac7aafc9eb443754208 Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Tue, 21 Jul 2026 17:06:21 -0700 Subject: [PATCH 1/3] feat(groom): configurable run cadence via GROOM_INTERVAL_DAYS interval gate (BE-4004) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GitHub Actions `schedule:` cron is static in the workflow file — there is no native runtime "every N days". Replace groom's hardcoded weekly cron with the standard frequent-base-cron + runtime-gate pattern so cadence is a live knob: - ci-groom.yml now fires DAILY; the effective cadence is a new runtime gate. - New reusable input `interval_days` (default 7 = today's weekly behavior), wired in the caller to repo Actions variable GROOM_INTERVAL_DAYS. Changing the variable retunes cadence (weekly -> every-3-days -> daily) with no file edit. - `.github/groom/interval.py`: the gate. At run start it derives the last REAL groom run from this caller's Actions run history (a run counts only if its finder job actually ran, so interval-skip ticks never reset the clock) and early-exits cheaply — before the finder — unless interval_days have elapsed. Durable across stateless CI runs, no net-new secret, only `actions: read`. Pure decision logic split from the gh I/O shell + a unittest suite, mirroring ledger.py. - `workflow_dispatch` always bypasses the interval gate (manual override). - Fail-open on any history-read error (mirrors the volume gate). The gate runs in the existing `gate` job before the finder; the volume gate is kept as a second throttle with its window matched to the effective cadence. --- .github/groom/README.md | 36 ++++ .github/groom/interval.py | 263 +++++++++++++++++++++++++++ .github/groom/tests/test_interval.py | 180 ++++++++++++++++++ .github/workflows/ci-groom.yml | 64 +++++-- .github/workflows/groom.yml | 146 +++++++++++++-- AGENTS.md | 5 +- README.md | 2 +- 7 files changed, 659 insertions(+), 37 deletions(-) create mode 100644 .github/groom/interval.py create mode 100644 .github/groom/tests/test_interval.py diff --git a/.github/groom/README.md b/.github/groom/README.md index 060e05f..efe328c 100644 --- a/.github/groom/README.md +++ b/.github/groom/README.md @@ -147,6 +147,42 @@ Single-signature probe (exit 0 = should file, 1 = suppressed): python3 .github/groom/ledger.py --repo owner/name --check "" ``` +## `interval.py` — the runtime cadence gate (BE-4004) + +GitHub Actions `schedule:` cron is **static in the workflow file** — there is no +native "every N days" input. So a caller fires on a **frequent (daily) base +cron**, and this gate turns that into an **effective every-`GROOM_INTERVAL_DAYS` +run**: at run start it early-exits unless the interval has elapsed since the last +real groom, so a skipped tick costs ~nothing (it never reaches the finder). + +- **The knob is a repo Actions variable, `GROOM_INTERVAL_DAYS`** (default `7` = + weekly, matching the original cron). The caller wires it to the reusable's + `interval_days` input (`interval_days: ${{ vars.GROOM_INTERVAL_DAYS || '7' }}`) + and re-evaluates it each run, so changing the variable retunes cadence — weekly + → every-3-days → daily — with **no workflow-file edit**, the same "live knob" + ergonomics as the per-repo caps. +- **Last-run state is derived from GitHub Actions run history**, not a writable + store: the GitHub-native option that needs **no net-new secret** and only + `actions: read`. A prior run "counts" only if it actually reached the finder + (its `Audit — finder` job ran, not `skipped` by this gate), so the + interval-skip ticks in between never reset the clock. (A repo variable would + need a `Variables: write` credential the run doesn't carry, and a missing grant + would fail *silently* into a daily over-spend — run history has no such trap.) +- **`workflow_dispatch` always runs** — a manual dispatch bypasses the gate. +- **Fail-open**, like the volume gate: any error reading history (API hiccup, no + history, unparseable timestamp) RUNS the audit rather than skip a due groom. + +The caller grants `actions: read` (a reusable workflow's token is capped by the +caller's grant, so without it the run-history read 403s and the gate fails open). +As with `ledger.py`, the pure decision logic is split from the thin `gh` I/O so it +is fully unit-testable with no network. + +```bash +python3 .github/groom/interval.py \ + --repo owner/name --workflow-file ci-groom.yml \ + --current-run-id 123 --interval-days 7 --event-name schedule +``` + - **`tests/`** — `unittest` suite, run by [`test-groom-scripts.yml`](../workflows/test-groom-scripts.yml). diff --git a/.github/groom/interval.py b/.github/groom/interval.py new file mode 100644 index 0000000..796ee8f --- /dev/null +++ b/.github/groom/interval.py @@ -0,0 +1,263 @@ +#!/usr/bin/env python3 +"""Runtime cadence gate for the stateless groom CI run (BE-4004). + +GitHub Actions `schedule:` cron is **static in the workflow file** — there is no +native "every N days" input. The standard pattern is therefore a **frequent base +cron + a runtime gate**: the caller fires on a daily cron, but the reusable +early-exits unless enough time has elapsed since the last real groom run. This +module is that gate — it decides, cheaply, whether a given tick should proceed to +the (expensive) finder or no-op. + +The effective cadence is the repo Actions variable **`GROOM_INTERVAL_DAYS`** +(default `7` = today's weekly behavior), read fresh each run. Changing the +variable retunes cadence with no workflow-file edit — the same "live knob" +ergonomics as the per-repo caps. + +**Last-run state is derived from GitHub Actions run history** — the GitHub-native +option that needs no net-new secret and no writable durable store (a repo +variable would need an extra `Variables: write` credential the run does not +otherwise carry, and a missing grant would fail *silently* into a daily +over-spend). A prior run "counts" as a real groom only if it actually reached the +finder (its `Audit — finder` job ran, i.e. was not `skipped` by this very gate), +so the interval-skip ticks in between never reset the clock. Run history is +durable across the stateless CI runs and readable with only `actions: read`. + +The gate is **fail-open**, matching the volume gate: any error deriving the last +run (API hiccup, unparseable timestamp, no history) RUNS the audit rather than +silently skipping a due groom. `workflow_dispatch` **always** bypasses the gate — +a manual run is an explicit override. + +The pure decision logic (`run_audited`, `days_since`, `interval_decision`) is +separated from the thin `gh` I/O shell (`fetch_workflow_runs`, `fetch_run_jobs`) +so it is fully unit-testable with no network. + +CLI (what the groom gate job calls): + + python3 .github/groom/interval.py \ + --repo owner/name --workflow-file ci-groom.yml \ + --current-run-id 123 --interval-days 7 --event-name schedule + +Prints a `{should_run, reason, interval_days, days_since, last_run_at}` decision +JSON to stdout; the gate step reads `.should_run`. Always exits 0 (the decision +is carried in the JSON, and any failure is folded into a fail-open `should_run`). +""" + +import argparse +import json +import re +import subprocess +import sys +from datetime import datetime, timezone + +# A prior run only resets the cadence clock if it actually ran the finder — the +# gate names that job `Audit — finder` (job id `audit_find`), and in a called +# reusable the jobs API reports it as " / Audit — finder". Match on +# either the display-name or job-id form (case-insensitive) to be robust to how +# GitHub renders nested-reusable job names. An interval-skip tick has this job +# `skipped`, so it never matches the audited conclusions below. +_FINDER_JOB_HINTS = ("finder", "audit_find") + +# A finder job that reached success OR failure spent the (billed) audit, so both +# count as a real run: counting a failure keeps a run that spent money but died +# at a later step (e.g. filing) from re-spending on the very next daily tick. +# `skipped` (the interval-skip case), `cancelled`, and a null conclusion do not. +_AUDITED_CONCLUSIONS = {"success", "failure"} + +# Default cadence when GROOM_INTERVAL_DAYS is unset/blank/garbage — 7 days keeps +# the documented weekly behavior (AC: unset variable stays weekly, matching today). +_DEFAULT_INTERVAL_DAYS = 7.0 + +# owner/name only — no path segments or URL metacharacters that could redirect +# the `gh api` endpoint (mirrors ledger.py's guard). +_REPO_RE = re.compile(r"^[A-Za-z0-9._-]+/[A-Za-z0-9._-]+$") + +# A workflow file basename the API accepts as a workflow id, e.g. `ci-groom.yml`. +_WORKFLOW_FILE_RE = re.compile(r"^[A-Za-z0-9._-]+\.ya?ml$") + +# Bound each `gh api` call so a stalled connection can't hang the gate until the +# coarse job timeout (mirrors ledger.py). +_FETCH_TIMEOUT_SECONDS = 30 + +# How many recent runs to scan back for the last real groom before giving up +# (fail-open). Far more than any sane interval's worth of daily skip-ticks. +_MAX_RUNS_SCANNED = 100 + + +def parse_interval_days(raw, default: float = _DEFAULT_INTERVAL_DAYS): + """Parse the GROOM_INTERVAL_DAYS value; fall back to the weekly default. + + Unset/blank/negative/non-numeric all fall back to `default` (7 = weekly) so a + misconfigured variable degrades to today's behavior rather than disabling the + gate. A value of exactly 0 is honored (0 = no throttle, run every tick). + """ + if raw is None: + return default + text = str(raw).strip() + if text == "": + return default + try: + value = float(text) + except ValueError: + return default + if value < 0: + return default + return value + + +def parse_iso8601_utc(ts: str) -> datetime: + """Parse a GitHub API timestamp (e.g. `2026-07-21T09:17:30Z`) as aware UTC.""" + text = (ts or "").strip() + if text.endswith("Z"): + text = text[:-1] + "+00:00" + dt = datetime.fromisoformat(text) + if dt.tzinfo is None: + dt = dt.replace(tzinfo=timezone.utc) + return dt.astimezone(timezone.utc) + + +def days_since(then_iso: str, now: datetime) -> float: + """Fractional days between an ISO timestamp and `now` (an aware datetime).""" + then = parse_iso8601_utc(then_iso) + return (now - then).total_seconds() / 86400.0 + + +def run_audited(jobs) -> bool: + """True if a run's jobs show the finder actually ran (not an interval-skip).""" + for job in jobs or []: + name = (job.get("name") or "").lower() + if any(hint in name for hint in _FINDER_JOB_HINTS) and job.get("conclusion") in _AUDITED_CONCLUSIONS: + return True + return False + + +def interval_decision(interval_days: float, last_run_iso, now: datetime) -> dict: + """Pure gate decision, given the last real run (or None) and now. + + - No prior real run -> run (first groom, fail-open). + - `interval_days <= 0` -> run (throttle disabled). + - `days_since(last) >= interval_days` -> run; else skip cheaply. + """ + if interval_days <= 0: + return {"should_run": True, "reason": f"interval_days={interval_days:g} (<=0) — throttle disabled, running.", + "days_since": None, "last_run_at": last_run_iso} + if not last_run_iso: + return {"should_run": True, "reason": "no prior groom run found in history — running (fail-open).", + "days_since": None, "last_run_at": None} + elapsed = days_since(last_run_iso, now) + if elapsed >= interval_days: + return {"should_run": True, + "reason": f"{elapsed:.2f} days since last run >= interval {interval_days:g} — running.", + "days_since": round(elapsed, 2), "last_run_at": last_run_iso} + return {"should_run": False, + "reason": f"skipped: {elapsed:.2f} days since last run < interval {interval_days:g}.", + "days_since": round(elapsed, 2), "last_run_at": last_run_iso} + + +def _api_json(args, run): + """Run `gh api ` with a timeout and parse the stdout JSON.""" + try: + result = run( + ["gh", "api", *args], + text=True, + capture_output=True, + timeout=_FETCH_TIMEOUT_SECONDS, + ) + except subprocess.TimeoutExpired as exc: + raise RuntimeError(f"gh api timed out after {_FETCH_TIMEOUT_SECONDS}s: {args}") from exc + if result.returncode != 0: + raise RuntimeError(f"gh api failed ({args}): {result.stderr.strip()}") + text = (result.stdout or "").strip() + if not text: + return {} + return json.loads(text) + + +def fetch_workflow_runs(repo: str, workflow_file: str, run=subprocess.run): + """Recent runs of the caller workflow, newest first (single page).""" + if not _REPO_RE.match(repo or ""): + raise ValueError(f"invalid repo {repo!r}: expected owner/name") + if not _WORKFLOW_FILE_RE.match(workflow_file or ""): + raise ValueError(f"invalid workflow file {workflow_file!r}: expected a *.yml basename") + payload = _api_json( + [f"/repos/{repo}/actions/workflows/{workflow_file}/runs?per_page={_MAX_RUNS_SCANNED}"], + run, + ) + return payload.get("workflow_runs", []) if isinstance(payload, dict) else [] + + +def fetch_run_jobs(repo: str, run_id, run=subprocess.run): + """The jobs of one workflow run (single page).""" + if not _REPO_RE.match(repo or ""): + raise ValueError(f"invalid repo {repo!r}: expected owner/name") + payload = _api_json([f"/repos/{repo}/actions/runs/{run_id}/jobs?per_page=100"], run) + return payload.get("jobs", []) if isinstance(payload, dict) else [] + + +def find_last_audited_run_at(repo, workflow_file, current_run_id, run=subprocess.run): + """`run_started_at` of the most recent completed run that ran the finder. + + Walks the caller workflow's runs newest-first, skips the current run and any + still-in-progress run, and returns the first whose finder job actually ran. + Returns None if none is found within the scanned window (-> fail-open run). + """ + current = str(current_run_id) if current_run_id is not None else None + for wf_run in fetch_workflow_runs(repo, workflow_file, run=run): + if current is not None and str(wf_run.get("id")) == current: + continue + if wf_run.get("status") != "completed": + continue + jobs = fetch_run_jobs(repo, wf_run.get("id"), run=run) + if run_audited(jobs): + return wf_run.get("run_started_at") + return None + + +def evaluate(repo, workflow_file, current_run_id, interval_days, event_name, now, run=subprocess.run) -> dict: + """Full gate decision, folding dispatch bypass + fail-open around the pure logic.""" + if event_name == "workflow_dispatch": + return {"should_run": True, "reason": "workflow_dispatch — interval gate bypassed (manual override).", + "interval_days": interval_days, "days_since": None, "last_run_at": None} + try: + last_run_iso = find_last_audited_run_at(repo, workflow_file, current_run_id, run=run) + except Exception as exc: # noqa: BLE001 — any failure to read history must fail OPEN, never skip a due groom. + return {"should_run": True, "reason": f"could not read run history ({exc}) — running (fail-open).", + "interval_days": interval_days, "days_since": None, "last_run_at": None} + decision = interval_decision(interval_days, last_run_iso, now) + decision["interval_days"] = interval_days + return decision + + +def main(argv=None): + parser = argparse.ArgumentParser(description="Groom runtime cadence gate (GROOM_INTERVAL_DAYS).") + parser.add_argument("--repo", required=True, help="owner/name of the target repo") + parser.add_argument("--workflow-file", required=True, help="caller workflow basename, e.g. ci-groom.yml") + parser.add_argument("--current-run-id", required=True, help="this run's id, to exclude it from history") + parser.add_argument("--interval-days", default="", help="raw GROOM_INTERVAL_DAYS value (blank -> default 7)") + parser.add_argument("--event-name", default="schedule", help="github.event_name (workflow_dispatch bypasses)") + parser.add_argument("--now", default=None, help="override 'now' as an ISO-8601 UTC timestamp (for testing)") + parser.add_argument("--out", help="write the decision JSON here (also printed to stdout)") + args = parser.parse_args(argv) + + interval_days = parse_interval_days(args.interval_days) + now = parse_iso8601_utc(args.now) if args.now else datetime.now(timezone.utc) + + try: + decision = evaluate( + args.repo, args.workflow_file, args.current_run_id, + interval_days, args.event_name, now, + ) + except Exception as exc: # noqa: BLE001 — belt-and-suspenders: an unexpected bug fails OPEN. + decision = {"should_run": True, "reason": f"gate error ({exc}) — running (fail-open).", + "interval_days": interval_days, "days_since": None, "last_run_at": None} + + payload = json.dumps(decision, indent=2) + if args.out: + with open(args.out, "w", encoding="utf-8") as f: + f.write(payload + "\n") + print(payload) + print(decision["reason"], file=sys.stderr) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/.github/groom/tests/test_interval.py b/.github/groom/tests/test_interval.py new file mode 100644 index 0000000..4d291c4 --- /dev/null +++ b/.github/groom/tests/test_interval.py @@ -0,0 +1,180 @@ +#!/usr/bin/env python3 +"""Tests for the groom runtime cadence gate (BE-4004). + +Core properties: +- A tick within GROOM_INTERVAL_DAYS of the last REAL groom run no-ops (skips); + a tick at/after the interval runs. +- The interval-skip ticks in between do NOT reset the clock (only a run whose + finder actually ran counts). +- `workflow_dispatch` always runs, regardless of the interval. +- The gate is fail-open: no history / an API error runs rather than skips. + +The pure logic runs with no network; the history I/O is exercised via a stubbed +`gh` subprocess. + +Run: python3 -m unittest discover -s .github/groom/tests -p 'test_*.py' -v +""" + +import importlib.util +import json +import os +import unittest +from datetime import datetime, timezone + +_MODULE_PATH = os.path.join(os.path.dirname(__file__), "..", "interval.py") +_spec = importlib.util.spec_from_file_location("groom_interval", _MODULE_PATH) +interval = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(interval) + + +NOW = datetime(2026, 7, 21, 9, 17, 0, tzinfo=timezone.utc) + + +def iso(days_ago: float) -> str: + """An ISO-8601 UTC timestamp `days_ago` days before NOW.""" + ts = NOW.timestamp() - days_ago * 86400.0 + return datetime.fromtimestamp(ts, tz=timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ") + + +class Result: + """A minimal subprocess.CompletedProcess stand-in.""" + + def __init__(self, stdout="", returncode=0, stderr=""): + self.stdout = stdout + self.returncode = returncode + self.stderr = stderr + + +def make_gh_stub(runs, jobs_by_run): + """A `gh api` stub: routes /runs vs /runs//jobs by URL.""" + + def _run(cmd, **kwargs): + url = cmd[-1] + if "/jobs" in url: + run_id = url.split("/actions/runs/")[1].split("/jobs")[0] + return Result(stdout=json.dumps({"jobs": jobs_by_run.get(run_id, [])})) + return Result(stdout=json.dumps({"workflow_runs": runs})) + + return _run + + +def finder_job(conclusion="success"): + return {"name": "groom / Audit — finder", "conclusion": conclusion} + + +class ParseIntervalDaysTest(unittest.TestCase): + def test_unset_blank_garbage_default_to_weekly(self): + for raw in (None, "", " ", "not-a-number", "-3"): + self.assertEqual(interval.parse_interval_days(raw), 7.0, raw) + + def test_numeric_values(self): + self.assertEqual(interval.parse_interval_days("3"), 3.0) + self.assertEqual(interval.parse_interval_days("1.5"), 1.5) + self.assertEqual(interval.parse_interval_days("0"), 0.0) + + +class RunAuditedTest(unittest.TestCase): + def test_finder_success_or_failure_counts(self): + self.assertTrue(interval.run_audited([finder_job("success")])) + self.assertTrue(interval.run_audited([finder_job("failure")])) + + def test_matches_job_id_form(self): + # Robust to GitHub rendering the nested job by id rather than display name. + self.assertTrue(interval.run_audited([{"name": "groom / audit_find", "conclusion": "success"}])) + + def test_skipped_or_missing_does_not_count(self): + self.assertFalse(interval.run_audited([finder_job("skipped")])) + self.assertFalse(interval.run_audited([finder_job("cancelled")])) + self.assertFalse(interval.run_audited([finder_job(None)])) + self.assertFalse(interval.run_audited([{"name": "groom / Gate", "conclusion": "success"}])) + self.assertFalse(interval.run_audited([])) + + +class IntervalDecisionTest(unittest.TestCase): + def test_within_interval_skips(self): + d = interval.interval_decision(7.0, iso(3), NOW) + self.assertFalse(d["should_run"]) + self.assertIn("skipped", d["reason"]) + + def test_at_or_after_interval_runs(self): + self.assertTrue(interval.interval_decision(7.0, iso(7), NOW)["should_run"]) + self.assertTrue(interval.interval_decision(7.0, iso(9.5), NOW)["should_run"]) + + def test_no_prior_run_runs(self): + self.assertTrue(interval.interval_decision(7.0, None, NOW)["should_run"]) + + def test_zero_interval_disables_throttle(self): + self.assertTrue(interval.interval_decision(0.0, iso(0.1), NOW)["should_run"]) + + +class EvaluateTest(unittest.TestCase): + def test_dispatch_always_runs_even_within_interval(self): + # A recent real run exists, but a manual dispatch bypasses the gate. + stub = make_gh_stub([{"id": 9, "status": "completed", "run_started_at": iso(0.5)}], + {"9": [finder_job("success")]}) + d = interval.evaluate("o/r", "ci-groom.yml", 100, 7.0, "workflow_dispatch", NOW, run=stub) + self.assertTrue(d["should_run"]) + self.assertIn("dispatch", d["reason"]) + + def test_skips_when_last_real_run_is_recent(self): + runs = [ + {"id": 100, "status": "in_progress", "run_started_at": iso(0)}, # current, excluded + {"id": 99, "status": "completed", "run_started_at": iso(1)}, # skip-tick + {"id": 98, "status": "completed", "run_started_at": iso(2)}, # skip-tick + {"id": 90, "status": "completed", "run_started_at": iso(3)}, # last REAL run + ] + jobs = { + "99": [finder_job("skipped")], + "98": [finder_job("skipped")], + "90": [finder_job("success")], + } + d = interval.evaluate("o/r", "ci-groom.yml", 100, 7.0, "schedule", NOW, run=make_gh_stub(runs, jobs)) + # Last real run was 3 days ago; skip-ticks at 1 and 2 days must NOT reset it. + self.assertFalse(d["should_run"]) + self.assertEqual(d["last_run_at"], iso(3)) + + def test_runs_when_last_real_run_is_old(self): + runs = [ + {"id": 100, "status": "in_progress", "run_started_at": iso(0)}, + {"id": 99, "status": "completed", "run_started_at": iso(1)}, # skip + {"id": 90, "status": "completed", "run_started_at": iso(8)}, # last real, 8d ago + ] + jobs = {"99": [finder_job("skipped")], "90": [finder_job("success")]} + d = interval.evaluate("o/r", "ci-groom.yml", 100, 7.0, "schedule", NOW, run=make_gh_stub(runs, jobs)) + self.assertTrue(d["should_run"]) + + def test_no_history_runs(self): + d = interval.evaluate("o/r", "ci-groom.yml", 100, 7.0, "schedule", NOW, run=make_gh_stub([], {})) + self.assertTrue(d["should_run"]) + + def test_api_error_fails_open(self): + def boom(cmd, **kwargs): + return Result(stdout="", returncode=1, stderr="boom") + + d = interval.evaluate("o/r", "ci-groom.yml", 100, 7.0, "schedule", NOW, run=boom) + self.assertTrue(d["should_run"]) + self.assertIn("fail-open", d["reason"]) + + def test_only_skip_ticks_in_history_runs(self): + # Every prior run was itself an interval-skip -> no real run found -> run. + runs = [ + {"id": 99, "status": "completed", "run_started_at": iso(1)}, + {"id": 98, "status": "completed", "run_started_at": iso(2)}, + ] + jobs = {"99": [finder_job("skipped")], "98": [finder_job("skipped")]} + d = interval.evaluate("o/r", "ci-groom.yml", 100, 7.0, "schedule", NOW, run=make_gh_stub(runs, jobs)) + self.assertTrue(d["should_run"]) + + +class FetchValidationTest(unittest.TestCase): + def test_bad_repo_rejected(self): + with self.assertRaises(ValueError): + interval.fetch_workflow_runs("not-a-repo", "ci-groom.yml", run=make_gh_stub([], {})) + + def test_bad_workflow_file_rejected(self): + with self.assertRaises(ValueError): + interval.fetch_workflow_runs("o/r", "ci-groom", run=make_gh_stub([], {})) + + +if __name__ == "__main__": + unittest.main() diff --git a/.github/workflows/ci-groom.yml b/.github/workflows/ci-groom.yml index fd4408e..53d6d36 100644 --- a/.github/workflows/ci-groom.yml +++ b/.github/workflows/ci-groom.yml @@ -10,10 +10,18 @@ name: CI - Groom # itself (same reason as ci-cursor-review.yml / ci-detect-unreviewed-merge.yml). # # GROOMER-ONLY mode: groom only FINDS and FILES `groom`-labeled GitHub issues — -# no commits, no PRs, no auto-builder, never merges. It runs weekly on a schedule -# (never on-PR), plus a manual dispatch carrying a `dry_run` toggle so the audit + -# dedup can be spot-checked against a studio groom run on the same commit BEFORE -# trusting live filing (the Phase 3 dry-run parity gate). +# no commits, no PRs, no auto-builder, never merges. It fires on a DAILY base +# cron (never on-PR), but the EFFECTIVE cadence is the runtime interval gate in +# the reusable (BE-4004): a tick within `GROOM_INTERVAL_DAYS` of the last real +# run no-ops cheaply before the finder ever spins up. Plus a manual dispatch +# carrying a `dry_run` toggle (which ALWAYS runs, bypassing the interval gate) so +# the audit + dedup can be spot-checked against a studio groom run on the same +# commit BEFORE trusting live filing (the Phase 3 dry-run parity gate). +# +# CADENCE KNOB (BE-4004): set repo Actions variable `GROOM_INTERVAL_DAYS` +# (default 7 = weekly, matching the original cron). Changing it retunes cadence +# (weekly -> every-3-days -> daily) with NO workflow-file edit: +# gh variable set GROOM_INTERVAL_DAYS --repo Comfy-Org/github-workflows --body 3 # # PREREQUISITE (operator): `secrets.ANTHROPIC_API_KEY` must be available to this # repo (org- or repo-level) — the finder + verifier agents bill through it. Until @@ -25,9 +33,12 @@ name: CI - Groom on: schedule: - # Weekly, Monday 09:17 UTC. Off-peak + a non-round minute to dodge the - # top-of-hour GitHub scheduler congestion. Never fires on a PR. - - cron: '17 9 * * 1' + # DAILY base tick at 09:17 UTC. Off-peak + a non-round minute to dodge the + # top-of-hour GitHub scheduler congestion. Never fires on a PR. This is the + # frequent base cron for the interval-gate pattern (GH Actions cron is static + # in the file — there is no runtime "every N days"); the reusable's interval + # gate turns this daily tick into an effective every-GROOM_INTERVAL_DAYS run. + - cron: '17 9 * * *' workflow_dispatch: inputs: dry_run: @@ -55,27 +66,42 @@ jobs: # caller's own github.token stays read-only. (Grant issues:write here ONLY # if you drop bot_app_id and fall back to filing as github-actions[bot].) contents: read - uses: Comfy-Org/github-workflows/.github/workflows/groom.yml@07154fb5d0d979017d79b822873cfbe86483bfb3 # main (groom.yml lands here — #49 / BE-3872) + # REQUIRED for the interval gate (BE-4004): the reusable reads this repo's + # Actions run history to find the last real groom run. A reusable workflow's + # token is capped by the caller's grant, so without this the gate fails open + # (runs every daily tick, over-spending). + actions: read + # ⚠️ BUMP-AT-MERGE (BE-4004): this pin (and workflows_ref below) must point at + # THIS PR's squash-merge SHA — the interval gate lives in the NEW groom.yml, + # so the placeholder below fails LOUD (unresolvable ref) until bumped, exactly + # the reusable-lands-then-bump-caller two-step the pilot used (#49 -> #53). + # After merge: gh api ... / edit both refs to the merge commit SHA. + uses: Comfy-Org/github-workflows/.github/workflows/groom.yml@REPLACE_AT_MERGE_WITH_THIS_PRS_SQUASH_SHA # BE-4004 interval gate — bump to merge SHA with: # File groom's issues as cloud-code-bot (App token), not github-actions[bot]. bot_app_id: ${{ vars.APP_ID }} - # Keep the assets ref (finder/verifier briefs + dedup ledger) in lock-step - # with the `uses:` ref above, so a run always loads the briefs that match - # the workflow filing them. - workflows_ref: 07154fb5d0d979017d79b822873cfbe86483bfb3 - # Manual dispatch can dry-run; the weekly schedule always files live + # Keep the assets ref (finder/verifier briefs + interval gate + dedup + # ledger) in lock-step with the `uses:` ref above, so a run always loads the + # scripts that match the workflow filing them. BUMP-AT-MERGE with `uses:`. + workflows_ref: REPLACE_AT_MERGE_WITH_THIS_PRS_SQUASH_SHA + # Manual dispatch can dry-run; the daily schedule always files live # (github.event.inputs is null on a schedule event -> '' != 'true' -> false). dry_run: ${{ github.event.inputs.dry_run == 'true' }} - # Match the volume-gate window to this caller's ~weekly cron, so a week - # with nothing merged skips the (expensive) audit. - cadence: 7 + # Effective cadence knob (BE-4004): the daily base cron only produces a real + # run every GROOM_INTERVAL_DAYS days. Re-evaluated each run, so changing the + # variable retunes cadence with no file edit. Unset -> 7 (weekly), today. + interval_days: ${{ vars.GROOM_INTERVAL_DAYS || '7' }} + # Match the volume-gate window to that same effective cadence, not the daily + # base cron — so the merge-activity check spans how often a real run happens. + cadence: ${{ vars.GROOM_INTERVAL_DAYS || '7' }} # A dry-run is the Phase 3 parity spot-check, so it must ALWAYS exercise the # full audit + dedup — bypass the volume gate when dispatched with - # dry_run:true. Otherwise a quiet week (nothing merged in `cadence` days) + # dry_run:true. Otherwise a quiet window (nothing merged in `cadence` days) # would gate the dry-run to nothing and print no parity summary, defeating - # the dispatch's whole purpose. Live runs (the weekly schedule, where inputs + # the dispatch's whole purpose. Live runs (the daily schedule, where inputs # is null -> '' != 'true' -> true, or a dry_run:false dispatch) keep the gate - # on so a quiescent repo still skips the spend. + # on so a quiescent repo still skips the spend. (Note: a schedule tick still + # passes the interval gate first — the volume gate is the second throttle.) volume_gate: ${{ github.event.inputs.dry_run != 'true' }} secrets: ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} diff --git a/.github/workflows/groom.yml b/.github/workflows/groom.yml index 5ab5016..0d237ca 100644 --- a/.github/workflows/groom.yml +++ b/.github/workflows/groom.yml @@ -28,8 +28,12 @@ name: Groom (reusable) # name: Groom # on: # schedule: -# - cron: '17 9 * * 1' # weekly, Monday 09:17 UTC — never on-PR -# workflow_dispatch: +# # Frequent BASE cron (daily). Effective cadence is the runtime interval +# # gate, not the cron: a tick within GROOM_INTERVAL_DAYS of the last real +# # run no-ops cheaply (before the finder). GH Actions cron is static in the +# # file, so a daily tick + a runtime gate is how you get a tunable cadence. +# - cron: '17 9 * * *' # daily 09:17 UTC — never on-PR +# workflow_dispatch: # always runs — a manual override bypasses the gate # inputs: # dry_run: # description: 'Run the audit but do NOT file issues' @@ -37,6 +41,10 @@ name: Groom (reusable) # default: false # permissions: # contents: read +# actions: read # REQUIRED — the interval gate reads this workflow's run +# # # history to find the last real groom run. A reusable +# # # workflow's token is capped by the caller's grant, so +# # # WITHOUT this the gate fails open (runs every daily tick). # # issues: write # REQUIRED ONLY if you OMIT bot_app_id and file as # # # github-actions[bot]: a reusable workflow cannot elevate # # # the caller's token, so the github.token fallback needs @@ -56,9 +64,19 @@ name: Groom (reusable) # bot_app_id: ${{ vars.APP_ID }} # workflows_ref: # pin assets to the same ref as `uses:` # dry_run: ${{ github.event.inputs.dry_run == 'true' }} +# # Cadence knob + the matching volume-gate window (BE-4004). +# interval_days: ${{ vars.GROOM_INTERVAL_DAYS || '7' }} +# cadence: ${{ vars.GROOM_INTERVAL_DAYS || '7' }} # secrets: # ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} # BOT_APP_PRIVATE_KEY: ${{ secrets.CLOUD_CODE_BOT_PRIVATE_KEY }} +# +# CADENCE (the runtime knob, BE-4004): wire `interval_days` to the repo Actions +# variable `GROOM_INTERVAL_DAYS` (default 7 = weekly). The caller re-evaluates it +# each run, and on the daily base cron the reusable early-exits unless that many +# days have elapsed since the last real groom run — so weekly -> every-3-days -> +# daily is a variable edit, no workflow-file change. `workflow_dispatch` always +# runs regardless. on: workflow_call: @@ -76,12 +94,27 @@ on: default: >- duplication, inconsistent patterns, missing abstractions, complexity hotspots, dead code + interval_days: + description: >- + Effective run cadence in days (BE-4004). On a frequent (daily) base + cron, the reusable early-exits unless at least this many days have + elapsed since the last REAL groom run — so the caller fires daily but a + run actually happens ~every `interval_days`. Wire it in the caller to a + repo Actions variable (GROOM_INTERVAL_DAYS, falling back to 7) to retune + cadence with no workflow-file edit — see the caller pattern in the + header. `workflow_dispatch` always runs regardless. Default 7 keeps the + original weekly behavior; 0 disables the throttle (run every base tick). + type: number + required: false + default: 7 cadence: description: >- Volume-gate window in days (see volume_gate). When the gate is on, the run is skipped if NO pull request merged into the default branch in the last `cadence` days — a quiescent repo has nothing new to groom. Set - this to roughly your caller's cron interval. + this to roughly your EFFECTIVE cadence — the same value you pass to + `interval_days` — so the merge window matches how often a real run can + happen (see the interval gate below). type: number required: false default: 7 @@ -185,20 +218,30 @@ jobs: runs-on: ubuntu-latest permissions: contents: read + # The interval gate lists THIS caller's Actions run history to find the + # last real groom run (see the "Interval gate" step). A reusable workflow's + # token is CAPPED by the caller's grant, so the CALLER must also grant + # actions:read — without it the run-history read 403s and the gate fails + # OPEN (runs every daily tick, over-spending). See the caller pattern. + actions: read outputs: should_run: ${{ steps.check.outputs.should_run }} steps: - - name: Validate sink + apply volume gate - id: check + - name: Load groom assets (interval gate) + # interval.py is loaded from THIS repo at the pinned ref — a single + # source of truth like the briefs + ledger, never the target checkout. + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + with: + repository: Comfy-Org/github-workflows + ref: ${{ inputs.workflows_ref }} + path: _groom_assets + persist-credentials: false + + - name: Validate sink env: - GH_TOKEN: ${{ github.token }} SINK: ${{ inputs.sink }} - VOLUME_GATE: ${{ inputs.volume_gate }} - CADENCE: ${{ inputs.cadence }} - REPO: ${{ github.repository }} run: | set -euo pipefail - # The GitHub sink is the only one implemented. `linear` is a documented # later phase — fail loudly instead of running the whole audit and then # silently filing nothing. @@ -207,19 +250,74 @@ jobs: exit 1 fi + - name: Interval gate (GROOM_INTERVAL_DAYS) + # Cadence gate: on a frequent (daily) base cron, skip cheaply unless + # GROOM_INTERVAL_DAYS have elapsed since the last REAL groom run (derived + # from run history). workflow_dispatch always bypasses. Runs BEFORE the + # finder ever spins up, so a skipped tick costs ~nothing. Fail-open. + id: interval + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + RUN_ID: ${{ github.run_id }} + EVENT_NAME: ${{ github.event_name }} + WF_REF: ${{ github.workflow_ref }} + # The caller wires this to `vars.GROOM_INTERVAL_DAYS` and re-evaluates it + # each run, so changing that variable retunes cadence with no file edit. + # Blank/garbage -> interval.py defaults to 7 (weekly), matching today. + INTERVAL_DAYS: ${{ inputs.interval_days }} + run: | + set -euo pipefail + # Derive the CALLER workflow file (e.g. ci-groom.yml) from workflow_ref + # `owner/repo/.github/workflows/@` so the run-history read is + # scoped to this caller, not every workflow in the repo. + WF_PATH="${WF_REF%@*}" + WF_FILE="${WF_PATH##*/}" + # interval.py always exits 0 with a fail-open decision in the JSON; the + # `else` is a belt-and-suspenders fallback for an unexpected crash. + if DECISION=$(python3 "$GROOM_ASSETS/interval.py" \ + --repo "$REPO" --workflow-file "$WF_FILE" \ + --current-run-id "$RUN_ID" --interval-days "$INTERVAL_DAYS" \ + --event-name "$EVENT_NAME"); then + echo "$DECISION" + RUN=$(echo "$DECISION" | jq -r '.should_run') + REASON=$(echo "$DECISION" | jq -r '.reason') + else + RUN=true + REASON="interval gate crashed — running (fail-open)." + fi + echo "Interval gate: $REASON" + if [ "$RUN" = "true" ]; then + echo "run=true" >> "$GITHUB_OUTPUT" + else + echo "run=false" >> "$GITHUB_OUTPUT" + fi + + - name: Volume gate (merge activity) + # Only reached when the interval gate passed. Skip a quiescent repo: + # nothing merged in the cadence window ⇒ nothing new to groom. + id: volume + if: steps.interval.outputs.run == 'true' + env: + GH_TOKEN: ${{ github.token }} + VOLUME_GATE: ${{ inputs.volume_gate }} + CADENCE: ${{ inputs.cadence }} + REPO: ${{ github.repository }} + run: | + set -euo pipefail + if [ "$VOLUME_GATE" != "true" ]; then echo "Volume gate off — running." - echo "should_run=true" >> "$GITHUB_OUTPUT" + echo "run=true" >> "$GITHUB_OUTPUT" exit 0 fi - # Skip a quiescent repo: nothing merged in the window ⇒ nothing new to - # groom. Fail-OPEN — any API hiccup runs the audit rather than silently + # Fail-OPEN — any API hiccup runs the audit rather than silently # skipping a due groom (mirrors the studio volume gate). SINCE=$(date -u -d "${CADENCE} days ago" +%Y-%m-%d 2>/dev/null || echo "") if [ -z "$SINCE" ]; then echo "Could not compute the cadence window — running (fail-open)." - echo "should_run=true" >> "$GITHUB_OUTPUT" + echo "run=true" >> "$GITHUB_OUTPUT" exit 0 fi @@ -229,12 +327,28 @@ jobs: if [ "$MERGED" = "-1" ]; then echo "Merge-count check failed — running (fail-open)." - echo "should_run=true" >> "$GITHUB_OUTPUT" + echo "run=true" >> "$GITHUB_OUTPUT" elif [ "$MERGED" -gt 0 ]; then echo "$MERGED PR(s) merged since $SINCE — running." - echo "should_run=true" >> "$GITHUB_OUTPUT" + echo "run=true" >> "$GITHUB_OUTPUT" else echo "No PRs merged since $SINCE (cadence ${CADENCE}d) — nothing new to groom, skipping." + echo "run=false" >> "$GITHUB_OUTPUT" + fi + + - name: Decide should_run + # Run only if BOTH gates passed: enough time elapsed (interval) AND new + # merge activity (volume). If the interval gate skipped, the volume step + # was skipped and its output is empty -> should_run=false. + id: check + env: + INTERVAL_RUN: ${{ steps.interval.outputs.run }} + VOLUME_RUN: ${{ steps.volume.outputs.run }} + run: | + set -euo pipefail + if [ "$INTERVAL_RUN" = "true" ] && [ "$VOLUME_RUN" = "true" ]; then + echo "should_run=true" >> "$GITHUB_OUTPUT" + else echo "should_run=false" >> "$GITHUB_OUTPUT" fi diff --git a/AGENTS.md b/AGENTS.md index 57046ac..ac89790 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -51,7 +51,10 @@ tests — run the matching command above for whatever you touched. (the two-phase prompts, single source of truth, loaded at run time), and `ledger.py`, the durable dedup/rejection memory that stops the stateless groom CI run from re-filing already-filed or human-rejected findings (it uses GitHub - issue state as the store — no new secret). Tests in `tests/`. + issue state as the store — no new secret), and `interval.py`, the runtime + cadence gate (`GROOM_INTERVAL_DAYS`) that early-exits a daily tick unless the + interval has elapsed since the last real run (derived from run history — no new + secret). Tests in `tests/`. - `.github/bump-callers/` — `bump-callers.sh`, the ONE fleet-agnostic script that opens SHA-bump PRs in consumer repos when a reusable workflow changes. Tests in `tests/`. diff --git a/README.md b/README.md index dbebc8e..1839e76 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ This repo is **public** so any repo — public or private, inside or outside the | [`assign-prs-to-author.yml`](.github/workflows/assign-prs-to-author.yml) | Housekeeping — assigns every open PR with no assignees to its author (bot-authored PRs skipped by default). Run on a schedule from a thin caller; useful when a team tracks PR ownership via assignees. The calling job needs `pull-requests: write` and `issues: write`. | | [`pr-size.yml`](.github/workflows/pr-size.yml) | PR-size cap — fails (or, in `mode: warn`, only reports) when a PR's net diff exceeds `max_lines` non-generated changed lines, keeping diffs reviewable. Excludes dependency lockfiles, `linguist-generated` files (read from the base ref, so a PR can't exempt itself), Go generated-code markers, and per-repo `extra_lockfiles` / `extra_generated_globs`. A `bypass_label` (default `oversized-ok`) waves through a legitimately large change; a sticky bot comment explains overages when `bot_app_id` + `BOT_APP_PRIVATE_KEY` are supplied (degrades to status + step summary without them). Counting logic + tests live in [`scripts/check-pr-size/`](scripts/check-pr-size). | | [`stale.yml`](.github/workflows/stale.yml) | Stale-PR sweeper (`actions/stale`) plus a Slack digest of what it touched. PRs inactive for N days are labeled `stale`; still-inactive PRs are closed. The digest header names the source repo so batches from different repos posted to the same channel are unambiguous. Thresholds, messages, exempt labels, and the Slack channel are inputs; the caller owns the schedule + dry-run toggle. The calling job needs `pull-requests: write` and `issues: write`. Optional `SLACK_BOT_TOKEN`. | -| [`groom.yml`](.github/workflows/groom.yml) | Scheduled/dispatch org-wide **code-cleanup sweep** (finds only — no commits, no PRs, never merges). A read-only FINDER agent scans a clean default-branch checkout (whole-repo, not a diff) for high-value refactors; an INDEPENDENT VERIFIER agent (fresh session) re-checks each as CONFIRM/DOWNGRADE/REJECT with a stable dedup signature; survivors are deduped against a durable GitHub-issue-state ledger and filed as `groom`-labeled GitHub issues (security-adjacent ones get `groom-security` — investigate, don't auto-implement). Mirrors the cursor-review topology: briefs + ledger live in [`.github/groom/`](.github/groom) as the single source of truth. The agent step holds no write credentials (the `audit` job is `contents: read` only, per `model-gap-detector.yml`); filing runs in a separate job as the bot you configure via `bot_app_id` (Comfy: cloud-code-bot). `dry_run` reports what it would file without opening issues. Requires `ANTHROPIC_API_KEY` (+ `BOT_APP_PRIVATE_KEY` when `bot_app_id` is set). | +| [`groom.yml`](.github/workflows/groom.yml) | Scheduled/dispatch org-wide **code-cleanup sweep** (finds only — no commits, no PRs, never merges). A read-only FINDER agent scans a clean default-branch checkout (whole-repo, not a diff) for high-value refactors; an INDEPENDENT VERIFIER agent (fresh session) re-checks each as CONFIRM/DOWNGRADE/REJECT with a stable dedup signature; survivors are deduped against a durable GitHub-issue-state ledger and filed as `groom`-labeled GitHub issues (security-adjacent ones get `groom-security` — investigate, don't auto-implement). Mirrors the cursor-review topology: briefs + ledger live in [`.github/groom/`](.github/groom) as the single source of truth. The agent step holds no write credentials (the `audit` job is `contents: read` only, per `model-gap-detector.yml`); filing runs in a separate job as the bot you configure via `bot_app_id` (Comfy: cloud-code-bot). `dry_run` reports what it would file without opening issues. Runs on a **daily base cron** with a runtime cadence gate: set repo Actions variable `GROOM_INTERVAL_DAYS` (default 7 = weekly) to retune how often a real run happens — weekly → every-3-days → daily — with no workflow-file edit; a tick within the interval no-ops before the finder (`workflow_dispatch` always runs). Needs `actions: read` (the gate reads run history for the last real run). Requires `ANTHROPIC_API_KEY` (+ `BOT_APP_PRIVATE_KEY` when `bot_app_id` is set). | | [`agents-md-integrity.yml`](.github/workflows/agents-md-integrity.yml) | Enforces the Comfy `AGENTS.md` standard on the caller repo: a top-level `AGENTS.md` must exist and stay under a hard line ceiling (`max_lines`, default 200; warns over `warn_lines`, default 150), a `CLAUDE.md` (if present) must be a thin `@AGENTS.md` shim rather than a divergent copy, no legacy `.cursorrules` (gated `forbid_cursorrules`), every nested monorepo `AGENTS.md` needs a sibling `@AGENTS.md` shim and to be under the ceiling (gated `check_nested`), and `AGENTS.md` should have a CODEOWNERS DRI (`require_codeowners`, warn-only by default). Fails with a non-zero exit + GitHub annotations so it wires in as a required status check. The checker lives in [`.github/agents-md-integrity/`](.github/agents-md-integrity) (pin `workflows_ref` to the same ref as `uses:`); no secrets required. | ## Usage From 8eb8e0e02286e6465295e35f318467528fe27285 Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Tue, 21 Jul 2026 20:48:48 -0700 Subject: [PATCH 2/3] docs(AGENTS): fix interval.py state description per CodeRabbit The cadence gate derives state from Actions run history, not GitHub issue+PR state (that's the ledger's store); drop the misplaced sentence. Co-Authored-By: Claude Fable 5 --- AGENTS.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 27655cd..12b0ac6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -54,9 +54,8 @@ tests — run the matching command above for whatever you touched. — and (BE-4003) recognizes auto-builder PR state (open/merged/closed) so a built finding is never re-proposed — and `interval.py`, the runtime cadence gate (`GROOM_INTERVAL_DAYS`) that early-exits a daily tick unless the interval - has elapsed since the last real run (derived from run history — no new - secret). It uses GitHub issue+PR state as the store — no new secret. Tests in - `tests/`. + has elapsed since the last real run (derived from Actions run history — no new + secret). Tests in `tests/`. - `.github/bump-callers/` — `bump-callers.sh`, the ONE fleet-agnostic script that opens SHA-bump PRs in consumer repos when a reusable workflow changes. Tests in `tests/`. From 160fdea0a05135a1b9590d7e4d85a71f0bcc74de Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Thu, 23 Jul 2026 14:02:53 -0700 Subject: [PATCH 3/3] fix(groom): normalize the volume-gate window through interval.py (BE-4004) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both gates are wired to the same GROOM_INTERVAL_DAYS knob in the caller, but only the interval gate normalized it — the volume gate fed inputs.cadence straight to `date -d`, so the two drifted on reachable values: -3 interval.py -> 7 (safe weekly default), but `date -d '-3 days ago'` is a cutoff 3 days in the FUTURE, matching no merged PR -> the volume gate skips EVERY run, silently disabling groom. 0 a legitimate "no throttle" for the interval gate, but a 0-day merge window is today-only, so most ticks are judged quiescent. Add normalize_cadence_days() next to parse_interval_days() (same blank/garbage/negative -> 7 degradation, then floored at 1 whole day) plus a flag-only `interval.py --normalize-cadence ` mode the volume gate shells out to, so one parser backs both gates. The floor removes no capability: `volume_gate: false` is the input that expresses "no merge-activity throttle". Also correct the README's groom permission claim — the audit jobs are not `contents: read` only; they also hold the `id-token: write` claude-code-action needs to mint its OIDC token. Reworded to "no repository write credentials", which is the property that actually matters. Both per CodeRabbit review on #56. --- .github/groom/README.md | 12 +++++++++ .github/groom/interval.py | 39 +++++++++++++++++++++++++++ .github/groom/tests/test_interval.py | 40 ++++++++++++++++++++++++++++ .github/workflows/groom.yml | 16 ++++++++--- README.md | 2 +- 5 files changed, 105 insertions(+), 4 deletions(-) diff --git a/.github/groom/README.md b/.github/groom/README.md index 07b3b1d..c362c19 100644 --- a/.github/groom/README.md +++ b/.github/groom/README.md @@ -190,6 +190,15 @@ real groom, so a skipped tick costs ~nothing (it never reaches the finder). - **`workflow_dispatch` always runs** — a manual dispatch bypasses the gate. - **Fail-open**, like the volume gate: any error reading history (API hiccup, no history, unparseable timestamp) RUNS the audit rather than skip a due groom. +- **One normalization for both gates.** The caller wires the same variable to + `cadence` (the volume gate's merge-activity window), so the volume gate routes + it through this module too — `interval.py --normalize-cadence "$CADENCE"` — + rather than feeding the raw value to `date -d`. Same degradation + (blank/garbage/negative → `7`), then floored at **1 whole day**. Without it the + gates drift on reachable values: `-3` becomes a *future* `date -d` cutoff that + matches no merged PR (skipping every run — groom silently off) while the + interval gate had safely degraded to weekly, and `0` (a legitimate "no + throttle") shrinks the merge window to today-only. The caller grants `actions: read` (a reusable workflow's token is capped by the caller's grant, so without it the run-history read 403s and the gate fails open). @@ -200,6 +209,9 @@ is fully unit-testable with no network. python3 .github/groom/interval.py \ --repo owner/name --workflow-file ci-groom.yml \ --current-run-id 123 --interval-days 7 --event-name schedule + +# Second mode — normalize the shared knob into the volume gate's window: +python3 .github/groom/interval.py --normalize-cadence "$GROOM_INTERVAL_DAYS" ``` - **`tests/`** — `unittest` suite, run by diff --git a/.github/groom/interval.py b/.github/groom/interval.py index 796ee8f..fff33f6 100644 --- a/.github/groom/interval.py +++ b/.github/groom/interval.py @@ -67,6 +67,13 @@ # the documented weekly behavior (AC: unset variable stays weekly, matching today). _DEFAULT_INTERVAL_DAYS = 7.0 +# Floor for the volume gate's merge-activity window. The volume gate shares this +# same cadence knob, but a sub-1-day lookback is meaningless for "did anything +# merge?": 0 collapses the window to today-only, so a repo that merged nothing +# in the last few hours is judged quiescent. (Negative values never reach here — +# parse_interval_days already folds them into the default.) +_MIN_CADENCE_DAYS = 1 + # owner/name only — no path segments or URL metacharacters that could redirect # the `gh api` endpoint (mirrors ledger.py's guard). _REPO_RE = re.compile(r"^[A-Za-z0-9._-]+/[A-Za-z0-9._-]+$") @@ -104,6 +111,27 @@ def parse_interval_days(raw, default: float = _DEFAULT_INTERVAL_DAYS): return value +def normalize_cadence_days(raw, default: float = _DEFAULT_INTERVAL_DAYS) -> int: + """Whole-day volume-gate window derived from the SAME raw value as the interval. + + Both gates are wired to `GROOM_INTERVAL_DAYS` in the caller, so they must + agree on what a given value means. Feeding the raw variable straight to + `date -d` drifts from `parse_interval_days` in two reachable ways: + + - `-3` -> `date -d '-3 days ago'` is a FUTURE cutoff that matches no merged + PR, so the volume gate skips EVERY run — silently disabling groom, where + the interval gate had degraded safely to the weekly default. + - `0` (a legitimate "no throttle" for the interval gate) collapses the + merge window to today-only, so most ticks are judged quiescent. + + Routing through `parse_interval_days` first, then flooring at one whole day, + keeps the two gates on one normalization. The floor takes nothing away: to + run with no merge-activity throttle at all, set `volume_gate: false` — that + is the input that expresses it, not a zero-width window. + """ + return max(_MIN_CADENCE_DAYS, int(parse_interval_days(raw, default))) + + def parse_iso8601_utc(ts: str) -> datetime: """Parse a GitHub API timestamp (e.g. `2026-07-21T09:17:30Z`) as aware UTC.""" text = (ts or "").strip() @@ -228,6 +256,17 @@ def evaluate(repo, workflow_file, current_run_id, interval_days, event_name, now def main(argv=None): + argv = list(sys.argv[1:] if argv is None else argv) + + # Second, tiny mode — no API calls, no other flags required: + # python3 interval.py --normalize-cadence "$CADENCE" # -> e.g. `7` + # The volume gate shells out to this so BOTH gates derive their window from + # this one parser instead of the volume gate feeding the raw Actions + # variable straight to `date -d` (see normalize_cadence_days). + if argv and argv[0] == "--normalize-cadence": + print(normalize_cadence_days(argv[1] if len(argv) > 1 else "")) + return 0 + parser = argparse.ArgumentParser(description="Groom runtime cadence gate (GROOM_INTERVAL_DAYS).") parser.add_argument("--repo", required=True, help="owner/name of the target repo") parser.add_argument("--workflow-file", required=True, help="caller workflow basename, e.g. ci-groom.yml") diff --git a/.github/groom/tests/test_interval.py b/.github/groom/tests/test_interval.py index 4d291c4..8d326af 100644 --- a/.github/groom/tests/test_interval.py +++ b/.github/groom/tests/test_interval.py @@ -8,6 +8,8 @@ finder actually ran counts). - `workflow_dispatch` always runs, regardless of the interval. - The gate is fail-open: no history / an API error runs rather than skips. +- The volume gate's window normalizes through the SAME parser (blank/garbage/ + negative -> 7, floored at 1 whole day), so the two gates can't drift apart. The pure logic runs with no network; the history I/O is exercised via a stubbed `gh` subprocess. @@ -15,7 +17,9 @@ Run: python3 -m unittest discover -s .github/groom/tests -p 'test_*.py' -v """ +import contextlib import importlib.util +import io import json import os import unittest @@ -73,6 +77,42 @@ def test_numeric_values(self): self.assertEqual(interval.parse_interval_days("0"), 0.0) +class NormalizeCadenceDaysTest(unittest.TestCase): + def test_unset_blank_garbage_negative_default_to_weekly(self): + # Same degradation as the interval gate — the two share one knob. + for raw in (None, "", " ", "not-a-number", "-3"): + self.assertEqual(interval.normalize_cadence_days(raw), 7, raw) + + def test_zero_and_fractions_floor_to_one_whole_day(self): + # 0 legitimately disables the interval throttle, but a 0-day merge + # window would judge almost every repo quiescent — floor at 1. + self.assertEqual(interval.normalize_cadence_days("0"), 1) + self.assertEqual(interval.normalize_cadence_days("0.5"), 1) + self.assertEqual(interval.normalize_cadence_days("1.9"), 1) + + def test_numeric_values_truncate_to_whole_days(self): + self.assertEqual(interval.normalize_cadence_days("3"), 3) + self.assertEqual(interval.normalize_cadence_days("7"), 7) + self.assertEqual(interval.normalize_cadence_days("14.7"), 14) + + def test_cli_mode_prints_normalized_value_without_other_flags(self): + # The volume gate shells out as `interval.py --normalize-cadence "$X"`, + # with none of the gate's required flags — it must not error out. + for raw, want in (("-3", "7"), ("0", "1"), ("3", "3"), ("", "7")): + buf = io.StringIO() + with contextlib.redirect_stdout(buf): + rc = interval.main(["--normalize-cadence", raw]) + self.assertEqual(rc, 0, raw) + self.assertEqual(buf.getvalue().strip(), want, raw) + + def test_cli_mode_with_missing_value_falls_back_to_default(self): + buf = io.StringIO() + with contextlib.redirect_stdout(buf): + rc = interval.main(["--normalize-cadence"]) + self.assertEqual(rc, 0) + self.assertEqual(buf.getvalue().strip(), "7") + + class RunAuditedTest(unittest.TestCase): def test_finder_success_or_failure_counts(self): self.assertTrue(interval.run_audited([finder_job("success")])) diff --git a/.github/workflows/groom.yml b/.github/workflows/groom.yml index 67c087c..ec198d9 100644 --- a/.github/workflows/groom.yml +++ b/.github/workflows/groom.yml @@ -141,7 +141,9 @@ on: last `cadence` days — a quiescent repo has nothing new to groom. Set this to roughly your EFFECTIVE cadence — the same value you pass to `interval_days` — so the merge window matches how often a real run can - happen (see the interval gate below). + happen (see the interval gate below). Normalized through the same + parser as `interval_days` (blank/garbage/negative -> 7) and floored at + 1 day, so wiring both to one Actions variable can't drift the gates. type: number required: false default: 7 @@ -377,9 +379,17 @@ jobs: exit 0 fi + # Normalize through interval.py so BOTH gates read the shared + # GROOM_INTERVAL_DAYS knob the same way — blank/garbage/negative -> 7, + # and a sub-1-day window floored to 1. Passing the raw value to + # `date -d` drifts: `-3` yields a FUTURE cutoff that matches no merged + # PR (skipping every run, silently disabling groom) while the interval + # gate had safely degraded to weekly. + CADENCE_DAYS=$(python3 "$GROOM_ASSETS/interval.py" --normalize-cadence "$CADENCE" || echo "") + # Fail-OPEN — any API hiccup runs the audit rather than silently # skipping a due groom (mirrors the studio volume gate). - SINCE=$(date -u -d "${CADENCE} days ago" +%Y-%m-%d 2>/dev/null || echo "") + SINCE=$(date -u -d "${CADENCE_DAYS:-} days ago" +%Y-%m-%d 2>/dev/null || echo "") if [ -z "$SINCE" ]; then echo "Could not compute the cadence window — running (fail-open)." echo "run=true" >> "$GITHUB_OUTPUT" @@ -397,7 +407,7 @@ jobs: echo "$MERGED PR(s) merged since $SINCE — running." echo "run=true" >> "$GITHUB_OUTPUT" else - echo "No PRs merged since $SINCE (cadence ${CADENCE}d) — nothing new to groom, skipping." + echo "No PRs merged since $SINCE (cadence ${CADENCE_DAYS}d) — nothing new to groom, skipping." echo "run=false" >> "$GITHUB_OUTPUT" fi diff --git a/README.md b/README.md index 8961bb0..8e12ef4 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ This repo is **public** so any repo — public or private, inside or outside the | [`assign-prs-to-author.yml`](.github/workflows/assign-prs-to-author.yml) | Housekeeping — assigns every open PR with no assignees to its author (bot-authored PRs skipped by default). Run on a schedule from a thin caller; useful when a team tracks PR ownership via assignees. The calling job needs `pull-requests: write` and `issues: write`. | | [`pr-size.yml`](.github/workflows/pr-size.yml) | PR-size cap — fails (or, in `mode: warn`, only reports) when a PR's net diff exceeds `max_lines` non-generated changed lines, keeping diffs reviewable. Excludes dependency lockfiles, `linguist-generated` files (read from the base ref, so a PR can't exempt itself), Go generated-code markers, and per-repo `extra_lockfiles` / `extra_generated_globs`. A `bypass_label` (default `oversized-ok`) waves through a legitimately large change; a sticky bot comment explains overages when `bot_app_id` + `BOT_APP_PRIVATE_KEY` are supplied (degrades to status + step summary without them). Counting logic + tests live in [`scripts/check-pr-size/`](scripts/check-pr-size). | | [`stale.yml`](.github/workflows/stale.yml) | Stale-PR sweeper (`actions/stale`) plus a Slack digest of what it touched. PRs inactive for N days are labeled `stale`; still-inactive PRs are closed. The digest header names the source repo so batches from different repos posted to the same channel are unambiguous. Thresholds, messages, exempt labels, and the Slack channel are inputs; the caller owns the schedule + dry-run toggle. The calling job needs `pull-requests: write` and `issues: write`. Optional `SLACK_BOT_TOKEN`. | -| [`groom.yml`](.github/workflows/groom.yml) | Scheduled/dispatch org-wide **code-cleanup sweep** (finds only — no commits, no PRs, never merges). A read-only FINDER agent scans a clean default-branch checkout (whole-repo, not a diff) for high-value refactors; an INDEPENDENT VERIFIER agent (fresh session) re-checks each as CONFIRM/DOWNGRADE/REJECT with a stable dedup signature; survivors are deduped against a durable GitHub-issue-state ledger and filed as `groom`-labeled GitHub issues (security-adjacent ones get `groom-security` — investigate, don't auto-implement). Mirrors the cursor-review topology: briefs + ledger live in [`.github/groom/`](.github/groom) as the single source of truth. The agent step holds no write credentials (the `audit` job is `contents: read` only, per `model-gap-detector.yml`); filing runs in a separate job as the bot you configure via `bot_app_id` (Comfy: cloud-code-bot). `dry_run` reports what it would file without opening issues. Runs on a **daily base cron** with a runtime cadence gate: set repo Actions variable `GROOM_INTERVAL_DAYS` (default 7 = weekly) to retune how often a real run happens — weekly → every-3-days → daily — with no workflow-file edit; a tick within the interval no-ops before the finder (`workflow_dispatch` always runs). The calling job must grant `contents: read` + `issues: write` + `pull-requests: read` + `id-token: write` + `actions: read` — the `file` / `build_select` jobs declare the first three (needed even with `bot_app_id` set), the finder/verifier agent jobs need `id-token: write` (claude-code-action mints its GitHub token via OIDC), and the interval gate needs `actions: read` (reads run history for the last real run); GitHub rejects a shorter grant at startup. Requires `ANTHROPIC_API_KEY` (+ `BOT_APP_PRIVATE_KEY` when `bot_app_id` is set). **Opt-in auto-builder** (`builder: true`, BE-4003): the top `max_prs` (default 5) CONFIRMED, non-security findings become **review-gated PRs** (full CI + cursor-review, **never auto-merged**) instead of issues; a credential-free `build` job emits only a patch artifact and a separate `build_pr` job opens the PR as the bot, preserving the security boundary. The ledger's PR-state (open/merged/closed) stops a built finding being re-proposed. Requires `bot_app_id`. | +| [`groom.yml`](.github/workflows/groom.yml) | Scheduled/dispatch org-wide **code-cleanup sweep** (finds only — no commits, no PRs, never merges). A read-only FINDER agent scans a clean default-branch checkout (whole-repo, not a diff) for high-value refactors; an INDEPENDENT VERIFIER agent (fresh session) re-checks each as CONFIRM/DOWNGRADE/REJECT with a stable dedup signature; survivors are deduped against a durable GitHub-issue-state ledger and filed as `groom`-labeled GitHub issues (security-adjacent ones get `groom-security` — investigate, don't auto-implement). Mirrors the cursor-review topology: briefs + ledger live in [`.github/groom/`](.github/groom) as the single source of truth. The agent step holds no repository write credentials (the `audit` jobs grant `contents: read` plus only the `id-token: write` claude-code-action needs to mint its OIDC token — no `issues` / `pull-requests` / write `contents` scope, per `model-gap-detector.yml`); filing runs in a separate job as the bot you configure via `bot_app_id` (Comfy: cloud-code-bot). `dry_run` reports what it would file without opening issues. Runs on a **daily base cron** with a runtime cadence gate: set repo Actions variable `GROOM_INTERVAL_DAYS` (default 7 = weekly) to retune how often a real run happens — weekly → every-3-days → daily — with no workflow-file edit; a tick within the interval no-ops before the finder (`workflow_dispatch` always runs). The calling job must grant `contents: read` + `issues: write` + `pull-requests: read` + `id-token: write` + `actions: read` — the `file` / `build_select` jobs declare the first three (needed even with `bot_app_id` set), the finder/verifier agent jobs need `id-token: write` (claude-code-action mints its GitHub token via OIDC), and the interval gate needs `actions: read` (reads run history for the last real run); GitHub rejects a shorter grant at startup. Requires `ANTHROPIC_API_KEY` (+ `BOT_APP_PRIVATE_KEY` when `bot_app_id` is set). **Opt-in auto-builder** (`builder: true`, BE-4003): the top `max_prs` (default 5) CONFIRMED, non-security findings become **review-gated PRs** (full CI + cursor-review, **never auto-merged**) instead of issues; a credential-free `build` job emits only a patch artifact and a separate `build_pr` job opens the PR as the bot, preserving the security boundary. The ledger's PR-state (open/merged/closed) stops a built finding being re-proposed. Requires `bot_app_id`. | | [`agents-md-integrity.yml`](.github/workflows/agents-md-integrity.yml) | Enforces the Comfy `AGENTS.md` standard on the caller repo: a top-level `AGENTS.md` must exist and stay under a hard line ceiling (`max_lines`, default 200; warns over `warn_lines`, default 150), a `CLAUDE.md` (if present) must be a thin `@AGENTS.md` shim rather than a divergent copy, no legacy `.cursorrules` (gated `forbid_cursorrules`), every nested monorepo `AGENTS.md` needs a sibling `@AGENTS.md` shim and to be under the ceiling (gated `check_nested`), and `AGENTS.md` should have a CODEOWNERS DRI (`require_codeowners`, warn-only by default). Fails with a non-zero exit + GitHub annotations so it wires in as a required status check. The checker lives in [`.github/agents-md-integrity/`](.github/agents-md-integrity) (pin `workflows_ref` to the same ref as `uses:`); no secrets required. | ## Usage