also looking at this
fix(pr): apply the security label so security-only can fire
#1409
4 files
+87
-60
| @@ -366,9 +366,14 @@ export class Buddy { | ||
| 366 | 366 | /** |
| 367 | 367 | * Every label a pull request for this group should carry. |
| 368 | 368 | * |
| 369 | * Three sources union here: the automatic set derived from the update types | |
| 370 | * and manifests in the group, whatever `pullRequest.labels` declares, and the | |
| 371 | * labels any matching rule asked for. | |
| 369 | * Four sources union here: the automatic set derived from the update types | |
| 370 | * and manifests in the group, `security.label` when the group resolves a | |
| 371 | * published advisory, whatever `pullRequest.labels` declares, and the labels | |
| 372 | * any matching rule asked for. | |
| 373 | * | |
| 374 | * The security label matters beyond decoration: the `security-only` | |
| 375 | * auto-merge condition tests for it, so a group that resolves an advisory | |
| 376 | * could never satisfy that condition while nothing applied the label. | |
| 372 | 377 | * |
| 373 | 378 | * Centralised because the five call sites had drifted — only the creation |
| 374 | 379 | * path folded in rule labels, so refreshing an existing pull request quietly |
| @@ -380,8 +385,15 @@ export class Buddy { | ||
| 380 | 385 | * @returns Labels to set on the pull request |
| 381 | 386 | */ |
| 382 | 387 | private labelsFor(group: UpdateGroup, prGenerator: PullRequestGenerator): string[] { |
| 388 | // Label groups that actually resolve a published advisory. This once | |
| 389 | // matched a hardcoded list of security-adjacent package names, which both | |
| 390 | // missed every real vulnerability outside that list and mislabelled | |
| 391 | // routine updates to packages that merely sound security-related. | |
| 392 | const resolvesAdvisory = group.updates.some(update => (update.securityAdvisories?.length ?? 0) > 0) | |
| 393 | ||
| 383 | 394 | return [...new Set([ |
| 384 | 395 | ...prGenerator.generateLabels(group), |
| 396 | ...(resolvesAdvisory ? [this.config.security?.label ?? 'security'] : []), | |
| 385 | 397 | ...(this.config.pullRequest?.labels ?? []), |
| 386 | 398 | ...this.groupEffectsFor(group.updates).labels, |
| 387 | 399 | ])] |
| @@ -2219,62 +2231,6 @@ export class Buddy { | ||
| 2219 | 2231 | return existingUpdates.size === newUpdates.length |
| 2220 | 2232 | } |
| 2221 | 2233 | |
| 2222 | /** | |
| 2223 | * Generate dynamic labels for PR based on update types and configuration | |
| 2224 | */ | |
| 2225 | private generatePRLabels(group: UpdateGroup): string[] { | |
| 2226 | const labels = new Set<string>() | |
| 2227 | ||
| 2228 | // Always add dependencies label | |
| 2229 | labels.add('dependencies') | |
| 2230 | ||
| 2231 | // Add update type specific labels | |
| 2232 | const updateTypes = group.updates.map(u => u.updateType) | |
| 2233 | const hasUpdates = { | |
| 2234 | major: updateTypes.includes('major'), | |
| 2235 | minor: updateTypes.includes('minor'), | |
| 2236 | patch: updateTypes.includes('patch'), | |
| 2237 | } | |
| 2238 | ||
| 2239 | // Add specific update type labels | |
| 2240 | if (hasUpdates.major) { | |
| 2241 | labels.add('major') | |
| 2242 | } | |
| 2243 | if (hasUpdates.minor) { | |
| 2244 | labels.add('minor') | |
| 2245 | } | |
| 2246 | if (hasUpdates.patch) { | |
| 2247 | labels.add('patch') | |
| 2248 | } | |
| 2249 | ||
| 2250 | // Add additional contextual labels | |
| 2251 | if (group.updates.length > 5) { | |
| 2252 | labels.add('dependencies') // Use standard dependencies label instead of bulk-update | |
| 2253 | } | |
| 2254 | ||
| 2255 | // Label groups that actually resolve a published advisory. This used to | |
| 2256 | // match a hardcoded list of security-adjacent package names, which both | |
| 2257 | // missed every real vulnerability outside that list and mislabelled | |
| 2258 | // routine updates to packages that merely sound security-related. | |
| 2259 | const resolvesAdvisory = group.updates.some(update => (update.securityAdvisories?.length ?? 0) > 0) | |
| 2260 | if (resolvesAdvisory) { | |
| 2261 | labels.add(this.config.security?.label ?? 'security') | |
| 2262 | } | |
| 2263 | ||
| 2264 | // Add configured labels from config if they exist (but avoid duplicates) | |
| 2265 | if (this.config.pullRequest?.labels) { | |
| 2266 | this.config.pullRequest.labels.forEach((label) => { | |
| 2267 | // Only add if it's not 'dependencies' since we always add that | |
| 2268 | if (label !== 'dependencies') { | |
| 2269 | labels.add(label) | |
| 2270 | } | |
| 2271 | }) | |
| 2272 | } | |
| 2273 | ||
| 2274 | // Convert to array and return | |
| 2275 | return Array.from(labels) | |
| 2276 | } | |
| 2277 | ||
| 2278 | 2234 | /** |
| 2279 | 2235 | * Check if a PR should be auto-closed due to configuration changes |
| 2280 | 2236 | * This handles cases where: |