diff --git a/.github/DEVELOPING_V3.md b/.github/DEVELOPING_V3.md index 399cd87d1da..bdfcad92ffb 100644 --- a/.github/DEVELOPING_V3.md +++ b/.github/DEVELOPING_V3.md @@ -133,20 +133,19 @@ of the breaking removal on `3.x`. original message and author. Nothing is ever squashed. Merge commits in the range (breaking PRs merged into `3.x`) are flattened away, and a breaking commit that also landed on `master` is dropped as empty. -3. Conflicts confined to **mechanical files** — tool-generated content with a deterministic - resolution (`pnpm-lock.yaml`, `packages/frontend/editor-ui/data/node-popularity.json`, +3. Conflicts confined to **non-lockfile mechanical files** — bot-maintained content with a + deterministic resolution (`packages/frontend/editor-ui/data/node-popularity.json`, `packages/@n8n/instance-ai/src/tools/nodes/credential-setupability.json`, and `.github/test-metrics/e2e-impact-map.json`) — are **auto-resolved during the replay**, - exactly as a human resolver would: the lockfile is regenerated with - `pnpm install --lockfile-only` (pnpm merges its own conflict markers), bot-maintained data - files take `master`'s side. The resolution is folded into the stalled commit, so this - still adds **no commit and no PR**. The list lives in `MECHANICAL_PATHS` in + by taking `master`'s side. The resolution is folded into the stalled commit, so this still + adds **no commit and no PR**. The list lives in `MECHANICAL_PATHS` in [`sync-master-to-3x.mjs`](./scripts/sync-master-to-3x.mjs). -4. On a **real code conflict**, `3.x` is left **untouched** and a **draft conflict PR** - (labeled `automation:v3-sync`) carrying the conflict markers is opened on - `sync/master-to-3x` — with the mechanical files already pre-resolved — plus a post to the - **`#alerts-v3-sync`** Slack channel. **Syncs pause until that PR is merged** — so - conflicts never pile up silently. +4. On a **real code conflict or any `pnpm-lock.yaml` conflict**, `3.x` is left **untouched** + and a **draft conflict PR** (labeled `automation:v3-sync`) carrying the conflict markers is + opened on `sync/master-to-3x`. Other mechanical files are already pre-resolved. The + lockfile stays unresolved because pnpm validation is not reliable while the merge index is + unresolved. The workflow also posts to the **`#alerts-v3-sync`** Slack channel. **Syncs + pause until that PR is merged**, so conflicts never pile up silently. Whatever route it takes, the sync verifies that the content it is about to push is **exactly the tree a merge of `3.x` and `master` produces** (`git merge-tree`), and that no conflict @@ -165,8 +164,8 @@ rewriting `3.x`. The conflict branch is `master` merged into `3.x` with the **conflict markers committed**, so you see exactly what clashed — and the required checks stay red until they're gone, so the PR -can't be merged half-resolved. Mechanical files arrive **pre-resolved** (listed in the PR -under "Auto-resolved for you"), so only the real code conflicts need you: +can't be merged half-resolved. Non-lockfile mechanical files arrive **pre-resolved** (listed +in the PR under "Auto-resolved for you"), so only the code and lockfile conflicts need you: ```bash git fetch origin sync/master-to-3x && git switch sync/master-to-3x @@ -174,9 +173,8 @@ git fetch origin sync/master-to-3x && git switch sync/master-to-3x git push origin sync/master-to-3x ``` -If the PR says the lockfile was **deferred** (a `package.json` / `pnpm-workspace.yaml` is -conflicted too), resolve the manifests first, then regenerate it with -`pnpm install --lockfile-only` and include the result in your fix commit. +If the PR says the lockfile was **deferred**, resolve the other conflicts 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 diff --git a/.github/WORKFLOWS.md b/.github/WORKFLOWS.md index 118f3b04a98..4e4d8e9616c 100644 --- a/.github/WORKFLOWS.md +++ b/.github/WORKFLOWS.md @@ -622,13 +622,13 @@ flags) and the long-lived `3.x` branch carries breaking changes. `util-sync-mast syncs daily by **replaying the `3.x`-only commits onto `master` and force-pushing `3.x`**, so a clean sync adds no commit and nothing is squashed. What it pushes is always verified to be exactly the tree a merge of `3.x` and `master` produces, and marker-free. Conflicts confined -to mechanical, tool-generated files (the pnpm lockfile, bot-maintained data files — see -`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`, 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 +to non-lockfile mechanical files (bot-maintained data files — see `MECHANICAL_PATHS` in +`sync-master-to-3x.mjs`) are auto-resolved during the replay. On a code or `pnpm-lock.yaml` +conflict, `3.x` is left untouched and a draft PR carrying the conflict markers (labeled +`automation:v3-sync`, with other mechanical files pre-resolved) is opened on +`sync/master-to-3x`. The lockfile is always left for the resolver. The PR names both ends of +the conflict — the breaking-commit authors and the `master` commits that touched the same +files — via `sync-conflict-owners.mjs`, posts to `#alerts-v3-sync`, and pauses 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[-]` images from `3.x` diff --git a/.github/scripts/sync-conflict-owners.mjs b/.github/scripts/sync-conflict-owners.mjs index c69dc4d7197..402d84d63cc 100644 --- a/.github/scripts/sync-conflict-owners.mjs +++ b/.github/scripts/sync-conflict-owners.mjs @@ -254,7 +254,7 @@ export function buildOutputs({ ? [ '', '> [!NOTE]', - '> `pnpm-lock.yaml` still carries its conflict markers because a manifest (`package.json` / `pnpm-workspace.yaml`) is conflicted too, or its regeneration failed. After resolving the manifests, regenerate it with `pnpm install --lockfile-only` and commit the result.', + '> `pnpm-lock.yaml` still carries its conflict markers. Resolve the other conflicts first. Then regenerate it with `pnpm install --lockfile-only` and commit the result.', ] : []), '', diff --git a/.github/scripts/sync-conflict-owners.test.mjs b/.github/scripts/sync-conflict-owners.test.mjs index 39f0dfcb330..f08514fd63a 100644 --- a/.github/scripts/sync-conflict-owners.test.mjs +++ b/.github/scripts/sync-conflict-owners.test.mjs @@ -210,12 +210,12 @@ test('buildOutputs lists mechanically pre-resolved files apart from the code con syncBranch: 'sync/master-to-3x', files: ['packages/cli/x.ts'], owners: ['alice'], - preResolved: ['pnpm-lock.yaml'], + preResolved: ['packages/frontend/editor-ui/data/node-popularity.json'], }); assert.match(out.body, /### Conflicted files\n- `packages\/cli\/x\.ts`/); assert.match(out.body, /### Auto-resolved for you/); assert.match(out.body, /resolved mechanically — no action needed/); - assert.ok(out.body.indexOf('pnpm-lock.yaml') > out.body.indexOf('Auto-resolved')); + assert.ok(out.body.indexOf('node-popularity.json') > out.body.indexOf('Auto-resolved')); }); test('buildOutputs carries the regen instruction when the lockfile was deferred', () => { diff --git a/.github/scripts/sync-master-to-3x.mjs b/.github/scripts/sync-master-to-3x.mjs index 2c2d30ccc0e..f3cff057b9b 100644 --- a/.github/scripts/sync-master-to-3x.mjs +++ b/.github/scripts/sync-master-to-3x.mjs @@ -24,20 +24,16 @@ * the human's fix commit is in the queue and does the real work. Stalls the strategy * option cannot settle (modify/delete — `-X` never resolves those) are resolved in * place toward the queue commit's side. Tree is then proven. - * 3. master conflicts with 3.x, but ONLY on mechanical files — tool-generated content - * with a deterministic resolution (the pnpm lockfile, bot-maintained data files). - * These are resolved in place while the replay is stopped, exactly as a human - * resolver would (regenerate the lockfile, take master's blob), and folded into the - * stalled commit — still no commit of its own and no PR. When the `-X theirs` route - * resolves lockfile hunks without stalling, the lockfile is reconciled at the tip - * instead (folded into the tip commit by amending — never a commit of its own). - * 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 — 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. + * 3. master conflicts with 3.x, but ONLY on non-lockfile mechanical files — bot-maintained + * data with a deterministic resolution. These are resolved in place while the replay + * is stopped and folded into the stalled commit — still no commit of its own and no PR. + * 4. The content does NOT reconcile on a real code path, or `pnpm-lock.yaml` conflicts → + * a genuinely new conflict. 3.x is left UNTOUCHED and a draft PR is opened on the sync + * branch carrying the conflict markers. Other mechanical files are pre-resolved. The + * lockfile is left for the resolver because pnpm validation is not reliable while the + * merge index is unresolved. Delete/modify conflicts leave no markers, so they are + * resolved toward 3.x and reported as an explicit decision instead. Syncs pause until + * the PR 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. A conflict git @@ -91,7 +87,8 @@ export const LOCKFILE = 'pnpm-lock.yaml'; /** * Paths whose conflicts are MECHANICAL: tool-generated files with a deterministic - * resolution, so no human judgement is lost by resolving them automatically. + * resolution during replay. Conflict PRs defer the lockfile but still pre-resolve the + * other paths. * Keys are exact repo-relative paths; values pick the resolution strategy: * - 'pnpm-regen': pnpm natively merges a conflicted lockfile when regenerating * (`pnpm install --lockfile-only`). @@ -134,14 +131,6 @@ export function classifyPaths(paths) { return { mechanical, code }; } -// A conflicted manifest makes lockfile regeneration meaningless until it is resolved -// (catalogs live in pnpm-workspace.yaml, so it counts as a manifest too). -export function blocksLockfileRegen(codePaths) { - return codePaths.some( - (p) => p === 'package.json' || p.endsWith('/package.json') || p === 'pnpm-workspace.yaml', - ); -} - /** Run the trusted frozen install without using a store restored by the sync job. */ export function validateLockfile(pnpm, env = process.env) { const validationDir = join(env.RUNNER_TEMP || tmpdir(), `n8n-sync-lockfile-${randomUUID()}`); @@ -340,29 +329,28 @@ export function reconcileLockfileAtTip({ git, pnpm, masterSha, log = console.log /** * Build the conflict branch: master merged into 3.x with the conflict markers committed as - * they are — except mechanical files, which are pre-resolved so the resolver only deals - * with real code conflicts. The remaining markers are the review surface: the resolver sees - * exactly what clashed, and the required checks stay red until they fix it, so the PR - * cannot be merged half-resolved (an auto-resolved branch would be green with master's - * change silently dropped). + * they are — except non-lockfile mechanical files, which are pre-resolved so the resolver + * only deals with code and lockfile conflicts. The remaining markers are the review surface: + * the resolver sees exactly what clashed, and the required checks stay red until they fix it, + * so the PR 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 regeneration or - * frozen-install validation fails — flagged via `lockfileDeferred` so the PR body carries - * the instruction. + * The lockfile is always left with its markers. pnpm install validation can give different + * results while the merge index is unresolved, even with isolated stores. `lockfileDeferred` + * makes the PR body carry the resolver instruction. * * 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[], 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. + * `files` is the code-conflict list, or the unresolved mechanical list when there are no + * code conflicts. Owners are attributed for both lists. */ export function buildConflictBranch({ git, @@ -386,8 +374,7 @@ export function buildConflictBranch({ const preResolved = []; let lockfileDeferred = false; for (const path of mechanical) { - const needsManifests = MECHANICAL_PATHS[path] === 'pnpm-regen' && blocksLockfileRegen(code); - if (needsManifests) { + if (path === LOCKFILE) { lockfileDeferred = true; continue; } @@ -398,7 +385,6 @@ export function buildConflictBranch({ // Degrade gracefully (e.g. a transient registry failure): leave the markers // for the resolver rather than failing the PR-opening path. log(`warning: could not pre-resolve ${path}: ${error.message}`); - if (MECHANICAL_PATHS[path] === 'pnpm-regen') lockfileDeferred = true; } } @@ -418,8 +404,13 @@ export function buildConflictBranch({ git(['add', '-A']); git(['commit', '--no-edit', '--no-verify']); + const preResolvedSet = new Set(preResolved); + const unresolvedMechanical = mechanical.filter((path) => !preResolvedSet.has(path)); return { - files: code.length > 0 ? code.filter((p) => !deleted.has(p)) : all, + files: + code.length > 0 + ? code.filter((path) => !deleted.has(path)) + : unresolvedMechanical.filter((path) => !deleted.has(path)), deleteConflicts, preResolved, lockfileDeferred, @@ -568,7 +559,7 @@ export async function sync({ if (!merged.ok) { const { mechanical, code } = classifyPaths(merged.conflictedPaths); - if (code.length === 0 && mechanical.length > 0) { + if (code.length === 0 && mechanical.length > 0 && !mechanical.includes(LOCKFILE)) { log( `master conflicts with ${target} only on mechanical files (${mechanical.join(', ')}); auto-resolving.`, ); @@ -609,7 +600,9 @@ export async function sync({ log('Mechanical auto-resolution did not complete; falling back to a conflict PR.'); } else { log( - `master conflicts with ${target} — leaving ${target} untouched and opening a conflict PR.`, + mechanical.includes(LOCKFILE) + ? `master conflicts with ${target} on ${LOCKFILE} — deferring the lockfile and opening a conflict PR.` + : `master conflicts with ${target} — leaving ${target} untouched and opening a conflict PR.`, ); } diff --git a/.github/scripts/sync-master-to-3x.test.mjs b/.github/scripts/sync-master-to-3x.test.mjs index b9ffc1b9eac..e8762937268 100644 --- a/.github/scripts/sync-master-to-3x.test.mjs +++ b/.github/scripts/sync-master-to-3x.test.mjs @@ -6,7 +6,6 @@ import { hasOpenConflictPr, mergeTree, classifyPaths, - blocksLockfileRegen, validateLockfile, resolveMechanicalPath, resolveQueueSidePath, @@ -153,7 +152,7 @@ test('mergeTree reports the tree when clean, and the conflicted paths when the s assert.deepEqual(res.conflictedPaths, [LOCKFILE, 'packages/cli/x.ts']); }); -test('classifyPaths and blocksLockfileRegen split mechanical from code conflicts', () => { +test('classifyPaths splits mechanical from code conflicts', () => { const { mechanical, code } = classifyPaths([ LOCKFILE, 'packages/cli/x.ts', @@ -166,11 +165,6 @@ test('classifyPaths and blocksLockfileRegen split mechanical from code conflicts '.github/test-metrics/e2e-impact-map.json', ]); assert.deepEqual(code, ['packages/cli/x.ts']); - - assert.equal(blocksLockfileRegen(['packages/cli/package.json']), true); - assert.equal(blocksLockfileRegen(['package.json']), true); - assert.equal(blocksLockfileRegen(['pnpm-workspace.yaml']), true); - assert.equal(blocksLockfileRegen(['packages/cli/x.ts']), false); }); test('validateLockfile uses fresh stores under the configured temporary directory', () => { @@ -702,52 +696,39 @@ test('sync fails without pushing when even the favoured replay cannot finish', a assert.equal(git.calls.filter((a) => a[0] === 'rebase' && a[1] === '--abort').length, 2); }); -test('sync auto-resolves a lockfile-only conflict during the replay — no PR, no commit', async () => { +test('sync opens a conflict PR for a lockfile-only conflict and defers the lockfile', async () => { const git = makeStub([ ...baseGitRoutes.filter((r) => !r[0](['merge-tree'])), [(a) => a[0] === 'merge-tree', conflictedMergeTree(LOCKFILE)], - [(a) => a[0] === 'rebase' && a[1] === '--continue', ''], - [isRebase, fail(`CONFLICT (content): Merge conflict in ${LOCKFILE}`)], + [(a) => a[0] === 'merge', fail(`CONFLICT (content): Merge conflict in ${LOCKFILE}`)], [isConflictedFiles, LOCKFILE], - [(a) => a[0] === 'diff' && a.includes('--quiet'), ''], // tip lockfile already consistent - [(a) => a[0] === 'diff-index', fail()], // staged resolution -> continue, not skip + [(a) => a[0] === 'log', 'breaking-sha'], + ]); + const gh = makeStub([ + ...noOpenPr, + [(a) => a[0] === 'pr' && a[1] === 'create', 'https://github.com/n8n-io/n8n/pull/99'], ]); - const gh = makeStub(noOpenPr); const pnpm = makeStub(); - await sync({ git, gh, pnpm, env, log: () => {} }); + await sync({ git, gh, pnpm, env, fetchFn: okFetch(['alice']), log: () => {} }); - // The stall regen and tip reconciliation each run regeneration and exact CI validation. - assert.deepEqual(pnpm.calls[0], ['install', '--lockfile-only', '--no-frozen-lockfile']); - const stallValidation = isolatedValidationPaths(pnpm.calls[1]); - assert.deepEqual(pnpm.calls[2], ['install', '--lockfile-only', '--no-frozen-lockfile']); - const tipValidation = isolatedValidationPaths(pnpm.calls[3]); - assert.notEqual(stallValidation.storeDir, tipValidation.storeDir); - assert.notEqual(stallValidation.virtualStoreDir, tipValidation.virtualStoreDir); - assert.equal(pnpm.calls.length, 4); - assert.ok(git.calls.some((a) => a[0] === 'add' && a.includes(LOCKFILE))); - assert.ok(git.calls.some((a) => a[0] === 'rebase' && a[1] === '--continue')); - const push = git.calls.find((a) => a[0] === 'push'); - assert.equal(push[1], `--force-with-lease=refs/heads/${TARGET_BRANCH}:${PRE_HEAD}`); - // No human surface: no merge commit, no amend, no conflict PR. + assert.equal(pnpm.calls.length, 0, 'the conflict-PR path must not run pnpm'); assert.equal( - git.calls.some((a) => a[0] === 'merge'), - false, - ); - assert.equal( - git.calls.some((a) => a[0] === 'commit'), - false, - ); - assert.equal( - gh.calls.some((a) => a[0] === 'pr' && a[1] === 'create'), + git.calls.some((a) => a[0] === 'rebase'), false, ); + const create = gh.calls.find((a) => a[0] === 'pr' && a[1] === 'create'); + assert.ok(create, 'expected a conflict PR'); + assert.match(create[create.indexOf('--body') + 1], /pnpm-lock\.yaml.*conflict markers/); + 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}`); }); test('sync falls back to a conflict PR when mechanical auto-resolution cannot complete', async () => { const git = makeStub([ ...baseGitRoutes.filter((r) => !r[0](['merge-tree'])), - [(a) => a[0] === 'merge-tree', conflictedMergeTree(LOCKFILE)], + [(a) => a[0] === 'merge-tree', conflictedMergeTree(POPULARITY)], [isRebase, fail('CONFLICT')], // Both replay attempts stall on a code file the endpoints do not reconcile. [isConflictedFiles, 'packages/cli/x.ts'], @@ -853,7 +834,7 @@ test('buildConflictBranch resolves marker-less delete/modify conflicts toward 3. assert.ok(git.calls.some((a) => a[0] === 'checkout' && a.includes('--ours'))); }); -test('buildConflictBranch pre-resolves mechanical files so only code conflicts remain', () => { +test('buildConflictBranch defers the lockfile with code and pre-resolves other mechanical files', () => { const git = makeStub([ [(a) => a[0] === 'merge', fail('CONFLICT')], [isConflictedFiles, `packages/cli/x.ts\n${LOCKFILE}\n${POPULARITY}`], @@ -869,20 +850,18 @@ test('buildConflictBranch pre-resolves mechanical files so only code conflicts r }); assert.deepEqual(files, ['packages/cli/x.ts']); - assert.deepEqual(preResolved, [LOCKFILE, POPULARITY]); - assert.equal(lockfileDeferred, false); - assert.deepEqual(pnpm.calls[0], ['install', '--lockfile-only', '--no-frozen-lockfile']); - isolatedValidationPaths(pnpm.calls[1]); - assert.equal(pnpm.calls.length, 2); + assert.deepEqual(preResolved, [POPULARITY]); + assert.equal(lockfileDeferred, true); + assert.equal(pnpm.calls.length, 0); assert.ok( git.calls.some((a) => a[0] === 'checkout' && a[1] === MASTER && a.includes(POPULARITY)), ); }); -test('buildConflictBranch defers the lockfile when a manifest is conflicted too', () => { +test('buildConflictBranch defers the lockfile with a code conflict without running pnpm', () => { const git = makeStub([ [(a) => a[0] === 'merge', fail('CONFLICT')], - [isConflictedFiles, `packages/cli/package.json\n${LOCKFILE}`], + [isConflictedFiles, `packages/cli/x.ts\n${LOCKFILE}`], ]); const pnpm = makeStub(); @@ -893,20 +872,19 @@ test('buildConflictBranch defers the lockfile when a manifest is conflicted too' log: () => {}, }); - assert.deepEqual(files, ['packages/cli/package.json']); + assert.deepEqual(files, ['packages/cli/x.ts']); assert.deepEqual(preResolved, []); assert.equal(lockfileDeferred, true); - assert.equal(pnpm.calls.length, 0, 'regen is meaningless until the manifests are resolved'); + assert.equal(pnpm.calls.length, 0, 'the conflict-PR path must not run pnpm'); }); -test('buildConflictBranch restores and defers a lockfile when the patched install fails', () => { +test('buildConflictBranch reports only the deferred lockfile after resolving another mechanical file', () => { const git = makeStub([ [(a) => a[0] === 'merge', fail('CONFLICT')], - [isConflictedFiles, `packages/cli/x.ts\n${LOCKFILE}`], - ]); - const pnpm = makeStub([ - [(a) => a.includes('--frozen-lockfile'), fail('ERR_PNPM_PATCH_FAILED')], + [isConflictedFiles, `${LOCKFILE}\n${POPULARITY}`], + [(a) => a[0] === 'cat-file', ''], ]); + const pnpm = makeStub(); const { files, preResolved, lockfileDeferred } = buildConflictBranch({ git, @@ -915,47 +893,17 @@ test('buildConflictBranch restores and defers a lockfile when the patched instal log: () => {}, }); - assert.deepEqual(files, ['packages/cli/x.ts']); - assert.deepEqual(preResolved, []); + assert.deepEqual(files, [LOCKFILE]); + assert.deepEqual(preResolved, [POPULARITY]); assert.equal(lockfileDeferred, true); - assert.deepEqual(pnpm.calls[0], ['install', '--lockfile-only', '--no-frozen-lockfile']); - isolatedValidationPaths(pnpm.calls[1]); - assert.equal(pnpm.calls.length, 2); + assert.equal(pnpm.calls.length, 0); assert.ok( - git.calls.some( - (a) => - a[0] === 'checkout' && a[1] === '--conflict=merge' && a.at(-1) === LOCKFILE, - ), - 'the committed conflict branch must contain the original lockfile markers', + git.calls.some((a) => a[0] === 'checkout' && a[1] === MASTER && a.includes(POPULARITY)), ); -}); - -test('buildConflictBranch leaves the original lockfile conflict untouched when regeneration fails', () => { - const git = makeStub([ - [(a) => a[0] === 'merge', fail('CONFLICT')], - [isConflictedFiles, `packages/cli/x.ts\n${LOCKFILE}`], - ]); - const pnpm = makeStub([[() => true, fail('ERR_PNPM_REGISTRY unreachable')]]); - - const { files, preResolved, lockfileDeferred } = buildConflictBranch({ - git, - pnpm, - masterSha: MASTER, - log: () => {}, - }); - - assert.deepEqual(files, ['packages/cli/x.ts']); - assert.deepEqual(preResolved, []); - assert.equal(lockfileDeferred, true); - assert.equal(pnpm.calls.length, 1); assert.equal( - git.calls.some((a) => a[0] === 'checkout' && a[1] === '--conflict=merge'), + git.calls.some((a) => a[0] === 'checkout' && a.includes(LOCKFILE)), false, - 'the initial failure leaves the unmodified conflict markers in place', - ); - assert.ok( - git.calls.some((a) => a[0] === 'commit'), - 'the conflict branch must still be committed', + 'the lockfile conflict markers must stay untouched', ); }); @@ -1014,7 +962,7 @@ test('sync opens a draft conflict PR and leaves 3.x untouched on a real conflict ); }); -test('sync reports only the code conflicts on a mixed conflict, with mechanical files pre-resolved', async () => { +test('sync reports the code conflict and defers the lockfile on a mixed conflict', async () => { const git = makeStub([ ...baseGitRoutes.filter((r) => !r[0](['merge-tree'])), [(a) => a[0] === 'merge-tree', conflictedMergeTree(LOCKFILE, 'packages/cli/x.ts')], @@ -1030,16 +978,13 @@ test('sync reports only the code conflicts on a mixed conflict, with mechanical await sync({ git, gh, pnpm, env, fetchFn: okFetch(['alice']), log: () => {} }); - assert.equal(pnpm.calls.length, 2, 'the lockfile is regenerated and validated'); + assert.equal(pnpm.calls.length, 0, 'the conflict-PR path must not run pnpm'); const create = gh.calls.find((a) => a[0] === 'pr' && a[1] === 'create'); const body = create[create.indexOf('--body') + 1]; assert.match(body, /### Conflicted files\n- `packages\/cli\/x\.ts`/); - assert.match(body, /### Auto-resolved for you/); - assert.ok( - body.indexOf(LOCKFILE) > body.indexOf('Auto-resolved'), - 'the lockfile belongs to the auto-resolved section', - ); + assert.doesNotMatch(body, /### Auto-resolved for you/); + assert.match(body, /pnpm-lock\.yaml.*conflict markers/); // Owner attribution is scoped to the real code conflicts only. const attributions = git.calls.filter((a) => a[0] === 'log' && a.includes('--format=%H')); diff --git a/.github/workflows/util-sync-master-to-3x.yml b/.github/workflows/util-sync-master-to-3x.yml index 53bdc9f48d6..b06a16bc908 100644 --- a/.github/workflows/util-sync-master-to-3x.yml +++ b/.github/workflows/util-sync-master-to-3x.yml @@ -9,12 +9,12 @@ # option alone (e.g. modify/delete) are resolved toward 3.x's side, under that same # verification. # -# Conflicts confined to mechanical, tool-generated files (the pnpm lockfile, bot-maintained -# 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. 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. +# Conflicts confined to non-lockfile mechanical files are resolved in place during the replay +# — no PR, no commit, no human. A code or pnpm-lock.yaml conflict leaves 3.x untouched. A +# draft conflict PR carries the code and lockfile markers, with other mechanical files already +# resolved. The lockfile is always left for the resolver. The workflow also notifies +# #alerts-v3-sync. 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