diff --git a/.github/DEVELOPING_V3.md b/.github/DEVELOPING_V3.md index 62045fc745f..09f57dd588f 100644 --- a/.github/DEVELOPING_V3.md +++ b/.github/DEVELOPING_V3.md @@ -186,8 +186,8 @@ automation can find that for you. The PR names the `master` commit behind each c 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. +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` never holds markers at its tip, so nightly images keep building; the merge commit that carries them drops out of `3.x`'s history at the next sync (the replay takes the queue's diff --git a/.github/scripts/sync-conflict-owners.mjs b/.github/scripts/sync-conflict-owners.mjs index ba5d6a74053..c69dc4d7197 100644 --- a/.github/scripts/sync-conflict-owners.mjs +++ b/.github/scripts/sync-conflict-owners.mjs @@ -180,7 +180,6 @@ export function buildOutputs({ masterCommits = new Map(), preResolved = [], lockfileDeferred = false, - abandoned = [], }) { // `- \`path\`` plus a nested line per master commit that touched it. const fileMd = (path, suffix = '') => { @@ -207,9 +206,6 @@ export function buildOutputs({ 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(); @@ -218,15 +214,11 @@ export function buildOutputs({ ? `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.` - : '', ] .filter(Boolean) .join(' '); const body = [ `Automated \`master\`→\`${targetBranch}\` sync hit a conflict.`, - ...(abandonedWarning ? ['', '> [!WARNING]', `> ${abandonedWarning}`] : []), '', 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.` diff --git a/.github/scripts/sync-conflict-owners.test.mjs b/.github/scripts/sync-conflict-owners.test.mjs index c45d2e8a8d8..39f0dfcb330 100644 --- a/.github/scripts/sync-conflict-owners.test.mjs +++ b/.github/scripts/sync-conflict-owners.test.mjs @@ -229,20 +229,6 @@ test('buildOutputs carries the regen instruction when the lockfile was deferred' assert.match(out.body, /pnpm install --lockfile-only/); }); -test('buildOutputs warns about conflict PRs that were closed without merging', () => { - const abandoned = [{ number: 42, url: 'https://github.com/n8n-io/n8n/pull/42' }]; - const out = buildOutputs({ - syncBranch: 'sync/master-to-3x', - files: ['x.ts'], - owners: ['alice'], - abandoned, - }); - assert.match(out.body, /#42\) was closed without being merged/); - assert.match(out.body, /Merge, don't close/); - assert.match(out.slack, //); - 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', diff --git a/.github/scripts/sync-master-to-3x.mjs b/.github/scripts/sync-master-to-3x.mjs index e1282ab67dc..637df9acdc5 100644 --- a/.github/scripts/sync-master-to-3x.mjs +++ b/.github/scripts/sync-master-to-3x.mjs @@ -410,29 +410,6 @@ export function buildConflictBranch({ }; } -// Conflict PRs that were recently closed WITHOUT being merged — closing resolves nothing, -// so the same conflict is about to come back; the new PR and Slack message call it out. -export function recentAbandonedConflictPrs( - gh, - { label = CONFLICT_LABEL, sinceDays = 14, now = Date.now() } = {}, -) { - const out = gh([ - 'pr', - 'list', - '--state', - 'closed', - '--label', - label, - '--json', - 'number,url,mergedAt,closedAt', - '--limit', - '10', - ]); - return JSON.parse(out || '[]').filter( - (pr) => !pr.mergedAt && pr.closedAt && now - Date.parse(pr.closedAt) < sinceDays * 86_400_000, - ); -} - /** * 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 @@ -469,13 +446,6 @@ export async function openConflictPr({ log, }); - let abandoned = []; - try { - abandoned = recentAbandonedConflictPrs(gh); - } catch (error) { - log(`warning: could not check for abandoned conflict PRs: ${error.message}`); - } - const { slack, body } = buildOutputs({ syncBranch: SYNC_BRANCH, targetBranch: target, @@ -485,7 +455,6 @@ export async function openConflictPr({ masterCommits, preResolved, lockfileDeferred, - abandoned, }); git(['push', '--force', pushUrl, `HEAD:refs/heads/${SYNC_BRANCH}`]); diff --git a/.github/scripts/sync-master-to-3x.test.mjs b/.github/scripts/sync-master-to-3x.test.mjs index 0a0f7fa184f..0a807f0e0c7 100644 --- a/.github/scripts/sync-master-to-3x.test.mjs +++ b/.github/scripts/sync-master-to-3x.test.mjs @@ -12,7 +12,6 @@ import { rebaseResolvingMechanical, reconcileWithMergeTreeAtTip, reconcileLockfileAtTip, - recentAbandonedConflictPrs, assertTreeMatches, assertNoMarkers, buildConflictBranch, @@ -430,33 +429,6 @@ test('reconcileLockfileAtTip folds an inconsistent lockfile into the tip commit, ); }); -test('recentAbandonedConflictPrs keeps only recently closed-unmerged conflict PRs', () => { - const now = Date.parse('2026-08-10T00:00:00Z'); - const gh = makeStub([ - [ - () => true, - JSON.stringify([ - { - number: 1, - url: 'u1', - mergedAt: '2026-08-01T00:00:00Z', - closedAt: '2026-08-01T00:00:00Z', - }, - { number: 2, url: 'u2', mergedAt: null, closedAt: '2026-07-01T00:00:00Z' }, // too old - { number: 3, url: 'u3', mergedAt: null, closedAt: '2026-08-08T00:00:00Z' }, - ]), - ], - ]); - - const abandoned = recentAbandonedConflictPrs(gh, { now }); - - assert.deepEqual( - abandoned.map((pr) => pr.number), - [3], - ); - assert.equal(gh.calls[0][gh.calls[0].indexOf('--state') + 1], 'closed'); -}); - test('sync replays and force-pushes with a lease, creating no commit', async () => { const git = makeStub([...baseGitRoutes, [isRebase, '']]); const gh = makeStub(noOpenPr); @@ -995,39 +967,6 @@ test('openConflictPr degrades gracefully when owner resolution fails', async () assert.equal(ownersSlack, 'Could not auto-attribute owners.'); }); -test('openConflictPr calls out a recently abandoned conflict PR', async () => { - const git = makeStub([[(a) => a[0] === 'log', 'sha1']]); - const closedAt = new Date(Date.now() - 2 * 86_400_000).toISOString(); - const gh = makeStub([ - [ - (a) => a[0] === 'pr' && a[1] === 'list' && a.includes('closed'), - JSON.stringify([ - { number: 42, url: 'https://github.com/n8n-io/n8n/pull/42', mergedAt: null, closedAt }, - ]), - ], - [(a) => a[0] === 'pr' && a[1] === 'create', 'https://github.com/n8n-io/n8n/pull/43'], - ]); - - const { ownersSlack } = await openConflictPr({ - git, - gh, - repo: 'n8n-io/n8n', - token: 't', - masterSha: MASTER, - preHead: PRE_HEAD, - pushUrl: 'https://push', - files: ['x.ts'], - fetchFn: okFetch(['alice']), - log: () => {}, - }); - - const create = gh.calls.find((a) => a[0] === 'pr' && a[1] === 'create'); - const body = create[create.indexOf('--body') + 1]; - assert.match(body, /#42\) was closed without being merged/); - assert.match(body, /Merge, don't close/); - assert.match(ownersSlack, //); -}); - 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'])),