From 9a702f2211415ab5cf174dd042bcd105df83105f Mon Sep 17 00:00:00 2001 From: Dmitrii Vasilev Date: Sat, 5 Sep 2026 06:21:48 +0700 Subject: [PATCH] fix(loop): a conflict does not say which side is behind, and "needs a rebase" was advice toward deleting finished code `land` reported nine conflicts and one remedy: rebase, or close as superseded. Taking that advice on #1302 would have destroyed landed work. #1302 is "expose Queen billing mode and quota authority in public research status", 205 insertions, accepted, its issue already closed. Replaying it onto today's base would have: - deleted `WorkerCapacityBreakdown`, which is #1308's work, carried and merged as PR #331 after this branch was cut - deleted the tree-load-failure handling the base gained since - REINTRODUCED a non-ASCII ellipsis into a path redaction the base already performs in ASCII - breaking L3 in the same stroke That branch is not waiting for a rebase. It is superseded in part, and what survives is a small delta belonging on today's base as new work. A conflict says two sides touched the same lines; it says nothing about which side is behind, and the advice depends entirely on that. So the report measures it: how far has the base travelled on the conflicting files since the fork point? CONFL queen-1302 2 conflicting file(s): ...queen-public-research.ts, ...test.ts | the base moved on these since the fork: N commit(s), M insertions - a rebase would replay OLD code over new; re-file what survives against today's base It is a measurement, not a verdict. The person still decides, with the number that decides it. AND THE COUNT WAS INFLATED. `git merge-tree --name-only` interleaves prose with paths - "Auto-merging X" and "CONFLICT (add/add): Merge conflict in X" are commentary about the file named on the line above. Counting them reported "6 conflicting path(s)" for two files. An inflated number is how a report stops being read, and this one had been doubling every conflict it printed. The reason line is no longer truncated to 96 characters either, which had been cutting off exactly the half that says what to do. selftest 145 pass 0 fail. --- trios/.trinity/loop/land.mjs | 54 ++++++++++++++++++++++++++++++-- trios/.trinity/loop/selftest.mjs | 13 ++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/trios/.trinity/loop/land.mjs b/trios/.trinity/loop/land.mjs index b284637b5b..dafa92d439 100644 --- a/trios/.trinity/loop/land.mjs +++ b/trios/.trinity/loop/land.mjs @@ -188,7 +188,53 @@ export function mergesCleanly(branch) { // With --write-tree the first line is the tree oid; conflicted paths follow. const lines = out.split('\n').filter(Boolean) if (lines.length <= 1) return { clean: true } - return { clean: false, why: `${lines.length - 1} conflicting path(s): ${lines.slice(1, 4).join(', ')}` } + // `merge-tree --name-only` interleaves PROSE with paths: "Auto-merging X" and + // "CONFLICT (add/add): Merge conflict in X" are commentary about the same file + // the line above names. Counting them made every conflict look twice as wide + // as it is - "6 conflicting path(s)" for three files, which is the kind of + // inflated number that makes a report stop being read. + const paths = lines.slice(1) + .filter((p) => !/^(Auto-merging|CONFLICT )/.test(p)) + .filter((p, i, a) => a.indexOf(p) === i) + const moved = baseMovedSince(branch, paths) + const since = moved + ? ` | the base moved on these since the fork: ${moved.commits} commit(s), ${moved.stat} - a rebase would replay OLD code over new; re-file what survives against today's base` + : '' + return { clean: false, why: `${paths.length} conflicting file(s): ${paths.slice(0, 3).join(', ')}${since}` } +} + + +/** + * HAS THE BASE MOVED ON PAST THIS BRANCH, or merely diverged from it? + * + * A conflict says two sides touched the same lines. It does NOT say which side + * is behind, and the advice that follows depends entirely on that. + * + * Measured 2026-09-05 on #1302, "expose Queen billing mode and quota authority + * in public research status". Rebasing it would have replayed a 205-line file + * over a base that had since gained `WorkerCapacityBreakdown` (#1308's landed + * work) and tree-load-failure handling - deleting both - and would have + * REINTRODUCED a non-ASCII ellipsis into a path redaction the base already does + * in ASCII, breaking L3 in the same stroke. + * + * That branch is not waiting for a rebase. It is superseded in part, and what + * survives is a small delta that belongs on today's base as new work. Telling a + * person "needs a rebase" would have been advice toward destroying finished + * code. + * + * So the report says how far the base has travelled on the conflicting files + * since the fork point. It is a measurement, not a verdict: the person still + * decides, but now with the number that decides it. + */ +export function baseMovedSince(branch, paths, run = sh) { + const fork = run(`git merge-base origin/${BASE} origin/${branch}`) + if (!fork || !paths.length) return null + const quoted = paths.map((p) => JSON.stringify(p)).join(' ') + const commits = run(`git rev-list --count ${fork}..origin/${BASE} -- ${quoted}`) + const stat = run(`git diff --shortstat ${fork}..origin/${BASE} -- ${quoted}`) + const n = Number(commits) + if (!Number.isFinite(n) || n === 0) return null + return { commits: n, stat: (stat || '').trim() } } export async function survey() { @@ -300,7 +346,11 @@ if (isMain) { const m = mergesCleanly(r.branch) if (m.clean) { r.clean = true; batch.push(r) } else { r.clean = false; r.why = m.why; skipped.push(r) } } - for (const r of skipped) console.log(` CONFL ${r.branch.padEnd(16)} ${r.why.slice(0, 96)}`) + for (const r of skipped) { + // The whole reason, not the first 96 characters of it. The truncation hid + // exactly the half that says what to DO about the conflict. + console.log(` CONFL ${r.branch.padEnd(16)} ${r.why}`) + } for (const r of batch) console.log(` land ${r.branch.padEnd(16)} ${r.why}`) if (skipped.length) console.log(` (${skipped.length} conflicting branch(es) skipped over, not counted against the batch)`) diff --git a/trios/.trinity/loop/selftest.mjs b/trios/.trinity/loop/selftest.mjs index 77981bf848..21c4fe1ede 100644 --- a/trios/.trinity/loop/selftest.mjs +++ b/trios/.trinity/loop/selftest.mjs @@ -1829,6 +1829,19 @@ check('close-done and land agree about what landed, because it is one rule', () if (/const landed = baseTree && mergedTree/.test(code)) throw new Error('the second copy of the rule must be gone, not merely bypassed') }) +check('merge-tree prose is not a conflicting path', async () => { + const { baseMovedSince } = await import('./land.mjs') + const code = codeOf('land.mjs') + // `--name-only` interleaves "Auto-merging X" and "CONFLICT (add/add): ..." + // with the paths. Counting them reported six conflicting paths for three + // files - the kind of inflated number that makes a report stop being read. + if (!/\^\(Auto-merging\|CONFLICT \)/.test(code)) throw new Error('the commentary lines must be filtered out') + if (!/a\.indexOf\(p\) === i/.test(code)) throw new Error('and a path named twice is one file') + // A base that has not moved says nothing rather than saying zero. + if (baseMovedSince('queen-1', [], () => 'abc') !== null) throw new Error('no paths, no claim') + if (baseMovedSince('queen-1', ['a.ts'], () => '0') !== null) throw new Error('zero commits is not a finding') +}) + check('the harness can fail an async check', async () => { // Guarding the fix above: before it, this file reported 0 failures while an // async case was rejecting into the void.