mirror of
https://github.com/n8n-io/n8n.git
synced 2026-09-19 01:45:48 +08:00
ci: Drop the abandoned-conflict-PR warning from the 3.x sync PR body (no-changelog) (#37098)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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.`
|
||||
|
||||
@@ -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, /<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',
|
||||
|
||||
@@ -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}`]);
|
||||
|
||||
@@ -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, /<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'])),
|
||||
|
||||
Reference in New Issue
Block a user