Skip to content

Commit 74fc034

Browse files
reggiCopilot
andcommitted
ci: enforce dedicated dependency commits
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c2d286ec-a386-457f-93ce-a51b8b44e24e
1 parent 9819323 commit 74fc034

3 files changed

Lines changed: 601 additions & 54 deletions

File tree

‎.github/workflows/deps-commits.yml‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,10 @@ jobs:
2020
name: Check Deps Commits
2121
# Skip bot-generated release PRs, which intentionally bump many
2222
# inter-workspace dependency versions inside a single `chore: release` commit.
23-
if: ${{ !startsWith(github.head_ref, 'release/') }}
23+
if: >-
24+
github.event.pull_request.user.login != 'github-actions[bot]' ||
25+
github.event.pull_request.head.repo.full_name != github.repository ||
26+
!startsWith(github.head_ref, 'release-please--branches--')
2427
runs-on: ubuntu-latest
2528
defaults:
2629
run:

‎scripts/check-deps-commits.js‎

Lines changed: 149 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -38,89 +38,179 @@ const readJSONAtRef = (ref, file) => {
3838
}
3939
}
4040

41-
// A first-party manifest is any package.json that is NOT inside a node_modules
42-
// directory. Bundled dependency manifests under node_modules churn as a
43-
// side-effect of lockfile/bundle regeneration and must be ignored.
44-
const isFirstPartyManifest = (file) =>
45-
(file === 'package.json' || file.endsWith('/package.json')) &&
46-
!file.startsWith('node_modules/') &&
47-
!file.includes('/node_modules/')
48-
49-
// Does a commit message mention this dependency by name? Historical formats:
41+
const workspacePatternsAtRef = (ref) => {
42+
const manifest = readJSONAtRef(ref, 'package.json')
43+
return Array.isArray(manifest?.workspaces) ? manifest.workspaces : []
44+
}
45+
46+
// The repository's workspace patterns contain literal path segments and `*`
47+
// segments. Match the declared package roots without including nested fixtures.
48+
const matchesWorkspace = (directory, pattern) => {
49+
const directoryParts = directory.split('/')
50+
const patternParts = pattern.replace(/\/$/, '').split('/')
51+
return directoryParts.length === patternParts.length &&
52+
patternParts.every((part, index) =>
53+
part === '*' || part === directoryParts[index])
54+
}
55+
56+
const isFirstPartyManifest = (file, workspacePatterns) => {
57+
if (file === 'package.json') {
58+
return true
59+
}
60+
if (!file.endsWith('/package.json')) {
61+
return false
62+
}
63+
const directory = file.slice(0, -'/package.json'.length)
64+
return workspacePatterns.some((pattern) =>
65+
matchesWorkspace(directory, pattern))
66+
}
67+
68+
// Does a commit subject mention this dependency by name? Historical formats:
5069
// deps: undici@6.27.0
5170
// deps(arborist): validate-npm-package-name ^8.0.0
5271
// deps!: bump sigstore from 2.x to 3.0.0
5372
// deps: remove read-package-json-fast
5473
// The name appears as a standalone token, optionally followed by `@version`.
55-
const mentionsDep = (message, dep) => {
56-
const tokens = message
74+
const mentionsDep = (subject, dep) => {
75+
const tokens = subject
5776
.split(/\s+/)
5877
.map((t) => t.replace(/^[`'"([,]+/, '').replace(/[`'").,;:]+$/, ''))
5978
return tokens.some((t) => t === dep || t.startsWith(`${dep}@`))
6079
}
6180

62-
const main = () => {
63-
const from = arg('--from')
64-
const to = arg('--to', 'HEAD')
65-
66-
if (!from) {
67-
throw new Error('Usage: node scripts/check-deps-commits.js --from <base> [--to <head>]')
68-
}
69-
70-
// Net set of first-party package.json files changed by the PR.
71-
const changedFiles = git('diff', '--name-only', `${from}...${to}`, '--', '*package.json')
81+
const dependencyChanges = (from, to) => {
82+
const workspacePatterns = [
83+
...new Set([
84+
...workspacePatternsAtRef(from),
85+
...workspacePatternsAtRef(to),
86+
]),
87+
]
88+
const changedFiles = git('diff', '--name-only', from, to, '--', '*package.json')
7289
.split('\n')
7390
.map((f) => f.trim())
7491
.filter(Boolean)
75-
.filter(isFirstPartyManifest)
92+
.filter((file) => isFirstPartyManifest(file, workspacePatterns))
7693

77-
// Collect every production dependency whose presence or version range changed.
78-
const changedDeps = new Map()
94+
const changes = new Map()
7995
for (const file of changedFiles) {
8096
const before = readJSONAtRef(from, file) || {}
8197
const after = readJSONAtRef(to, file) || {}
8298
for (const type of DEP_TYPES) {
8399
const b = before[type] || {}
84100
const a = after[type] || {}
85-
for (const name of new Set([...Object.keys(b), ...Object.keys(a)])) {
86-
if (b[name] !== a[name]) {
87-
if (!changedDeps.has(name)) {
88-
changedDeps.set(name, { files: new Set(), from: b[name], to: a[name] })
89-
}
90-
const entry = changedDeps.get(name)
91-
entry.files.add(file)
92-
entry.to = a[name] ?? entry.to
101+
for (const dep of new Set([...Object.keys(b), ...Object.keys(a)])) {
102+
if (b[dep] !== a[dep]) {
103+
changes.set(`${file}\0${dep}`, {
104+
dep,
105+
file,
106+
from: b[dep],
107+
to: a[dep],
108+
})
93109
}
94110
}
95111
}
96112
}
113+
return changes
114+
}
115+
116+
const main = () => {
117+
const from = arg('--from')
118+
const to = arg('--to', 'HEAD')
119+
120+
if (!from) {
121+
throw new Error('Usage: node scripts/check-deps-commits.js --from <base> [--to <head>]')
122+
}
123+
124+
const mergeBase = git('merge-base', from, to).trim()
125+
const changedDeps = dependencyChanges(mergeBase, to)
97126

98127
if (!changedDeps.size) {
99128
process.stdout.write('OK: no production dependency changes detected.\n')
100129
return
101130
}
102131

103-
// Gather the `deps:` commits in the range. Records are separated by an ASCII
104-
// record separator and fields by an ASCII unit separator so multi-line commit
105-
// bodies are handled safely.
106-
const commits = git('log', '--format=%H%x1f%B%x1e', `${from}..${to}`)
132+
// The merge-base range excludes commits inherited from the base branch.
133+
// Inspect every commit so dependency commits from a merged topic branch and
134+
// changes introduced while resolving a merge are both validated.
135+
const commits = git(
136+
'log',
137+
'--reverse',
138+
'--topo-order',
139+
'--format=%H%x1f%P%x1f%s%x1e',
140+
`${mergeBase}..${to}`
141+
)
107142
.split('\x1e')
108143
.map((c) => c.trim())
109144
.filter(Boolean)
110145
.map((c) => {
111-
const sep = c.indexOf('\x1f')
112-
return { hash: c.slice(0, sep), message: c.slice(sep + 1) }
146+
const [hash, parents, subject] = c.split('\x1f')
147+
return { hash, parents: parents.split(' ').filter(Boolean), subject }
113148
})
114-
const depsCommits = commits.filter((c) => DEPS_SUBJECT.test(c.message.split('\n')[0].trim()))
115149

116-
const uncovered = []
117-
for (const [dep, { files, to: version }] of changedDeps) {
118-
if (!depsCommits.some((c) => mentionsDep(c.message, dep))) {
119-
// `version` is undefined when the dependency was removed.
120-
uncovered.push({ dep, files: [...files], version })
150+
const initialState = new Map(
151+
[...changedDeps].map(([key]) => [
152+
key,
153+
{ covered: false, invalidCommits: [] },
154+
])
155+
)
156+
const stateByCommit = new Map([[mergeBase, initialState]])
157+
for (const { hash, parents, subject } of commits) {
158+
const state = new Map(stateByCommit.get(parents[0]))
159+
160+
if (parents.length > 1) {
161+
for (const [key, { dep, file }] of changedDeps) {
162+
const version = readJSONAtRef(hash, file)?.dependencies?.[dep]
163+
const inheritedFrom = parents.find((parent) =>
164+
readJSONAtRef(parent, file)?.dependencies?.[dep] === version)
165+
if (inheritedFrom) {
166+
state.set(key, stateByCommit.get(inheritedFrom).get(key))
167+
} else {
168+
const previous = state.get(key)
169+
state.set(key, {
170+
covered: previous.covered,
171+
invalidCommits: [
172+
...previous.invalidCommits,
173+
{ hash, subject },
174+
],
175+
})
176+
}
177+
}
178+
stateByCommit.set(hash, state)
179+
continue
180+
}
181+
182+
for (const [key, { dep }] of dependencyChanges(parents[0], hash)) {
183+
if (!changedDeps.has(key)) {
184+
continue
185+
}
186+
if (
187+
DEPS_SUBJECT.test(subject) &&
188+
mentionsDep(subject, dep)
189+
) {
190+
state.set(key, { covered: true, invalidCommits: [] })
191+
} else {
192+
const previous = state.get(key)
193+
state.set(key, {
194+
covered: previous.covered,
195+
invalidCommits: [
196+
...previous.invalidCommits,
197+
{ hash, subject },
198+
],
199+
})
200+
}
121201
}
202+
stateByCommit.set(hash, state)
122203
}
123204

205+
const toHash = git('rev-parse', `${to}^{commit}`).trim()
206+
const finalState = stateByCommit.get(toHash)
207+
const uncovered = [...changedDeps].filter(
208+
([key]) => {
209+
const state = finalState.get(key)
210+
return !state.covered || state.invalidCommits.length
211+
}
212+
)
213+
124214
if (!uncovered.length) {
125215
process.stdout.write(
126216
`OK: all ${changedDeps.size} production dependency change(s) have a matching deps: commit.\n`
@@ -140,19 +230,25 @@ const main = () => {
140230

141231
// One suggestion per (dependency, manifest) so each change is isolated into a
142232
// scoped `deps(<workspace>):` commit that names the exact dependency+version.
143-
const suggestions = uncovered.flatMap(({ dep, files, version }) =>
144-
files.map((file) => {
145-
const scope = scopeOf(file)
146-
const type = scope ? `deps(${scope})` : 'deps'
147-
return version ? ` ${type}: ${dep}@${version}` : ` ${type}: remove ${dep}`
148-
})
149-
)
233+
const suggestions = uncovered.map(([, { dep, file, to: version }]) => {
234+
const scope = scopeOf(file)
235+
const type = scope ? `deps(${scope})` : 'deps'
236+
return version ? ` ${type}: ${dep}@${version}` : ` ${type}: remove ${dep}`
237+
})
150238

151239
const lines = [
152240
'Production dependency changes must each have a dedicated `deps:` commit that names the dependency.',
153241
'',
154-
'The following production dependency changes are missing a matching deps: commit:',
155-
...uncovered.map(({ dep, files }) => ` - ${dep} (changed in: ${files.join(', ')})`),
242+
'The following production dependency changes are not isolated in a matching deps: commit:',
243+
...uncovered.map(([key, { dep, file }]) => {
244+
const invalidCommitEntries = finalState.get(key)?.invalidCommits
245+
const details = invalidCommitEntries
246+
? `; also changed by ${invalidCommitEntries
247+
.map(({ hash, subject }) => `${hash.slice(0, 7)} "${subject}"`)
248+
.join(', ')}`
249+
: ''
250+
return ` - ${dep} (changed in: ${file}${details})`
251+
}),
156252
'',
157253
'Add a separate commit for each, e.g.:',
158254
...suggestions,

0 commit comments

Comments
 (0)