ci: Handle marker-less delete/modify conflicts in the master→3.x sync (#37072)

.github/DEVELOPING_V3.md

@@ -177,6 +177,14 @@ If the PR says the lockfile was **deferred** (a `package.json` / `pnpm-workspace
 conflicted too), resolve the manifests first, then regenerate it with
 `pnpm install --lockfile-only` and include the result in your fix commit.
 
+Watch for the **"Deleted on one side, changed on the other"** section. Git leaves no markers
+for a delete/modify, so the branch looks clean where it is not: the merge keeps `3.x`'s side
+(its deletion, or its file when `master` deleted it). Confirm that is right, and check
+whether `master`'s change has to be carried over by hand — when a breaking commit re-recorded
+or renamed a file, `master`'s edit to the old one usually belongs on the replacement, and no
+automation can find that for you. The PR names the `master` commit behind each conflicted
+path so you can see what the change was.
+
 Then **merge the PR with the normal merge button.** `master`'s commits arrive as-is and your
 fix stays its own commit. **Never close a conflict PR unmerged** — closing resolves nothing,
 the same conflict reopens on the next sync, and the new PR will call out the abandoned one.
@@ -194,9 +202,10 @@ is squashed, and the tree guard proves the result is exactly the merge of `3.x`
 `master`.
 
 **Who gets pinged.** The conflict is attributed to the authors of the `3.x` commits behind the
-conflicted files (`.github/scripts/sync-conflict-owners.mjs`, mapped to GitHub accounts).
-Those authors are **requested as reviewers** on the conflict PR and listed in the
-`#alerts-v3-sync` message.
+conflicted files, plus the `master` commits that touched the same files
+(`.github/scripts/sync-conflict-owners.mjs`, mapped to GitHub accounts). Both sides are named
+in the PR body and the `#alerts-v3-sync` message. **Nobody is requested as a reviewer** — the
+resolver picks the PR up themselves.
 
 ## Trialing v3
 

.github/WORKFLOWS.md

@@ -415,9 +415,11 @@ to mechanical, tool-generated files (the pnpm lockfile, bot-maintained data file
 `MECHANICAL_PATHS` in `sync-master-to-3x.mjs`) are auto-resolved during the replay; the tree
 check then applies to every path except those files. On a real code conflict `3.x` is left
 untouched and a draft PR carrying the conflict markers (labeled `automation:v3-sync`, with
-mechanical files pre-resolved) is opened on `sync/master-to-3x`, requesting the
-breaking-commit authors as reviewers via `sync-conflict-owners.mjs`, posting to
-`#alerts-v3-sync` and pausing further syncs until it is resolved and merged normally.
+mechanical files pre-resolved) is opened on `sync/master-to-3x`, naming both ends of the
+conflict — the breaking-commit authors and the `master` commits that touched the same files
+— via `sync-conflict-owners.mjs`, posting to `#alerts-v3-sync` and pausing further syncs
+until it is resolved and merged normally. Delete/modify conflicts have no markers to carry,
+so they are resolved toward `3.x` and listed as an explicit decision in the PR body.
 `build-v3-nightly.yml` publishes `n8nio/n8n:v3-nightly[-<date>]` images from `3.x`
 by calling `docker-build-push.yml` with `ref: 3.x` + `date_tag`. On Mondays it also
 retags that run's n8n + runners manifests as a release candidate (by digest on GHCR, so

.github/scripts/sync-conflict-owners.mjs

@@ -1,6 +1,7 @@
 #!/usr/bin/env node
 /**
- * Attributes a master→3.x sync conflict to the authors of the breaking commits.
+ * Attributes a master→3.x sync conflict to BOTH sides: the authors of the breaking commits
+ * that diverged 3.x, and the master commits that touched the same files.
  *
  * Since 3.x = master + breaking commits, the commits that diverged 3.x are exactly
  * `<base>..<tip>` (where <base> is the fetched master SHA and <tip> the pre-rebase 3.x
@@ -8,12 +9,18 @@
  * conflict; their GitHub authors are who to nudge. `conflictedFiles` must run while the
  * rebase is stopped, i.e. with unmerged paths present in the index.
  *
+ * The master half is the other end of the same conflict, and knowing which master commit
+ * touched the file is usually what makes it resolvable (a fixture re-recorded by a
+ * dependency bump reads as an unexplained clash without it). Nobody is requested as a
+ * reviewer: the PR body and the Slack post name both sides, and the resolver picks
+ * themselves.
+ *
  * git log gives only the author name/email, and ~2/3 of n8n authors commit with a
- * non-noreply email that carries no GitHub username. So the conflicted files → breaking
- * SHAs analysis is done locally, and a SINGLE GraphQL call maps those few SHAs to GitHub
+ * non-noreply email that carries no GitHub username. So the conflicted files → commit
+ * analysis is done locally, and a SINGLE GraphQL call maps those few SHAs to GitHub
  * logins. Bot- and unlinked-account commits resolve to a null user and are skipped.
  *
- * Emits a JSON object to stdout: { ownersCsv, slack, body }.
+ * Emits a JSON object to stdout: { slack, body }.
  *
  * Usage:
  *   node .github/scripts/sync-conflict-owners.mjs --base <masterSha> --sync-branch <name>
@@ -47,10 +54,39 @@ export function breakingShas(base, files, git = runGit, tip = 'HEAD') {
 	return [...shas];
 }
 
-// Resolve commit SHAs to GitHub logins in one GraphQL call. Commits whose author has
-// no linked account (unverified email, bots) resolve to a null user and are dropped.
-export async function resolveLogins(repo, shas, token, fetchFn = fetch) {
-	if (shas.length === 0) return [];
+// Master commits that touched the given files since the branches diverged — the other end
+// of the conflict. Capped per file: the point is naming the change, not a full log.
+export function masterCommitsByFile(base, files, git = runGit, tip = 'FETCH_HEAD', limit = 3) {
+	const byFile = new Map();
+	for (const file of files) {
+		const out = git([
+			'log',
+			`${base}..${tip}`,
+			`--max-count=${limit}`,
+			'--format=%H %h %s',
+			'--',
+			file,
+		]);
+		const commits = out
+			.split('\n')
+			.filter(Boolean)
+			.map((line) => {
+				const [sha, short, ...rest] = line.split(' ');
+				return { sha, short, subject: rest.join(' ') };
+			});
+		byFile.set(file, commits);
+	}
+	return byFile;
+}
+
+/**
+ * Resolve commit SHAs to GitHub logins in one GraphQL call. Commits whose author has no
+ * linked account (unverified email, bots) resolve to a null user and are left out.
+ *
+ * @returns {Promise<Map<string, string>>} SHA → login, for the SHAs that resolved.
+ */
+export async function resolveCommitAuthors(repo, shas, token, fetchFn = fetch) {
+	if (shas.length === 0) return new Map();
 	const [owner, name] = repo.split('/');
 	const aliases = shas
 		.map((sha, i) => `c${i}: object(oid: "${sha}") { ... on Commit { author { user { login } } } }`)
@@ -71,35 +107,117 @@ export async function resolveLogins(repo, shas, token, fetchFn = fetch) {
 	if (json.errors) throw new Error(`GitHub GraphQL error: ${JSON.stringify(json.errors)}`);
 
 	const repository = json.data?.repository ?? {};
-	const logins = new Set();
-	for (const node of Object.values(repository)) {
-		const login = node?.author?.user?.login;
-		if (login) logins.add(login);
-	}
-	return [...logins].sort();
+	const authors = new Map();
+	shas.forEach((sha, i) => {
+		const login = repository[`c${i}`]?.author?.user?.login;
+		if (login) authors.set(sha, login);
+	});
+	return authors;
 }
 
-// Build the conflict-PR body, reviewer CSV, and Slack owner line.
+/**
+ * Both ends of a conflict, in one GraphQL call: the logins behind the target-side
+ * (breaking) commits, and the master commits that touched each conflicted path with their
+ * authors. Degrades to empty attribution — a transient API failure must still let the
+ * conflict PR open.
+ *
+ * `base` is the fetched master SHA (= the target branch's base, since 3.x = master + N),
+ * `tip` the pre-merge target tip, and the master half is read from where the two diverged.
+ */
+export async function gatherAttribution({
+	repo,
+	token,
+	files,
+	base,
+	tip = 'HEAD',
+	git = runGit,
+	fetchFn = fetch,
+	log = console.error,
+}) {
+	const targetShas = breakingShas(base, files, git, tip);
+
+	let byFile = new Map();
+	try {
+		byFile = masterCommitsByFile(git(['merge-base', tip, base]), files, git, base);
+	} catch (error) {
+		log(`warning: could not read the master side of the conflict: ${error.message}`);
+	}
+
+	const masterShas = [...byFile.values()].flat().map((c) => c.sha);
+	let authors = new Map();
+	try {
+		authors = await resolveCommitAuthors(repo, [...targetShas, ...masterShas], token, fetchFn);
+	} catch (error) {
+		log(`warning: could not resolve owners: ${error.message}`);
+	}
+
+	return {
+		owners: [...new Set(targetShas.map((sha) => authors.get(sha)).filter(Boolean))].sort(),
+		masterCommits: new Map(
+			[...byFile].map(([file, commits]) => [
+				file,
+				commits.map((c) => ({ ...c, login: authors.get(c.sha) })),
+			]),
+		),
+	};
+}
+
+/**
+ * Build the conflict-PR body and the Slack owner line.
+ *
+ * `files` are the marker-carrying conflicts. `deleteConflicts` are the ones git left
+ * WITHOUT markers ({ path, deletedBy: 'target' | 'master' }) — they need their own section
+ * or a resolver reads the branch as "nothing to fix here". `masterCommits` maps a
+ * conflicted path to the master commits that touched it, so each entry names both ends of
+ * the clash. Nobody is requested as a reviewer.
+ */
 export function buildOutputs({
 	syncBranch,
 	targetBranch = '3.x',
 	files,
 	owners,
+	deleteConflicts = [],
+	masterCommits = new Map(),
 	preResolved = [],
 	lockfileDeferred = false,
 	abandoned = [],
 }) {
-	const filesMd = files.map((f) => `- \`${f}\``).join('\n') || '_none detected_';
+	// `- \`path\`` plus a nested line per master commit that touched it.
+	const fileMd = (path, suffix = '') => {
+		const commits = masterCommits.get(path) ?? [];
+		return [
+			`- \`${path}\`${suffix}`,
+			...commits.map(
+				({ short, subject, login }) =>
+					`  - master: \`${short}\` ${subject}${login ? ` — @${login}` : ''}`,
+			),
+		].join('\n');
+	};
+	const filesMd = files.map((f) => fileMd(f)).join('\n') || '_none detected_';
+	const deleteMd = deleteConflicts
+		.map(({ path, deletedBy }) =>
+			fileMd(
+				path,
+				deletedBy === 'master'
+					? ` — deleted on master, changed on \`${targetBranch}\`; the merge kept \`${targetBranch}\`'s file`
+					: ` — deleted on \`${targetBranch}\`, changed on master; the merge kept \`${targetBranch}\`'s deletion`,
+			),
+		)
+		.join('\n');
 	const ownersMd = owners.length
 		? owners.map((o) => `- @${o}`).join('\n')
 		: '_Could not auto-attribute — review the conflicted files manually._';
 	const abandonedWarning = abandoned.length
 		? `A previous PR for this recurring conflict (${abandoned.map((pr) => `#${pr.number}`).join(', ')}) was closed without being merged. Closing resolves nothing — the conflict comes back on the next sync. **Merge, don't close.**`
 		: '';
+	const masterOwners = [
+		...new Set([...masterCommits.values()].flat().flatMap((c) => (c.login ? [c.login] : []))),
+	].sort();
 	const slack = [
 		owners.length
 			? `Likely owners (GitHub): ${owners.map((o) => `@${o}`).join(' ')}`
 			: 'Could not auto-attribute owners.',
+		masterOwners.length ? `· master side: ${masterOwners.map((o) => `@${o}`).join(' ')}` : '',
 		abandoned.length
 			? `⚠️ ${abandoned.map((pr) => `<${pr.url}|#${pr.number}>`).join(', ')} was closed without merging and the conflict is back — merge this one, don't close it.`
 			: '',
@@ -110,18 +228,28 @@ export function buildOutputs({
 		`Automated \`master\`→\`${targetBranch}\` sync hit a conflict.`,
 		...(abandonedWarning ? ['', '> [!WARNING]', `> ${abandonedWarning}`] : []),
 		'',
-		`**\`${targetBranch}\` was not touched.** This branch is \`master\` merged into \`${targetBranch}\` with the **conflict markers committed**, so you can see exactly what clashed. The required checks stay red until they are resolved, so this PR cannot be merged half-done.`,
+		files.length
+			? `**\`${targetBranch}\` was not touched.** This branch is \`master\` merged into \`${targetBranch}\` with the conflicts committed exactly as git left them — **conflict markers included** — so you can see what clashed. The required checks stay red until they are resolved, so this PR cannot be merged half-done.`
+			: `**\`${targetBranch}\` was not touched.** This branch is \`master\` merged into \`${targetBranch}\` exactly as git left it. Nothing here carries conflict markers, so the checks can go green on a merge that was resolved by default — read the decision below before merging.`,
 		'',
 		'### How to resolve',
 		`1. \`git fetch origin ${syncBranch} && git switch ${syncBranch}\``,
-		'2. Fix the conflict markers and commit them **in one commit of your own** (rather than amending the merge).',
+		'2. Resolve everything listed below and commit it **in one commit of your own** (rather than amending the merge).',
 		`3. \`git push origin ${syncBranch}\``,
 		`4. **Merge this PR with the normal merge button.** \`master\`'s commits come in as-is and your fix stays its own commit — nothing is squashed. Never close this PR unmerged: closing resolves nothing and the same conflict reopens on the next sync.`,
 		'',
 		`**Daily syncs are paused until this PR is merged.** The next sync then replays \`${targetBranch}\` onto master linearly again — which also drops this merge commit and its markers out of \`${targetBranch}\`'s history — and verifies the result is exactly what a merge would produce.`,
-		'',
-		'### Conflicted files',
-		filesMd,
+		// An empty file list is only worth printing when there is no other section either:
+		// "_none detected_" is then the signal that detection itself came up short.
+		...(files.length || !deleteConflicts.length ? ['', '### Conflicted files', filesMd] : []),
+		...(deleteConflicts.length
+			? [
+					'',
+					'### Deleted on one side, changed on the other',
+					`Git leaves **no conflict markers** for these, so the branch looks clean where it is not — the merge kept \`${targetBranch}\`'s side. Decide whether that is right, and whether the other side's change has to be carried over (onto a re-recorded or renamed replacement file, typically):`,
+					deleteMd,
+				]
+			: []),
 		...(preResolved.length
 			? [
 					'',
@@ -139,10 +267,10 @@ export function buildOutputs({
 			: []),
 		'',
 		'### Likely owners',
-		`Authors of the ${targetBranch} commits behind the conflicted files, requested as reviewers:`,
+		`Authors of the ${targetBranch} commits behind the conflicted files — nobody is requested as a reviewer, so pick this up between yourselves:`,
 		ownersMd,
 	].join('\n');
-	return { ownersCsv: owners.join(','), slack, body };
+	return { slack, body };
 }
 
 async function main() {
@@ -164,16 +292,7 @@ async function main() {
 	if (!token) throw new Error('GH_TOKEN / GITHUB_TOKEN env var is required');
 
 	const files = conflictedFiles();
-	const shas = breakingShas(base, files);
-
-	// Degrade gracefully: a transient API failure should still open the PR
-	// (unattributed) rather than fail the whole sync.
-	let owners = [];
-	try {
-		owners = await resolveLogins(repo, shas, token);
-	} catch (error) {
-		console.error(`warning: could not resolve owners: ${error.message}`);
-	}
+	const { owners, masterCommits } = await gatherAttribution({ repo, token, files, base });
 
 	process.stdout.write(
 		JSON.stringify(
@@ -182,6 +301,7 @@ async function main() {
 				targetBranch: values['target-branch'],
 				files,
 				owners,
+				masterCommits,
 			}),
 		),
 	);

.github/scripts/sync-conflict-owners.test.mjs

@@ -1,7 +1,13 @@
 import assert from 'node:assert/strict';
 import { test } from 'node:test';
 
-import { breakingShas, resolveLogins, buildOutputs } from './sync-conflict-owners.mjs';
+import {
+	breakingShas,
+	masterCommitsByFile,
+	resolveCommitAuthors,
+	gatherAttribution,
+	buildOutputs,
+} from './sync-conflict-owners.mjs';
 
 test('breakingShas collects unique SHAs across the conflicted files only', () => {
 	const calls = [];
@@ -29,7 +35,97 @@ test('breakingShas can be scoped to an explicit tip (the pre-rebase 3.x tip)', (
 	assert.deepEqual(calls[0], ['log', 'BASE..PREHEAD', '--format=%H', '--', 'a.ts']);
 });
 
-test('resolveLogins maps SHAs to logins in one call, dropping unlinked/bot authors', async () => {
+test('masterCommitsByFile reads the master side of each conflicted file, capped', () => {
+	const calls = [];
+	const git = (args) => {
+		calls.push(args);
+		return args.at(-1) === 'a.ts'
+			? 'full1 short1 fix(core): a thing (#1)\nfull2 short2 chore: another'
+			: '';
+	};
+
+	const byFile = masterCommitsByFile('BASE', ['a.ts', 'b.ts'], git, 'MASTER', 3);
+
+	assert.deepEqual(byFile.get('a.ts'), [
+		{ sha: 'full1', short: 'short1', subject: 'fix(core): a thing (#1)' },
+		{ sha: 'full2', short: 'short2', subject: 'chore: another' },
+	]);
+	assert.deepEqual(byFile.get('b.ts'), []);
+	assert.deepEqual(calls[0], [
+		'log',
+		'BASE..MASTER',
+		'--max-count=3',
+		'--format=%H %h %s',
+		'--',
+		'a.ts',
+	]);
+});
+
+test('gatherAttribution resolves both sides of the conflict in a single call', async () => {
+	const git = (args) => {
+		if (args[0] === 'merge-base') return 'DIVERGED';
+		if (args.includes('--format=%H')) return 'breaking-sha';
+		if (args.includes('--format=%H %h %s')) return 'master-sha msha build(core): bump deps (#2)';
+		return '';
+	};
+	let queried = [];
+	const fetchFn = async (_url, opts) => {
+		queried.push(JSON.parse(opts.body).query);
+		return {
+			ok: true,
+			json: async () => ({
+				data: {
+					repository: {
+						c0: { author: { user: { login: 'alice' } } },
+						c1: { author: { user: { login: 'bob' } } },
+					},
+				},
+			}),
+		};
+	};
+
+	const { owners, masterCommits } = await gatherAttribution({
+		repo: 'n8n-io/n8n',
+		token: 't',
+		files: ['a.ts'],
+		base: 'MASTER',
+		tip: 'PREHEAD',
+		git,
+		fetchFn,
+	});
+
+	assert.equal(queried.length, 1);
+	assert.deepEqual(owners, ['alice']);
+	assert.deepEqual(masterCommits.get('a.ts'), [
+		{ sha: 'master-sha', short: 'msha', subject: 'build(core): bump deps (#2)', login: 'bob' },
+	]);
+});
+
+test('gatherAttribution still attributes the 3.x side when the master side cannot be read', async () => {
+	const git = (args) => {
+		if (args[0] === 'merge-base') throw new Error('no merge base');
+		return 'breaking-sha';
+	};
+	const fetchFn = async () => ({
+		ok: true,
+		json: async () => ({ data: { repository: { c0: { author: { user: { login: 'alice' } } } } } }),
+	});
+
+	const { owners, masterCommits } = await gatherAttribution({
+		repo: 'r',
+		token: 't',
+		files: ['a.ts'],
+		base: 'MASTER',
+		git,
+		fetchFn,
+		log: () => {},
+	});
+
+	assert.deepEqual(owners, ['alice']);
+	assert.equal(masterCommits.size, 0);
+});
+
+test('resolveCommitAuthors maps SHAs to logins in one call, dropping unlinked/bot authors', async () => {
 	let calls = 0;
 	const fetchFn = async (url, opts) => {
 		calls++;
@@ -50,35 +146,40 @@ test('resolveLogins maps SHAs to logins in one call, dropping unlinked/bot autho
 			}),
 		};
 	};
-	const owners = await resolveLogins('n8n-io/n8n', ['sha1', 'sha2', 'sha3'], 't', fetchFn);
+	const authors = await resolveCommitAuthors('n8n-io/n8n', ['sha1', 'sha2', 'sha3'], 't', fetchFn);
 	assert.equal(calls, 1); // single batched request
-	assert.deepEqual(owners, ['alice', 'bob']); // sorted, deduped, null dropped
+	assert.deepEqual(
+		[...authors],
+		[
+			['sha1', 'bob'],
+			['sha2', 'alice'],
+		],
+	); // sha3's null author is dropped
 });
 
-test('resolveLogins makes no request when there are no SHAs', async () => {
+test('resolveCommitAuthors makes no request when there are no SHAs', async () => {
 	let calls = 0;
 	const fetchFn = async () => {
 		calls++;
 		return { ok: true, json: async () => ({ data: { repository: {} } }) };
 	};
-	assert.deepEqual(await resolveLogins('r', [], 't', fetchFn), []);
+	assert.equal((await resolveCommitAuthors('r', [], 't', fetchFn)).size, 0);
 	assert.equal(calls, 0);
 });
 
-test('resolveLogins throws on API/GraphQL errors (caller degrades gracefully)', async () => {
+test('resolveCommitAuthors throws on API/GraphQL errors (caller degrades gracefully)', async () => {
 	const httpError = async () => ({ ok: false, status: 502, json: async () => ({}) });
 	const gqlError = async () => ({ ok: true, json: async () => ({ errors: [{ message: 'bad' }] }) });
-	await assert.rejects(resolveLogins('r', ['s'], 't', httpError), /502/);
-	await assert.rejects(resolveLogins('r', ['s'], 't', gqlError), /GraphQL error/);
+	await assert.rejects(resolveCommitAuthors('r', ['s'], 't', httpError), /502/);
+	await assert.rejects(resolveCommitAuthors('r', ['s'], 't', gqlError), /GraphQL error/);
 });
 
-test('buildOutputs formats reviewers, slack line, and PR body with owners', () => {
+test('buildOutputs formats the slack line and PR body with owners', () => {
 	const out = buildOutputs({
 		syncBranch: 'sync/master-to-3x',
 		files: ['packages/cli/x.ts'],
 		owners: ['alice', 'bob'],
 	});
-	assert.equal(out.ownersCsv, 'alice,bob');
 	assert.equal(out.slack, 'Likely owners (GitHub): @alice @bob');
 	assert.match(out.body, /### Conflicted files/);
 	assert.match(out.body, /- `packages\/cli\/x\.ts`/);
@@ -86,7 +187,8 @@ test('buildOutputs formats reviewers, slack line, and PR body with owners', () =
 	assert.match(out.body, /- @bob/);
 	assert.match(out.body, /Daily syncs are paused until this PR is merged/);
 	// Markers are the review surface; the fix goes in the resolver's own commit.
-	assert.match(out.body, /conflict markers committed/);
+	assert.match(out.body, /conflict markers included/);
+	assert.match(out.body, /nobody is requested as a reviewer/);
 	assert.match(out.body, /in one commit of your own/);
 	assert.match(out.body, /Merge this PR with the normal merge button/);
 	assert.match(out.body, /`3\.x` was not touched/);
@@ -99,7 +201,6 @@ test('buildOutputs degrades gracefully when nothing could be attributed', () =>
 		files: ['x.ts'],
 		owners: [],
 	});
-	assert.equal(out.ownersCsv, '');
 	assert.equal(out.slack, 'Could not auto-attribute owners.');
 	assert.match(out.body, /Could not auto-attribute/);
 });
@@ -141,3 +242,52 @@ test('buildOutputs warns about conflict PRs that were closed without merging', (
 	assert.match(out.slack, /<https:\/\/github\.com\/n8n-io\/n8n\/pull\/42\|#42>/);
 	assert.match(out.slack, /merge this one, don't close it/);
 });
+
+test('buildOutputs names the master commit behind each conflicted file', () => {
+	const out = buildOutputs({
+		syncBranch: 'sync/master-to-3x',
+		files: ['packages/cli/x.ts'],
+		owners: ['alice'],
+		masterCommits: new Map([
+			[
+				'packages/cli/x.ts',
+				[{ short: 'abc1234', subject: 'build(core): bump deps (#2)', login: 'bob' }],
+			],
+		]),
+	});
+	assert.match(
+		out.body,
+		/- `packages\/cli\/x\.ts`\n {2}- master: `abc1234` build\(core\): bump deps \(#2\) — @bob/,
+	);
+	assert.equal(out.slack, 'Likely owners (GitHub): @alice · master side: @bob');
+});
+
+test('buildOutputs gives marker-less delete/modify conflicts their own section', () => {
+	const out = buildOutputs({
+		syncBranch: 'sync/master-to-3x',
+		files: [],
+		owners: ['alice'],
+		deleteConflicts: [
+			{ path: 'fixtures/a.json', deletedBy: 'target' },
+			{ path: 'src/b.ts', deletedBy: 'master' },
+		],
+		masterCommits: new Map([
+			['fixtures/a.json', [{ short: 'abc1234', subject: 'build: re-record (#2)' }]],
+		]),
+	});
+	assert.match(out.body, /### Deleted on one side, changed on the other/);
+	// Nothing keeps the checks red here, so the body must not promise that it does.
+	assert.match(out.body, /the checks can go green on a merge that was resolved by default/);
+	assert.match(out.body, /no conflict markers/);
+	assert.match(
+		out.body,
+		/- `fixtures\/a\.json` — deleted on `3\.x`, changed on master; the merge kept `3\.x`'s deletion/,
+	);
+	assert.match(
+		out.body,
+		/- `src\/b\.ts` — deleted on master, changed on `3\.x`; the merge kept `3\.x`'s file/,
+	);
+	assert.match(out.body, / {2}- master: `abc1234` build: re-record \(#2\)/);
+	// Nothing carries markers, so the misleading "Conflicted files" list is dropped.
+	assert.equal(/### Conflicted files/.test(out.body), false);
+});

.github/scripts/sync-master-to-3x.mjs

@@ -34,16 +34,19 @@
  *   4. The content does NOT reconcile on a real code path → a genuinely new conflict. 3.x
  *      is left UNTOUCHED and a draft PR is opened on the sync branch carrying the conflict
  *      markers — with the mechanical files pre-resolved, so the resolver only deals with
- *      real code — attributed to the authors of the breaking commits behind the conflicted
- *      files. Syncs pause until it is merged.
+ *      real code — naming both ends of the clash: the authors of the breaking commits and
+ *      the master commits that touched the same files. Delete/modify conflicts leave no
+ *      markers, so they are resolved toward 3.x and reported as an explicit decision
+ *      instead. Syncs pause until it is merged.
  *
  * The conflict branch carries the conflict markers, so the resolver sees exactly what clashed
- * and the required checks stay red until they fix it in a commit of their own. That PR is
- * merged with the normal GitHub merge button — master's commits arrive as-is and the fix stays
- * its own commit. NEVER close a conflict PR unmerged: closing resolves nothing and the same
- * conflict reopens on the next sync. 3.x itself never has markers at its tip (nightly images
- * build from it), and the merge commit holding them is dropped from its history by the next
- * replay.
+ * and the required checks stay red until they fix it in a commit of their own. A conflict git
+ * left without markers (delete/modify) has no such gate: the PR body carries the decision that
+ * was made by default and says as much. That PR is merged with the normal GitHub merge button
+ * — master's commits arrive as-is and the fix stays its own commit. NEVER close a conflict PR
+ * unmerged: closing resolves nothing and the same conflict reopens on the next sync. 3.x
+ * itself never has markers at its tip (nightly images build from it), and the merge commit
+ * holding them is dropped from its history by the next replay.
  *
  * Runs from a checkout of the target branch (fetch-depth 0). Assumes credentials are NOT
  * persisted by checkout — pushes go through an explicit token URL.
@@ -71,12 +74,7 @@ import {
 	mergeTree,
 	runGit,
 } from './branch-replay.mjs';
-import {
-	conflictedFiles,
-	breakingShas,
-	resolveLogins,
-	buildOutputs,
-} from './sync-conflict-owners.mjs';
+import { conflictedFiles, gatherAttribution, buildOutputs } from './sync-conflict-owners.mjs';
 
 // Re-exported so this module stays the single entry point for the master→3.x flow, tests
 // included. The implementations are branch-pair-agnostic and shared with the bundle replay.
@@ -184,6 +182,34 @@ export function resolveQueueSidePath({ git, path, log = console.log }) {
 	}
 }
 
+/**
+ * The unmerged paths git left as delete/modify: one side removed the file, the other
+ * changed it. These need naming separately, because a real merge leaves NO conflict
+ * markers for them — the working tree simply holds the surviving side, so committing the
+ * merge as-is silently takes that side with nothing for a resolver to look at.
+ *
+ * `ls-files -u` lines are `<mode> <oid> <stage>\t<path>`: stage 1 base, 2 ours (the target
+ * branch), 3 theirs (master). A base with one side missing is the delete; no base at all is
+ * an add/add, which is a normal content conflict and keeps its markers.
+ *
+ * @returns {Array<{ path: string, deletedBy: 'target' | 'master' }>}
+ */
+export function deleteModifyConflicts(git, paths) {
+	const out = [];
+	for (const path of paths) {
+		const stages = new Set(
+			git(['ls-files', '-u', '--', path])
+				.split('\n')
+				.map((line) => /^\S+ \S+ (\d)\t/.exec(line)?.[1])
+				.filter(Boolean),
+		);
+		if (!stages.has('1')) continue;
+		if (!stages.has('2')) out.push({ path, deletedBy: 'target' });
+		else if (!stages.has('3')) out.push({ path, deletedBy: 'master' });
+	}
+	return out;
+}
+
 /**
  * Replay the 3.x-only commits onto master, resolving stalls that involve ONLY mechanical
  * paths in place — folded into the stalled commit exactly as a human's `rebase --continue`
@@ -305,6 +331,11 @@ export function writeGithubOutput(obj, env = process.env) {
  * cannot be merged half-resolved (an auto-resolved branch would be green with master's
  * change silently dropped).
  *
+ * Delete/modify conflicts have no markers to leave, so they are resolved toward 3.x's side
+ * — the same side the replay favours — and reported separately. Left to `add -A` they would
+ * commit master's surviving blob instead, re-adding a file 3.x deleted on purpose with
+ * nothing in the diff to suggest a decision was made.
+ *
  * The lockfile is left with its markers when a manifest is among the code conflicts
  * (regenerating is meaningless until the manifests are resolved) or when the regen fails
  * transiently — flagged via `lockfileDeferred` so the PR body carries the instruction.
@@ -312,11 +343,18 @@ export function writeGithubOutput(obj, env = process.env) {
  * 3.x never carries the markers at its tip, and not for long in its history either: this
  * merge commit is dropped by the next replay, which takes the queue's commits only.
  *
- * @returns {{ files: string[], preResolved: string[], lockfileDeferred: boolean }}
- *   `files` is the list the PR reports and attributes owners for: the code conflicts,
- *   or every conflict when none are code (a fallback after a failed auto-resolution).
+ * @returns {{ files: string[], deleteConflicts: Array<{path: string, deletedBy: string}>,
+ *   preResolved: string[], lockfileDeferred: boolean }}
+ *   `files` is the marker-carrying list the PR reports, or every conflict when none are
+ *   code (a fallback after a failed auto-resolution); owners are attributed for both lists.
  */
-export function buildConflictBranch({ git, pnpm, masterSha, log = console.log }) {
+export function buildConflictBranch({
+	git,
+	pnpm,
+	masterSha,
+	target = TARGET_BRANCH,
+	log = console.log,
+}) {
 	const merge = attempt(git, ['merge', '--no-edit', masterSha]);
 	if (merge.ok) {
 		// merge-tree said these conflict; if a real merge disagrees, don't guess.
@@ -348,9 +386,28 @@ export function buildConflictBranch({ git, pnpm, masterSha, log = console.log })
 		}
 	}
 
+	// No markers to leave behind for these — resolve toward 3.x and report them instead.
+	const deleteConflicts = deleteModifyConflicts(git, code);
+	for (const { path, deletedBy } of deleteConflicts) {
+		if (deletedBy === 'target') {
+			log(`Keeping ${target}'s deletion of ${path} (master modified it)...`);
+			git(['rm', '--force', '--', path]);
+		} else {
+			log(`Keeping ${target}'s ${path} (master deleted it)...`);
+			git(['checkout', '--ours', '--', path]);
+			git(['add', '--', path]);
+		}
+	}
+	const deleted = new Set(deleteConflicts.map((c) => c.path));
+
 	git(['add', '-A']);
 	git(['commit', '--no-edit', '--no-verify']);
-	return { files: code.length > 0 ? code : all, preResolved, lockfileDeferred };
+	return {
+		files: code.length > 0 ? code.filter((p) => !deleted.has(p)) : all,
+		deleteConflicts,
+		preResolved,
+		lockfileDeferred,
+	};
 }
 
 // Conflict PRs that were recently closed WITHOUT being merged — closing resolves nothing,
@@ -377,8 +434,10 @@ export function recentAbandonedConflictPrs(
 }
 
 /**
- * Push the marker-carrying conflict branch and open a draft PR, attributing it to the
- * authors of the breaking commits behind the conflicted files. 3.x is left untouched.
+ * Push the marker-carrying conflict branch and open a draft PR naming both ends of the
+ * conflict: the authors of the breaking commits behind the conflicted files, and the master
+ * commits that touched the same files. Nobody is requested as a reviewer — the PR body and
+ * the Slack post are the ping. 3.x is left untouched.
  *
  * @returns {Promise<{ prUrl: string, ownersSlack: string }>}
  */
@@ -392,22 +451,23 @@ export async function openConflictPr({
 	pushUrl,
 	target = TARGET_BRANCH,
 	files = [],
+	deleteConflicts = [],
 	preResolved = [],
 	lockfileDeferred = false,
 	fetchFn = fetch,
 	log = console.log,
 }) {
 	// Attribute against the pre-merge tip: HEAD is the merge commit by now.
-	const shas = breakingShas(masterSha, files, git, preHead);
-
-	// Degrade gracefully: a transient API failure should still open the PR
-	// (unattributed) rather than fail the whole sync.
-	let owners = [];
-	try {
-		owners = await resolveLogins(repo, shas, token, fetchFn);
-	} catch (error) {
-		log(`warning: could not resolve owners: ${error.message}`);
-	}
+	const { owners, masterCommits } = await gatherAttribution({
+		repo,
+		token,
+		files: [...files, ...deleteConflicts.map((c) => c.path)],
+		base: masterSha,
+		tip: preHead,
+		git,
+		fetchFn,
+		log,
+	});
 
 	let abandoned = [];
 	try {
@@ -416,11 +476,13 @@ export async function openConflictPr({
 		log(`warning: could not check for abandoned conflict PRs: ${error.message}`);
 	}
 
-	const { ownersCsv, slack, body } = buildOutputs({
+	const { slack, body } = buildOutputs({
 		syncBranch: SYNC_BRANCH,
 		targetBranch: target,
 		files,
 		owners,
+		deleteConflicts,
+		masterCommits,
 		preResolved,
 		lockfileDeferred,
 		abandoned,
@@ -455,16 +517,6 @@ export async function openConflictPr({
 		body,
 	]);
 
-	// Request owners as reviewers (best-effort: the API rejects the PR author
-	// and non-collaborators, so a failure here must not fail the sync).
-	if (ownersCsv) {
-		try {
-			gh(['pr', 'edit', prUrl, '--add-reviewer', ownersCsv]);
-		} catch {
-			log(`::warning::could not request some reviewers: ${ownersCsv}`);
-		}
-	}
-
 	return { prUrl, ownersSlack: slack };
 }
 
@@ -576,10 +628,11 @@ export async function sync({
 			);
 		}
 
-		const { files, preResolved, lockfileDeferred } = buildConflictBranch({
+		const { files, deleteConflicts, preResolved, lockfileDeferred } = buildConflictBranch({
 			git,
 			pnpm,
 			masterSha,
+			target,
 			log,
 		});
 		const { prUrl, ownersSlack } = await openConflictPr({
@@ -592,6 +645,7 @@ export async function sync({
 			pushUrl,
 			target,
 			files,
+			deleteConflicts,
 			preResolved,
 			lockfileDeferred,
 			fetchFn,

.github/scripts/sync-master-to-3x.test.mjs

@@ -8,6 +8,7 @@ import {
 	blocksLockfileRegen,
 	resolveMechanicalPath,
 	resolveQueueSidePath,
+	deleteModifyConflicts,
 	rebaseResolvingMechanical,
 	reconcileWithMergeTreeAtTip,
 	reconcileLockfileAtTip,
@@ -749,6 +750,11 @@ test('buildConflictBranch commits the conflicted state, markers and all', () =>
 	assert.deepEqual(preResolved, []);
 	assert.equal(lockfileDeferred, false);
 	assert.deepEqual(git.calls[0], ['merge', '--no-edit', MASTER]);
+	assert.equal(
+		git.calls.some((a) => a[0] === 'rm'),
+		false,
+		'a content conflict keeps its markers',
+	);
 	assert.ok(git.calls.some((a) => a[0] === 'add' && a[1] === '-A'));
 	assert.ok(git.calls.some((a) => a[0] === 'commit' && a.includes('--no-edit')));
 	// The markers ARE the review surface here, so nothing may auto-resolve them.
@@ -762,6 +768,53 @@ test('buildConflictBranch commits the conflicted state, markers and all', () =>
 	);
 });
 
+test('deleteModifyConflicts tells the deleting side apart, and ignores add/add', () => {
+	const stages = {
+		'gone-on-3x.json': '100644 oid 1\tgone-on-3x.json\n100644 oid 3\tgone-on-3x.json',
+		'gone-on-master.ts': '100644 oid 1\tgone-on-master.ts\n100644 oid 2\tgone-on-master.ts',
+		'both-added.ts': '100644 oid 2\tboth-added.ts\n100644 oid 3\tboth-added.ts',
+		'content.ts': '100644 oid 1\tcontent.ts\n100644 oid 2\tcontent.ts\n100644 oid 3\tcontent.ts',
+	};
+	const git = (args) => stages[args.at(-1)] ?? '';
+
+	assert.deepEqual(deleteModifyConflicts(git, Object.keys(stages)), [
+		{ path: 'gone-on-3x.json', deletedBy: 'target' },
+		{ path: 'gone-on-master.ts', deletedBy: 'master' },
+	]);
+});
+
+test('buildConflictBranch resolves marker-less delete/modify conflicts toward 3.x', () => {
+	const git = makeStub([
+		[(a) => a[0] === 'merge', fail('CONFLICT (modify/delete): fixtures/a.json')],
+		[isConflictedFiles, 'fixtures/a.json\nsrc/b.ts'],
+		[
+			(a) => a[0] === 'ls-files',
+			(a) =>
+				a.at(-1) === 'fixtures/a.json'
+					? '100644 oid 1\tfixtures/a.json\n100644 oid 3\tfixtures/a.json'
+					: '100644 oid 1\tsrc/b.ts\n100644 oid 2\tsrc/b.ts',
+		],
+	]);
+
+	const { files, deleteConflicts } = buildConflictBranch({
+		git,
+		pnpm: makeStub(),
+		masterSha: MASTER,
+		log: () => {},
+	});
+
+	// Both are marker-less, so neither belongs in the "conflicted files" list.
+	assert.deepEqual(files, []);
+	assert.deepEqual(deleteConflicts, [
+		{ path: 'fixtures/a.json', deletedBy: 'target' },
+		{ path: 'src/b.ts', deletedBy: 'master' },
+	]);
+	// 3.x deleted the fixture: keep the deletion rather than master's re-added blob.
+	assert.ok(git.calls.some((a) => a[0] === 'rm' && a.at(-1) === 'fixtures/a.json'));
+	// master deleted the source file: keep 3.x's.
+	assert.ok(git.calls.some((a) => a[0] === 'checkout' && a.includes('--ours')));
+});
+
 test('buildConflictBranch pre-resolves mechanical files so only code conflicts remain', () => {
 	const git = makeStub([
 		[(a) => a[0] === 'merge', fail('CONFLICT')],
@@ -864,11 +917,14 @@ test('sync opens a draft conflict PR and leaves 3.x untouched on a real conflict
 	const body = create[create.indexOf('--body') + 1];
 	assert.match(body, /Merge this PR with the normal merge button/);
 	assert.match(body, /nothing is squashed/);
-	assert.match(body, /conflict markers committed/);
+	assert.match(body, /conflict markers included/);
+	assert.match(body, /- @alice/);
 
-	// Owner requested as reviewer.
-	const edit = gh.calls.find((a) => a[0] === 'pr' && a[1] === 'edit');
-	assert.equal(edit[edit.indexOf('--add-reviewer') + 1], 'alice');
+	// Reviewers are never requested — the body and the Slack post are the ping.
+	assert.equal(
+		gh.calls.some((a) => a[0] === 'pr' && a[1] === 'edit'),
+		false,
+	);
 
 	// Only the sync branch is pushed — 3.x must not move.
 	const pushes = git.calls.filter((a) => a[0] === 'push');
@@ -937,11 +993,6 @@ test('openConflictPr degrades gracefully when owner resolution fails', async ()
 
 	assert.equal(prUrl, 'https://github.com/n8n-io/n8n/pull/1');
 	assert.equal(ownersSlack, 'Could not auto-attribute owners.');
-	// No reviewer request when there are no owners.
-	assert.equal(
-		gh.calls.some((a) => a[0] === 'pr' && a[1] === 'edit'),
-		false,
-	);
 });
 
 test('openConflictPr calls out a recently abandoned conflict PR', async () => {
@@ -976,3 +1027,40 @@ test('openConflictPr calls out a recently abandoned conflict PR', async () => {
 	assert.match(body, /Merge, don't close/);
 	assert.match(ownersSlack, /<https:\/\/github\.com\/n8n-io\/n8n\/pull\/42\|#42>/);
 });
+
+test('sync reports a marker-less delete/modify conflict as its own decision, with the master commit', async () => {
+	const git = makeStub([
+		...baseGitRoutes.filter((r) => !r[0](['merge-tree']) && !r[0](['merge-base'])),
+		[(a) => a[0] === 'merge-base' && a[1] === '--is-ancestor', fail()],
+		[(a) => a[0] === 'merge-base', 'DIVERGED'],
+		[(a) => a[0] === 'merge-tree', conflictedMergeTree('fixtures/a.json')],
+		[(a) => a[0] === 'merge', fail('CONFLICT (modify/delete): fixtures/a.json')],
+		[isConflictedFiles, 'fixtures/a.json'],
+		[(a) => a[0] === 'ls-files', '100644 oid 1\tfixtures/a.json\n100644 oid 3\tfixtures/a.json'],
+		[(a) => a[0] === 'log' && a.includes('--format=%H'), 'breaking-sha'],
+		[(a) => a[0] === 'log' && a.includes('--format=%H %h %s'), 'master-sha msha build: bump (#2)'],
+	]);
+	const gh = makeStub([
+		...noOpenPr,
+		[(a) => a[0] === 'pr' && a[1] === 'create', 'https://github.com/n8n-io/n8n/pull/99'],
+	]);
+
+	await sync({
+		git,
+		gh,
+		pnpm: makeStub(),
+		env,
+		fetchFn: okFetch(['alice', 'bob']),
+		log: () => {},
+	});
+
+	const create = gh.calls.find((a) => a[0] === 'pr' && a[1] === 'create');
+	const body = create[create.indexOf('--body') + 1];
+	assert.match(body, /### Deleted on one side, changed on the other/);
+	assert.match(body, /- `fixtures\/a\.json` — deleted on `3\.x`, changed on master/);
+	assert.match(body, / {2}- master: `msha` build: bump \(#2\) — @bob/);
+	// 3.x is still untouched — the decision is the resolver's, only the PR moves.
+	const pushes = git.calls.filter((a) => a[0] === 'push');
+	assert.equal(pushes.length, 1);
+	assert.equal(pushes[0].at(-1), `HEAD:refs/heads/${SYNC_BRANCH}`);
+});

.github/workflows/util-sync-master-to-3x.yml

@@ -13,11 +13,13 @@
 # data files) are resolved in place during the replay — no PR, no commit, no human. When
 # master genuinely conflicts with 3.x on real code, 3.x is left untouched: a draft conflict
 # PR carrying the conflict markers (mechanical files pre-resolved) is opened on the sync
-# branch and #alerts-v3-sync is notified. The resolver fixes the markers in a commit of their
-# own and merges that PR with the normal merge button — never closes it, since closing
-# resolves nothing and the conflict returns. No further syncs run until it is merged. 3.x
-# itself never has markers at its tip (nightly images build from it) and the merge commit
-# holding them drops out of its history at the next replay.
+# branch and #alerts-v3-sync is notified. Delete/modify conflicts, which git cannot express
+# as markers, are resolved toward 3.x's side and listed in that PR as an explicit decision.
+# The resolver fixes the markers in a commit of their own and merges that PR with the normal
+# merge button — never closes it, since closing resolves nothing and the conflict returns.
+# No further syncs run until it is merged. 3.x itself never has markers at its tip (nightly
+# images build from it) and the merge commit holding them drops out of its history at the
+# next replay.
 #
 # See .github/DEVELOPING_V3.md for the full v3 development model.