From fe2a3bd8a3353d2270fb7ae4dccb25b6317ca6e4 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 31 Oct 2026 08:11:36 -0700 Subject: [PATCH 1/3] chore(quest): claim quest/m0/required-heading.md Quest-Claim: eac64b24020188db From 1245cf019e7ff2581e0522c8c8e0b5ca04afa4cc Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 31 Oct 2026 08:17:19 -0700 Subject: [PATCH 2/3] fix(merge): drop a Required heading the driver empties When the quest resolve driver's list merge leaves a Required section with nothing but blank lines, drop the heading with it, as quest rm does, so the merged tree passes quest check. Other empty list sections stay for quest check to report. Co-Authored-By: Claude Opus 5.5 --- quest/m0/README.md | 1 - quest/m0/required-heading.md | 25 ---------- src/core/merge.ts | 89 ++++++++++++++++++++++++++++++++++-- test/resolve.test.ts | 40 +++++++++++++++- 4 files changed, 123 insertions(+), 32 deletions(-) delete mode 100644 quest/m0/required-heading.md diff --git a/quest/m0/README.md b/quest/m0/README.md index 9322d412..8d5a4626 100644 --- a/quest/m0/README.md +++ b/quest/m0/README.md @@ -44,7 +44,6 @@ ownership-manifest design: - [Local mergeability](/quest/m0/land-mergeability.md) - `quest land` merges with the quest driver instead of ejecting on GitHub's conflict flag - [Accurate head_moved](/quest/m0/land-head-moved.md) - the gate's own base-merge push and batch-mate failures are not reported as a moved head - [Queue-time checks](/quest/m0/land-queue-checks.md) - queued PRs are checked against each other and `quest check` runs per PR before batching -- [Drop emptied Required headings](/quest/m0/required-heading.md) - a clean driver merge that empties a `Required` list passes `quest check` - [Codex cloud off](/quest/m0/codex-cloud-off.md) - automatic Codex cloud reviews stop once no open PR relies on them - [Review handles expired and environment logins](/quest/m0/review-login.md) - `quest review` says to log in again instead of failing with a 401, and reviews with credentials from the environment - [Live sandbox test](/quest/m0/review-sandbox.md) - the real reviewer CLIs run nothing a reviewed head plants diff --git a/quest/m0/required-heading.md b/quest/m0/required-heading.md deleted file mode 100644 index b3ed7bdd..00000000 --- a/quest/m0/required-heading.md +++ /dev/null @@ -1,25 +0,0 @@ -# [XS] The merge driver drops a Required heading it empties - -## Goal - -After a merge where the `quest resolve` driver removes the last entry from -an epic's `Required` list, the `## Required` heading is gone too, and -`quest check` passes on the merged tree. - -## Plan - -Decided 2026-10-31 (user, /quest-plan of the follow-ups from a moq-dev/moq -`/quest-complete` run). [moq-dev/moq#7251](https://github.com/moq-dev/moq/pull/7251) -merged cleanly, then failed `quest check`. The driver had emptied a -`Required` list and left its heading, but the guide says an empty `Required` -section is deleted. - -`resolveLists` (`src/core/merge.ts`) merges only the conflicting entries, -so it never sees the heading. After resolving, drop a `Required` heading -whose section is now empty. Leave other empty sections to `quest check`, which -should report them. Test it with both sides each removing one of the last two -entries. - -## Related - -- [Visible merge-driver fallback](/quest/m0/helpers/merge-driver.md) - the driver's other gap diff --git a/src/core/merge.ts b/src/core/merge.ts index 17134b72..b96b243e 100644 --- a/src/core/merge.ts +++ b/src/core/merge.ts @@ -7,6 +7,8 @@ // inserts a new quest next to them. Such a conflict merges item by item: keep // what neither side removed and add what either side inserted. Anything else, // including prose, a heading, or an item both sides rewrote, stays a conflict. +// A `Required` list the merge empties loses its heading too, since an empty +// one would block its quest forever. import { parse } from "./doc"; import { LIST_SECTIONS } from "./rules"; @@ -27,9 +29,10 @@ export type Chunk = Clean | Conflict; const SIDES = ["ours", "base", "theirs"] as const; type Side = (typeof SIDES)[number]; -/** A list entry as one side has it: the destination of its opening link, and its source lines. */ +/** A list entry as one side has it: the destination of its opening link, its section, and its source lines. */ interface Item { key: string; + section: string; lines: string[]; } @@ -103,7 +106,9 @@ export function render(chunks: Chunk[], size: number, diff3: boolean, labels = L * removed, with the one side's edit when only one side changed one, and * inserted are the entries either side added, after the entry that precedes * them on that side, ours before theirs. Entries are matched by their opening - * link's destination. Every other conflict is returned as it was. + * link's destination. A `Required` section this leaves with nothing but + * blank lines is dropped with its heading. Every other conflict is returned as + * it was. */ export function resolveLists(path: string, merged: Chunk[]): Chunk[] { const items = { @@ -111,10 +116,80 @@ export function resolveLists(path: string, merged: Chunk[]): Chunk[] { base: lists(path, merged, "base"), theirs: lists(path, merged, "theirs"), }; - return merged.map((chunk, index) => { + const emptied = new Set(); + const resolved = merged.map((chunk, index) => { const [ours, base, theirs] = SIDES.map((side) => items[side].get(index)); if (ours === undefined || base === undefined || theirs === undefined) return chunk; - return mergeItems(ours, base, theirs) ?? chunk; + const lines = mergeItems(ours, base, theirs); + if (lines === null) return chunk; + if (lines.length === 0 && base.some((item) => item.section === "Required")) emptied.add(index); + return lines; + }); + return emptied.size === 0 ? resolved : dropEmptyRequired(path, resolved, emptied); +} + +/** + * `merged` without each `Required` heading whose section holds one of the + * `emptied` resolutions and otherwise only blank lines, all outside any + * conflict, along with that section's lines. As `quest rm` does, a blank line + * left doubled by the removal goes too. + */ +function dropEmptyRequired(path: string, merged: Chunk[], emptied: Set): Chunk[] { + // The document with every conflict taken from ours; `clean` marks lines outside a conflict. + const lines: string[] = []; + const clean: boolean[] = []; + const at: number[] = []; + merged.forEach((chunk, index) => { + if (emptied.has(index)) at.push(lines.length); + const own = Array.isArray(chunk) ? chunk : chunk.ours; + lines.push(...own); + clean.push(...own.map(() => Array.isArray(chunk))); + }); + const doc = parse(path, lines.join("\n")); + // A trailing newline leaves an empty last element, which is not a line. + const count = lines.length > 0 && lines[lines.length - 1] === "" ? lines.length - 1 : lines.length; + + // Indices, 0-based, of the lines to drop. + const dropped = new Set(); + doc.headings.forEach((heading, index) => { + if (heading.text !== "Required" || !clean[heading.line - 1]) return; + const next = doc.headings[index + 1]; + if (next !== undefined && !clean[next.line - 1]) return; + // The section's body is the 0-based lines from `heading.line` up to `end`. + const end = next === undefined ? count : next.line - 1; + if (!at.some((line) => line >= heading.line && line <= end)) return; + const body = Array.from({ length: end - heading.line }, (_, i) => heading.line + i); + if (!body.every((i) => clean[i] && lines[i].trim() === "")) return; + dropped.add(heading.line - 1); + for (const i of body) dropped.add(i); + }); + if (dropped.size === 0) return merged; + + // Removing a block between two blank lines would leave both; keep one. + let line = 0; + let gap = false; + let blank = false; + return merged.map((chunk) => { + if (!Array.isArray(chunk)) { + line += chunk.ours.length; + gap = false; + blank = false; + return chunk; + } + const kept: string[] = []; + for (const text of chunk) { + if (dropped.has(line++)) { + gap = true; + continue; + } + const empty = text.trim() === ""; + if (!(gap && empty && blank)) { + kept.push(text); + blank = empty; + } + gap = false; + } + return kept; }); } @@ -156,7 +231,11 @@ function lists(path: string, merged: Chunk[], side: Side): Map { return ok; }); if (!valid || next !== last + 1) continue; - const items = entries.map((entry) => ({ key: entry.target!, lines: lines.slice(entry.line - 1, entry.end) })); + const items = entries.map((entry) => ({ + key: entry.target!, + section: entry.section!, + lines: lines.slice(entry.line - 1, entry.end), + })); if (items.every((item) => selfContained(path, item))) out.set(index, items); } return out; diff --git a/test/resolve.test.ts b/test/resolve.test.ts index 50225e1c..021ace61 100644 --- a/test/resolve.test.ts +++ b/test/resolve.test.ts @@ -7,7 +7,7 @@ import { fileURLToPath } from "node:url"; import { describe, expect, test } from "vitest"; -import { chunks, render, resolveLists } from "../src/core"; +import { check, chunks, parse, render, resolveLists } from "../src/core"; import { type ResolveOptions, resolveFile } from "../src/cli/resolve"; import { heal, init, uninstall } from "../src/cli/setup"; import { tempDir } from "./fixture"; @@ -200,6 +200,44 @@ describe("resolve", () => { conflicts: 0, }); }); + + describe("a Required list both sides emptied", () => { + const plan = "# [S] Quest\n\n## Goal\n\nGoal.\n\n## Plan\n\nPlan.\n"; + const related = `\n## Related\n\n${item("r")}\n`; + const empty = (text: string) => + check([parse("quest/README.md", text)], () => true).filter((f) => f.message.includes("is empty")); + + for (const [name, tail] of [ + ["at the end", ""], + ["before another section", related], + ] as const) { + test(`loses its heading ${name}`, () => { + const base = `${quest([item("a"), item("b")])}${tail}`; + // Each side finished one of the last two blockers. + const { text, conflicts } = merge(base, `${quest([item("b")])}${tail}`, `${quest([item("a")])}${tail}`); + expect({ text, conflicts }).toEqual({ text: `${plan}${tail}`, conflicts: 0 }); + expect(empty(text)).toEqual([]); + }); + } + + test("loses its heading beside a conflict left in prose", () => { + const { text, conflicts } = merge( + quest([item("a"), item("b")], "Base plan."), + quest([item("b")], "Our plan."), + quest([item("a")], "Their plan."), + ); + expect(conflicts).toBe(1); + expect(text).toContain("<<<<<<< ours\nOur plan.\n=======\nTheir plan.\n>>>>>>> theirs\n"); + expect(text).not.toContain("## Required"); + }); + + test("keeps a Related heading for quest check to report", () => { + const doc = (names: string[]) => `${plan}\n## Related\n\n${names.map((n) => item(n)).join("\n")}\n`; + const { text, conflicts } = merge(doc(["a", "b"]), doc(["b"]), doc(["a"])); + expect({ text, conflicts }).toEqual({ text: `${plan}\n## Related\n\n`, conflicts: 0 }); + expect(empty(text)).toHaveLength(1); + }); + }); }); const bin = fileURLToPath(new URL("../src/cli/bin.ts", import.meta.url)); From a7f3e3988683d6aff69cf1d3950d700a6dfbfc60 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 31 Oct 2026 08:28:56 -0700 Subject: [PATCH 3/3] fix(merge): keep a Required heading while a conflict in it remains A conflict whose ours side has no lines left no trace in the blank-body check, so the heading went even though resolving that conflict could put an entry back under it. Track every remaining conflict's position instead. Co-Authored-By: Claude Opus 5.5 --- src/core/merge.ts | 13 +++++++++---- test/resolve.test.ts | 8 ++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/core/merge.ts b/src/core/merge.ts index b96b243e..a6f8cbc0 100644 --- a/src/core/merge.ts +++ b/src/core/merge.ts @@ -130,17 +130,21 @@ export function resolveLists(path: string, merged: Chunk[]): Chunk[] { /** * `merged` without each `Required` heading whose section holds one of the - * `emptied` resolutions and otherwise only blank lines, all outside any - * conflict, along with that section's lines. As `quest rm` does, a blank line + * `emptied` resolutions and otherwise only blank lines, with no conflict left + * in it, along with that section's lines. As `quest rm` does, a blank line * left doubled by the removal goes too. */ function dropEmptyRequired(path: string, merged: Chunk[], emptied: Set): Chunk[] { // The document with every conflict taken from ours; `clean` marks lines outside a conflict. + // `at` and `open` hold where each emptied list and each remaining conflict + // sits, as the index of the line after it, since either can have no lines. const lines: string[] = []; const clean: boolean[] = []; const at: number[] = []; + const open: number[] = []; merged.forEach((chunk, index) => { if (emptied.has(index)) at.push(lines.length); + if (!Array.isArray(chunk)) open.push(lines.length); const own = Array.isArray(chunk) ? chunk : chunk.ours; lines.push(...own); clean.push(...own.map(() => Array.isArray(chunk))); @@ -157,9 +161,10 @@ function dropEmptyRequired(path: string, merged: Chunk[], emptied: Set): if (next !== undefined && !clean[next.line - 1]) return; // The section's body is the 0-based lines from `heading.line` up to `end`. const end = next === undefined ? count : next.line - 1; - if (!at.some((line) => line >= heading.line && line <= end)) return; + const inside = (line: number) => line >= heading.line && line <= end; + if (!at.some(inside) || open.some(inside)) return; const body = Array.from({ length: end - heading.line }, (_, i) => heading.line + i); - if (!body.every((i) => clean[i] && lines[i].trim() === "")) return; + if (!body.every((i) => lines[i].trim() === "")) return; dropped.add(heading.line - 1); for (const i of body) dropped.add(i); }); diff --git a/test/resolve.test.ts b/test/resolve.test.ts index 021ace61..312c75f2 100644 --- a/test/resolve.test.ts +++ b/test/resolve.test.ts @@ -231,6 +231,14 @@ describe("resolve", () => { expect(text).not.toContain("## Required"); }); + test("keeps its heading while a conflict in it remains", () => { + // Ours removing b against theirs editing it stays a conflict, even with no lines on ours. + const head = ["# [S] Quest", "", "## Goal", "", "Goal.", "", "## Required", ""]; + const kept = { ours: [], base: [item("b")], theirs: [item("b", "edited")] }; + const merged = [head, { ours: [], base: [item("a")], theirs: [] }, [""], kept, [""]]; + expect(resolveLists("quest/README.md", merged)).toEqual([head, [], [""], kept, [""]]); + }); + test("keeps a Related heading for quest check to report", () => { const doc = (names: string[]) => `${plan}\n## Related\n\n${names.map((n) => item(n)).join("\n")}\n`; const { text, conflicts } = merge(doc(["a", "b"]), doc(["b"]), doc(["a"]));