From 71da3c51ffb732cf5d4c5cf59a9cb4e7bf4381bf Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 5 Nov 2026 20:32:38 +0000 Subject: [PATCH] fix(publish): Resolve merge conflicts caused only by the version bump When the merge target changed lines next to the version strings the release branch bumped, both merge strategies conflict and the release branch is left unmerged, which also skips the post-release bump. After the resolve strategy fails, re-merge each conflicted file with git merge-file --diff3 and resolve a hunk when the release side only replaced version tokens with the released version: keep the target's lines and apply the same replacement to them. Any other conflict still aborts the merge as before. Co-Authored-By: BYK <126780+BYK@users.noreply.github.com> --- docs/src/content/docs/release-workflow.md | 4 + src/commands/__tests__/publish.test.ts | 109 ++++++- src/commands/publish.ts | 50 ++- src/utils/__tests__/versionConflicts.test.ts | 157 ++++++++++ src/utils/versionConflicts.ts | 311 +++++++++++++++++++ 5 files changed, 629 insertions(+), 2 deletions(-) create mode 100644 src/utils/__tests__/versionConflicts.test.ts create mode 100644 src/utils/versionConflicts.ts diff --git a/docs/src/content/docs/release-workflow.md b/docs/src/content/docs/release-workflow.md index 560bd829c..d885a039c 100644 --- a/docs/src/content/docs/release-workflow.md +++ b/docs/src/content/docs/release-workflow.md @@ -222,6 +222,10 @@ After publishing, Craft merges the release branch into the merge target and deletes the branch. The merge target defaults to the repository's default branch; override it with `--merge-target`. +- If the merge target changed lines next to the version strings the release + branch bumped, Craft resolves those conflicts itself: it keeps the merge + target's lines and applies the version bump to them. Any other conflict + aborts the merge and leaves it to you; the release is still published. - `--no-merge` skips the merge. `--keep-branch` merges but keeps the branch. - Set `noMerge: true` in `.craft.yml` to never merge. Craft turns this on automatically for compiled Node.js GitHub Actions (repositories with a diff --git a/src/commands/__tests__/publish.test.ts b/src/commands/__tests__/publish.test.ts index b4191f506..5c6d105d6 100644 --- a/src/commands/__tests__/publish.test.ts +++ b/src/commands/__tests__/publish.test.ts @@ -1,4 +1,4 @@ -import { mkdir, mkdtemp, rm } from 'fs/promises'; +import { mkdir, mkdtemp, readFile, rm, writeFile } from 'fs/promises'; import { tmpdir } from 'os'; import { vi, describe, test, expect, beforeEach, type Mock } from 'vitest'; import { join as pathJoin } from 'path'; @@ -702,6 +702,113 @@ describe('handleReleaseBranch', () => { expect(git.branch).not.toHaveBeenCalled(); }); + describe('with a version bump conflict', () => { + const skill = (version: string, description: string) => + `---\nname: cli-init\nversion: ${version}\ndescription: ${description}\nrequires: sentry\n---\n`; + + /** + * Builds the situation from a real release: the release branch bumps a + * version in a file whose neighbouring line changed on the target branch. + */ + async function createConflictingRepository(releaseSkill: string) { + const directory = await mkdtemp( + pathJoin(tmpdir(), 'craft-publish-conflict-'), + ); + const repository = pathJoin(directory, 'repository'); + const remote = pathJoin(directory, 'remote.git'); + const branch = 'release/0.48.0'; + + await createGitClient(directory).raw('init', '--bare', remote); + await mkdir(repository); + const git = createGitClient(repository); + await git.raw('init', '-b', 'main'); + await git.addConfig('user.name', 'Craft Test'); + await git.addConfig('user.email', 'craft@example.com'); + await git.addConfig('commit.gpgsign', 'false'); + await git.addRemote('origin', remote); + + const skillPath = pathJoin(repository, 'SKILL.md'); + await writeFile(skillPath, skill('0.48.0-dev.0', 'Old description')); + await git.add('SKILL.md'); + await git.commit('initial'); + + await git.checkoutLocalBranch(branch); + await writeFile(skillPath, releaseSkill); + await git.commit('release: 0.48.0', ['--all']); + const releaseRevision = (await git.revparse('HEAD')).trim(); + + await git.checkout('main'); + await writeFile(skillPath, skill('0.48.0-dev.0', 'New description')); + await git.commit('docs: Update description', ['--all']); + await git.raw('push', 'origin', 'main', branch); + + return { directory, git, branch, releaseRevision, skillPath }; + } + + test('resolves and pushes the merge when only the version conflicts', async () => { + const { directory, git, branch, releaseRevision, skillPath } = + await createConflictingRepository(skill('0.48.0', 'Old description')); + + try { + await handleReleaseBranch( + git, + 'origin', + branch, + releaseRevision, + 'main', + false, + branch, + '0.48.0', + ); + + expect(await readFile(skillPath, 'utf-8')).toBe( + skill('0.48.0', 'New description'), + ); + const pushed = (await git.revparse('origin/main')).trim(); + expect(pushed).toBe((await git.revparse('HEAD')).trim()); + expect( + (await git.raw('rev-list', '--parents', '-n', '1', pushed)) + .trim() + .split(' '), + ).toContain(releaseRevision); + expect(await git.raw('ls-remote', '--heads', 'origin', branch)).toBe( + '', + ); + } finally { + await rm(directory, { recursive: true, force: true }); + } + }); + + test('aborts the merge when the release changed more than the version', async () => { + const { directory, git, branch, releaseRevision, skillPath } = + await createConflictingRepository(skill('0.48.0', 'Hotfix')); + + try { + const error = await handleReleaseBranch( + git, + 'origin', + branch, + releaseRevision, + 'main', + false, + branch, + '0.48.0', + ).catch((e: unknown) => e); + + expect(error).toBeInstanceOf(MergeConflictError); + expect((error as MergeConflictError).conflictedFiles).toEqual([ + 'SKILL.md', + ]); + expect(await readFile(skillPath, 'utf-8')).toBe( + skill('0.48.0-dev.0', 'New description'), + ); + expect((await git.status()).conflicted).toEqual([]); + } finally { + await rm(directory, { recursive: true, force: true }); + } + }); + }); + test('resolves default branch when mergeTarget is not provided', async () => { const { getDefaultBranch } = await import('../../utils/git'); vi.mocked(getDefaultBranch).mockResolvedValue('master'); diff --git a/src/commands/publish.ts b/src/commands/publish.ts index 2b4c8f1ae..1983751e4 100644 --- a/src/commands/publish.ts +++ b/src/commands/publish.ts @@ -59,6 +59,7 @@ import { buildReleaseCommandEnv } from '../utils/releaseCommandEnv'; import { getPublishStatePath } from '../utils/publishState'; import { DEFAULT_CALVER_CONFIG, getNextCalVerPeriod } from '../utils/calver'; import { runPreReleaseCommand } from '../utils/preReleaseCommand'; +import { resolveVersionBumpMergeConflicts } from '../utils/versionConflicts'; /** Default path to post-release script, relative to project root */ const DEFAULT_POST_RELEASE_SCRIPT_PATH = join('scripts', 'post-release.sh'); @@ -482,6 +483,8 @@ export class BranchCleanupError extends Error { * @param [mergeTarget] Branch name to merge the release branch into * @param keepBranch If set to "true", the branch will not be deleted * @param mergeSource Revision to merge, defaults to the release branch + * @param releaseVersion Released version, used to resolve conflicts that + * only come from the release branch's version bump */ export async function handleReleaseBranch( git: SimpleGit, @@ -491,6 +494,7 @@ export async function handleReleaseBranch( mergeTarget?: string, keepBranch = false, mergeSource = branch, + releaseVersion?: string, ): Promise { if (!mergeTarget) { mergeTarget = await getDefaultBranch(git, remoteName); @@ -519,7 +523,16 @@ export async function handleReleaseBranch( // Retry with the resolve strategy which handles criss-cross ambiguities // differently and often succeeds where ort fails on files like CHANGELOG.md try { - await git.merge(['-s', 'resolve', '--no-ff', '--no-edit', mergeSource]); + try { + await git.merge(['-s', 'resolve', '--no-ff', '--no-edit', mergeSource]); + } catch (resolveError) { + // The target branch may have changed lines next to the version + // strings the release branch bumped. Those conflicts have an obvious + // resolution. + if (!(await tryResolveVersionBumpConflicts(git, releaseVersion))) { + throw resolveError; + } + } } catch (resolveError) { // Resolve also failed — capture conflict details before aborting let conflictedFiles: string[] = []; @@ -603,6 +616,40 @@ export async function handleReleaseBranch( } } +/** + * Commits the merge resolution when every conflict of the in-progress merge + * only comes from the release version bump. + * + * @returns Whether the merge was resolved and committed + */ +async function tryResolveVersionBumpConflicts( + git: SimpleGit, + releaseVersion: string | undefined, +): Promise { + if (!releaseVersion) { + return false; + } + try { + const resolvedFiles = await resolveVersionBumpMergeConflicts( + git, + releaseVersion, + ); + if (!resolvedFiles) { + return false; + } + logger.info( + `Resolved conflicts caused only by the ${releaseVersion} version bump in: ${resolvedFiles.join(', ')}`, + ); + return true; + } catch (resolutionError) { + logger.debug( + 'Could not auto-resolve the version bump conflicts:', + resolutionError, + ); + return false; + } +} + /** * Pull with `--rebase`, aborting the rebase if the pull fails so the * repository isn't left mid-rebase. @@ -1004,6 +1051,7 @@ export async function publishMain(argv: PublishOptions): Promise { mergeTarget, argv.keepBranch, rev ? revision : branchName, + newVersion, ); mergedInto = mergeTarget; } catch (mergeError) { diff --git a/src/utils/__tests__/versionConflicts.test.ts b/src/utils/__tests__/versionConflicts.test.ts new file mode 100644 index 000000000..9df87b0ed --- /dev/null +++ b/src/utils/__tests__/versionConflicts.test.ts @@ -0,0 +1,157 @@ +import { describe, expect, test } from 'vitest'; + +import { + resolveVersionBumpConflicts, + resolveVersionBumpHunk, +} from '../versionConflicts'; + +const OURS = '<'.repeat(32); +const BASE = '|'.repeat(32); +const SEP = '='.repeat(32); +const THEIRS = '>'.repeat(32); + +function conflict(ours: string, base: string, theirs: string): string { + return `${OURS} ours\n${ours}${BASE} base\n${base}${SEP}\n${theirs}${THEIRS} theirs\n`; +} + +describe('resolveVersionBumpHunk', () => { + test('applies the version bump to the target branch side', () => { + expect( + resolveVersionBumpHunk( + 'version: 0.48.0-dev.0\ndescription: New description\n', + 'version: 0.48.0-dev.0\ndescription: Old description\n', + 'version: 0.48.0\ndescription: Old description\n', + '0.48.0', + ), + ).toBe('version: 0.48.0\ndescription: New description\n'); + }); + + test('bumps every occurrence of the old version in the hunk', () => { + expect( + resolveVersionBumpHunk( + '"version": "1.1.0",\n"dependencies": { "core": "1.1.0", "x": "2" }\n', + '"version": "1.1.0",\n"dependencies": { "core": "1.1.0" }\n', + '"version": "1.2.0",\n"dependencies": { "core": "1.2.0" }\n', + '1.2.0', + ), + ).toBe( + '"version": "1.2.0",\n"dependencies": { "core": "1.2.0", "x": "2" }\n', + ); + }); + + test('handles PEP 440 style development versions', () => { + expect( + resolveVersionBumpHunk( + 'version = "2.1.0.dev0"\nrequires-python = ">=3.10"\n', + 'version = "2.1.0.dev0"\nrequires-python = ">=3.9"\n', + 'version = "2.1.0"\nrequires-python = ">=3.9"\n', + '2.1.0', + ), + ).toBe('version = "2.1.0"\nrequires-python = ">=3.10"\n'); + }); + + test('does not touch versions that only contain the old version', () => { + expect( + resolveVersionBumpHunk( + 'version: 1.0.0\nmin: 11.0.0\nnext: 1.0.0-rc.1\nnew line\n', + 'version: 1.0.0\nmin: 11.0.0\nnext: 1.0.0-rc.1\n', + 'version: 1.1.0\nmin: 11.0.0\nnext: 1.0.0-rc.1\n', + '1.1.0', + ), + ).toBe('version: 1.1.0\nmin: 11.0.0\nnext: 1.0.0-rc.1\nnew line\n'); + }); + + test('refuses when the release side changed more than versions', () => { + expect( + resolveVersionBumpHunk( + 'version: 1.0.0\ndescription: Target\n', + 'version: 1.0.0\ndescription: Base\n', + 'version: 1.1.0\ndescription: Hotfix\n', + '1.1.0', + ), + ).toBeNull(); + }); + + test('refuses when the release side added a line with the version', () => { + expect( + resolveVersionBumpHunk( + '## Unreleased\n\n- Target change\n', + '## Unreleased\n', + '## 1.1.0\n\n- Release change\n', + '1.1.0', + ), + ).toBeNull(); + }); + + test('refuses when the release side bumped to another version', () => { + expect( + resolveVersionBumpHunk( + 'dep: 1.0.0\nother: x\n', + 'dep: 1.0.0\n', + 'dep: 3.0.0\n', + '1.1.0', + ), + ).toBeNull(); + }); + + test('refuses when the target branch changed the version itself', () => { + expect( + resolveVersionBumpHunk( + 'version: 2.0.0-dev.0\ndescription: Target\n', + 'version: 1.1.0-dev.0\ndescription: Base\n', + 'version: 1.1.0\ndescription: Base\n', + '1.1.0', + ), + ).toBeNull(); + }); +}); + +describe('resolveVersionBumpConflicts', () => { + test('resolves every hunk and keeps the merged content around them', () => { + const merged = [ + '---\n', + 'name: init\n', + conflict( + 'version: 0.48.0-dev.0\ndescription: New\n', + 'version: 0.48.0-dev.0\ndescription: Old\n', + 'version: 0.48.0\ndescription: Old\n', + ), + 'requires: sentry\n', + '=======\n', + conflict('v0.48.0-dev.0 (new)\n', 'v0.48.0-dev.0\n', 'v0.48.0\n'), + ].join(''); + + expect(resolveVersionBumpConflicts(merged, '0.48.0')).toBe( + '---\nname: init\nversion: 0.48.0\ndescription: New\nrequires: sentry\n=======\nv0.48.0 (new)\n', + ); + }); + + test('refuses when any hunk is not a version bump', () => { + const merged = [ + conflict( + 'version: 1.0.0\na\n', + 'version: 1.0.0\nb\n', + 'version: 1.1.0\nb\n', + ), + conflict('x\n', 'y\n', 'z\n'), + ].join(''); + + expect(resolveVersionBumpConflicts(merged, '1.1.0')).toBeNull(); + }); + + test('refuses conflicts without a base section', () => { + expect( + resolveVersionBumpConflicts( + `${OURS}\nversion: 1.0.0\n${SEP}\nversion: 1.1.0\n${THEIRS}\n`, + '1.1.0', + ), + ).toBeNull(); + }); + + test('refuses unterminated conflicts and content without conflicts', () => { + expect( + resolveVersionBumpConflicts(`${OURS}\na\n${BASE}\nb\n`, '1.1.0'), + ).toBeNull(); + expect(resolveVersionBumpConflicts('no conflicts\n', '1.1.0')).toBeNull(); + }); +}); diff --git a/src/utils/versionConflicts.ts b/src/utils/versionConflicts.ts new file mode 100644 index 000000000..5c79dbf8b --- /dev/null +++ b/src/utils/versionConflicts.ts @@ -0,0 +1,311 @@ +import { writeFile } from 'fs/promises'; +import path from 'path'; +import type { SimpleGit } from 'simple-git'; + +import { logger } from '../logger'; +import { safeFs } from './dryRun'; +import { withTempDir } from './files'; + +/** + * Conflict marker size passed to `git merge-file`. Large enough that file + * content (e.g. a Markdown setext heading underlined with `=======`) is not + * mistaken for a marker. + */ +const MARKER_SIZE = 32; +const OURS_MARKER = '<'.repeat(MARKER_SIZE); +const BASE_MARKER = '|'.repeat(MARKER_SIZE); +const SEPARATOR_MARKER = '='.repeat(MARKER_SIZE); +const THEIRS_MARKER = '>'.repeat(MARKER_SIZE); + +/** + * A version-like token: at least `MAJOR.MINOR`, optionally followed by + * dot, dash or plus separated segments (`1.2.3`, `1.2.3-dev.0`, + * `1.2.3.dev0`, `24.1.0+build.5`). + */ +const VERSION_PATTERN = '\\d+\\.[0-9A-Za-z]+(?:[-+.][0-9A-Za-z]+)*'; + +function escapeRegExp(text: string): string { + return text.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + +/** + * Matches `version` only where it is a whole version token, so `1.2.3` does + * not match inside `11.2.3` or `1.2.3-dev.0`. + */ +function wholeVersionRegExp(version: string): RegExp { + return new RegExp( + `(? v !== version)); + if (oldVersions.size === 0) { + return null; + } + + let resolved = ours; + for (const oldVersion of oldVersions) { + const oldVersionRegExp = wholeVersionRegExp(oldVersion); + if (resolved.search(oldVersionRegExp) === -1) { + // The target branch changed the version line itself; let a human + // decide which version it should have. + return null; + } + resolved = resolved.replace(oldVersionRegExp, version); + } + return resolved; +} + +function isMarker(line: string, marker: string): boolean { + if (!line.startsWith(marker)) { + return false; + } + const next = line.charAt(marker.length); + return next === '' || next === ' ' || next === '\n' || next === '\r'; +} + +/** + * Resolves every conflict hunk in `git merge-file --diff3` output that only + * conflicts because of a version bump on the release side. + * + * @param merged Output of `git merge-file -p --diff3` with markers of + * `MARKER_SIZE` characters + * @param version The released version + * @returns The fully resolved content, or null if any hunk cannot be + * resolved this way + */ +export function resolveVersionBumpConflicts( + merged: string, + version: string, +): string | null { + const lines = merged.split(/(?<=\n)/); + const output: string[] = []; + let state: 'outside' | 'ours' | 'base' | 'theirs' = 'outside'; + let ours = ''; + let base = ''; + let theirs = ''; + let resolvedAny = false; + + for (const line of lines) { + switch (state) { + case 'outside': + if (isMarker(line, OURS_MARKER)) { + state = 'ours'; + ours = base = theirs = ''; + } else { + output.push(line); + } + break; + case 'ours': + if (isMarker(line, BASE_MARKER)) { + state = 'base'; + } else if ( + isMarker(line, SEPARATOR_MARKER) || + isMarker(line, OURS_MARKER) || + isMarker(line, THEIRS_MARKER) + ) { + // Without the base section we cannot tell what the release changed. + return null; + } else { + ours += line; + } + break; + case 'base': + if (isMarker(line, SEPARATOR_MARKER)) { + state = 'theirs'; + } else if ( + isMarker(line, OURS_MARKER) || + isMarker(line, BASE_MARKER) || + isMarker(line, THEIRS_MARKER) + ) { + return null; + } else { + base += line; + } + break; + case 'theirs': + if (isMarker(line, THEIRS_MARKER)) { + const resolved = resolveVersionBumpHunk(ours, base, theirs, version); + if (resolved === null) { + return null; + } + output.push(resolved); + resolvedAny = true; + state = 'outside'; + } else if ( + isMarker(line, OURS_MARKER) || + isMarker(line, BASE_MARKER) || + isMarker(line, SEPARATOR_MARKER) + ) { + return null; + } else { + theirs += line; + } + break; + } + } + + if (state !== 'outside' || !resolvedAny) { + return null; + } + return output.join(''); +} + +interface UnmergedEntry { + base?: string; + ours?: string; + theirs?: string; +} + +/** + * Lists unmerged index entries as repository-root-relative paths mapped to + * the blob of each conflict stage. + */ +async function getUnmergedEntries( + git: SimpleGit, +): Promise> { + const output = await git.raw(['ls-files', '-u', '-z', '--full-name']); + const entries = new Map(); + for (const record of output.split('\0')) { + // " \t" + const match = /^\d+ ([0-9a-f]+) ([123])\t(.+)$/s.exec(record); + if (!match) { + continue; + } + const [, object, stage, file] = match; + const entry = entries.get(file) ?? {}; + if (stage === '1') { + entry.base = object; + } else if (stage === '2') { + entry.ours = object; + } else { + entry.theirs = object; + } + entries.set(file, entry); + } + return entries; +} + +/** + * Resolves an in-progress merge whose conflicts are caused only by the + * release version bump, then commits the merge. + * + * The release branch bumps version strings (e.g. `1.2.0-dev.0` to `1.2.0`). + * When the target branch changed lines next to those strings after the + * release branch was cut, Git reports a conflict even though the intent of + * both sides is clear. For each such hunk, this keeps the target branch's + * content and applies the version bump to it. + * + * Nothing is written unless every conflicted file can be resolved this way. + * + * @param git Git client with a conflicted merge in progress + * @param version The released version that the release branch bumped to + * @returns The resolved files, or null if the merge was left untouched + */ +export async function resolveVersionBumpMergeConflicts( + git: SimpleGit, + version: string, +): Promise { + const entries = await getUnmergedEntries(git); + if (entries.size === 0) { + return null; + } + + const repoRoot = (await git.revparse(['--show-toplevel'])).trim(); + const resolutions = new Map(); + + for (const [file, entry] of entries) { + if (!entry.base || !entry.ours || !entry.theirs) { + // Add/add, modify/delete, and rename conflicts are not version bumps. + logger.debug(`Cannot auto-resolve "${file}": not a content conflict.`); + return null; + } + const stages = { + ours: entry.ours, + base: entry.base, + theirs: entry.theirs, + }; + const merged = await withTempDir( + async directory => { + const paths: string[] = []; + for (const [name, object] of Object.entries(stages)) { + const content = await git.raw(['cat-file', 'blob', object]); + // Skip binary and non-UTF-8 files: they would not survive the + // round trip through a JS string. + if (content.includes('\0') || content.includes('\uFFFD')) { + return null; + } + const stagePath = path.join(directory, name); + await writeFile(stagePath, content); + paths.push(stagePath); + } + // Exits with the number of conflicts. simple-git only rejects when + // there is also stderr output, which `-q` suppresses for conflicts. + return git.raw([ + 'merge-file', + '-p', + '-q', + '--diff3', + `--marker-size=${MARKER_SIZE}`, + ...paths, + ]); + }, + true, + 'craft-merge-', + ); + if (merged === null) { + logger.debug(`Cannot auto-resolve "${file}": not a UTF-8 text file.`); + return null; + } + + const resolved = resolveVersionBumpConflicts(merged, version); + if (resolved === null) { + logger.debug( + `Cannot auto-resolve "${file}": the conflict is not only a version bump.`, + ); + return null; + } + resolutions.set(file, resolved); + } + + for (const [file, content] of resolutions) { + const filePath = path.join(repoRoot, file); + await safeFs.writeFile(filePath, content); + await git.add(filePath); + } + await git.raw(['commit', '--no-edit']); + return [...resolutions.keys()]; +}