From 4a9b78c7a9c40a6e8af04b80697155167f1297be Mon Sep 17 00:00:00 2001 From: Imani Manyara Date: Mon, 2 Nov 2026 14:19:43 -0500 Subject: [PATCH 1/3] Cap the reconciler's new pull requests per run Going live was unsafe for one reason: volume. Measured 2026-11-02 with a dry run across all ten accounts in policy.json, the first apply run would have opened 118 pull requests across 256 repositories -- 64 in laranail, 38 in simtabi, 15 in ichava, 1 in simbi-prjs-clt. Against the 773 pull requests in the week of 2026-09-30 that made "fewer, fuller PRs" a rule, that is not a switch anyone should flip blind, so it has stayed dry-run and the standard has stayed unenforced instead. policy.json gains `max-new-per-run: 5`: at most five NEW pull requests per account per run, overridable with --max-new, and 0 for no limit. A scheduled week then opens at most five per drifting account and the estate converges over a few weeks rather than in one burst. Two details that matter more than the cap itself: A repository whose pull request is already open is still refreshed past the budget. Leaving a stale one open is worse than opening nothing, and the budget is about new ones. A repository held back reads `deferred:rate-limit`, never `skipped:`, and the summary says how many were deferred. `skipped` means a decision was taken about that repository; a budget says nothing about it. Without that distinction a capped run reads as a clean estate -- which is exactly how the defects in #15 and #16 hid for as long as they did. Four tests, proven red before green: the budget holds, deferring is not skipping, an open pull request is still refreshed past it, and 0 means no limit. Disabling the cap turns two of them red. --- dependabot/README.md | 20 ++++++- dependabot/policy.json | 88 ++++++++++++++++++++++-------- dependabot/reconcile.py | 25 ++++++++- dependabot/tests/test_reconcile.py | 55 +++++++++++++++++++ 4 files changed, 163 insertions(+), 25 deletions(-) diff --git a/dependabot/README.md b/dependabot/README.md index 6e00285..b2b4a2e 100644 --- a/dependabot/README.md +++ b/dependabot/README.md @@ -28,7 +28,8 @@ Every run prints a table and appends it to the job summary: for the reconciler ` | `exclude` | `[{"repo": "owner/name", "reason": "..."}]` | Repositories neither script touches. A reason is required. | | `schedule` | weekly, Monday 06:00, America/New_York | The documented convention. | | `cooldown` | 3 days, 7 for majors | A release is not proposed the day it ships. | -| `open-pull-requests-limit` | `10` | Per ecosystem entry. | +| `open-pull-requests-limit` | `10` | Per ecosystem entry. Dependabot's own cap, not the reconciler's. | +| `max-new-per-run` | `5` | The reconciler's own cap: at most this many **new** pull requests per account per run. A pull request already open is still refreshed past it. `0` removes the limit. | | `labels` | `["dependencies"]` | Applied to every Dependabot pull request. | | `commit-message-prefix` | per ecosystem | `chore(actions)`, `deps(composer)`, `deps(npm)`, … | | `groups` | `{ecosystem}-minor-and-patch`, minor + patch | One grouped pull request per ecosystem; majors stay one each. | @@ -85,6 +86,23 @@ Dependency Merge Bot this section used to describe was never created; its job is Both workflows ship as dry runs. +> **The reconciler is rate limited, and that is what makes going live safe.** Measured +> 2026-11-02, before the limit existed: a first apply run would have opened **118 pull +> requests across 256 repositories** -- 64 in laranail, 38 in simtabi, 15 in ichava, 1 in +> simbi-prjs-clt. Against the 773 pull requests in the week of 2026-09-30 that made +> "fewer, fuller PRs" a rule, that is not a switch to flip blind. +> +> `policy.json`'s `max-new-per-run` (5) caps **new** pull requests per account per run, so a +> scheduled week opens at most 5 x the number of accounts with drift and the estate converges +> over a few weeks instead of in one burst. `--max-new N` overrides it; `--max-new 0` removes +> the limit. A repository whose pull request is **already open** is still refreshed past the +> budget -- leaving a stale one open is worse than opening nothing. +> +> A repository held back reads `deferred:rate-limit`, never `skipped:`, and the summary says +> how many were deferred. That distinction is load-bearing: `skipped` means a decision was +> taken about that repository, and a budget says nothing about it. Without it a capped run +> looks like a clean estate -- which is exactly how the two defects in #15 and #16 hid. + | Step | How | |---|---| | Try one run | Actions → the workflow → Run workflow, with `apply` ticked. | diff --git a/dependabot/policy.json b/dependabot/policy.json index c2b94db..77562cd 100644 --- a/dependabot/policy.json +++ b/dependabot/policy.json @@ -1,26 +1,68 @@ { - "version": 1, - "accounts": { - "orgs": ["brandgulp", "ichava", "laranail", "mukoracms", "simsange", "simtabi", "tayariai", "tusente", "simbi-prjs-clt", "simbi-prjs-int"], - "users": ["imanimanyara"] - }, - "exclude": [ - { - "repo": "laranail/wire-elements-pro", - "reason": "A mirror of the commercial Wire Elements Pro package: its dependencies follow upstream releases, not Dependabot." - } + "version": 1, + "accounts": { + "orgs": [ + "brandgulp", + "ichava", + "laranail", + "mukoracms", + "simsange", + "simtabi", + "tayariai", + "tusente", + "simbi-prjs-clt", + "simbi-prjs-int" ], - "schedule": {"interval": "weekly", "day": "monday", "time": "06:00", "timezone": "America/New_York"}, - "cooldown": {"default-days": 3, "semver-major-days": 7}, - "open-pull-requests-limit": 10, - "labels": ["dependencies"], - "commit-message-prefix": {"github-actions": "chore(actions)", "composer": "deps(composer)", "npm": "deps(npm)", "pip": "deps(pip)", "gomod": "deps(go)", "cargo": "deps(cargo)", "docker": "deps(docker)", "bundler": "deps(bundler)"}, - "groups": {"name-pattern": "{ecosystem}-minor-and-patch", "update-types": ["minor", "patch"]}, - "merge": { - "default": "actions", - "overrides": {}, - "hold-label": "automerge:hold", - "majors": "never" - }, - "local-block": {"begin": "# >>> local (kept by the generator)", "end": "# <<< local"} + "users": [ + "imanimanyara" + ] + }, + "exclude": [ + { + "repo": "laranail/wire-elements-pro", + "reason": "A mirror of the commercial Wire Elements Pro package: its dependencies follow upstream releases, not Dependabot." + } + ], + "schedule": { + "interval": "weekly", + "day": "monday", + "time": "06:00", + "timezone": "America/New_York" + }, + "cooldown": { + "default-days": 3, + "semver-major-days": 7 + }, + "open-pull-requests-limit": 10, + "labels": [ + "dependencies" + ], + "commit-message-prefix": { + "github-actions": "chore(actions)", + "composer": "deps(composer)", + "npm": "deps(npm)", + "pip": "deps(pip)", + "gomod": "deps(go)", + "cargo": "deps(cargo)", + "docker": "deps(docker)", + "bundler": "deps(bundler)" + }, + "groups": { + "name-pattern": "{ecosystem}-minor-and-patch", + "update-types": [ + "minor", + "patch" + ] + }, + "merge": { + "default": "actions", + "overrides": {}, + "hold-label": "automerge:hold", + "majors": "never" + }, + "local-block": { + "begin": "# >>> local (kept by the generator)", + "end": "# <<< local" + }, + "max-new-per-run": 5 } diff --git a/dependabot/reconcile.py b/dependabot/reconcile.py index 39b6e7f..736f37d 100644 --- a/dependabot/reconcile.py +++ b/dependabot/reconcile.py @@ -191,6 +191,10 @@ def main(argv=None, gh=None, shell=None): ap.add_argument("--policy", default=common.DEFAULT_POLICY) ap.add_argument("--apply", action="store_true", help="push branches and open/refresh PRs") ap.add_argument("--repo", action="append", default=[], help="limit to these names (repeatable)") + ap.add_argument("--max-new", type=int, default=None, + help="open at most N NEW pull requests this run (default: policy " + "max-new-per-run). Refreshing one that is already open does not count. " + "0 means no limit.") ap.add_argument("--git-name", default=os.environ.get("RECONCILE_GIT_NAME", "Imani Manyara")) ap.add_argument("--git-email", default=os.environ.get("RECONCILE_GIT_EMAIL", "imanimanyara@users.noreply.github.com")) opts = ap.parse_args(argv) @@ -200,6 +204,8 @@ def main(argv=None, gh=None, shell=None): policy = common.load_policy(opts.policy) kind = common.account_kind(policy, opts.owner) + if opts.max_new is None: + opts.max_new = int(policy.get("max-new-per-run", 0)) if not os.path.isfile(opts.generator_script): print(f"reconcile: generator not found at {opts.generator_script}", file=sys.stderr) return 2 @@ -213,9 +219,21 @@ def main(argv=None, gh=None, shell=None): errors = 0 workdir = tempfile.mkdtemp(prefix="dependabot-reconcile-") try: + opened = 0 for repo in active: try: + if opts.max_new and opened >= opts.max_new: + # The budget is for NEW pull requests. A repository whose pull request is + # already open still gets refreshed, because leaving a stale one open is + # worse than opening nothing. + if not open_pr(gh, repo["full_name"], opts.owner): + rows.append((repo["full_name"], "deferred:rate-limit", + f"budget of {opts.max_new} new pull requests spent; " + "the next run takes this one")) + continue status, note = reconcile_repo(repo, opts.owner, gh, shell, opts, policy, workdir) + if status in ("opened", "would-open"): + opened += 1 except Exception as exc: # one repository's failure must not stop the others errors += 1 status, note = "error", str(exc)[:300] @@ -227,7 +245,12 @@ def main(argv=None, gh=None, shell=None): rows.sort(key=lambda r: r[0].lower()) mode = "apply" if opts.apply else "dry run" - head = f"## Dependabot reconcile: {opts.owner} ({mode})\n\n{len(repos)} repositories listed, {len(active)} active.\n\n" + deferred = sum(1 for r in rows if r[1] == "deferred:rate-limit") + budget = (f" Budget: {opts.max_new} new pull requests per run" + + (f", {deferred} deferred to a later run." if deferred else ", none deferred.") + if opts.max_new else "") + head = (f"## Dependabot reconcile: {opts.owner} ({mode})\n\n" + f"{len(repos)} repositories listed, {len(active)} active.{budget}\n\n") text = head + common.markdown_table(["repo", "status", "note"], rows) print(text) common.write_summary(text) diff --git a/dependabot/tests/test_reconcile.py b/dependabot/tests/test_reconcile.py index eb527f4..05cb8c7 100644 --- a/dependabot/tests/test_reconcile.py +++ b/dependabot/tests/test_reconcile.py @@ -208,6 +208,61 @@ def test_write_never_carries_strict(self): self.assertTrue(writes, "drifting repositories are written") self.assertFalse([c for c in writes if "--strict" in c]) + # --- the per-run budget ------------------------------------------------------------- + # Going live without one meant 118 pull requests across 256 repositories in a single + # weekly run, measured 2026-11-02. These three cases are what make the switch safe to + # flip: the budget holds, deferring is not skipping, and an open pull request is still + # refreshed once the budget is spent. + + def three_drifting(self): + routes = self.base_routes() + routes["orgs/simtabi/repos"] = [repo("d1"), repo("d2"), repo("d3")] + return routes, {"d1": 1, "d2": 1, "d3": 1} + + def test_the_budget_caps_new_pull_requests(self): + routes, rc = self.three_drifting() + captured = [] + orig = common.write_summary + common.write_summary = captured.append + try: + code, gh, _ = self.run_main(routes, rc, "--apply", "--max-new", "1") + finally: + common.write_summary = orig + self.assertEqual(code, 0) + self.assertEqual(len([r for r in gh.runs if r[1] == "create"]), 1) + self.assertEqual(captured[0].count("deferred:rate-limit"), 2) + + def test_a_deferred_repository_is_not_reported_as_skipped(self): + # "skipped" means a decision was taken about that repository. A budget says nothing + # about it, so the status has to read differently or a capped run looks like a clean + # estate. + routes, rc = self.three_drifting() + captured = [] + orig = common.write_summary + common.write_summary = captured.append + try: + self.run_main(routes, rc, "--apply", "--max-new", "1") + finally: + common.write_summary = orig + self.assertIn("Budget: 1 new pull requests per run, 2 deferred", captured[0]) + self.assertNotIn("skipped:rate", captured[0]) + + def test_an_open_pull_request_is_still_refreshed_past_the_budget(self): + # pr-stale already has one open; leaving it stale would be worse than opening nothing. + routes = self.base_routes() + routes["orgs/simtabi/repos"] = [repo("d1"), repo("pr-stale")] + code, gh, _ = self.run_main(routes, {"d1": 1, "pr-stale": 1}, "--apply", "--max-new", "1") + self.assertEqual(code, 0) + verbs = sorted(r[1] for r in gh.runs) + self.assertIn("create", verbs) + self.assertIn("edit", verbs) + + def test_no_budget_means_no_limit(self): + routes, rc = self.three_drifting() + code, gh, _ = self.run_main(routes, rc, "--apply", "--max-new", "0") + self.assertEqual(code, 0) + self.assertEqual(len([r for r in gh.runs if r[1] == "create"]), 3) + def test_the_rewrite_uses_render_not_check(self): # `check --write` creates or appends; it does not rewrite a file that is merely # wrong, which is what drift is. Measured 2026-11-02: every repository sampled came From 6116ff739aec81fb472d6747ede682fc6969c3a1 Mon Sep 17 00:00:00 2001 From: Imani Manyara Date: Mon, 2 Nov 2026 14:21:38 -0500 Subject: [PATCH 2/3] Spend the budget where a new pull request is decided The first placement checked the budget before doing any work, which is cheaper and misreports. A repository that conforms, or whose pull request is already up to date, came back `deferred:rate-limit` with nobody having looked at it. The real laranail dry run showed it: 85 listed, 77 active, "63 deferred" -- when only 64 of the 77 drift at all. A summary that inflates the backlog is the same class of defect as one that hides it. The budget is now an object passed into reconcile_repo and spent at the one point a NEW pull request is decided, after every other branch has returned. So `conforms`, `skipped:pr-up-to-date`, `skipped:branch-without-open-pr` and the rest keep their own status, and only a repository that would genuinely have had a pull request opened is deferred. test_the_budget_does_not_mislabel_a_repository_it_never_reached pins it: two drifting repositories and one conforming, budget 1, and the conforming one must read `conforms` with exactly one deferral. Reinstating the early spend turns seven tests red, including that one. --- dependabot/reconcile.py | 39 +++++++++++++++++++----------- dependabot/tests/test_reconcile.py | 16 ++++++++++++ 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/dependabot/reconcile.py b/dependabot/reconcile.py index 736f37d..dda5904 100644 --- a/dependabot/reconcile.py +++ b/dependabot/reconcile.py @@ -114,9 +114,24 @@ def open_pr(gh, full_name, owner): return gh.api(f"repos/{full_name}/pulls/{prs[0]['number']}") -def reconcile_repo(repo, owner, gh, shell, opts, policy, workdir): +class Budget: + """How many NEW pull requests this run may still open. 0 means no limit.""" + + def __init__(self, limit): + self.limit = limit + self.spent = 0 + + def take(self): + if self.limit and self.spent >= self.limit: + return False + self.spent += 1 + return True + + +def reconcile_repo(repo, owner, gh, shell, opts, policy, workdir, budget=None): """(status, note). Raises on errors; the caller records them.""" name, default = repo["full_name"], repo["default_branch"] + budget = budget if budget is not None else Budget(0) if not default or default == BRANCH: return "skipped:default-branch-is-the-reconciler-branch", "" dest = os.path.join(workdir, name.replace("/", "__")) @@ -160,6 +175,12 @@ def reconcile_repo(repo, owner, gh, shell, opts, policy, workdir): lease = branch_head(gh, name) if lease: return "skipped:branch-without-open-pr", f"`{BRANCH}` exists with no open PR; delete it to re-propose" + if not budget.take(): + # Only a NEW pull request spends the budget. The branches above return before this, + # so a repository that conforms, is already up to date, or is blocked for another + # reason keeps its own status rather than being reported as deferred. + return "deferred:rate-limit", (f"{', '.join(changed)} -- budget of {budget.limit} new " + "pull requests spent; the next run takes this one") status = "opened" if opts.apply else "would-open" if not opts.apply: @@ -219,21 +240,11 @@ def main(argv=None, gh=None, shell=None): errors = 0 workdir = tempfile.mkdtemp(prefix="dependabot-reconcile-") try: - opened = 0 + budget = Budget(opts.max_new) for repo in active: try: - if opts.max_new and opened >= opts.max_new: - # The budget is for NEW pull requests. A repository whose pull request is - # already open still gets refreshed, because leaving a stale one open is - # worse than opening nothing. - if not open_pr(gh, repo["full_name"], opts.owner): - rows.append((repo["full_name"], "deferred:rate-limit", - f"budget of {opts.max_new} new pull requests spent; " - "the next run takes this one")) - continue - status, note = reconcile_repo(repo, opts.owner, gh, shell, opts, policy, workdir) - if status in ("opened", "would-open"): - opened += 1 + status, note = reconcile_repo(repo, opts.owner, gh, shell, opts, policy, workdir, + budget) except Exception as exc: # one repository's failure must not stop the others errors += 1 status, note = "error", str(exc)[:300] diff --git a/dependabot/tests/test_reconcile.py b/dependabot/tests/test_reconcile.py index 05cb8c7..5a0b8a7 100644 --- a/dependabot/tests/test_reconcile.py +++ b/dependabot/tests/test_reconcile.py @@ -263,6 +263,22 @@ def test_no_budget_means_no_limit(self): self.assertEqual(code, 0) self.assertEqual(len([r for r in gh.runs if r[1] == "create"]), 3) + def test_the_budget_does_not_mislabel_a_repository_it_never_reached(self): + # The budget was first spent before the work, so a conforming repository came back + # `deferred` without anyone having looked at it -- laranail read 63 deferred where + # only 64 of 77 drift. Only a NEW pull request may spend it. + routes = self.base_routes() + routes["orgs/simtabi/repos"] = [repo("d1"), repo("d2"), repo("conforming")] + captured = [] + orig = common.write_summary + common.write_summary = captured.append + try: + self.run_main(routes, {"d1": 1, "d2": 1, "conforming": 0}, "--apply", "--max-new", "1") + finally: + common.write_summary = orig + self.assertRegex(captured[0], r"\| simtabi/conforming \| conforms \|") + self.assertEqual(captured[0].count("deferred:rate-limit"), 1) + def test_the_rewrite_uses_render_not_check(self): # `check --write` creates or appends; it does not rewrite a file that is merely # wrong, which is what drift is. Measured 2026-11-02: every repository sampled came From 3b8a33f9f25eec391c90f10e7e9d7c72c8a94556 Mon Sep 17 00:00:00 2001 From: Imani Manyara Date: Mon, 2 Nov 2026 14:21:51 -0500 Subject: [PATCH 3/3] Record where the budget is spent, and why not earlier The first placement checked it before the work, so a conforming repository read `deferred` with nobody having looked at it. laranail's real dry run showed 63 deferred where only 64 of 77 drift. --- dependabot/README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dependabot/README.md b/dependabot/README.md index b2b4a2e..2f02f0c 100644 --- a/dependabot/README.md +++ b/dependabot/README.md @@ -98,6 +98,11 @@ Both workflows ship as dry runs. > the limit. A repository whose pull request is **already open** is still refreshed past the > budget -- leaving a stale one open is worse than opening nothing. > +> The budget is spent at the moment a **new** pull request is decided, not before the work. +> Spending it earlier is cheaper and misreports: a conforming repository came back `deferred` +> with nobody having looked at it, and laranail's summary read 63 deferred when only 64 of its +> 77 active repositories drift at all. +> > A repository held back reads `deferred:rate-limit`, never `skipped:`, and the summary says > how many were deferred. That distinction is load-bearing: `skipped` means a decision was > taken about that repository, and a budget says nothing about it. Without it a capped run