diff --git a/bin/gh-prs-merge.ts b/bin/gh-prs-merge.ts index 19e793f..45b32b7 100755 --- a/bin/gh-prs-merge.ts +++ b/bin/gh-prs-merge.ts @@ -82,6 +82,11 @@ Fixing (--fix): rebuilds it (lockfile-only, no scripts), pushes a merge commit (never a force push) and parks it. Same-repo PRs only; forks stay FIXME + version-line conflict a package.json whose only conflict is its + "version" line (two PRs each bumped the release): + keeps the head's version if it is ahead of the + base, else bumps the base's patch. Combines with + the lockfile repair above Never repaired: Any other DIRTY PR. update-branch cannot merge over a conflict, so it is diff --git a/package.json b/package.json index 0e5ed66..79ea2b3 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@profullstack/cli-tools", - "version": "0.69.0", + "version": "0.70.0", "private": true, "description": "Local command-line tools, in TypeScript, exposed on PATH.", "type": "module", diff --git a/src/lockfile-conflicts.ts b/src/lockfile-conflicts.ts index eff8e53..e326246 100644 --- a/src/lockfile-conflicts.ts +++ b/src/lockfile-conflicts.ts @@ -1,4 +1,4 @@ -import { mkdir, mkdtemp, rm } from 'node:fs/promises'; +import { mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'; import { homedir } from 'node:os'; import { basename, dirname, join } from 'node:path'; import { run, type RunResult } from './exec.ts'; @@ -27,11 +27,60 @@ export const LOCKFILES: Readonly basename(path) in LOCKFILES; +const VERSION_LINE = /^(\s*"version"\s*:\s*")(\d+)\.(\d+)\.(\d+)("\s*,?\s*)$/; + +/** + * Resolves a package.json whose every conflict hunk is one "version" line a + * side: two PRs each bumped the release, which is bookkeeping, not a + * disagreement. Keeps the head's version when it is already ahead of the base, + * otherwise the base's version with the patch bumped, so the PR still ships as + * a release of its own. Anything else in a hunk (or a prerelease) returns null + * and the PR stays a FIXME. Handles merge and diff3/zdiff3 markers. + */ +export function resolveVersionConflict(text: string): string | null { + const out: string[] = []; + let state: 'out' | 'ours' | 'base' | 'theirs' = 'out'; + let ours: string[] = []; + let theirs: string[] = []; + let hunks = 0; + for (const line of text.split('\n')) { + if (state === 'out' && line.startsWith('<<<<<<< ')) { + state = 'ours'; + ours = []; + theirs = []; + } else if (state === 'ours' && line.startsWith('||||||| ')) { + state = 'base'; + } else if ((state === 'ours' || state === 'base') && line === '=======') { + state = 'theirs'; + } else if (state === 'theirs' && line.startsWith('>>>>>>> ')) { + const a = ours.length === 1 ? VERSION_LINE.exec(ours[0]!) : null; + const b = theirs.length === 1 ? VERSION_LINE.exec(theirs[0]!) : null; + if (!a || !b) return null; + const head = [a[2], a[3], a[4]].map(Number); + const base = [b[2], b[3], b[4]].map(Number); + const cmp = head[0]! - base[0]! || head[1]! - base[1]! || head[2]! - base[2]!; + const version = cmp > 0 ? head : [base[0], base[1], base[2]! + 1]; + out.push(`${b[1]}${version.join('.')}${b[5]}`); + hunks++; + state = 'out'; + } else if (state === 'ours') { + ours.push(line); + } else if (state === 'theirs') { + theirs.push(line); + } else if (state === 'out') { + out.push(line); + } + } + return state === 'out' && hunks > 0 ? out.join('\n') : null; +} + +const isManifest = (path: string): boolean => basename(path) === 'package.json'; + export type LockfileOutcome = | { kind: 'resolved'; files: string[] } | { kind: 'manual'; reason: string }; -/** Resolves a DIRTY PR when every conflicting file is a lockfile. */ +/** Resolves a DIRTY PR when every conflict is a lockfile or a package.json version line. */ export type LockfileResolver = (pr: PullRequest) => Promise; const tail = (result: RunResult): string => @@ -48,6 +97,10 @@ const tail = (result: RunResult): string => export async function resolveLockfileConflicts( pr: PullRequest, exec: typeof run = run, + fs: { + read: (path: string) => Promise; + write: (path: string, text: string) => Promise; + } = { read: (p) => readFile(p, 'utf8'), write: (p, t) => writeFile(p, t) }, ): Promise { const parsed = parsePullRequestUrl(pr.url); if (!parsed) return { kind: 'manual', reason: `cannot parse ${pr.url}` }; @@ -101,12 +154,23 @@ export async function resolveLockfileConflicts( return { kind: 'manual', reason: `git merge failed: ${tail(merge)}` }; } - const other = conflicted.filter((f) => !isLockfile(f)); + const other = conflicted.filter((f) => !isLockfile(f) && !isManifest(f)); if (other.length > 0) { return { kind: 'manual', reason: `conflicts outside lockfiles: ${other.join(', ')}` }; } - for (const file of conflicted) { + // Manifests first: the lockfiles below are rebuilt from them. + for (const file of conflicted.filter(isManifest)) { + const resolved = resolveVersionConflict(await fs.read(join(dir, file))); + if (resolved === null) { + return { kind: 'manual', reason: `${file} conflicts beyond its version line` }; + } + await fs.write(join(dir, file), resolved); + const add = await git(['add', '--', file]); + if (add.code !== 0) return { kind: 'manual', reason: `git add ${file}: ${tail(add)}` }; + } + + for (const file of conflicted.filter(isLockfile)) { // Merging base into head, "theirs" is the base: the lockfile main already // resolved, which the branch's own changes are then applied on top of. const take = await git(['checkout', '--theirs', '--', file]); @@ -127,7 +191,7 @@ export async function resolveLockfileConflicts( } const body = - conflicted.length > 0 ? `\n\nRegenerated ${conflicted.join(', ')} by gh-prs-merge --fix.` : ''; + conflicted.length > 0 ? `\n\nResolved ${conflicted.join(', ')} by gh-prs-merge --fix.` : ''; const commit = await git(['commit', '--no-verify', '-m', `${message}${body}`]); if (commit.code !== 0) return { kind: 'manual', reason: `commit failed: ${tail(commit)}` }; diff --git a/src/prs-merge.ts b/src/prs-merge.ts index b5e6716..a1f786a 100644 --- a/src/prs-merge.ts +++ b/src/prs-merge.ts @@ -372,7 +372,7 @@ export async function sweep( if (pr.mergeStateStatus === 'DIRTY' && options.resolveLockfiles) { // Both sides bumped dependencies and only the lockfile collided. That // is not a disagreement: the merged manifests say what both wanted. - emit({ kind: 'fixing', url, text: `${reason}; trying a lockfile-only conflict resolution` }); + emit({ kind: 'fixing', url, text: `${reason}; trying a lockfile/version-line conflict resolution` }); let outcome: Awaited>; try { outcome = await options.resolveLockfiles(pr); diff --git a/test/lockfile-conflicts.test.ts b/test/lockfile-conflicts.test.ts index ac1789f..0a936de 100644 --- a/test/lockfile-conflicts.test.ts +++ b/test/lockfile-conflicts.test.ts @@ -1,7 +1,57 @@ import { describe, expect, it } from 'vitest'; import type { RunResult } from '../src/exec.ts'; import type { PullRequest } from '../src/gh.ts'; -import { isLockfile, resolveLockfileConflicts } from '../src/lockfile-conflicts.ts'; +import { + isLockfile, + resolveLockfileConflicts, + resolveVersionConflict, +} from '../src/lockfile-conflicts.ts'; + +/** package.json after `git merge main` into a branch: ours is the head, theirs the base. */ +const conflict = (head: string, base: string, diff3 = false) => + [ + '{', + ' "name": "nixamp",', + '<<<<<<< HEAD', + ` "version": "${head}",`, + ...(diff3 ? ['||||||| abc123', ' "version": "0.52.1",'] : []), + '=======', + ` "version": "${base}",`, + '>>>>>>> FETCH_HEAD', + ' "type": "module"', + '}', + '', + ].join('\n'); + +describe('version-line conflict resolution', () => { + it('bumps the base patch when the head is behind it', () => { + expect(resolveVersionConflict(conflict('0.52.2', '0.53.3'))).toBe( + '{\n "name": "nixamp",\n "version": "0.53.4",\n "type": "module"\n}\n', + ); + }); + + it('keeps the head version when it is already ahead', () => { + expect(resolveVersionConflict(conflict('1.0.0', '0.53.3'))).toContain('"version": "1.0.0",'); + }); + + it('bumps rather than ties when both sides picked the same next version', () => { + expect(resolveVersionConflict(conflict('0.53.4', '0.53.4'))).toContain('"version": "0.53.5",'); + }); + + it('reads diff3 markers', () => { + expect(resolveVersionConflict(conflict('0.52.2', '0.53.3', true))).toContain( + '"version": "0.53.4",', + ); + }); + + it('refuses anything that is not one version line a side', () => { + expect(resolveVersionConflict(conflict('0.52.2-beta.1', '0.53.3'))).toBeNull(); + expect( + resolveVersionConflict(conflict('0.52.2', '0.53.3').replace('=======', ' "private": true,\n=======')), + ).toBeNull(); + expect(resolveVersionConflict('{"version": "1.0.0"}\n')).toBeNull(); + }); +}); const pr: PullRequest = { url: 'https://github.com/acme/repo/pull/7', @@ -96,6 +146,38 @@ describe('lockfile conflict resolution', () => { expect(calls.some((c) => c.startsWith('git push'))).toBe(false); }); + it('resolves a version-only package.json before rebuilding the lockfile', async () => { + const { exec, calls } = stubExec({ conflicted: ['package.json', 'bun.lock'] }); + const written: Record = {}; + const fs = { + read: async () => conflict('0.52.2', '0.53.3'), + write: async (path: string, text: string) => { + written[path] = text; + calls.push(`write ${path.split('/').pop()}`); + }, + }; + const outcome = await resolveLockfileConflicts(pr, exec, fs); + + expect(outcome).toEqual({ kind: 'resolved', files: ['package.json', 'bun.lock'] }); + expect(Object.values(written)[0]).toContain('"version": "0.53.4",'); + expect(calls.indexOf('write package.json')).toBeLessThan( + calls.indexOf('bun install --lockfile-only --ignore-scripts'), + ); + }); + + it('declines a package.json conflict that is more than the version', async () => { + const { exec, calls } = stubExec({ conflicted: ['package.json'] }); + const fs = { + read: async () => conflict('0.52.2', '0.53.3').replace('"version": "0.52.2",', '"version": "0.52.2",\n "private": true,'), + write: async () => {}, + }; + expect(await resolveLockfileConflicts(pr, exec, fs)).toEqual({ + kind: 'manual', + reason: 'package.json conflicts beyond its version line', + }); + expect(calls.some((c) => c.startsWith('git push'))).toBe(false); + }); + it('reports a refused push instead of claiming the repair', async () => { const { exec } = stubExec({ conflicted: ['bun.lock'], pushFails: true }); const outcome = await resolveLockfileConflicts(pr, exec);