diff --git a/docs/branch-review-records/23787765990717e03f13cc7032add47d5a8e7d5b869c94ad78c36a062211e4e5.record.md b/docs/branch-review-records/23787765990717e03f13cc7032add47d5a8e7d5b869c94ad78c36a062211e4e5.record.md new file mode 100644 index 0000000000..cb84bc9742 --- /dev/null +++ b/docs/branch-review-records/23787765990717e03f13cc7032add47d5a8e7d5b869c94ad78c36a062211e4e5.record.md @@ -0,0 +1 @@ +| 2026-08-17 | 2014 | 1c88ada9c023918764dcf2a01914829e3b93e902 | PR #2014 review-and-fix | High-confidence defects fixed; review threads resolved | audit-merge-loss --self-test + --since; prettier check; npm run check:branch-review-ledger; npm run check-outstanding-issues; required CI SUCCESS | diff --git a/docs/branch-review-records/4405a36b7fedb006b4f8a9947a9e66aec8589e5a59de4871bbab616ad8a91c5c.record.md b/docs/branch-review-records/4405a36b7fedb006b4f8a9947a9e66aec8589e5a59de4871bbab616ad8a91c5c.record.md new file mode 100644 index 0000000000..fd098b9254 --- /dev/null +++ b/docs/branch-review-records/4405a36b7fedb006b4f8a9947a9e66aec8589e5a59de4871bbab616ad8a91c5c.record.md @@ -0,0 +1 @@ +| 2026-08-17 | PR (branch claude/p1-ledger-324-318-316-xag5sy, #324 follow-up) | 91d3ccfcef79f02c140141eb560f941626039443 | scripts/audit-merge-loss.mjs + tests/merge-loss-audit.test.ts + one #324 inbox request (1d35d652); advisory only, no CI wiring, no schedule, exit code behaviour unchanged | Authored handoff, owner-approved scope (tab fix + mechanism classifier only). Fixed a defect that had disabled the reconciliation exemption since it was written: treeEntryReader split ls-tree on a literal backslash-t instead of a tab, keeping the path on the entry, so the cross-path inbox-to-applied comparison could never match. 14-day window before/after: 51 findings / 255 flagged files / filesExempted 0 -> 11 findings / 66 flagged / 189 exempted. Escaped originally because all tests injected entryAt directly; closed by extracting parseTreeEntry and testing it against real ls-tree output. Added classifyRemoval, which blames the oldest commit whose tree entry already matches the pre-landing entry and reports merge-resolution vs deliberate-commit vs unknown, sorting merge-resolution first. Over the window 14 of 66 flagged files were merge-resolution (13 from acf78bf) and all 52 others had explanatory single-parent subjects. Re-verified three genuine unrepaired losses against current main: #1800 (wiring and all three tests gone), #1804 (also-matches back in forms, guards reverted, apparently untracked), #1796 (Node 26 allowance gone, apparently untracked). The (a) schedule, (b) triage-owner and (c) one-tool-vs-two decisions remain OPEN and were deliberately not implemented; nothing was made blocking and no finding is auto-closed. | verify:pr-local executable scope, 9 checks completed, failed: (none) - lint, typecheck, full unit suite, check:rag:fixtures, check:medication-interactions, check:medication-lexicon-report. Focused tests/merge-loss-audit.test.ts 29 passed (was 16). Mutation-verified three ways: backslash-t reintroduction fails 3 tests + self-test; newest-first walk fails the blame-the-oldest test; unknown-as-deliberate fails 2 tests. verify:ui NOT run - Playwright chromium-1194 vs pinned 1234 (#255/#312) fails closed in this container; no browser coverage claimed and none needed for a non-UI script. | diff --git a/docs/outstanding-issues-inbox/1d35d652-d833-4934-890c-84b6837581cd.json b/docs/outstanding-issues-inbox/1d35d652-d833-4934-890c-84b6837581cd.json new file mode 100644 index 0000000000..0b22f0d69a --- /dev/null +++ b/docs/outstanding-issues-inbox/1d35d652-d833-4934-890c-84b6837581cd.json @@ -0,0 +1,10 @@ +{ + "version": 2, + "id": "1d35d652-d833-4934-890c-84b6837581cd", + "createdOn": "2026-08-17", + "action": "update", + "payload": { + "id": "#324", + "detail": "**Outcome:** the file-level merge-loss detector is delivered; one authoritative row now tracks its remaining operational decision. **Delivered:** PR #1944 added scripts/audit-merge-loss.mjs through npm run audit:merge-loss and focused tests. It compares every changed file in a bounded main-history window with the landing commit's first parent, then reports possible reverts for human review. The implementation independently rediscovered the acf78bf casualties, including the #1803 token-retirement loss, and deliberately remains advisory because blob equality cannot distinguish a deliberate revert from an accidental merge-resolution loss. **Remaining:** decide whether it runs after merges or on a schedule, who triages positive findings, and whether the separate branch-versus-squash inbox-request-loss case should be a second detector or a mode of the same tool. A scheduled or required check without a named human disposition path would become ignorable noise. **Stop:** do not reimplement the delivered script, and do not make either detector blocking or auto-close findings until that ownership decision exists. SIGNAL-TO-NOISE CHARACTERISED AND TWO FIXES LANDED 2026-08-15 (owner-approved in session; still advisory, still unscheduled, still not blocking). (1) DEFECT FOUND AND FIXED: treeEntryReader split ls-tree output on the literal two-character sequence backslash-t rather than a tab, so the tree entry kept the filename. Same-path comparisons were unaffected, which is why the tool still found real losses, but isReconciliationMove compares an inbox path against its applied/ path, so the exemption could never match. Measured at 8069188 over 14 days: 51 findings / 255 flagged files / filesExempted 0, versus 11 findings / 66 flagged files / 189 exempted after a one-character fix - the exemption the script's own docstring says exists to stop inbox noise burying the genuine #1803 signal had been dead since it was written. Root cause of the escape: every test injected entryAt directly, so bare entries compared equal whether or not the path was stripped. Closed permanently by extracting parseTreeEntry as an exported pure function and testing it against real ls-tree output; mutation-verified (reintroducing backslash-t fails 3 tests plus the self-test). (2) MECHANISM CLASSIFIER ADDED: classifyRemoval walks the commits touching each flagged file between the landing and the ref, oldest first, takes the first whose tree entry already equals the pre-landing entry, and reports whether that commit was a merge (accidental) or single-parent (usually deliberate, and its subject says why). This is what makes the report triageable: over the window, 14 of 66 flagged files were merge-resolution removals with 13 from the single documented bad merge acf78bf, while all 52 others had explanatory single-parent subjects such as 'Re-land the --shadow-tight retirement', 'rework the viewer for phone and PWA reading' and 'ci: speed iteration without weakening gates'. Merge-resolution findings now sort first; unknown is reported rather than guessed. Mutation-verified in three directions (tab bug, newest-first walk, unknown-as-deliberate). GENUINE STILL-UNREPAIRED LOSSES, re-verified against main after it advanced past 8069188: #1800 fuzzy catalogue wiring is absent from therapies.ts, specifiers.ts and factsheets-data.ts AND all three of its tests carry zero fuzzy assertions so nothing can go red (tracked by #330); #1804's removal of UniversalSearchAlsoMatches from forms mode is reverted so the component is back at forms-search-results-page.tsx lines 44 and 894 with its guard assertions reverted, APPARENTLY UNTRACKED; #1796's ALLOWED_NODE_MAJOR_VERSIONS [24, 26] allowance is gone so worker/validate-runtime.ts still hard-codes nodeMajor() !== 24, APPARENTLY UNTRACKED; #1803 and #1807 lost design-system doc status rows while their code landed, so docs and code disagree. NEXT - the three decisions this row exists for are still open and are deliberately NOT implemented: (a) schedule, recommended weekly on a 14-day window rather than post-merge, because a post-merge trigger fires roughly 380 times per 14 days here and at merge time the loss has not happened yet; (b) triage owner, recommended routing to a pinned issue reusing the live-drift routing already covered by tests/live-drift-workflow.test.ts, with one named human, and not a required check; (c) recommended ONE tool with a --mode flag rather than a second detector, since the inbox case shares the window, landing enumeration and tree-entry comparison and differs only in paths and exemptions. Also recommended: the phantom-SHA class (a ledger record asserting a fix at 720e7027, an object that does not exist) is a DIFFERENT family - a ledger assertion with no landed content, checkable with git cat-file -e - and should get its own row rather than being folded into this tool. Stop unchanged: do not make either detector blocking or auto-close findings until the ownership decision exists." + } +} diff --git a/scripts/audit-merge-loss.mjs b/scripts/audit-merge-loss.mjs index 053179ca53..9f34cdaaed 100644 --- a/scripts/audit-merge-loss.mjs +++ b/scripts/audit-merge-loss.mjs @@ -108,6 +108,57 @@ export function isReconciliationMove(file, landingRef, ref, entryAt) { return requestAtLanding !== null && entryAt(ref, `${INBOX}/applied/${match[1]}`) === requestAtLanding; } +/** + * Where a landing's contribution to one file disappeared, and by what mechanism. + * + * Walk the commits that touched `file` between the landing and the ref OLDEST + * FIRST and take the first one whose tree entry already equals the pre-landing + * entry. That is where the contribution stopped being present; a later commit + * that also matches is just carrying the absence forward. + * + * The mechanism is the triage signal, and it is the reason this tool can be read + * at all. A merge commit dropping a side is the accidental case the audit exists + * for. A single-parent commit is somebody choosing to remove the change, and its + * subject almost always says why. Measured over the 2026-08-01..15 window: of 66 + * flagged non-inbox files, 14 were merge-resolution removals and 13 of those came + * from the single documented bad merge (acf78bf) — while every one of the other 52 + * had an explanatory single-parent subject ("Re-land the --shadow-tight + * retirement", "rework the viewer for phone and PWA reading", "ci: speed + * iteration without weakening gates"). + * + * This does NOT turn the audit into a verdict, and deliberately does not filter: + * a deliberate-looking commit can still be a mistake, and `unknown` means the + * walk found no commit matching the pre-landing entry, which is a question rather + * than a clean bill. It orders the reading list so the accidental cases are not + * buried under the deliberate ones. + * + * Pure, with git access injected, so it is testable without a repository. + * + * @param {{ file: string, landingSha: string, preEntry: string | null, ref: string, + * historyOf: (landingSha: string, ref: string, file: string) => Array<{ sha: string, subject: string }>, + * entryAt: (ref: string, file: string) => string | null, + * isMergeCommit: (sha: string) => boolean }} options + */ +export function classifyRemoval({ file, landingSha, preEntry, ref, historyOf, entryAt, isMergeCommit }) { + const commits = historyOf(landingSha, ref, file) ?? []; + for (const commit of commits) { + if (entryAt(commit.sha, file) !== preEntry) continue; + return { + mechanism: isMergeCommit(commit.sha) ? "merge-resolution" : "deliberate-commit", + sha: commit.sha, + subject: commit.subject, + }; + } + return { mechanism: "unknown", sha: undefined, subject: undefined }; +} + +const MECHANISMS = ["merge-resolution", "deliberate-commit", "unknown"]; + +/** How many of a finding's files went in a merge resolution — the ordering key. */ +function mergeResolutionCount(finding) { + return finding.revertedFiles.filter((entry) => entry.removal?.mechanism === "merge-resolution").length; +} + /** * Compare each landing's contribution against the ref's current state. * @@ -120,18 +171,28 @@ export function isReconciliationMove(file, landingRef, ref, entryAt) { * gone again" case. A file the pull request DELETED and which is still absent * does not match, because its pre-landing blob existed — the deletion survived. * + * `removalOf` is optional. When given, each reported file carries the commit and + * mechanism that removed it (see classifyRemoval) and findings are ordered so the + * merge-resolution cases come first. Omitting it leaves the finding shape and the + * original ordering untouched. + * * Pure and exported for the self-test and focused tests. * * @typedef {{ sha: string, date: string, subject: string, pullNumber: number | undefined, * preRef: string, files?: string[] }} Landing + * @typedef {{ mechanism: "merge-resolution" | "deliberate-commit" | "unknown", + * sha: string | undefined, subject: string | undefined }} Removal * @param {{ landings?: Landing[], entryAt: (ref: string, file: string) => string | null, - * ref?: string }} options + * ref?: string, + * removalOf?: (context: { file: string, landingSha: string, preEntry: string | null }) => Removal | undefined + * }} options */ -export function classifyMergeLoss({ landings = [], entryAt, ref = DEFAULT_REF }) { +export function classifyMergeLoss({ landings = [], entryAt, ref = DEFAULT_REF, removalOf }) { const findings = []; let filesCompared = 0; let filesExempted = 0; const skipped = []; + const mechanismCounts = Object.fromEntries(MECHANISMS.map((mechanism) => [mechanism, 0])); for (const landing of landings) { if (landing.pullNumber === undefined) { skipped.push(landing); @@ -147,7 +208,13 @@ export function classifyMergeLoss({ landings = [], entryAt, ref = DEFAULT_REF }) filesExempted += 1; continue; } - reverted.push({ file, absent: now === null }); + // `removalOf` stays optional so the finding shape is unchanged for callers + // that only want the comparison — the mechanism costs a `git log` per + // flagged file, which is worth paying for findings and not for every + // compared file. + const removal = removalOf ? removalOf({ file, landingSha: landing.sha, preEntry: before }) : undefined; + if (removal) mechanismCounts[removal.mechanism] += 1; + reverted.push(removal ? { file, absent: now === null, removal } : { file, absent: now === null }); } if (reverted.length > 0) { findings.push({ @@ -160,8 +227,23 @@ export function classifyMergeLoss({ landings = [], entryAt, ref = DEFAULT_REF }) }); } } - findings.sort((a, b) => b.revertedFiles.length - a.revertedFiles.length || a.pullNumber - b.pullNumber); - return { findings, scannedLandings: landings.length - skipped.length, skipped, filesCompared, filesExempted }; + // Merge-resolution removals first: they are the accidental case. With no + // `removalOf` every count is 0, so this falls through to the original + // most-of-the-landing-missing order. + findings.sort( + (a, b) => + mergeResolutionCount(b) - mergeResolutionCount(a) || + b.revertedFiles.length - a.revertedFiles.length || + a.pullNumber - b.pullNumber, + ); + return { + findings, + scannedLandings: landings.length - skipped.length, + skipped, + filesCompared, + filesExempted, + mechanismCounts, + }; } function resolveArgs(argv) { @@ -191,18 +273,93 @@ function collectLandings(ref, since) { }); } +/** + * The mode/type/OID prefix of one `git ls-tree` line, with the path removed. + * + * `ls-tree` renders ` SP SP TAB `, so the split is on a + * REAL tab. This is exported because getting it wrong is invisible to any test + * that injects `entryAt` directly, which is how the original `"\\t"` (a literal + * backslash-t, matching nothing) survived a green suite: leaving the path on the + * entry is harmless for the same-path comparison that finds losses, and breaks + * only the cross-path reconciliation check, which compares an inbox request + * against its `applied/` record. That silently turned every reconciled request + * back into a finding — 189 of 255 flagged files on a 14-day window, with + * `filesExempted` reporting 0 — burying the genuine signal the exemption exists + * to protect. Parse it here, once, and test it against real `ls-tree` output. + */ +export function parseTreeEntry(raw) { + if (raw === undefined || raw === null) return null; + const line = String(raw); + if (line.length === 0) return null; + return line.split("\t", 1)[0]; +} + function treeEntryReader() { const cache = new Map(); return (ref, file) => { const key = `${ref}:${file}`; if (!cache.has(key)) { - const entry = tryGit(["ls-tree", ref, "--", file]); - cache.set(key, entry ? entry.split("\\t", 1)[0] : null); + cache.set(key, parseTreeEntry(tryGit(["ls-tree", ref, "--", file]))); + } + return cache.get(key); + }; +} + +/** + * Commits touching `file` between a landing and the ref, oldest first. + * + * `--full-history` matters: default history simplification hides the merge + * commits that are exactly what this walk is looking for, so without it a + * merge-resolution removal reports as `unknown`. + */ +function historyReader() { + const cache = new Map(); + return (landingSha, ref, file) => { + const key = `${landingSha}..${ref}:${file}`; + if (!cache.has(key)) { + const raw = tryGit([ + "log", + "--reverse", + "--first-parent", + "--full-history", + "--format=%H%x1f%s", + `${landingSha}..${ref}`, + "--", + file, + ]); + cache.set( + key, + String(raw ?? "") + .split(/\r?\n/) + .filter((line) => line.trim().length > 0) + .map((line) => { + const [sha, ...rest] = line.split("\x1f"); + return { sha, subject: rest.join("\x1f") }; + }), + ); } return cache.get(key); }; } +function mergeCommitReader() { + const cache = new Map(); + return (sha) => { + if (!cache.has(sha)) { + const raw = tryGit(["rev-list", "--parents", "-n", "1", sha]); + // ` …` — more than one parent means a merge. + cache.set(sha, raw === undefined ? false : raw.trim().split(/\s+/).length - 1 > 1); + } + return cache.get(sha); + }; +} + +const MECHANISM_LABEL = { + "merge-resolution": "MERGE-RESOLUTION", + "deliberate-commit": "deliberate commit", + unknown: "unresolved", +}; + function report(result, { ref, since, strict }) { const { findings, scannedLandings, skipped, filesCompared, filesExempted } = result; console.log( @@ -228,6 +385,18 @@ function report(result, { ref, since, strict }) { console.log(`[merge-loss] ${findings.length} landing(s) look reverted — HUMAN CONFIRMATION REQUIRED.`); console.log("A deliberate later revert is identical to an accidental one at blob level, so this is a"); console.log("question, not a verdict. For each entry below, decide whether the change was meant to go."); + const counts = result.mechanismCounts ?? {}; + if (MECHANISMS.some((mechanism) => counts[mechanism] > 0)) { + console.log(""); + console.log( + `[merge-loss] removal mechanism across ${MECHANISMS.reduce((total, m) => total + counts[m], 0)} flagged file(s): ` + + `${counts["merge-resolution"]} merge-resolution, ${counts["deliberate-commit"]} deliberate commit, ` + + `${counts.unknown} unresolved.`, + ); + console.log("Merge-resolution removals are the accidental case and are listed FIRST. A deliberate commit"); + console.log("with an explanatory subject is usually a real decision — check it, but check the others first."); + console.log("`unresolved` means no commit in the window matched the pre-merge entry; treat it as a question."); + } for (const finding of findings) { console.log(""); console.log(` PR #${finding.pullNumber} — ${finding.subject}`); @@ -237,6 +406,10 @@ function report(result, { ref, since, strict }) { ); for (const entry of finding.revertedFiles) { console.log(` - ${entry.file}${entry.absent ? " (added by the PR, absent now)" : ""}`); + if (entry.removal) { + const where = entry.removal.sha ? ` ${entry.removal.sha.slice(0, 12)} — ${entry.removal.subject}` : ""; + console.log(` via ${MECHANISM_LABEL[entry.removal.mechanism]}:${where}`); + } } console.log(` Inspect: git diff ${finding.sha}^1 ${finding.sha} -- `); } @@ -303,6 +476,55 @@ function selfTest() { if (reconciled.findings.length !== 0 || reconciled.filesExempted !== 1) { throw new Error("self-test failed: a reconciled inbox request was reported as a merge loss"); } + + // Regression guard for the `"\\t"` bug. The check above injects bare entries, so + // it passes either way; this one runs real `ls-tree` output through the real + // parser, where leaving the path attached makes the cross-path comparison fail. + if (parseTreeEntry("100644 blob eee\tsome/path.json") !== "100644 blob eee") { + throw new Error("self-test failed: parseTreeEntry did not strip the path from an ls-tree line"); + } + if (parseTreeEntry(undefined) !== null || parseTreeEntry("") !== null) { + throw new Error("self-test failed: parseTreeEntry did not treat a missing path as null"); + } + const applied = `${INBOX}/applied/${path.posix.basename(request)}`; + const lsTree = (file) => parseTreeEntry(`100644 blob eee\t${file}`); + const realistic = classifyMergeLoss({ + ref: "head", + entryAt: (reference, file) => { + if (reference === "s6" && file === request) return lsTree(file); + return reference === "head" && file === applied ? lsTree(file) : null; + }, + landings: [{ sha: "s6", date: "d", subject: "r (#6)", pullNumber: 6, preRef: "pre", files: [request] }], + }); + if (realistic.findings.length !== 0 || realistic.filesExempted !== 1) { + throw new Error("self-test failed: the reconciliation exemption does not survive real ls-tree entries"); + } + + const history = { "s7..head:lost.ts": [{ sha: "m1", subject: "Merge remote-tracking branch 'origin/main'" }] }; + const removal = classifyRemoval({ + file: "lost.ts", + landingSha: "s7", + preEntry: "pre-entry", + ref: "head", + historyOf: (landingSha, reference, file) => history[`${landingSha}..${reference}:${file}`] ?? [], + entryAt: () => "pre-entry", + isMergeCommit: (sha) => sha === "m1", + }); + if (removal.mechanism !== "merge-resolution" || removal.sha !== "m1") { + throw new Error("self-test failed: a merge-resolution removal was not classified"); + } + const unresolved = classifyRemoval({ + file: "lost.ts", + landingSha: "s7", + preEntry: "pre-entry", + ref: "head", + historyOf: () => [], + entryAt: () => "pre-entry", + isMergeCommit: () => false, + }); + if (unresolved.mechanism !== "unknown") { + throw new Error("self-test failed: an empty history should classify as unknown, not as deliberate"); + } console.error("merge-loss audit self-test passed."); } @@ -310,7 +532,8 @@ function main() { if (process.argv.includes("--self-test")) return selfTest(); const options = resolveArgs(process.argv.slice(2)); - if (tryGit(["rev-parse", "--is-shallow-repository"]) === "true") { + const shallow = tryGit(["rev-parse", "--is-shallow-repository"]); + if (shallow?.trim() !== "false") { console.error("[merge-loss] this is a shallow clone; pre-merge parents are unavailable and a clean sweep here"); console.error("[merge-loss] would be meaningless. Re-run after `git fetch --unshallow`."); process.exitCode = 1; @@ -323,7 +546,16 @@ function main() { } const landings = collectLandings(options.ref, options.since); - const result = classifyMergeLoss({ landings, entryAt: treeEntryReader(), ref: options.ref }); + const entryAt = treeEntryReader(); + const historyOf = historyReader(); + const isMergeCommit = mergeCommitReader(); + const result = classifyMergeLoss({ + landings, + entryAt, + ref: options.ref, + removalOf: ({ file, landingSha, preEntry }) => + classifyRemoval({ file, landingSha, preEntry, ref: options.ref, historyOf, entryAt, isMergeCommit }), + }); if (options.json) { console.log(JSON.stringify({ ref: options.ref, sinceDays: options.since, ...result }, null, 2)); process.exitCode = options.strict && result.findings.length > 0 ? 1 : 0; diff --git a/tests/merge-loss-audit.test.ts b/tests/merge-loss-audit.test.ts index b8320d098f..c2078bc741 100644 --- a/tests/merge-loss-audit.test.ts +++ b/tests/merge-loss-audit.test.ts @@ -2,9 +2,11 @@ import { describe, expect, it } from "vitest"; import { classifyMergeLoss, + classifyRemoval, isReconciliationMove, parseLogEntries, parsePullNumber, + parseTreeEntry, } from "../scripts/audit-merge-loss.mjs"; const INBOX = "docs/outstanding-issues-inbox"; @@ -149,4 +151,142 @@ describe("merge-loss reconciliation exemption", () => { expect(result.findings).toHaveLength(1); expect(result.filesExempted).toBe(0); }); + + it("exempts a reconciled request when entries come from real ls-tree output", () => { + // The guard the `"\\t"` bug needed. Every other test in this file injects bare + // entries, which compare equal whether or not the path was stripped; only a + // realistic entry exposes a cross-path comparison that can never match. + const lsTree = (file: string) => parseTreeEntry(`100644 blob eee\t${file}`); + const result = classifyMergeLoss({ + ref: "head", + entryAt: reader({ [`sha1915:${request}`]: lsTree(request)!, [`head:${applied}`]: lsTree(applied)! }), + landings: [landing(1915, [request])], + }); + expect(result.findings).toEqual([]); + expect(result.filesExempted).toBe(1); + }); +}); + +describe("merge-loss tree-entry parsing", () => { + it("strips the path from an ls-tree line", () => { + // ` SP SP TAB ` — the separator is a real tab. + expect(parseTreeEntry("100644 blob aa5bc159018775a3ba075fa846ff4c1fda01e236\tpackage.json")).toBe( + "100644 blob aa5bc159018775a3ba075fa846ff4c1fda01e236", + ); + }); + + it("gives two paths sharing a blob the same entry", () => { + // This is what the reconciliation exemption depends on. + const inbox = parseTreeEntry("100644 blob eee\tdocs/outstanding-issues-inbox/x.json"); + const moved = parseTreeEntry("100644 blob eee\tdocs/outstanding-issues-inbox/applied/x.json"); + expect(inbox).toBe(moved); + }); + + it("keeps a mode difference visible", () => { + expect(parseTreeEntry("100644 blob aaa\ts.sh")).not.toBe(parseTreeEntry("100755 blob aaa\ts.sh")); + }); + + it("treats a missing path as null", () => { + expect(parseTreeEntry(undefined)).toBeNull(); + expect(parseTreeEntry("")).toBeNull(); + }); +}); + +describe("merge-loss removal mechanism", () => { + const walk = (commits: Array<{ sha: string; subject: string }>, merges: string[] = []) => ({ + historyOf: () => commits, + entryAt: () => "pre-entry" as string | null, + isMergeCommit: (sha: string) => merges.includes(sha), + }); + const base = { file: "lost.ts", landingSha: "s1", preEntry: "pre-entry", ref: "head" }; + + it("classifies a merge commit as a merge resolution", () => { + const removal = classifyRemoval({ + ...base, + ...walk( + [{ sha: "acf78bf4", subject: "Merge remote-tracking branch 'origin/main' into probe2-1815" }], + ["acf78bf4"], + ), + }); + expect(removal).toMatchObject({ mechanism: "merge-resolution", sha: "acf78bf4" }); + }); + + it("classifies a single-parent commit as deliberate", () => { + const removal = classifyRemoval({ + ...base, + ...walk([{ sha: "8ca147d5", subject: "ci: speed iteration without weakening gates (#1926)" }]), + }); + expect(removal).toMatchObject({ + mechanism: "deliberate-commit", + subject: "ci: speed iteration without weakening gates (#1926)", + }); + }); + + it("blames the oldest matching commit, not a later one carrying the absence forward", () => { + const removal = classifyRemoval({ + ...base, + historyOf: () => [ + { sha: "culprit", subject: "Merge branch 'main'" }, + { sha: "later", subject: "unrelated touch" }, + ], + entryAt: () => "pre-entry", + isMergeCommit: (sha: string) => sha === "culprit", + }); + expect(removal).toMatchObject({ mechanism: "merge-resolution", sha: "culprit" }); + }); + + it("reports unknown rather than guessing when no commit matches the pre-merge entry", () => { + const removal = classifyRemoval({ + ...base, + historyOf: () => [{ sha: "c1", subject: "something else" }], + entryAt: () => "a-different-entry", + isMergeCommit: () => false, + }); + expect(removal.mechanism).toBe("unknown"); + expect(removal.sha).toBeUndefined(); + }); + + it("reports unknown for an empty history", () => { + expect(classifyRemoval({ ...base, ...walk([]) }).mechanism).toBe("unknown"); + }); +}); + +describe("merge-loss mechanism reporting", () => { + const entryAt = reader({ "pre:a.ts": "1", "head:a.ts": "1", "pre:b.ts": "2", "head:b.ts": "2" }); + + it("leaves the finding shape and order untouched when no mechanism reader is given", () => { + const result = classifyMergeLoss({ ref: "head", entryAt, landings: [landing(10, ["a.ts"])] }); + expect(result.findings[0].revertedFiles).toEqual([{ file: "a.ts", absent: false }]); + expect(result.mechanismCounts).toEqual({ "merge-resolution": 0, "deliberate-commit": 0, unknown: 0 }); + }); + + it("attaches the mechanism and counts it", () => { + const result = classifyMergeLoss({ + ref: "head", + entryAt, + landings: [landing(10, ["a.ts"])], + removalOf: () => ({ mechanism: "merge-resolution", sha: "acf78bf4", subject: "Merge origin/main" }), + }); + expect(result.findings[0].revertedFiles[0]).toMatchObject({ + file: "a.ts", + removal: { mechanism: "merge-resolution", sha: "acf78bf4" }, + }); + expect(result.mechanismCounts["merge-resolution"]).toBe(1); + }); + + it("lists merge-resolution findings before larger deliberate ones", () => { + // #1800 lost 6 files to acf78bf; #1815 lost 47 to a re-land. The accidental + // case must not be buried under the bigger deliberate one. + const result = classifyMergeLoss({ + ref: "head", + entryAt, + landings: [landing(1815, ["a.ts", "b.ts"]), landing(1800, ["a.ts"])], + removalOf: ({ landingSha }: { landingSha: string }) => + landingSha === "sha1800" + ? { mechanism: "merge-resolution", sha: "acf78bf4", subject: "Merge origin/main" } + : { mechanism: "deliberate-commit", sha: "f89fbcc7", subject: "Re-land the retirement" }, + }); + expect(result.findings.map((finding) => finding.pullNumber)).toEqual([1800, 1815]); + expect(result.mechanismCounts).toEqual({ "merge-resolution": 1, "deliberate-commit": 2, unknown: 0 }); + }); });