also looking at this
fix(rules): consult a rule's autoMerge when deciding to merge
#1422
2 files
+178
-1
| @@ -1,4 +1,5 @@ | ||
| 1 | import type { BuddyConfig, PackageUpdate, PRManifest } from '../types' | |
| 1 | import type { BuddyConfig, PackageUpdate, PRManifest, PRManifestUpdate } from '../types' | |
| 2 | import { groupsToRules, resolveRuleEffects } from '../rules/engine' | |
| 2 | 3 | import { parseManifest } from './pr-manifest' |
| 3 | 4 | |
| 4 | 5 | /** Branch prefix every buddy pull request is opened from. */ |
| @@ -116,9 +117,74 @@ export function evaluateAutoMerge( | ||
| 116 | 117 | if (manifest.truncated) |
| 117 | 118 | return { eligible: false, reason: `PR #${pr.number} has a truncated manifest, so its full update set cannot be verified` } |
| 118 | 119 | |
| 120 | // A rule that names auto-merge explicitly settles it in either direction; | |
| 121 | // the global conditions are the default for everything it does not cover. | |
| 122 | const ruled = evaluateRuleAutoMerge(manifest, config) | |
| 123 | if (ruled) | |
| 124 | return ruled | |
| 125 | ||
| 119 | 126 | return evaluateConditions(manifest, labels, settings) |
| 120 | 127 | } |
| 121 | 128 | |
| 129 | /** | |
| 130 | * Whether a package rule decides auto-merge for this pull request. | |
| 131 | * | |
| 132 | * `autoMerge` on a rule was resolved into `ResolvedEffects` and merged by | |
| 133 | * `mergeGroupEffects`, and nothing ever read either — so a rule saying "these | |
| 134 | * are safe to merge without review" did nothing, and one saying the opposite | |
| 135 | * did nothing either. | |
| 136 | * | |
| 137 | * The all-or-nothing reading is `mergeGroupEffects`'s, not a new invention: a | |
| 138 | * group qualifies only when every update in it is covered. One package no rule | |
| 139 | * has vouched for sends the whole pull request back to the global conditions. | |
| 140 | * | |
| 141 | * @param manifest - The pull request's embedded update manifest | |
| 142 | * @param config - Configuration supplying the rules | |
| 143 | * @returns A decision when a rule settles it, `null` to fall through | |
| 144 | */ | |
| 145 | function evaluateRuleAutoMerge(manifest: PRManifest, config: BuddyConfig): AutoMergeDecision | null { | |
| 146 | const rules = [...groupsToRules(config.packages?.groups), ...(config.packages?.rules ?? [])] | |
| 147 | if (rules.length === 0 || manifest.updates.length === 0) | |
| 148 | return null | |
| 149 | ||
| 150 | // A manifest sheds `type` and `dependencyType` before it sheds rows, and | |
| 151 | // does not mark itself truncated when it does. Rules match on both, so | |
| 152 | // evaluating one against a manifest missing them would decide on data that | |
| 153 | // is not there — fall through to the conditions instead. | |
| 154 | const complete = manifest.updates.every(update => update.type && update.dependencyType) | |
| 155 | if (!complete) | |
| 156 | return null | |
| 157 | ||
| 158 | const effects = manifest.updates.map(update => resolveRuleEffects(toPackageUpdate(update), rules)) | |
| 159 | ||
| 160 | if (effects.some(effect => effect.autoMerge === false)) | |
| 161 | return { eligible: false, reason: 'a package rule holds one of these updates back from auto-merge' } | |
| 162 | ||
| 163 | if (effects.every(effect => effect.autoMerge === true)) | |
| 164 | return { eligible: true, reason: 'every update is covered by a package rule that enables auto-merge' } | |
| 165 | ||
| 166 | return null | |
| 167 | } | |
| 168 | ||
| 169 | /** | |
| 170 | * Rebuild the update a manifest row describes, for rule matching. | |
| 171 | * | |
| 172 | * Only the fields `ruleMatches` reads are reconstructed; the manifest carries | |
| 173 | * nothing else and nothing here needs more. | |
| 174 | * | |
| 175 | * @param row - A manifest row | |
| 176 | */ | |
| 177 | function toPackageUpdate(row: PRManifestUpdate): PackageUpdate { | |
| 178 | return { | |
| 179 | name: row.name, | |
| 180 | currentVersion: row.current, | |
| 181 | newVersion: row.target, | |
| 182 | updateType: row.type, | |
| 183 | file: row.file, | |
| 184 | dependencyType: row.dependencyType, | |
| 185 | } as PackageUpdate | |
| 186 | } | |
| 187 | ||
| 122 | 188 | function evaluateConditions( |
| 123 | 189 | manifest: PRManifest, |
| 124 | 190 | labels: string[], |
| @@ -219,8 +285,13 @@ export function evaluateAutoMergeForUpdates( | ||
| 219 | 285 | target: update.newVersion, |
| 220 | 286 | type: update.updateType, |
| 221 | 287 | file: update.file, |
| 288 | dependencyType: update.dependencyType, | |
| 222 | 289 | })), |
| 223 | 290 | } |
| 224 | 291 | |
| 292 | const ruled = evaluateRuleAutoMerge(manifest, config) | |
| 293 | if (ruled) | |
| 294 | return ruled | |
| 295 | ||
| 225 | 296 | return evaluateConditions(manifest, labels, settings) |
| 226 | 297 | } |