Skip to content

Commit ecbd6d7

Browse files
committed
Don't gate PRs on open or closed state
1 parent 2f1bd98 commit ecbd6d7

File tree

11 files changed

+244
-268
lines changed

11 files changed

+244
-268
lines changed

src/commands/fix/agent-fix.mts

Lines changed: 82 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,10 @@ import {
1515
} from '@socketsecurity/registry/lib/packages'
1616
import { naturalCompare } from '@socketsecurity/registry/lib/sorts'
1717
import { isNonEmptyString } from '@socketsecurity/registry/lib/strings'
18+
import { pluralize } from '@socketsecurity/registry/lib/words'
1819

19-
import { getActiveBranchesForPackage } from './fix-branch-helpers.mts'
20+
import { getPrsForPurl } from './fix-branch-helpers.mts'
21+
import { getFixEnv } from './fix-env-helpers.mts'
2022
import { getActualTree } from './get-actual-tree.mts'
2123
import {
2224
getSocketBranchName,
@@ -28,12 +30,11 @@ import {
2830
gitUnstagedModifiedFiles,
2931
} from './git.mts'
3032
import {
31-
cleanupOpenPrs,
33+
cleanupPrs,
3234
enablePrAutoMerge,
3335
openPr,
34-
prExistForBranch,
3536
setGitRemoteGithubRepoUrl,
36-
} from './open-pr.mts'
37+
} from './pull-request.mts'
3738
import constants from '../../constants.mts'
3839
import {
3940
findBestPatchVersion,
@@ -50,14 +51,15 @@ import { getCveInfoFromAlertsMap } from '../../utils/socket-package-alert.mts'
5051
import { idToPurl } from '../../utils/spec.mts'
5152
import { getOverridesData } from '../optimize/get-overrides-by-agent.mts'
5253

53-
import type { CiEnv } from './fix-env-helpers.mts'
54-
import type { PrMatch } from './open-pr.mts'
5554
import type { NodeClass } from '../../shadow/npm/arborist/types.mts'
5655
import type { CResult } from '../../types.mts'
5756
import type { EnvDetails } from '../../utils/package-environment.mts'
5857
import type { RangeStyle } from '../../utils/semver.mts'
5958
import type { AlertsByPurl } from '../../utils/socket-package-alert.mts'
60-
import type { EditablePackageJson } from '@socketsecurity/registry/lib/packages'
59+
import type {
60+
EditablePackageJson,
61+
Packument,
62+
} from '@socketsecurity/registry/lib/packages'
6163
import type { Spinner } from '@socketsecurity/registry/lib/spinner'
6264

6365
export type FixConfig = {
@@ -80,7 +82,7 @@ export type InstallOptions = {
8082

8183
export type InstallPhaseHandler = (
8284
editablePkgJson: EditablePackageJson,
83-
name: string,
85+
packument: Packument,
8486
oldVersion: string,
8587
newVersion: string,
8688
vulnerableVersionRange: string,
@@ -109,11 +111,12 @@ export async function agentFix(
109111
afterInstall?: InstallPhaseHandler | undefined
110112
revertInstall?: InstallPhaseHandler | undefined
111113
},
112-
ciEnv: CiEnv | null,
113-
openPrs: PrMatch[],
114114
fixConfig: FixConfig,
115115
): Promise<CResult<{ fixed: boolean }>> {
116116
const { pkgPath: rootPath } = pkgEnvDetails
117+
118+
const fixEnv = await getFixEnv()
119+
117120
const {
118121
autoMerge,
119122
cwd,
@@ -128,7 +131,7 @@ export async function agentFix(
128131
let count = 0
129132

130133
const infoByPartialPurl = getCveInfoFromAlertsMap(alertsMap, {
131-
limit: Math.max(limit, openPrs.length),
134+
exclude: { upgradable: true },
132135
})
133136
if (!infoByPartialPurl) {
134137
spinner?.stop()
@@ -141,8 +144,14 @@ export async function agentFix(
141144
return { ok: true, data: { fixed: false } }
142145
}
143146

144-
if (isDebug('notice')) {
145-
debugFn('notice', 'found: cves for', Array.from(infoByPartialPurl.keys()))
147+
if (isDebug('notice,inspect')) {
148+
const partialPurls = Array.from(infoByPartialPurl.keys())
149+
const { length: purlsCount } = partialPurls
150+
debugFn(
151+
'notice',
152+
`found: ${purlsCount} ${pluralize('PURL', purlsCount)} with CVEs`,
153+
)
154+
debugDir('inspect', { partialPurls })
146155
}
147156

148157
// Lazily access constants.packumentCache.
@@ -190,10 +199,11 @@ export async function agentFix(
190199

191200
const infos = Array.from(infoEntry[1].values())
192201
if (!infos.length) {
202+
debugFn('notice', `miss: CVEs expected, but not found, for ${name}`)
193203
continue infoEntriesLoop
194204
}
195205

196-
logger.log(`Processing vulns for ${name}:`)
206+
logger.log(`Processing vulns for ${name}`)
197207
logger.indent()
198208
spinner?.indent()
199209

@@ -208,12 +218,8 @@ export async function agentFix(
208218
continue infoEntriesLoop
209219
}
210220

211-
const activeBranches = getActiveBranchesForPackage(
212-
ciEnv,
213-
infoEntry[0],
214-
openPrs,
215-
)
216221
const availableVersions = Object.keys(packument.versions)
222+
const prs = getPrsForPurl(fixEnv, infoEntry[0])
217223
const warningsForAfter = new Set<string>()
218224

219225
// eslint-disable-next-line no-unused-labels
@@ -230,18 +236,17 @@ export async function agentFix(
230236
const workspace = isWorkspaceRoot
231237
? 'root'
232238
: path.relative(rootPath, pkgPath)
233-
const branchWorkspace = ciEnv
239+
const branchWorkspace = fixEnv.isCi
234240
? getSocketBranchWorkspaceComponent(workspace)
235241
: ''
236-
237242
// actualTree may not be defined on the first iteration of pkgJsonPathsLoop.
238243
if (!actualTree) {
239-
if (!ciEnv) {
244+
if (!fixEnv.isCi) {
240245
// eslint-disable-next-line no-await-in-loop
241246
await removeNodeModules(cwd)
242247
}
243248
const maybeActualTree =
244-
ciEnv && existsSync(path.join(rootPath, 'node_modules'))
249+
fixEnv.isCi && existsSync(path.join(rootPath, 'node_modules'))
245250
? // eslint-disable-next-line no-await-in-loop
246251
await getActualTree(cwd)
247252
: // eslint-disable-next-line no-await-in-loop
@@ -282,7 +287,7 @@ export async function agentFix(
282287

283288
let hasAnnouncedWorkspace = false
284289
let workspaceLogCallCount = logger.logCallCount
285-
if (isDebug()) {
290+
if (isDebug('notice')) {
286291
debugFn('notice', `check: workspace ${workspace}`)
287292
hasAnnouncedWorkspace = true
288293
workspaceLogCallCount = logger.logCallCount
@@ -319,23 +324,34 @@ export async function agentFix(
319324
continue infosLoop
320325
}
321326
if (semver.gte(oldVersion, newVersion)) {
322-
debugFn('notice', `skip: ${oldId} is >= ${newVersion}`)
327+
debugFn('silly', `skip: ${oldId} is >= ${newVersion}`)
328+
continue infosLoop
329+
}
330+
const branch = getSocketBranchName(oldPurl, newVersion, workspace)
331+
const pr = prs.find(
332+
({ parsedBranch: b }) =>
333+
b.workspace === branchWorkspace && b.newVersion === newVersion,
334+
)
335+
if (pr) {
336+
debugFn('notice', `skip: PR #${pr.number} for ${name} exists`)
337+
if (++count >= limit) {
338+
cleanupInfoEntriesLoop()
339+
break infoEntriesLoop
340+
}
323341
continue infosLoop
324342
}
325343
if (
326-
activeBranches.find(
327-
b =>
328-
b.workspace === branchWorkspace && b.newVersion === newVersion,
329-
)
344+
fixEnv.isCi &&
345+
// eslint-disable-next-line no-await-in-loop
346+
(await gitRemoteBranchExists(branch, cwd))
330347
) {
331-
debugFn('notice', `skip: open PR found for ${name}@${newVersion}`)
348+
debugFn('notice', `skip: remote branch "${branch}" exists`)
332349
if (++count >= limit) {
333350
cleanupInfoEntriesLoop()
334351
break infoEntriesLoop
335352
}
336353
continue infosLoop
337354
}
338-
339355
const { overrides: oldOverrides } = getOverridesData(
340356
pkgEnvDetails,
341357
editablePkgJson.content,
@@ -351,26 +367,40 @@ export async function agentFix(
351367
// eslint-disable-next-line no-await-in-loop
352368
await beforeInstall(
353369
editablePkgJson,
354-
name,
370+
packument,
355371
oldVersion,
356372
newVersion,
357373
vulnerableVersionRange,
358374
fixConfig,
359375
)
376+
360377
updatePackageJsonFromNode(
361378
editablePkgJson,
362379
actualTree,
363380
node,
364381
newVersion,
365382
rangeStyle,
366383
)
384+
367385
// eslint-disable-next-line no-await-in-loop
368-
if (!(await editablePkgJson.save({ ignoreWhitespace: true }))) {
369-
debugFn('notice', `skip: ${workspace}/package.json unchanged`)
386+
const unstagedCResult = await gitUnstagedModifiedFiles(cwd)
387+
const moddedFilepaths = unstagedCResult.ok
388+
? unstagedCResult.data.filter(filepath => {
389+
const basename = path.basename(filepath)
390+
return (
391+
basename === 'package.json' ||
392+
basename === pkgEnvDetails.lockName
393+
)
394+
})
395+
: []
396+
if (!moddedFilepaths.length) {
397+
logger.warn(
398+
'Unexpected condition: Nothing to commit, skipping PR creation.',
399+
)
370400
// Reset things just in case.
371-
if (ciEnv) {
401+
if (fixEnv.isCi) {
372402
// eslint-disable-next-line no-await-in-loop
373-
await gitResetAndClean(ciEnv.baseBranch, cwd)
403+
await gitResetAndClean(fixEnv.baseBranch, cwd)
374404
}
375405
continue infosLoop
376406
}
@@ -402,7 +432,7 @@ export async function agentFix(
402432
// eslint-disable-next-line no-await-in-loop
403433
await afterInstall(
404434
editablePkgJson,
405-
name,
435+
packument,
406436
oldVersion,
407437
newVersion,
408438
vulnerableVersionRange,
@@ -426,68 +456,26 @@ export async function agentFix(
426456
spinner?.stop()
427457

428458
// Check repoInfo to make TypeScript happy.
429-
if (!errored && ciEnv?.repoInfo) {
459+
if (!errored && fixEnv.isCi && fixEnv.repoInfo) {
430460
try {
431-
// eslint-disable-next-line no-await-in-loop
432-
const unstagedCResult = await gitUnstagedModifiedFiles(cwd)
433-
if (!unstagedCResult.ok) {
434-
logger.warn(
435-
'Unexpected condition: Nothing to commit, skipping PR creation.',
436-
)
437-
continue
438-
}
439-
const moddedFilepaths = unstagedCResult.data.filter(filepath => {
440-
const basename = path.basename(filepath)
441-
return (
442-
basename === 'package.json' ||
443-
basename === pkgEnvDetails.lockName
444-
)
445-
})
446-
if (!moddedFilepaths.length) {
447-
logger.warn(
448-
'Unexpected condition: Nothing to commit, skipping PR creation.',
449-
)
450-
continue infosLoop
451-
}
452-
453-
const branch = getSocketBranchName(oldPurl, newVersion, workspace)
454-
let skipPr = false
455461
if (
456-
// eslint-disable-next-line no-await-in-loop
457-
await prExistForBranch(
458-
ciEnv.repoInfo.owner,
459-
ciEnv.repoInfo.repo,
460-
branch,
461-
)
462-
) {
463-
skipPr = true
464-
debugFn('notice', `skip: branch "${branch}" exists`)
465-
}
466-
// eslint-disable-next-line no-await-in-loop
467-
else if (await gitRemoteBranchExists(branch, cwd)) {
468-
skipPr = true
469-
debugFn('notice', `skip: remote branch "${branch}" exists`)
470-
} else if (
471462
// eslint-disable-next-line no-await-in-loop
472463
!(await gitCreateAndPushBranch(
473464
branch,
474465
getSocketCommitMessage(oldPurl, newVersion, workspace),
475466
moddedFilepaths,
476467
{
477468
cwd,
478-
email: ciEnv.gitEmail,
479-
user: ciEnv.gitUser,
469+
email: fixEnv.gitEmail,
470+
user: fixEnv.gitUser,
480471
},
481472
))
482473
) {
483-
skipPr = true
484474
logger.warn(
485475
'Unexpected condition: Push failed, skipping PR creation.',
486476
)
487-
}
488-
if (skipPr) {
489477
// eslint-disable-next-line no-await-in-loop
490-
await gitResetAndClean(ciEnv.baseBranch, cwd)
478+
await gitResetAndClean(fixEnv.baseBranch, cwd)
491479
// eslint-disable-next-line no-await-in-loop
492480
const maybeActualTree = await installer(pkgEnvDetails, {
493481
cwd,
@@ -508,26 +496,26 @@ export async function agentFix(
508496
// eslint-disable-next-line no-await-in-loop
509497
await Promise.allSettled([
510498
setGitRemoteGithubRepoUrl(
511-
ciEnv.repoInfo.owner,
512-
ciEnv.repoInfo.repo,
513-
ciEnv.githubToken!,
499+
fixEnv.repoInfo.owner,
500+
fixEnv.repoInfo.repo,
501+
fixEnv.githubToken!,
514502
cwd,
515503
),
516-
cleanupOpenPrs(ciEnv.repoInfo.owner, ciEnv.repoInfo.repo, {
504+
cleanupPrs(fixEnv.repoInfo.owner, fixEnv.repoInfo.repo, {
517505
newVersion,
518506
purl: oldPurl,
519507
workspace,
520508
}),
521509
])
522510
// eslint-disable-next-line no-await-in-loop
523511
const prResponse = await openPr(
524-
ciEnv.repoInfo.owner,
525-
ciEnv.repoInfo.repo,
512+
fixEnv.repoInfo.owner,
513+
fixEnv.repoInfo.repo,
526514
branch,
527515
oldPurl,
528516
newVersion,
529517
{
530-
baseBranch: ciEnv.baseBranch,
518+
baseBranch: fixEnv.baseBranch,
531519
cwd,
532520
workspace,
533521
},
@@ -561,10 +549,10 @@ export async function agentFix(
561549
}
562550
}
563551

564-
if (ciEnv) {
552+
if (fixEnv.isCi) {
565553
spinner?.start()
566554
// eslint-disable-next-line no-await-in-loop
567-
await gitResetAndClean(ciEnv.baseBranch, cwd)
555+
await gitResetAndClean(fixEnv.baseBranch, cwd)
568556
// eslint-disable-next-line no-await-in-loop
569557
const maybeActualTree = await installer(pkgEnvDetails, {
570558
cwd,
@@ -578,12 +566,12 @@ export async function agentFix(
578566
}
579567
}
580568
if (errored) {
581-
if (!ciEnv) {
569+
if (!fixEnv.isCi) {
582570
spinner?.start()
583571
// eslint-disable-next-line no-await-in-loop
584572
await revertInstall(
585573
editablePkgJson,
586-
name,
574+
packument,
587575
oldVersion,
588576
newVersion,
589577
vulnerableVersionRange,

0 commit comments

Comments
 (0)