ReviewOS
Reload

also looking at this

stacks/buddy

fix(rules): apply the per-rule minimum release age, which nothing read

#1419
Merged glennmichael123 wants to merge fix/rule-release-age into main
3 files +223 -7
src/buddy.tsmodified+23-5
@@ -1924,29 +1924,47 @@ export class Buddy {
19241924 * Filter updates by minimum release age requirement
19251925 */
19261926 private async filterUpdatesByMinimumReleaseAge(updates: PackageUpdate[]): Promise<PackageUpdate[]> {
1927 const minimumReleaseAge = this.config.packages?.minimumReleaseAge ?? 0
1927 const globalAge = this.config.packages?.minimumReleaseAge ?? 0
1928 const rules = this.activeRules()
19281929
1929 // If no minimum age is set, return all updates
1930 if (minimumReleaseAge === 0) {
1930 // A rule can set a hold where the global is zero, so the early return has
1931 // to account for both. Checking only the global here is what made
1932 // `rules[].minimumReleaseAge` inert: it was resolved into the effects and
1933 // nothing ever read it.
1934 const ruleSetsAnAge = rules.some(rule => rule.minimumReleaseAge !== undefined)
1935 if (globalAge === 0 && !ruleSetsAnAge) {
19311936 return updates
19321937 }
19331938
1934 this.logger.info(`Applying minimum release age filter (${minimumReleaseAge} minutes) in parallel...`)
1939 this.logger.info(`Applying minimum release age filter (global ${globalAge} minutes${
1940 ruleSetsAnAge ? ', with per-rule overrides' : ''
1941 }) in parallel...`)
19351942
19361943 // PARALLEL: Check all updates concurrently
19371944 const ageCheckPromises = updates.map(async (update) => {
19381945 try {
1946 // Resolved per update, because a rule matching this package may hold
1947 // it longer — or shorter — than the repository default.
1948 const required = (rules.length > 0
1949 ? resolveRuleEffects(update, rules).minimumReleaseAge
1950 : undefined) ?? globalAge
1951
1952 if (required === 0) {
1953 return update
1954 }
1955
19391956 const meetsRequirement = await this.registryClient.meetsMinimumReleaseAge(
19401957 update.name,
19411958 update.newVersion,
19421959 update.dependencyType,
1960 required,
19431961 )
19441962
19451963 if (meetsRequirement) {
19461964 return update
19471965 }
19481966 else {
1949 this.logger.debug(`Filtered out ${update.name}@${update.newVersion} (${update.dependencyType}) due to minimum release age requirement`)
1967 this.logger.debug(`Filtered out ${update.name}@${update.newVersion} (${update.dependencyType}) — held for ${required} minutes after publication`)
19501968 return null
19511969 }
19521970 }
src/registry/registry-client.tsmodified+9-2
@@ -1721,8 +1721,15 @@ export class RegistryClient {
17211721 /**
17221722 * Check if a package version meets the minimum release age requirement
17231723 */
1724 async meetsMinimumReleaseAge(packageName: string, version: string, dependencyType?: string): Promise<boolean> {
1725 const minimumReleaseAge = this.config?.packages?.minimumReleaseAge ?? 0
1724 async meetsMinimumReleaseAge(
1725 packageName: string,
1726 version: string,
1727 dependencyType?: string,
1728 requiredMinutes?: number,
1729 ): Promise<boolean> {
1730 // A package rule can set its own hold, so the caller may already know what
1731 // this package's age requirement is; the global value is the fallback.
1732 const minimumReleaseAge = requiredMinutes ?? this.config?.packages?.minimumReleaseAge ?? 0
17261733 const excludeList = this.config?.packages?.minimumReleaseAgeExclude ?? []
17271734
17281735 // If no minimum age is set, allow all packages
test/rule-release-age.test.tsadded+191-0
@@ -0,0 +1,191 @@
1import type { BuddyConfig, PackageUpdate } from '../src/types'
2import { describe, expect, it } from 'bun:test'
3import { Buddy } from '../src/buddy'
4
5/** An update, with only the fields the age filter reads. */
6function update(name: string): PackageUpdate {
7 return {
8 name,
9 currentVersion: '1.0.0',
10 newVersion: '1.1.0',
11 updateType: 'minor',
12 dependencyType: 'dependencies',
13 file: 'package.json',
14 } as PackageUpdate
15}
16
17interface Asked {
18 name: string
19 required?: number
20}
21
22/**
23 * Run the filter with the age check stubbed, recording what each package was
24 * asked to satisfy.
25 *
26 * `holdsFor` decides the verdict: a package is kept when the age it is
27 * required to meet is at or below what the fixture says it has actually aged.
28 */
29function filter(config: BuddyConfig, updates: PackageUpdate[], agedMinutes: Record<string, number>) {
30 const buddy = new Buddy(config, process.cwd())
31 const asked: Asked[] = []
32
33 ;(buddy as any).registryClient.meetsMinimumReleaseAge = async (
34 name: string,
35 _version: string,
36 _type?: string,
37 required?: number,
38 ) => {
39 asked.push({ name, required })
40 return (agedMinutes[name] ?? 0) >= (required ?? 0)
41 }
42
43 return (buddy as any).filterUpdatesByMinimumReleaseAge(updates)
44 .then((kept: PackageUpdate[]) => ({ kept: kept.map(u => u.name), asked }))
45}
46
47/**
48 * `packages.rules[].minimumReleaseAge` was declared on `PackageRule`, resolved
49 * into `ResolvedEffects`, validated by `config-validation`, and converted from
50 * Renovate's prose durations — and nothing ever read the resolved value. Both
51 * age gates read the global setting only, so a rule holding one risky package
52 * for a week did nothing at all.
53 */
54describe('per-rule minimum release age', () => {
55 it('success case - a rule holds a package the global would have let through', async () => {
56 // The headline: no global age at all, and the rule still holds.
57 const { kept } = await filter(
58 {
59 packages: {
60 strategy: 'all',
61 rules: [{ matchPackages: ['react'], minimumReleaseAge: 10080 }],
62 },
63 },
64 [update('react'), update('lodash')],
65 { react: 60, lodash: 60 },
66 )
67
68 expect(kept).toEqual(['lodash'])
69 })
70
71 it('success case - a rule can hold longer than the global', async () => {
72 const { kept, asked } = await filter(
73 {
74 packages: {
75 strategy: 'all',
76 minimumReleaseAge: 60,
77 rules: [{ matchPackages: ['react'], minimumReleaseAge: 10080 }],
78 },
79 },
80 [update('react'), update('lodash')],
81 { react: 1440, lodash: 1440 },
82 )
83
84 expect(kept).toEqual(['lodash'])
85 expect(asked).toEqual([
86 { name: 'react', required: 10080 },
87 { name: 'lodash', required: 60 },
88 ])
89 })
90
91 it('success case - a rule can release sooner than the global', async () => {
92 // Overriding downward matters as much: a repository holding everything for
93 // a week still wants its own packages to move.
94 const { kept } = await filter(
95 {
96 packages: {
97 strategy: 'all',
98 minimumReleaseAge: 10080,
99 rules: [{ matchPackages: ['@acme/*'], minimumReleaseAge: 0 }],
100 },
101 },
102 [update('@acme/ui'), update('lodash')],
103 { '@acme/ui': 5, 'lodash': 5 },
104 )
105
106 expect(kept).toEqual(['@acme/ui'])
107 })
108
109 it('success case - packages no rule matches keep the global hold', async () => {
110 const { asked } = await filter(
111 {
112 packages: {
113 strategy: 'all',
114 minimumReleaseAge: 60,
115 rules: [{ matchPackages: ['react'], minimumReleaseAge: 10080 }],
116 },
117 },
118 [update('lodash')],
119 { lodash: 99999 },
120 )
121
122 expect(asked).toEqual([{ name: 'lodash', required: 60 }])
123 })
124
125 it('edge case - a zero global with no rule ages skips the check entirely', async () => {
126 // The filter is a network round trip per update; it must not run when
127 // nothing has asked for a hold.
128 const { kept, asked } = await filter(
129 { packages: { strategy: 'all', minimumReleaseAge: 0 } },
130 [update('react')],
131 {},
132 )
133
134 expect(kept).toEqual(['react'])
135 expect(asked).toEqual([])
136 })
137
138 it('edge case - a rule with no age of its own falls back to the global', async () => {
139 const { asked } = await filter(
140 {
141 packages: {
142 strategy: 'all',
143 minimumReleaseAge: 60,
144 rules: [{ matchPackages: ['react'], groupName: 'React' }],
145 },
146 },
147 [update('react')],
148 { react: 99999 },
149 )
150
151 expect(asked).toEqual([{ name: 'react', required: 60 }])
152 })
153
154 it('failure case - a hold is not defeated by pairing it with auto-merge', async () => {
155 // The dangerous shape. A rule saying "hold this for a week, then merge it
156 // without review" was proposing and auto-merging a version published
157 // minutes earlier, because only the auto-merge half was ever read.
158 const { kept } = await filter(
159 {
160 packages: {
161 strategy: 'all',
162 rules: [{ matchPackages: ['react'], minimumReleaseAge: 10080, autoMerge: true }],
163 },
164 },
165 [update('react')],
166 { react: 5 },
167 )
168
169 expect(kept).toEqual([])
170 })
171
172 it('success case - the later of two matching rules wins', async () => {
173 // `minimumReleaseAge` is a single value, so a broad rule sets a default
174 // and a narrow one refines it — which is what the docs promise.
175 const { asked } = await filter(
176 {
177 packages: {
178 strategy: 'all',
179 rules: [
180 { matchPackages: ['**'], minimumReleaseAge: 60 },
181 { matchPackages: ['react'], minimumReleaseAge: 10080 },
182 ],
183 },
184 },
185 [update('react')],
186 { react: 99999 },
187 )
188
189 expect(asked).toEqual([{ name: 'react', required: 10080 }])
190 })
191})