Tracking issue for an upstream defect we are working around. Nothing to do here until it is fixed upstream; this exists so the workaround is not mistaken for a preference.
What is broken upstream
safe-settings does not merge arrays nested inside a matched element. Declaring a required_status_checks rule in both suborgs/all.yml and repos/<repo>.yml emits two rules rather than one combined rule.
Root cause, located while debugging #116: lib/mergeArrayBy.js:14 declares mergeBy with eight parameters, and lib/mergeDeep.js:333 calls it with seven — so the Octokit client binds to options and githubContext is undefined. options is handed straight to deepmerge at mergeArrayBy.js:24; an Octokit client has no arrayMerge, so deepmerge falls back to its default, which concatenates.
That is why rulesets merge by name as intended while the rules inside them do not merge by type, despite type being in NAME_FIELDS for exactly that purpose.
Upstream: github/safe-settings#723 (open since 2024-12-30, labelled bug), and probably #722. Neither names the mechanism; we have added it.
What it did to us
Attempted in #126, reverted in #128. Every payload carried two rules, and GitHub's response split by prior state:
| repo already had such a rule |
PUT |
outcome |
| no (6 repos) |
422 Invalid rule 'required_status_checks': |
rules never applied |
| yes (7 repos) |
200 |
the second rule won, the first was silently discarded |
The 200 path is the dangerous one: reqstool-client went from 8 required contexts to 5 with no error anywhere and a green sync. Protection was reduced silently.
Our workaround
Every repo's full context list lives in its own safe-settings/repos/<repo>.yml. Nothing goes in suborgs/all.yml. all.yml carries a comment recording why.
The cost is repeating Analyze (actions), CodeQL and DCO in 13 files. That is the safe shape and it is deliberate — see #116 for the derivation of each list.
When upstream fixes it
Revisit the hoist. Exactly three contexts are universal across all 13 repos, so the saving is 39 duplicated lines down to 3. Two near-universal contexts must stay per-repo regardless: Analyze (javascript-typescript) does not report on reqstool.github.io, and check / Validate PR title does not report on reqstool-regression or reqstool-ai.
Verify any retry with a harness that replicates safe-settings' real seven-argument call. Ours originally passed its own arrayMerge into the options slot — something the calling code never does — so it reported one correctly merged rule and we shipped on that. Supplying valid options is precisely what the upstream bug is. A harness passing {} reports two rules for the same input.
Also worth doing independently
We pin safe-settings 2.1.18; 2.1.21 is current. The call site and signature are unchanged on main-enterprise, so upgrading does not fix this — but the intervening releases are worth reviewing on their own merits.
Tracking issue for an upstream defect we are working around. Nothing to do here until it is fixed upstream; this exists so the workaround is not mistaken for a preference.
What is broken upstream
safe-settings does not merge arrays nested inside a matched element. Declaring a
required_status_checksrule in bothsuborgs/all.ymlandrepos/<repo>.ymlemits two rules rather than one combined rule.Root cause, located while debugging #116:
lib/mergeArrayBy.js:14declaresmergeBywith eight parameters, andlib/mergeDeep.js:333calls it with seven — so the Octokit client binds tooptionsandgithubContextisundefined.optionsis handed straight to deepmerge atmergeArrayBy.js:24; an Octokit client has noarrayMerge, so deepmerge falls back to its default, which concatenates.That is why
rulesetsmerge bynameas intended while therulesinside them do not merge bytype, despitetypebeing inNAME_FIELDSfor exactly that purpose.Upstream: github/safe-settings#723 (open since 2024-12-30, labelled
bug), and probably #722. Neither names the mechanism; we have added it.What it did to us
Attempted in #126, reverted in #128. Every payload carried two rules, and GitHub's response split by prior state:
422 Invalid rule 'required_status_checks':200The
200path is the dangerous one:reqstool-clientwent from 8 required contexts to 5 with no error anywhere and a green sync. Protection was reduced silently.Our workaround
Every repo's full context list lives in its own
safe-settings/repos/<repo>.yml. Nothing goes insuborgs/all.yml.all.ymlcarries a comment recording why.The cost is repeating
Analyze (actions),CodeQLandDCOin 13 files. That is the safe shape and it is deliberate — see #116 for the derivation of each list.When upstream fixes it
Revisit the hoist. Exactly three contexts are universal across all 13 repos, so the saving is 39 duplicated lines down to 3. Two near-universal contexts must stay per-repo regardless:
Analyze (javascript-typescript)does not report onreqstool.github.io, andcheck / Validate PR titledoes not report onreqstool-regressionorreqstool-ai.Verify any retry with a harness that replicates safe-settings' real seven-argument call. Ours originally passed its own
arrayMergeinto theoptionsslot — something the calling code never does — so it reported one correctly merged rule and we shipped on that. Supplying valid options is precisely what the upstream bug is. A harness passing{}reports two rules for the same input.Also worth doing independently
We pin safe-settings
2.1.18;2.1.21is current. The call site and signature are unchanged onmain-enterprise, so upgrading does not fix this — but the intervening releases are worth reviewing on their own merits.