Inventory: include any repo with issues+PRs enabled instead of a fork allow-list - #103
Open
silug wants to merge 3 commits into
Open
Inventory: include any repo with issues+PRs enabled instead of a fork allow-list#103silug wants to merge 3 commits into
silug wants to merge 3 commits into
Conversation
…allow-list The org now disables issues and pull requests on every fork that exists only as a mirror, so those two flags are the signal separating maintained repos (forks included) from mirrors — puppetsync's whole output is a PR, so a repo that can't receive one is definitionally out. The exclude_forks/include_forks allow-list is retired (passing the old keys warns and is ignored); the puppetsync-ignore topic and exclude globs remain as escape hatches. Two repos stay out via explicit exclude globs: pupmod-voxpupuli-selinux until #83 is fixed and its CI is green, and pupmod-simp-haveged, which never received the new baseline (the old fork rule silently dropped it) and is likely to be archived soon — at which point exclude_archived makes that entry redundant. Verified against the live org: the generated inventory matches the last real run's snapshot except for pupmod-simp-rngd, a brand-new module discovered automatically. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
hcaballero2
reviewed
Sep 3, 2026
hcaballero2
left a comment
hcaballero2
left a comment
Contributor
There was a problem hiding this comment.
Automated review (recall-biased, high effort). Findings verified against the PR head and today's live org listing (261 repos; the shipped config selects 79 — the PR text's 80 is just org drift since it was written). Specs pass (10/10).
Most severe first: (1) the flag gate breaks exclude_archived: false (47 of 67 glob-matching archived repos silently dropped), (2) list_github_repos.json task metadata still documents the retired keys. The rest are robustness/altitude and test/cleanup notes — details inline.
* The gate no longer applies to archived repos: archival turns the flags off too, so `exclude_archived: false` was silently dropping 47 of 67 archived repos. An archived sweep now sees all of them again. * Retired keys (include_forks/exclude_forks) and any unknown key fail the task with an explanation, instead of a stderr warning a plan run never shows and a typo'd `excludes:` no-oping silently. * Missing or null has_issues/has_pull_requests fields are "cannot tell": the repo is included and reported by name, so a changed API shape can neither empty the inventory nor quietly disarm the gate. * Every include-matching repo the gate alone removed is reported by name in one aggregated line (41 mirror forks against today's org), and a new `force_include` glob list is the lever for a maintained repo whose flags are off (GitHub creates forks with issues disabled). * The task returns its notices in a `warnings` array and the plan prints them, since task stderr is invisible from `bolt plan run`. * The rule is stated once, in the task header; README, github-org.yaml, the task metadata and the inline comment point there. The metadata no longer advertises the retired keys. * Specs: dead `fork` fixture keys dropped; PRs-only-off and archived- flags-off fixtures added so both halves of "EITHER" and the archived edge are each independently tested; retired/unknown keys, force_include, the missing-field report and the gate report are covered; a plan spec checks the warnings reach out::message. * github-org.yaml says why haveged is held by exclude rather than topic. Live check: the task reproduces the last fleet run's 79-repo inventory exactly, with one warning line naming the 41 excluded mirrors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
hcaballero2
reviewed
Sep 3, 2026
hcaballero2
reviewed
Sep 3, 2026
hcaballero2
reviewed
Sep 3, 2026
hcaballero2
reviewed
Sep 3, 2026
…ude is a true force * select_repos maps every repo to exactly one verdict (kept?, reason) and derives both the selection and the reports from those, so a future early-return cannot keep the selection right while skewing the reports. * force_include now bypasses the include filters as well as the contributions gate (rules 1-4 still apply): listing a name means "I want this repo, whatever its name or flags", which is what the retired include_forks meant. A differently-named fork no longer no-ops silently, and a force_include glob matching nothing in the org is reported. * The gate report says "matching the include filters", since a repo can reach the gate via include_topics without matching a glob. * The task metadata is a pointer at the header, like README and the repolist, instead of a second full copy of the rule. Live check unchanged: 79 repos, one warning naming the 41 mirrors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Re-opened from #101 (which GitHub marked "merged" after an accidental push to
mainthat was reset; its commits never landed). Same change, squashed to one commit.Now that issues and pull requests are disabled on every fork that exists only as a mirror, the dynamic inventory no longer needs the
include_forksallow-list: a repo is included iff issues AND pull requests are enabled (plus the existing name globs / topic / archived / empty filters). puppetsync's whole output is a PR, so a repo that can't receive one is definitionally out of scope.exclude_forks/include_forksare retired; passing them warns and is ignored. Thepuppetsync-ignoretopic andexcludeglobs remain as escape hatches.pupmod-voxpupuli-selinux(PRs enabled, so the new rule would include it) stays out via an explicitexcludeglob until GitHub PR stages can't use an existing fork-network sibling (e.g. silug/puppet-selinux for pupmod-voxpupuli-selinux) #83 is fixed and its CI is green — preserving the earlier deliberate hold.pupmod-simp-haveged— a maintained fork the old allow-list silently dropped, so it missed the recent fleet sessions and never received the new baseline. Since it is also likely to be archived soon, it is held out via an explicitexcludeglob alongside pupmod-voxpupuli-selinux (archival will make the entry redundant).Verified against the live org (260 repos → 80 selected): the generated inventory matches the last real fleet run's snapshot except for
pupmod-simp-rngd, a brand-new module discovered automatically, as intended.Review follow-ups (5e2e8d0)
The gate no longer applies to archived repos (an
exclude_archived: falsesweep sees all of them); retired and unknownrepos_sourcekeys fail the task instead of warning on invisible stderr; missing/null flags mean "cannot tell" — included and reported by name; every include-matching repo the gate removed is named in one warning line that the plan prints (41 mirror forks today); aforce_includeglob list is the lever for a maintained repo with the flags off; the rule is stated once in the task header with README/repolist/metadata pointing there. Live check: the task reproduces the last fleet run's 79-repo inventory exactly.Task specs rewritten for the new rule (either flag off ⇒ skipped; missing flags ⇒ included, so older API shapes can't empty the inventory; retired-key warning). 231 rspec + 28 BoltSpec examples green.
🤖 Generated with Claude Code