From 850bb00e5df3d59f32963d32871bb3abfcefe59c Mon Sep 17 00:00:00 2001 From: Shahin Saadati Date: Wed, 5 Aug 2026 10:15:36 -0700 Subject: [PATCH 1/3] refactor(ci): make dependabot.yml static and glob-based MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes the requirement that contributors regenerate .github/dependabot.yml when they add a recipe. That file enumerated one entry per directory and was produced by a generator nobody ran automatically. Adding a recipe meant running it and committing the result in the same PR, enforced by a CI gate. The requirement appeared in no contributor-facing doc — not CONTRIBUTING.md, not docs/recipe-checklist.md, not the recipe handbook — and `uv run validate` could not catch it, so the first anyone heard of it was a red X. #2473 hit exactly this after running every check the checklist asks for. Dependabot supports globs natively, so the enumeration was never necessary: directories: ["/core/**/*", "/contrib/**/*", "/skills/**/*"] All 21 generated entries were byte-identical apart from the path, so nothing configurable is lost. 499 lines become 133, and adding a recipe now requires no change here at all. Removed with it: generate_dependabot.py, the "Check dependabot.yml is up to date" gate, and the Sync Dependabot Config workflow. The orphan-PR cleanup that workflow also performed is still needed — Dependabot never closes PRs for a directory that disappeared — so it moves to a scheduled dependabot-housekeeping.yml. That job needs `contents: write`, since `--delete-branch` is a git-ref write, and sets `persist-credentials: false` because the script only ever talks to the API via `gh`. gomod, maven, and gradle are deliberately NOT declared. No Go, Java, or Kotlin recipe exists yet, and a glob matching zero directories is the likeliest way to get the whole file rejected — which would silently stop every dependency update, security ones included. The omission is guarded, not silent: test_dependabot_config.py fails the first such recipe with an explicit instruction to add its block. What cannot be verified before merge ------------------------------------ Dependabot only reads this file from the default branch, so its real behaviour is unobservable until then. Two things to check at /network/updates within minutes of merging: - all 20 manifest directories are listed (i.e. the globs resolved), and - `open-pull-requests-limit` is per-entry, not per-directory — 25 assumes per-entry. The patterns are validated as far as they can be offline: test_dependabot_config.py checks every directory the scanner finds is matched by a glob, that glob roots equal recipe_manifests.SCAN_ROOTS (a mismatch would make the cleanup delete a live root's PRs), that retired roots are not globbed, and that limits cover the directory count. The matcher itself is pinned with negative cases so those tests cannot pass vacuously. The config also validates against the published dependabot-2.0 JSON schema. Rollback is `git revert` of this commit; Dependabot re-reads on the next push to main. Note for the first run: the housekeeping job will close 19 long-stale PRs, all targeting retired python/agents/ roots. Verified by dry run. Dispatch it manually with dry_run first if you want to eyeball the list. --- .github/dependabot.yml | 502 +++--------------- .github/policy.yml | 7 +- .../scripts/close_orphan_dependabot_prs.py | 24 +- .github/scripts/generate_dependabot.py | 210 -------- .../scripts/tests/test_dependabot_config.py | 277 ++++++++++ .github/workflows/dependabot-auto-merge.yml | 8 +- .github/workflows/dependabot-housekeeping.yml | 87 +++ .github/workflows/sync-dependabot-config.yml | 112 ---- .../workflows/validate-recipe-structure.yml | 37 +- 9 files changed, 459 insertions(+), 805 deletions(-) delete mode 100644 .github/scripts/generate_dependabot.py create mode 100644 .github/scripts/tests/test_dependabot_config.py create mode 100644 .github/workflows/dependabot-housekeeping.yml delete mode 100644 .github/workflows/sync-dependabot-config.yml diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 206c645d8d..ae0a82da3a 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -1,206 +1,48 @@ -# GENERATED FILE — every line below is written by -# .github/scripts/generate_dependabot.py. Do not edit it by hand. +# Dependabot configuration. # -# Nothing triggers that script for you — there is no push hook, schedule, or -# bot that keeps this in sync. Whoever adds or removes a recipe has to run it -# and commit the result in the same pull request: +# This file is STATIC and hand-maintained. Adding, removing, or renaming a +# recipe requires NO change here — Dependabot resolves the glob patterns +# below against the tree on every run and discovers manifests itself. # -# python .github/scripts/generate_dependabot.py +# It did not always work that way. This file used to enumerate one entry per +# directory and was produced by a generator that contributors had to remember +# to re-run, with a CI gate failing their PR when they forgot. The requirement +# appeared in no contributor-facing doc and `uv run validate` could not catch +# it, so the first anyone heard of it was a red X on their PR. All 21 +# generated entries were byte-identical apart from the path, so the globs +# below lose no configurability. # -# CI enforces this — the "Check dependabot.yml is up to date" job fails the -# PR if this file no longer matches the recipe tree. +# What to change here, and when: +# - A new recipe, anywhere under core/, contrib/, or skills/ -> nothing. +# - A new top-level recipe root -> add it to +# every `directories` list below, and to SCAN_ROOTS in +# .github/scripts/recipe_manifests.py. +# - A new language whose ecosystem is not listed below -> add an +# entry for it. # -# Ecosystems tracked: uv (Python), gomod (Go), maven (Java), -# gradle (Kotlin), npm (TypeScript/JS), github-actions +# Ecosystem/manifest mapping (mirrored by the detectors in +# .github/scripts/recipe_manifests.py, which the orphan-PR cleanup uses): +# uv Python uv.lock +# npm TS/JS package.json with dependencies or devDependencies +# gomod Go go.mod +# maven Java pom.xml +# gradle Kotlin build.gradle.kts + settings.gradle.kts +# +# `**/*` is globstar: the directory layer below the root and everything +# beneath it, recursively. Recursion is required, not cosmetic — manifests sit +# at four different depths today, from `core/rag-vector-search` (a legacy flat +# path) down to `core/python/long-horizon-harness/web/server`. +# +# Retired roots (`frozen_paths` in .github/policy.yml: python/agents, +# java/agents, ...) are deliberately not globbed. They are closed to new work. version: 2 updates: - - package-ecosystem: "npm" - directory: "/core/python/deep-search/frontend" - schedule: - interval: weekly - day: monday - time: "02:00" - commit-message: - prefix: chore - include: scope - labels: - - dependencies - open-pull-requests-limit: 1 - # Wait a few days after a release before opening a PR, so the community - # catches broken releases before we auto-merge them. Security updates - # bypass this cooldown and fire immediately. - cooldown: - default-days: 7 - semver-major-days: 14 - groups: - all-dependencies: - patterns: - - "*" - - - package-ecosystem: "npm" - directory: "/core/python/genmedia-for-commerce/frontend" - schedule: - interval: weekly - day: monday - time: "02:00" - commit-message: - prefix: chore - include: scope - labels: - - dependencies - open-pull-requests-limit: 1 - # Wait a few days after a release before opening a PR, so the community - # catches broken releases before we auto-merge them. Security updates - # bypass this cooldown and fire immediately. - cooldown: - default-days: 7 - semver-major-days: 14 - groups: - all-dependencies: - patterns: - - "*" - - - package-ecosystem: "npm" - directory: "/core/python/long-horizon-harness/web" - schedule: - interval: weekly - day: monday - time: "02:00" - commit-message: - prefix: chore - include: scope - labels: - - dependencies - open-pull-requests-limit: 1 - # Wait a few days after a release before opening a PR, so the community - # catches broken releases before we auto-merge them. Security updates - # bypass this cooldown and fire immediately. - cooldown: - default-days: 7 - semver-major-days: 14 - groups: - all-dependencies: - patterns: - - "*" - - - package-ecosystem: "npm" - directory: "/core/python/long-horizon-harness/web/server" - schedule: - interval: weekly - day: monday - time: "02:00" - commit-message: - prefix: chore - include: scope - labels: - - dependencies - open-pull-requests-limit: 1 - # Wait a few days after a release before opening a PR, so the community - # catches broken releases before we auto-merge them. Security updates - # bypass this cooldown and fire immediately. - cooldown: - default-days: 7 - semver-major-days: 14 - groups: - all-dependencies: - patterns: - - "*" - - - package-ecosystem: "uv" - directory: "/contrib/python/financial-advisor" - schedule: - interval: weekly - day: monday - time: "02:00" - commit-message: - prefix: chore - include: scope - labels: - - dependencies - open-pull-requests-limit: 1 - # Wait a few days after a release before opening a PR, so the community - # catches broken releases before we auto-merge them. Security updates - # bypass this cooldown and fire immediately. - cooldown: - default-days: 7 - semver-major-days: 14 - groups: - all-dependencies: - patterns: - - "*" - - - package-ecosystem: "uv" - directory: "/contrib/python/market-research-agent" - schedule: - interval: weekly - day: monday - time: "02:00" - commit-message: - prefix: chore - include: scope - labels: - - dependencies - open-pull-requests-limit: 1 - # Wait a few days after a release before opening a PR, so the community - # catches broken releases before we auto-merge them. Security updates - # bypass this cooldown and fire immediately. - cooldown: - default-days: 7 - semver-major-days: 14 - groups: - all-dependencies: - patterns: - - "*" - - - package-ecosystem: "uv" - directory: "/core/python/ambient-expense-agent" - schedule: - interval: weekly - day: monday - time: "02:00" - commit-message: - prefix: chore - include: scope - labels: - - dependencies - open-pull-requests-limit: 1 - # Wait a few days after a release before opening a PR, so the community - # catches broken releases before we auto-merge them. Security updates - # bypass this cooldown and fire immediately. - cooldown: - default-days: 7 - semver-major-days: 14 - groups: - all-dependencies: - patterns: - - "*" - - - package-ecosystem: "uv" - directory: "/core/python/ambient-expense-agent/frontend" - schedule: - interval: weekly - day: monday - time: "02:00" - commit-message: - prefix: chore - include: scope - labels: - - dependencies - open-pull-requests-limit: 1 - # Wait a few days after a release before opening a PR, so the community - # catches broken releases before we auto-merge them. Security updates - # bypass this cooldown and fire immediately. - cooldown: - default-days: 7 - semver-major-days: 14 - groups: - all-dependencies: - patterns: - - "*" - - package-ecosystem: "uv" - directory: "/core/python/cross-session-memory" + directories: + - "/core/**/*" + - "/contrib/**/*" + - "/skills/**/*" schedule: interval: weekly day: monday @@ -210,227 +52,30 @@ updates: include: scope labels: - dependencies - open-pull-requests-limit: 1 + # Was 1 per directory across 16 enumerated entries. A single entry now + # covers every directory, so the cap is raised to keep throughput + # comparable. + open-pull-requests-limit: 25 # Wait a few days after a release before opening a PR, so the community # catches broken releases before we auto-merge them. Security updates # bypass this cooldown and fire immediately. cooldown: default-days: 7 semver-major-days: 14 + # No `group-by` — without it Dependabot opens one PR per directory, which + # preserves the previous one-PR-per-recipe behaviour. Adding + # `group-by: dependency-name` would collapse a bump across every recipe + # into a single repo-wide PR. groups: all-dependencies: patterns: - "*" - - package-ecosystem: "uv" - directory: "/core/python/deep-search" - schedule: - interval: weekly - day: monday - time: "02:00" - commit-message: - prefix: chore - include: scope - labels: - - dependencies - open-pull-requests-limit: 1 - # Wait a few days after a release before opening a PR, so the community - # catches broken releases before we auto-merge them. Security updates - # bypass this cooldown and fire immediately. - cooldown: - default-days: 7 - semver-major-days: 14 - groups: - all-dependencies: - patterns: - - "*" - - - package-ecosystem: "uv" - directory: "/core/python/genmedia-for-commerce" - schedule: - interval: weekly - day: monday - time: "02:00" - commit-message: - prefix: chore - include: scope - labels: - - dependencies - open-pull-requests-limit: 1 - # Wait a few days after a release before opening a PR, so the community - # catches broken releases before we auto-merge them. Security updates - # bypass this cooldown and fire immediately. - cooldown: - default-days: 7 - semver-major-days: 14 - groups: - all-dependencies: - patterns: - - "*" - - - package-ecosystem: "uv" - directory: "/core/python/long-horizon-harness" - schedule: - interval: weekly - day: monday - time: "02:00" - commit-message: - prefix: chore - include: scope - labels: - - dependencies - open-pull-requests-limit: 1 - # Wait a few days after a release before opening a PR, so the community - # catches broken releases before we auto-merge them. Security updates - # bypass this cooldown and fire immediately. - cooldown: - default-days: 7 - semver-major-days: 14 - groups: - all-dependencies: - patterns: - - "*" - - - package-ecosystem: "uv" - directory: "/core/python/oauth-user-consent-flow" - schedule: - interval: weekly - day: monday - time: "02:00" - commit-message: - prefix: chore - include: scope - labels: - - dependencies - open-pull-requests-limit: 1 - # Wait a few days after a release before opening a PR, so the community - # catches broken releases before we auto-merge them. Security updates - # bypass this cooldown and fire immediately. - cooldown: - default-days: 7 - semver-major-days: 14 - groups: - all-dependencies: - patterns: - - "*" - - - package-ecosystem: "uv" - directory: "/core/python/rag-agent-search" - schedule: - interval: weekly - day: monday - time: "02:00" - commit-message: - prefix: chore - include: scope - labels: - - dependencies - open-pull-requests-limit: 1 - # Wait a few days after a release before opening a PR, so the community - # catches broken releases before we auto-merge them. Security updates - # bypass this cooldown and fire immediately. - cooldown: - default-days: 7 - semver-major-days: 14 - groups: - all-dependencies: - patterns: - - "*" - - - package-ecosystem: "uv" - directory: "/core/python/rag-vector-search" - schedule: - interval: weekly - day: monday - time: "02:00" - commit-message: - prefix: chore - include: scope - labels: - - dependencies - open-pull-requests-limit: 1 - # Wait a few days after a release before opening a PR, so the community - # catches broken releases before we auto-merge them. Security updates - # bypass this cooldown and fire immediately. - cooldown: - default-days: 7 - semver-major-days: 14 - groups: - all-dependencies: - patterns: - - "*" - - - package-ecosystem: "uv" - directory: "/core/python/rag-vector-search/data_ingestion" - schedule: - interval: weekly - day: monday - time: "02:00" - commit-message: - prefix: chore - include: scope - labels: - - dependencies - open-pull-requests-limit: 1 - # Wait a few days after a release before opening a PR, so the community - # catches broken releases before we auto-merge them. Security updates - # bypass this cooldown and fire immediately. - cooldown: - default-days: 7 - semver-major-days: 14 - groups: - all-dependencies: - patterns: - - "*" - - - package-ecosystem: "uv" - directory: "/core/python/safety-plugins" - schedule: - interval: weekly - day: monday - time: "02:00" - commit-message: - prefix: chore - include: scope - labels: - - dependencies - open-pull-requests-limit: 1 - # Wait a few days after a release before opening a PR, so the community - # catches broken releases before we auto-merge them. Security updates - # bypass this cooldown and fire immediately. - cooldown: - default-days: 7 - semver-major-days: 14 - groups: - all-dependencies: - patterns: - - "*" - - - package-ecosystem: "uv" - directory: "/core/rag-agent-search" - schedule: - interval: weekly - day: monday - time: "02:00" - commit-message: - prefix: chore - include: scope - labels: - - dependencies - open-pull-requests-limit: 1 - # Wait a few days after a release before opening a PR, so the community - # catches broken releases before we auto-merge them. Security updates - # bypass this cooldown and fire immediately. - cooldown: - default-days: 7 - semver-major-days: 14 - groups: - all-dependencies: - patterns: - - "*" - - - package-ecosystem: "uv" - directory: "/core/rag-vector-search" + - package-ecosystem: "npm" + directories: + - "/core/**/*" + - "/contrib/**/*" + - "/skills/**/*" schedule: interval: weekly day: monday @@ -440,10 +85,7 @@ updates: include: scope labels: - dependencies - open-pull-requests-limit: 1 - # Wait a few days after a release before opening a PR, so the community - # catches broken releases before we auto-merge them. Security updates - # bypass this cooldown and fire immediately. + open-pull-requests-limit: 10 cooldown: default-days: 7 semver-major-days: 14 @@ -452,29 +94,21 @@ updates: patterns: - "*" - - package-ecosystem: "uv" - directory: "/core/rag-vector-search/data_ingestion" - schedule: - interval: weekly - day: monday - time: "02:00" - commit-message: - prefix: chore - include: scope - labels: - - dependencies - open-pull-requests-limit: 1 - # Wait a few days after a release before opening a PR, so the community - # catches broken releases before we auto-merge them. Security updates - # bypass this cooldown and fire immediately. - cooldown: - default-days: 7 - semver-major-days: 14 - groups: - all-dependencies: - patterns: - - "*" + # gomod / maven / gradle are deliberately NOT declared. No Go, Java, or + # Kotlin recipe exists yet, and a glob matching zero directories is the + # likeliest way to get this whole file rejected — which would silently stop + # every dependency update, security ones included. That is not a risk worth + # taking for languages the repo does not yet host. + # + # This is not a silent gap. recipe_manifests.py still detects all five + # ecosystems, and test_dependabot_config.py asserts that every ecosystem + # found in the tree has an entry here — so the first Go/Java/Kotlin recipe + # fails CI with a message telling you to add its block, rather than merging + # green and quietly receiving no updates. + # Not a recipe ecosystem: Dependabot reads /.github/workflows and any + # action.yml at the repo root. `directory: "/"` is what GitHub documents for + # github-actions; it is not a glob. - package-ecosystem: "github-actions" directory: "/" schedule: @@ -488,9 +122,9 @@ updates: - dependencies - github-actions open-pull-requests-limit: 1 - # Wait a few days after a release before opening a PR, so the community - # catches broken releases before we auto-merge them. Security updates - # bypass this cooldown and fire immediately. + # `semver-major-days` is invalid for github-actions — its releases (v1, + # v2, ...) are not semver, and Dependabot rejects the whole config if it + # appears here. cooldown: default-days: 7 groups: diff --git a/.github/policy.yml b/.github/policy.yml index e7b3bd0bad..4d5adf6f3b 100644 --- a/.github/policy.yml +++ b/.github/policy.yml @@ -250,9 +250,10 @@ required_files: # Every Go recipe is its own module. go.mod is required for three # reasons, in increasing order of subtlety: # 1. Without it the recipe cannot be built, tested, or versioned. - # 2. .github/scripts/generate_dependabot.py detects Go recipes by - # the presence of go.mod (`_is_gomod`), so a recipe without one - # silently receives no dependency updates. + # 2. Dependabot's `gomod` ecosystem keys off go.mod, so a recipe + # without one silently receives no dependency updates. The orphan + # cleanup in .github/scripts/recipe_manifests.py detects Go + # recipes the same way (`_is_gomod`). # 3. .github/workflows/go-format.yml groups changed files by their # owning go.mod and SKIPS any file it cannot attribute to a # module — because gci's `localmodule` import section reads diff --git a/.github/scripts/close_orphan_dependabot_prs.py b/.github/scripts/close_orphan_dependabot_prs.py index 250ae1f73e..6db9b9068d 100755 --- a/.github/scripts/close_orphan_dependabot_prs.py +++ b/.github/scripts/close_orphan_dependabot_prs.py @@ -10,25 +10,25 @@ Where the live set comes from ----------------------------- -The recipe tree, via recipe_manifests.scan() — the same scanner -generate_dependabot.py uses to decide what goes INTO dependabot.yml. +The tree, via recipe_manifests.scan() — NOT .github/dependabot.yml. -This script used to recover the set by regex-scanning the enumerated -`directory:` keys back out of that generated file. Reading the shared scanner -directly is both simpler and safer: rediscovering by parsing meant a -formatting change to the generator's output could silently empty this set, -and an empty set makes every open Dependabot PR look orphaned. +This script used to recover the set by regex-scanning enumerated `directory:` +keys out of dependabot.yml. That file now uses glob patterns (`directories:`) +so that adding a recipe requires no config change, which leaves nothing to +parse: the old parser would have found zero pairs, judged every open +Dependabot PR an orphan, and closed them all with --delete-branch. -It also stops assuming dependabot.yml enumerates directories at all, which -matters the moment that file moves to glob patterns. +Scanning the tree is also the more accurate source. The globs in +dependabot.yml are resolved by Dependabot against the tree, so the tree is +what actually determines which directories are live. Uses `gh pr close --delete-branch` with no explanatory comment: the GitHub GraphQL `addComment` mutation has an anti-abuse throttle that trips on large batches (observed in practice at ~80 comments in a burst). The close+delete- -branch itself is the audit signal; the workflow log enumerates every closed -PR. +branch itself is the audit signal; the workflow log below enumerates every +closed PR. -Invoked by .github/workflows/sync-dependabot-config.yml. +Invoked by .github/workflows/dependabot-housekeeping.yml. Requires: `gh` on PATH, GITHUB_TOKEN in the environment. diff --git a/.github/scripts/generate_dependabot.py b/.github/scripts/generate_dependabot.py deleted file mode 100644 index ca6b455f6f..0000000000 --- a/.github/scripts/generate_dependabot.py +++ /dev/null @@ -1,210 +0,0 @@ -#!/usr/bin/env python3 -""" -Auto-generates .github/dependabot.yml by scanning core/, contrib/, and skills/ -for language-specific manifest files across all 5 ADK languages. - -Manifest detection lives in recipe_manifests.py, shared with -close_orphan_dependabot_prs.py — see that module for the ecosystem/manifest -mapping. This script only renders what the scanner finds. One copy matters: -if the generator and the orphan cleanup disagreed about which directories are -dependency-managed, the cleanup would close PRs for directories the generator -had just declared live. - -Usage ------ - python .github/scripts/generate_dependabot.py # writes in-place - python .github/scripts/generate_dependabot.py --check # exit 1 if stale - -Who runs this -------------- -There is no automatic regeneration on push. main is protected, so the -github-actions bot cannot commit to it. Instead: - - 1. You add or remove a recipe — run this script and commit the updated - .github/dependabot.yml in the SAME pull request. The freshness gate in - .github/workflows/validate-recipe-structure.yml fails your PR otherwise. - 2. A maintainer can run the "Sync Dependabot Config" workflow manually from - the Actions tab to catch any drift that slipped through; it opens a small - PR with the regenerated file. - -DO NOT edit .github/dependabot.yml by hand — this script owns it. -""" - -from __future__ import annotations - -import argparse -import difflib -import sys -from pathlib import Path - -import recipe_manifests - -# --------------------------------------------------------------------------- -# Paths -# --------------------------------------------------------------------------- - -REPO_ROOT = Path(__file__).resolve().parent.parent.parent -OUTPUT_FILE = REPO_ROOT / ".github" / "dependabot.yml" - -# --------------------------------------------------------------------------- -# Scanner -# --------------------------------------------------------------------------- - - -def scan(repo_root: Path) -> list[dict]: - """Return one entry dict per detected (ecosystem, directory) pair. - - Detection lives in recipe_manifests.py, which is shared with - close_orphan_dependabot_prs.py. Keeping one copy matters: if the two ever - disagreed about which directories are dependency-managed, the cleanup - would close PRs for directories this generator had just declared live. - """ - return [ - {"package-ecosystem": ecosystem, "directory": directory} - for ecosystem, directory in recipe_manifests.scan(repo_root) - ] - - -# --------------------------------------------------------------------------- -# YAML renderer -# Keeps zero runtime dependencies — uses only stdlib. -# --------------------------------------------------------------------------- - - -def _render_entry( - ecosystem: str, directory: str, *, extra_labels: list[str] | None = None -) -> str: - labels_lines = " - dependencies" - if extra_labels: - labels_lines += "\n" + "\n".join(f" - {lb}" for lb in extra_labels) - - group_name = ( - "all-actions" if ecosystem == "github-actions" else "all-dependencies" - ) - - # `semver-major-days` is only valid for ecosystems that follow semver. - # github-actions releases (v1, v2, ...) don't, and Dependabot rejects the - # whole config if it appears there. - if ecosystem == "github-actions": - cooldown_lines = " default-days: 7" - else: - cooldown_lines = " default-days: 7\n semver-major-days: 14" - - return f"""\ - - package-ecosystem: "{ecosystem}" - directory: "{directory}" - schedule: - interval: weekly - day: monday - time: "02:00" - commit-message: - prefix: chore - include: scope - labels: -{labels_lines} - open-pull-requests-limit: 1 - # Wait a few days after a release before opening a PR, so the community - # catches broken releases before we auto-merge them. Security updates - # bypass this cooldown and fire immediately. - cooldown: -{cooldown_lines} - groups: - {group_name}: - patterns: - - "*" -""" - - -def render(entries: list[dict]) -> str: - header = """\ -# GENERATED FILE — every line below is written by -# .github/scripts/generate_dependabot.py. Do not edit it by hand. -# -# Nothing triggers that script for you — there is no push hook, schedule, or -# bot that keeps this in sync. Whoever adds or removes a recipe has to run it -# and commit the result in the same pull request: -# -# python .github/scripts/generate_dependabot.py -# -# CI enforces this — the "Check dependabot.yml is up to date" job fails the -# PR if this file no longer matches the recipe tree. -# -# Ecosystems tracked: uv (Python), gomod (Go), maven (Java), -# gradle (Kotlin), npm (TypeScript/JS), github-actions - -version: 2 -updates: -""" - body_parts = [ - _render_entry(e["package-ecosystem"], e["directory"]) for e in entries - ] - # Entries configured unconditionally rather than discovered. They live in - # recipe_manifests so close_orphan_dependabot_prs.py recognises exactly - # this set; defining them here alone would leave the cleanup treating - # their PRs as orphans and closing them with --delete-branch. - body_parts.extend( - _render_entry(ecosystem, directory, extra_labels=extra_labels) - for ecosystem, directory, extra_labels in ( - recipe_manifests.STATIC_ENTRIES - ) - ) - return header + "\n".join(body_parts) - - -# --------------------------------------------------------------------------- -# Main -# --------------------------------------------------------------------------- - - -def main(argv: list[str] | None = None) -> int: - parser = argparse.ArgumentParser( - description="Generate .github/dependabot.yml" - ) - parser.add_argument( - "--check", - action="store_true", - help="Exit 1 if dependabot.yml is stale (CI mode); do not write.", - ) - args = parser.parse_args(argv) - - entries = scan(REPO_ROOT) - content = render(entries) - - current = ( - OUTPUT_FILE.read_text(encoding="utf-8") if OUTPUT_FILE.is_file() else "" - ) - - if content == current: - print(f"dependabot.yml is up to date ({len(entries)} recipe entries).") - return 0 - - if args.check: - print("dependabot.yml is STALE — it does not match the recipe tree.") - print("") - print("Fix it by running this from the repo root and committing the") - print("result in this same pull request:") - print("") - print(" python .github/scripts/generate_dependabot.py") - print("") - print("Missing (+) and stale (-) lines:") - print("") - diff = difflib.unified_diff( - current.splitlines(keepends=True), - content.splitlines(keepends=True), - fromfile="committed .github/dependabot.yml", - tofile="expected from the current recipe tree", - ) - sys.stdout.writelines(diff) - return 1 - - OUTPUT_FILE.write_text(content, encoding="utf-8") - print( - f"dependabot.yml written: {len(entries)} recipe entries + github-actions." - ) - for e in entries: - print(f" [{e['package-ecosystem']:8s}] {e['directory']}") - return 0 - - -if __name__ == "__main__": - sys.exit(main()) diff --git a/.github/scripts/tests/test_dependabot_config.py b/.github/scripts/tests/test_dependabot_config.py new file mode 100644 index 0000000000..3280e2e029 --- /dev/null +++ b/.github/scripts/tests/test_dependabot_config.py @@ -0,0 +1,277 @@ +# Copyright 2026 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Consistency tests for .github/dependabot.yml. + +That file is static and glob-based, so nothing regenerates it and no CI gate +compares it to the tree. Two things could therefore drift unnoticed: + + 1. The globs stop covering directories that actually hold manifests — for + example someone adds a recipe root, or narrows a pattern in a way that + silently drops recursion. Recipes then receive no dependency updates, + and nothing says so. + 2. The globs and recipe_manifests.SCAN_ROOTS disagree about which roots + exist. That one is worse than it looks: the orphan cleanup treats + anything outside SCAN_ROOTS as dead, so a root present in the config but + missing from SCAN_ROOTS means its PRs get closed with --delete-branch. + +These tests pin both. They cannot verify Dependabot's own glob implementation +— only a merge to the default branch can — so they check the patterns under +standard globstar semantics, which is what the GitHub documentation describes. +""" + +from pathlib import Path + +import pytest +import recipe_manifests as rm +import yaml + +CONFIG_PATH = Path(__file__).resolve().parent.parent.parent / "dependabot.yml" + + +@pytest.fixture(scope="module") +def config() -> dict: + return yaml.safe_load(CONFIG_PATH.read_text(encoding="utf-8")) + + +@pytest.fixture(scope="module") +def globbed_entries(config) -> list[dict]: + """Entries that use `directories` (i.e. everything but github-actions).""" + return [u for u in config["updates"] if "directories" in u] + + +def _segments_match(pattern: list[str], path: list[str]) -> bool: + """Standard globstar matching over path segments. + + `**` matches zero or more segments; `*` matches exactly one, and never + spans a separator. + + Implemented rather than delegated to Path.glob() because expanding + `/core/**/*` against the working tree walks .venv and node_modules and + takes minutes. Matching the scanner's output against the pattern is the + same question asked in the cheap direction. + """ + if not pattern: + return not path + head, rest = pattern[0], pattern[1:] + if head == "**": + # Zero segments, or consume one and retry. + if _segments_match(rest, path): + return True + return bool(path) and _segments_match(pattern, path[1:]) + if not path: + return False + if head != "*" and head != path[0]: + return False + return _segments_match(rest, path[1:]) + + +def _matches_any(patterns: list[str], directory: str) -> bool: + segs = directory.strip("/").split("/") + return any(_segments_match(p.strip("/").split("/"), segs) for p in patterns) + + +# --------------------------------------------------------------------------- +# The matcher itself +# +# Every config test below depends on _segments_match. A matcher that returned +# True unconditionally would make all of them pass while proving nothing, so +# it is pinned first — negative cases included. +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + ("pattern", "directory", "expected"), + [ + # The depth question the globs hinge on: does `/core/**/*` reach a + # directory only one level below the root? Under standard globstar + # `**` matches zero segments, so yes. + ("/core/**/*", "/core/rag-vector-search", True), + ("/core/**/*", "/core/python/deep-search", True), + ("/core/**/*", "/core/python/rag/data_ingestion", True), + ("/core/**/*", "/core/python/lhh/web/server", True), + # The root itself holds no manifest and should not match. + ("/core/**/*", "/core", False), + # Other roots must not be caught by this pattern. + ("/core/**/*", "/contrib/python/x", False), + ("/core/**/*", "/skills/retail/x", False), + # Retired roots must not match. + ("/core/**/*", "/python/agents/legacy", False), + # A single `*` never spans a separator. + ("/core/*", "/core/python/deep-search", False), + ("/core/*", "/core/rag-vector-search", True), + # A directory that merely starts with the root name is not the root. + ("/core/**/*", "/coreutils/x", False), + ], +) +def test_segments_match(pattern, directory, expected): + assert _matches_any([pattern], directory) is expected + + +def test_matcher_is_not_vacuously_true(): + assert not _matches_any(["/nope/**/*"], "/core/python/deep-search") + assert not _matches_any([], "/core/python/deep-search") + + +# --------------------------------------------------------------------------- +# The config +# --------------------------------------------------------------------------- + + +def test_config_is_valid_yaml_with_version_2(config): + assert config["version"] == 2 + assert config["updates"] + + +def test_every_detected_manifest_directory_is_covered_by_a_glob(config): + """The core invariant: if the scanner can see a manifest there, the config + must be telling Dependabot to look there.""" + by_eco = {u["package-ecosystem"]: u for u in config["updates"]} + + uncovered = [] + for ecosystem, directory in rm.scan(): + entry = by_eco.get(ecosystem) + assert entry is not None, ( + f"{directory} has a {ecosystem} manifest but dependabot.yml " + f"declares no {ecosystem} entry — it receives no updates" + ) + if not _matches_any(entry["directories"], directory): + uncovered.append((ecosystem, directory)) + + assert not uncovered, ( + "these directories hold manifests but no glob matches them, so they " + f"receive no dependency updates: {uncovered}" + ) + + +def test_globs_reach_every_depth_present_in_the_tree(config): + """Recursion is load-bearing, not decorative. Manifests sit anywhere from + one to four levels below a root (`core/rag-vector-search` up to + `core/python/long-horizon-harness/web/server`), so a pattern that only + matched a fixed depth would silently drop the rest.""" + depths = {d.strip("/").count("/") + 1 for _, d in rm.scan()} + assert len(depths) > 1, ( + "expected manifests at several depths; if the tree flattened, this " + "test no longer proves recursion is exercised" + ) + + by_eco = {u["package-ecosystem"]: u for u in config["updates"]} + for ecosystem, directory in rm.scan(): + patterns = by_eco[ecosystem]["directories"] + assert _matches_any(patterns, directory), ( + f"{directory} (depth {directory.strip('/').count('/') + 1}) is " + f"not matched by {patterns}" + ) + + +def test_glob_roots_and_scan_roots_agree(globbed_entries): + """A root in the config but not in SCAN_ROOTS is actively dangerous: the + orphan cleanup treats anything outside SCAN_ROOTS as dead and closes its + PRs with --delete-branch.""" + for entry in globbed_entries: + roots = {p.strip("/").split("/")[0] for p in entry["directories"]} + assert roots == set(rm.SCAN_ROOTS), ( + f"{entry['package-ecosystem']} globs {sorted(roots)} but " + f"recipe_manifests.SCAN_ROOTS is {sorted(rm.SCAN_ROOTS)}" + ) + + +def test_retired_roots_are_not_globbed(globbed_entries): + """Retired roots (`frozen_paths` in .github/policy.yml) are closed to new + work. Globbing them would resurrect dependency updates for recipes the + repo has deliberately stopped maintaining.""" + policy = yaml.safe_load( + (rm.REPO_ROOT / ".github" / "policy.yml").read_text(encoding="utf-8") + ) + retired = {p.split("/")[0] for p in policy["frozen_paths"]} + for entry in globbed_entries: + roots = {p.strip("/").split("/")[0] for p in entry["directories"]} + assert not (roots & retired), ( + f"{entry['package-ecosystem']} globs retired root(s) " + f"{sorted(roots & retired)}" + ) + + +def test_every_ecosystem_present_in_the_tree_has_a_config_entry(config): + """The guard that lets dependabot.yml safely omit ecosystems it does not + need yet. + + recipe_manifests.py detects five ecosystems; the config declares only + those that have recipes, because a glob matching zero directories risks + the whole file being rejected — which would stop every dependency update, + security ones included. + + This test keeps that from becoming a silent gap: the first Go, Java, or + Kotlin recipe fails here with an explicit instruction, rather than + merging green and quietly receiving no updates. + """ + configured = {u["package-ecosystem"] for u in config["updates"]} + present = {eco for eco, _ in rm.scan()} + missing = present - configured + assert not missing, ( + f"{sorted(missing)} manifests exist in the tree but dependabot.yml " + "declares no entry for them, so those recipes receive no dependency " + "updates. Add a block for each, copying the shape of the `uv` entry." + ) + + +def test_github_actions_entry_uses_a_plain_directory(config): + """GitHub documents `directory: "/"` for github-actions; it is not a glob, + and Dependabot looks in /.github/workflows regardless.""" + entry = next( + u + for u in config["updates"] + if u["package-ecosystem"] == "github-actions" + ) + assert entry["directory"] == "/" + assert "directories" not in entry + + +def test_github_actions_entry_declares_no_semver_cooldown(config): + """github-actions releases (v1, v2, ...) are not semver. Dependabot + rejects the WHOLE config if semver-major-days appears here, which would + silently disable every ecosystem.""" + entry = next( + u + for u in config["updates"] + if u["package-ecosystem"] == "github-actions" + ) + assert "semver-major-days" not in entry.get("cooldown", {}) + + +def test_no_entry_sets_group_by(globbed_entries): + """Without `group-by`, Dependabot opens one PR per directory, matching the + behaviour the previous per-directory config had. Setting + `group-by: dependency-name` would collapse a bump across every recipe into + a single repo-wide PR.""" + for entry in globbed_entries: + for group in entry.get("groups", {}).values(): + assert "group-by" not in group + + +def test_open_pr_limits_leave_room_for_every_directory(config): + """If the limit is enforced per entry rather than per directory, a cap + below the directory count would throttle updates for whichever recipes + happen to lose the race.""" + counts: dict[str, int] = {} + for ecosystem, _ in rm.scan(): + counts[ecosystem] = counts.get(ecosystem, 0) + 1 + + for entry in config["updates"]: + eco = entry["package-ecosystem"] + needed = counts.get(eco, 0) + if needed: + assert entry["open-pull-requests-limit"] >= needed, ( + f"{eco} covers {needed} directories but caps open PRs at " + f"{entry['open-pull-requests-limit']}" + ) diff --git a/.github/workflows/dependabot-auto-merge.yml b/.github/workflows/dependabot-auto-merge.yml index 9490ab7e79..6558f4a3c8 100644 --- a/.github/workflows/dependabot-auto-merge.yml +++ b/.github/workflows/dependabot-auto-merge.yml @@ -40,10 +40,10 @@ on: # zizmor: ignore[dangerous-triggers] - "Validate Recipe Structure" - "🐍 Python Format PR Checks" - "Python Dependency Policy" - # Go. .github/scripts/generate_dependabot.py already ships a live - # `gomod` detector, so Dependabot starts opening Go dependency PRs as - # soon as the first go.mod lands under core/ or contrib/. Without this - # entry those PRs would never be re-evaluated for auto-merge. + # Go. .github/dependabot.yml already declares a globbed `gomod` entry, + # so Dependabot starts opening Go dependency PRs as soon as the first + # go.mod lands under core/, contrib/, or skills/. Without this entry + # those PRs would never be re-evaluated for auto-merge. - "🐹 Go Format PR Checks" types: - completed diff --git a/.github/workflows/dependabot-housekeeping.yml b/.github/workflows/dependabot-housekeeping.yml new file mode 100644 index 0000000000..6090339c2f --- /dev/null +++ b/.github/workflows/dependabot-housekeeping.yml @@ -0,0 +1,87 @@ +name: Dependabot Housekeeping + +# Closes Dependabot PRs whose target directory no longer exists. +# +# When a recipe is removed or renamed, Dependabot stops opening PRs for the +# old directory but never closes the ones already open, so they linger +# indefinitely. This sweeps them up. +# +# Replaces the old "Sync Dependabot Config" workflow, which regenerated +# .github/dependabot.yml and opened a PR with the result. That file is now +# static and glob-based — nothing to regenerate — but the orphan cleanup it +# also performed is still needed, so it survives here on its own schedule. +# +# The cleanup derives the set of live directories by scanning the recipe tree +# (.github/scripts/recipe_manifests.py), not by parsing dependabot.yml, which +# no longer enumerates directories. +# +# Because closing is destructive (`--delete-branch`), the script refuses to +# act when the scan comes back empty, and refuses to close a batch larger than +# its --max-close limit. If this job fails with "exceeds the --max-close +# limit", read the listed PRs before raising it: a large batch means healthy +# PRs were reclassified, not that a lot of recipes were removed. + +on: + schedule: + # Mondays at 04:00 UTC, a couple of hours after the 02:00 Dependabot + # schedule in .github/dependabot.yml. Ordering barely matters — an orphan + # is by definition an old PR for a directory that is gone, so it cannot be + # something Dependabot opened minutes earlier — but running after the + # weekly wave keeps the log for a given week in one place. + - cron: "0 4 * * 1" + workflow_dispatch: + inputs: + dry_run: + description: "List orphans without closing them" + type: boolean + default: false + +permissions: + # `contents: write` is required by `--delete-branch`, not by the checkout: + # deleting a branch is a git-ref write. Without it every close fails with + # "Resource not accessible by integration" after the PR has already been + # closed, leaving the branch behind. + contents: write + pull-requests: write + +concurrency: + group: ${{ github.workflow }} + cancel-in-progress: false + +jobs: + close-orphans: + name: Close orphaned Dependabot PRs + runs-on: ubuntu-latest + timeout-minutes: 10 + + steps: + - name: Checkout repository + uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # ratchet:actions/checkout@v4 + with: + # The whole tree at one commit: the script decides what to close by + # scanning for recipe manifests, so a sparse checkout would + # under-report live directories and make valid PRs look orphaned. + # History is irrelevant, hence depth 1. + fetch-depth: 1 + # This job runs with `contents: write` but never uses git — the + # script talks to the API via `gh` with GITHUB_TOKEN from the + # environment. Leaving a push-capable credential in .git/config + # would be attack surface for nothing. + persist-credentials: false + + - name: Set up Python + uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # ratchet:actions/setup-python@v5 + with: + python-version: "3.11" + + - name: Close orphaned Dependabot PRs + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + DRY_RUN: ${{ github.event.inputs.dry_run }} + run: | + set -euo pipefail + if [ "${DRY_RUN:-false}" = "true" ]; then + python .github/scripts/close_orphan_dependabot_prs.py --dry-run + else + python .github/scripts/close_orphan_dependabot_prs.py + fi diff --git a/.github/workflows/sync-dependabot-config.yml b/.github/workflows/sync-dependabot-config.yml deleted file mode 100644 index 5175e789b7..0000000000 --- a/.github/workflows/sync-dependabot-config.yml +++ /dev/null @@ -1,112 +0,0 @@ -name: Sync Dependabot Config - -# Regenerates .github/dependabot.yml by scanning core/, contrib/, and skills/ -# for all 5 ADK language ecosystems, then opens a PR with the result. -# -# Triggers -# -------- -# workflow_dispatch — manual trigger from the Actions tab -# -# Why not auto-trigger on push to main? -# main is protected (requires PR review + code-owner sign-off + CLA). -# The github-actions[bot] cannot push directly, so auto-triggering would -# fail silently on every recipe add/remove. Instead: -# 1. A contributor adds/removes a recipe and their PR merges to main. -# 2. A maintainer (or CI reminder) runs this workflow manually. -# 3. This workflow opens a small sync PR that a code owner approves. -# -# The PR-time freshness check — the "Check dependabot.yml is up to date" job -# in .github/workflows/validate-recipe-structure.yml — fails the recipe PR -# itself if the contributor forgot to regenerate the file, giving an earlier -# signal before this manual step is needed. That job is what actually keeps -# the file in sync today; this workflow has not yet been exercised, so verify -# it end to end before relying on it. - -on: - workflow_dispatch: - -permissions: - contents: write - pull-requests: write - -jobs: - sync: - name: Regenerate and open PR - runs-on: ubuntu-latest - - steps: - - name: Checkout repository - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # ratchet:actions/checkout@v4 - with: - fetch-depth: 1 - - - name: Set up Python - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # ratchet:actions/setup-python@v5 - with: - python-version: "3.11" - - - name: Generate dependabot.yml - run: python .github/scripts/generate_dependabot.py - - - name: Check for changes - id: diff - run: | - if git diff --quiet .github/dependabot.yml; then - echo "changed=false" >> "$GITHUB_OUTPUT" - echo "dependabot.yml is already up to date — nothing to do." - else - echo "changed=true" >> "$GITHUB_OUTPUT" - echo "dependabot.yml has changes:" - git diff .github/dependabot.yml - fi - - - name: Create branch, commit, and open PR - if: steps.diff.outputs.changed == 'true' - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - set -euo pipefail - - BRANCH="chore/sync-dependabot-config-$(date +%Y%m%d-%H%M%S)" - - git config user.name "github-actions[bot]" - git config user.email "github-actions[bot]@users.noreply.github.com" - - git checkout -b "$BRANCH" - git add .github/dependabot.yml - - # DCO sign-off required by repo policy (web_commit_signoff_required). - git commit \ - -m "chore: sync dependabot config" \ - -m "Auto-generated by the Sync Dependabot Config workflow." \ - -m "Signed-off-by: github-actions[bot] " - - git push origin "$BRANCH" - - gh pr create \ - --title "chore: sync dependabot config" \ - --body "$(cat <<'EOF' - ## Summary - - Auto-generated by the **Sync Dependabot Config** workflow. - - `.github/dependabot.yml` was regenerated from the current state of `core/`, - `contrib/`, and `skills/` using `.github/scripts/generate_dependabot.py`. - - No manual edits -- review the diff to confirm the recipe list looks correct, - then approve and merge. - EOF - )" \ - --base main \ - --head "$BRANCH" \ - --label "dependencies" - - # Belt-and-braces cleanup: on EVERY sync run, close any open Dependabot - # PR whose target directory no longer appears in dependabot.yml. When a - # directory is removed from config Dependabot itself never closes the - # PRs it opened for that directory, so without this step they linger as - # zombies. Cheap when there is nothing to close. - - name: Close orphaned Dependabot PRs - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: python .github/scripts/close_orphan_dependabot_prs.py diff --git a/.github/workflows/validate-recipe-structure.yml b/.github/workflows/validate-recipe-structure.yml index c68c8c18a4..ef288ede58 100644 --- a/.github/workflows/validate-recipe-structure.yml +++ b/.github/workflows/validate-recipe-structure.yml @@ -26,12 +26,6 @@ on: - '.github/schemas/manifest-schema.json' - '.github/policy.yml' - '.github/workflows/validate-recipe-structure.yml' - - '.github/scripts/generate_dependabot.py' - # The detectors the freshness gate depends on live here, not in - # the generator, so a PR touching only this file would otherwise - # change what dependabot.yml should contain without ever running - # the check that says so. - - '.github/scripts/recipe_manifests.py' - 'tools/validate.py' - 'tools/validate_manifest.py' - 'tools/validate_structure.py' @@ -50,12 +44,6 @@ on: - '.github/schemas/manifest-schema.json' - '.github/policy.yml' - '.github/workflows/validate-recipe-structure.yml' - - '.github/scripts/generate_dependabot.py' - # The detectors the freshness gate depends on live here, not in - # the generator, so a PR touching only this file would otherwise - # change what dependabot.yml should contain without ever running - # the check that says so. - - '.github/scripts/recipe_manifests.py' - 'tools/validate.py' - 'tools/validate_manifest.py' - 'tools/validate_structure.py' @@ -278,21 +266,10 @@ jobs: - name: Check every recipe sits at its required path run: uv run python tools/validate_placement.py - check-dependabot-config: - name: Check dependabot.yml is up to date - runs-on: ubuntu-latest - timeout-minutes: 5 - - steps: - - name: Checkout repository - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # ratchet:actions/checkout@v4 - - - name: Set up Python - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # ratchet:actions/setup-python@v5 - with: - python-version: "3.11" - - - name: Check dependabot.yml is fresh - run: | - python .github/scripts/generate_dependabot.py --check - echo "[PASS] dependabot.yml matches the current recipe tree." + # The `check-dependabot-config` job used to live here. It failed a + # contributor's PR when .github/dependabot.yml no longer matched the recipe + # tree, because that file enumerated one entry per directory and had to be + # regenerated by hand on every recipe add or remove. + # + # dependabot.yml is now static and glob-based, so there is nothing to keep + # in sync and nothing to gate. See the header of that file. From 85d38be7f4df02cfea7fbd0938fc15dddbd12914 Mon Sep 17 00:00:00 2001 From: Shahin Saadati Date: Wed, 5 Aug 2026 10:54:38 -0700 Subject: [PATCH 2/3] fix(ci): address review on the dependabot glob cutover MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - dependabot-auto-merge.yml said dependabot.yml "already declares a globbed gomod entry". Stale: the gomod block was dropped from this PR before it was opened, and that claim was the only stated reason for keeping the Go gate in the workflow_run list. Rewritten to say the gate fires on nothing today and why it is listed anyway. - policy.yml implied a go.mod is sufficient for a Go recipe to receive updates. It is necessary but not sufficient right now, since no gomod entry exists; notes that the first Go recipe must add one, and that test_dependabot_config.py enforces it rather than leaving it to memory. - dependabot.yml claimed all 21 generated entries were byte-identical apart from the path. 20 were; the static github-actions entry was never generated from the tree and has always had its own label, group name and cooldown. - tools-tests.yml did not list .github/dependabot.yml in either paths: filter, so a dependabot.yml-only PR would skip the tests that load it — precisely the case they exist for, now that the file is hand-edited. This is the same rationale already documented there for policy.yml and schemas/. - dependabot-housekeeping.yml told the reader to raise --max-close, but the only dispatch input was dry_run and neither branch passed the flag. Adds a max_close input and builds the argument list from both. - dependabot-housekeeping.yml was about to be the only actions/setup-python left in the directory; switched to astral-sh/setup-uv with `uv run --no-project`, matching the other six Python workflows. The script is stdlib-only, so there is nothing to sync. - test_globs_reach_every_depth_present_in_the_tree re-ran the assertion test_every_detected_manifest_directory_is_covered_by_a_glob already makes over the same rm.scan(), and its `depths` set fed nothing. Reduced to the part that was load-bearing: a canary asserting the tree still spans several depths, without which the coverage test would keep passing while silently no longer exercising `**` recursion. - test_no_entry_sets_group_by was named for entries but only inspected groups. An entry-level `group-by` is in fact schema-invalid (additionalProperties: false on an update entry, and the key exists only under groups.), so nothing could slip through — but the name should not have to be checked against the schema to be believed. Now asserts both levels, renamed, with the reasoning written down. --- .github/dependabot.yml | 8 +-- .github/policy.yml | 11 ++-- .../scripts/tests/test_dependabot_config.py | 52 ++++++++++++------- .github/workflows/dependabot-auto-merge.yml | 10 ++-- .github/workflows/dependabot-housekeeping.yml | 37 ++++++++++--- .github/workflows/tools-tests.yml | 16 ++++-- 6 files changed, 93 insertions(+), 41 deletions(-) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index ae0a82da3a..b6a35778b1 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -8,9 +8,11 @@ # directory and was produced by a generator that contributors had to remember # to re-run, with a CI gate failing their PR when they forgot. The requirement # appeared in no contributor-facing doc and `uv run validate` could not catch -# it, so the first anyone heard of it was a red X on their PR. All 21 -# generated entries were byte-identical apart from the path, so the globs -# below lose no configurability. +# it, so the first anyone heard of it was a red X on their PR. All 20 +# generated RECIPE entries were byte-identical apart from the path, so the +# globs below lose no configurability. The static github-actions entry at the +# bottom was never generated from the tree — it has always carried its own +# extra label, group name and cooldown — and is unchanged. # # What to change here, and when: # - A new recipe, anywhere under core/, contrib/, or skills/ -> nothing. diff --git a/.github/policy.yml b/.github/policy.yml index 4d5adf6f3b..e4042fec96 100644 --- a/.github/policy.yml +++ b/.github/policy.yml @@ -251,9 +251,14 @@ required_files: # reasons, in increasing order of subtlety: # 1. Without it the recipe cannot be built, tested, or versioned. # 2. Dependabot's `gomod` ecosystem keys off go.mod, so a recipe - # without one silently receives no dependency updates. The orphan - # cleanup in .github/scripts/recipe_manifests.py detects Go - # recipes the same way (`_is_gomod`). + # without one silently receives no dependency updates. Note that + # go.mod alone is not sufficient today: .github/dependabot.yml + # declares no `gomod` entry yet, so the FIRST Go recipe must add + # one. That is enforced, not left to memory — + # test_dependabot_config.py fails any ecosystem present in the + # tree but missing from the config. The orphan cleanup in + # .github/scripts/recipe_manifests.py detects Go recipes the same + # way (`_is_gomod`). # 3. .github/workflows/go-format.yml groups changed files by their # owning go.mod and SKIPS any file it cannot attribute to a # module — because gci's `localmodule` import section reads diff --git a/.github/scripts/tests/test_dependabot_config.py b/.github/scripts/tests/test_dependabot_config.py index 3280e2e029..e830fe4518 100644 --- a/.github/scripts/tests/test_dependabot_config.py +++ b/.github/scripts/tests/test_dependabot_config.py @@ -154,25 +154,23 @@ def test_every_detected_manifest_directory_is_covered_by_a_glob(config): ) -def test_globs_reach_every_depth_present_in_the_tree(config): - """Recursion is load-bearing, not decorative. Manifests sit anywhere from - one to four levels below a root (`core/rag-vector-search` up to - `core/python/long-horizon-harness/web/server`), so a pattern that only - matched a fixed depth would silently drop the rest.""" +def test_tree_still_spans_several_manifest_depths(): + """Canary for the coverage test above, which only proves the globs reach + every depth that actually EXISTS. Manifests sit one to four levels below + a root today (`core/rag-vector-search` up to + `core/python/long-horizon-harness/web/server`), which is why recursion is + load-bearing rather than decorative. + + If the tree ever flattened to a single depth, the coverage test would + keep passing while silently no longer exercising recursion — so assert + the premise separately rather than re-walking the same scan. + """ depths = {d.strip("/").count("/") + 1 for _, d in rm.scan()} assert len(depths) > 1, ( - "expected manifests at several depths; if the tree flattened, this " - "test no longer proves recursion is exercised" + f"all manifests are now at depth {depths}; the glob-coverage test no " + "longer demonstrates that `**` recursion works" ) - by_eco = {u["package-ecosystem"]: u for u in config["updates"]} - for ecosystem, directory in rm.scan(): - patterns = by_eco[ecosystem]["directories"] - assert _matches_any(patterns, directory), ( - f"{directory} (depth {directory.strip('/').count('/') + 1}) is " - f"not matched by {patterns}" - ) - def test_glob_roots_and_scan_roots_agree(globbed_entries): """A root in the config but not in SCAN_ROOTS is actively dangerous: the @@ -249,14 +247,28 @@ def test_github_actions_entry_declares_no_semver_cooldown(config): assert "semver-major-days" not in entry.get("cooldown", {}) -def test_no_entry_sets_group_by(globbed_entries): +def test_group_by_is_not_set_anywhere(globbed_entries): """Without `group-by`, Dependabot opens one PR per directory, matching the behaviour the previous per-directory config had. Setting - `group-by: dependency-name` would collapse a bump across every recipe into - a single repo-wide PR.""" + `group-by: dependency-name` would collapse a bump across every recipe + into a single repo-wide PR. + + The real home for the key is `groups..group-by`; the schema puts it + nowhere else and sets `additionalProperties: false` on an update entry, + so an entry-level `group-by` is rejected outright. Both levels are + checked anyway, so the test's scope matches its name and a reader does + not have to go and confirm that against the schema. + """ for entry in globbed_entries: - for group in entry.get("groups", {}).values(): - assert "group-by" not in group + assert "group-by" not in entry, ( + f"{entry['package-ecosystem']}: `group-by` at entry level is not " + "valid config and would have Dependabot reject the whole file" + ) + for name, group in entry.get("groups", {}).items(): + assert "group-by" not in group, ( + f"{entry['package-ecosystem']}.groups.{name}: `group-by` " + "collapses every directory's update into one repo-wide PR" + ) def test_open_pr_limits_leave_room_for_every_directory(config): diff --git a/.github/workflows/dependabot-auto-merge.yml b/.github/workflows/dependabot-auto-merge.yml index 6558f4a3c8..8cea5b4c57 100644 --- a/.github/workflows/dependabot-auto-merge.yml +++ b/.github/workflows/dependabot-auto-merge.yml @@ -40,10 +40,12 @@ on: # zizmor: ignore[dangerous-triggers] - "Validate Recipe Structure" - "🐍 Python Format PR Checks" - "Python Dependency Policy" - # Go. .github/dependabot.yml already declares a globbed `gomod` entry, - # so Dependabot starts opening Go dependency PRs as soon as the first - # go.mod lands under core/, contrib/, or skills/. Without this entry - # those PRs would never be re-evaluated for auto-merge. + # Go. Fires on nothing today: no Go recipe exists, and + # .github/dependabot.yml deliberately declares no `gomod` entry (see + # the comment there for why). Listed in advance because adding that + # entry and adding the first go.mod are one change, and forgetting this + # line would leave those PRs never re-evaluated for auto-merge — a + # silent failure rather than a loud one. - "🐹 Go Format PR Checks" types: - completed diff --git a/.github/workflows/dependabot-housekeeping.yml b/.github/workflows/dependabot-housekeeping.yml index 6090339c2f..9114ef7c00 100644 --- a/.github/workflows/dependabot-housekeeping.yml +++ b/.github/workflows/dependabot-housekeeping.yml @@ -18,8 +18,10 @@ name: Dependabot Housekeeping # Because closing is destructive (`--delete-branch`), the script refuses to # act when the scan comes back empty, and refuses to close a batch larger than # its --max-close limit. If this job fails with "exceeds the --max-close -# limit", read the listed PRs before raising it: a large batch means healthy -# PRs were reclassified, not that a lot of recipes were removed. +# limit", read the listed PRs first: a large batch usually means healthy PRs +# were reclassified, not that a lot of recipes were removed. Once you are +# satisfied, re-run it from the Actions tab with the `max_close` input set to +# the reported count. on: schedule: @@ -35,6 +37,13 @@ on: description: "List orphans without closing them" type: boolean default: false + max_close: + description: >- + Max orphans to close in one run. Blank uses the script default. + Raise it only after reviewing a dry run. + type: string + required: false + default: "" permissions: # `contents: write` is required by `--delete-branch`, not by the checkout: @@ -69,19 +78,31 @@ jobs: # would be attack surface for nothing. persist-credentials: false - - name: Set up Python - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # ratchet:actions/setup-python@v5 + # setup-uv rather than setup-python, matching every other Python + # workflow in this directory. `uv run --no-project` is used below: the + # script imports only the standard library plus its sibling + # recipe_manifests, so there is nothing to sync and no lockfile to read. + - name: Install uv + uses: astral-sh/setup-uv@445689ea25e0de0a23313031f5fe577c74ae45a1 # ratchet:astral-sh/setup-uv@v6.3.0 with: + # Minimum supported Python version, as in tools-tests.yml. python-version: "3.11" - name: Close orphaned Dependabot PRs env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - DRY_RUN: ${{ github.event.inputs.dry_run }} + # Both are empty on a schedule run, which is why each is defaulted + # below rather than interpolated straight into the command. + DRY_RUN: ${{ inputs.dry_run }} + MAX_CLOSE: ${{ inputs.max_close }} run: | set -euo pipefail + args=() if [ "${DRY_RUN:-false}" = "true" ]; then - python .github/scripts/close_orphan_dependabot_prs.py --dry-run - else - python .github/scripts/close_orphan_dependabot_prs.py + args+=(--dry-run) fi + if [ -n "${MAX_CLOSE:-}" ]; then + args+=(--max-close "${MAX_CLOSE}") + fi + uv run --no-project python \ + .github/scripts/close_orphan_dependabot_prs.py "${args[@]}" diff --git a/.github/workflows/tools-tests.yml b/.github/workflows/tools-tests.yml index fcd26c9758..9ad9e4049d 100644 --- a/.github/workflows/tools-tests.yml +++ b/.github/workflows/tools-tests.yml @@ -10,22 +10,31 @@ on: - "tools/**" - ".agents/**" - ".github/scripts/**" - # The three .github entries below are not cosmetic. Several tests load - # the REAL repo config rather than a fixture, so editing one of these - # files can break the suite even though nothing under tools/ changed: + # The .github entries below are not cosmetic. Several tests load the + # REAL repo config rather than a fixture, so editing one of these files + # can break the suite even though nothing under tools/ changed: # .github/policy.yml # tools/tests/test_check_frozen_paths.py:: # test_real_policy_file_declares_the_retired_roots calls # load_frozen_paths() with no argument, which falls back to # POLICY_PATH (tools/check_frozen_paths.py:38). + # .github/scripts/tests/test_dependabot_config.py:: + # test_retired_roots_are_not_globbed reads frozen_paths directly. # .github/schemas/manifest-schema.json # test_validate_manifest.py and test_validate_structure.py call # load_schema(), which hardcodes SCHEMA_PATH and takes no override # parameter (tools/validate_manifest.py:40,113). + # .github/dependabot.yml + # .github/scripts/tests/test_dependabot_config.py loads it from + # CONFIG_PATH to check its globs still cover every recipe manifest. + # That file is hand-edited now that no generator owns it, so a + # dependabot.yml-only PR is exactly the case that must run these + # tests. # Without these entries such a PR merges green and the failure only # surfaces later, on an unrelated PR that happens to touch tools/. - ".github/policy.yml" - ".github/schemas/**" + - ".github/dependabot.yml" - "pyproject.toml" - "uv.lock" - ".github/workflows/tools-tests.yml" @@ -41,6 +50,7 @@ on: - ".github/scripts/**" - ".github/policy.yml" - ".github/schemas/**" + - ".github/dependabot.yml" - "pyproject.toml" - "uv.lock" - ".github/workflows/tools-tests.yml" From 278e99b7956f823f086e4cba9ebecf61fb7db24a Mon Sep 17 00:00:00 2001 From: Shahin Saadati Date: Wed, 5 Aug 2026 11:08:42 -0700 Subject: [PATCH 3/3] fix(ci): keep recipe_manifests accurate once the generator is gone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-on from the audit finding about the counterfactual docstring — the same defect reappeared one commit later, in the opposite direction. recipe_manifests.py's docstring was rewritten in the parent PR to describe the world where generate_dependabot.py exists. This commit deletes that script, so the module was again describing a script that is not there. Rewritten for the tree as it now is. Adds test_static_entries_match_recipe_manifests, which was the real gap behind both wordings. STATIC_ENTRIES has to agree with the non-glob entries in dependabot.yml: one configured there but missing from the list has its PRs classified as orphans and closed with --delete-branch, at roughly one grouped PR a week — well under the --max-close circuit breaker. While a generator owned the file it kept the two in step by construction. Nothing does now, so the invariant is asserted instead, in both directions. --- .github/scripts/recipe_manifests.py | 54 ++++++++++--------- .../scripts/tests/test_dependabot_config.py | 35 ++++++++++++ 2 files changed, 64 insertions(+), 25 deletions(-) diff --git a/.github/scripts/recipe_manifests.py b/.github/scripts/recipe_manifests.py index 96058acb2e..835b698719 100644 --- a/.github/scripts/recipe_manifests.py +++ b/.github/scripts/recipe_manifests.py @@ -7,23 +7,25 @@ Why this exists as its own module --------------------------------- -Two scripts need the same answer, and they must never disagree: - - generate_dependabot.py decides what goes INTO .github/dependabot.yml - close_orphan_dependabot_prs.py decides which open Dependabot PRs target a - directory that no longer exists, and closes - them with --delete-branch - -The second used to recover the list by regex-scanning the enumerated -`directory:` keys back out of the file the first had written. Re-deriving by -parsing was fragile in a destructive code path: a formatting change to the -generated output could silently empty the set, and an empty set makes every -open Dependabot PR look orphaned. - -Sharing the scanner removes that class of bug outright — there is one -definition of "dependency-managed directory", so the generator and the -cleanup cannot drift apart. STATIC_ENTRIES below exists for the same reason, -for the entries that are configured unconditionally rather than discovered. +`.github/dependabot.yml` is static and glob-based — Dependabot resolves those +globs against the tree itself, so nothing here feeds that file. + +close_orphan_dependabot_prs.py still needs the concrete list, to decide +whether an open Dependabot PR targets a directory that no longer exists. It +closes what it decides with --delete-branch, so the answer has to be right. + +It used to recover the list by regex-scanning the enumerated `directory:` +keys out of dependabot.yml. That stopped being possible the moment the file +switched to globs: the old parser would have found nothing, judged every open +Dependabot PR an orphan, and closed the lot. Scanning the tree is also the +more faithful source, since the tree is what Dependabot resolves those globs +against. + +STATIC_ENTRIES below covers the other half — entries configured +unconditionally rather than discovered. Nothing in the tree can turn them up, +so they are listed once here and asserted against the real config by +tests/test_dependabot_config.py. An entry present in dependabot.yml but +missing from that list would have its PRs closed as orphans. Zero third-party dependencies, matching its callers. """ @@ -112,14 +114,16 @@ def _is_npm(d: Path) -> bool: # /.github/workflows and any root action.yml, so its directory is always "/" # and no amount of scanning the recipe tree would turn it up. # -# Shared for the same reason as the detectors above, and this half is the -# dangerous one. generate_dependabot.py appends these to dependabot.yml; -# close_orphan_dependabot_prs.py adds them to the set of live pairs so their -# PRs are never treated as orphans. When the two lists were written out -# separately, adding a static entry to the generator alone meant the cleanup -# did not recognise it and closed its PRs with --delete-branch — and because -# such an entry produces roughly one grouped PR a week, the --max-close -# circuit breaker would never trip on it. +# close_orphan_dependabot_prs.py folds these into the set of live pairs so +# their PRs are never treated as orphans. Because they cannot be discovered, +# an entry that exists in dependabot.yml but not in this list has its PRs +# closed with --delete-branch — and since such an entry produces roughly one +# grouped PR a week, the --max-close circuit breaker would never trip on it. +# tests/test_dependabot_config.py asserts the two agree, in both directions. +# +# `extra_labels` is carried because dependabot.yml gives github-actions its +# own extra label; keeping it here means the list fully describes the entry +# rather than half of it. # # Each tuple is (package-ecosystem, directory, extra_labels). STATIC_ENTRIES: list[tuple[str, str, list[str]]] = [ diff --git a/.github/scripts/tests/test_dependabot_config.py b/.github/scripts/tests/test_dependabot_config.py index e830fe4518..3cf78e11b6 100644 --- a/.github/scripts/tests/test_dependabot_config.py +++ b/.github/scripts/tests/test_dependabot_config.py @@ -223,6 +223,41 @@ def test_every_ecosystem_present_in_the_tree_has_a_config_entry(config): ) +def test_static_entries_match_recipe_manifests(config): + """Every non-glob entry in the config must be in STATIC_ENTRIES. + + A static entry is configured, never discovered, so + close_orphan_dependabot_prs.py can only know about it by being told. + One present here but missing from STATIC_ENTRIES has its PRs classified + as orphans and closed with --delete-branch — and since such an entry + produces roughly one grouped PR a week, the --max-close circuit breaker + would never trip on it. + + While a generator owned this file it kept the two in step. Nothing does + now, which is exactly why this assertion exists. + """ + configured_static = { + (u["package-ecosystem"], u["directory"]) + for u in config["updates"] + if "directory" in u + } + known_static = set(rm.static_pairs()) + + unknown = configured_static - known_static + assert not unknown, ( + f"{sorted(unknown)} are configured in dependabot.yml but missing from " + "recipe_manifests.STATIC_ENTRIES, so the orphan cleanup would close " + "their PRs with --delete-branch. Add them there." + ) + + stale = known_static - configured_static + assert not stale, ( + f"{sorted(stale)} are in recipe_manifests.STATIC_ENTRIES but not in " + "dependabot.yml. Harmless, but the list is now describing an entry " + "that does not exist — remove it." + ) + + def test_github_actions_entry_uses_a_plain_directory(config): """GitHub documents `directory: "/"` for github-actions; it is not a glob, and Dependabot looks in /.github/workflows regardless."""