diff --git a/quest/m0/README.md b/quest/m0/README.md index d15661d1..94cb18d3 100644 --- a/quest/m0/README.md +++ b/quest/m0/README.md @@ -86,7 +86,6 @@ ownership-manifest design: - [Merge through a host gate](/quest/m0/host-land/README.md) - the host's fixer containers, and the frozen batched gate for the repositories not yet switched - [The land fixer resolves every mechanical failure](/quest/m0/land-fixer/README.md) - lockfile merges, protected edits as ready commits, workflow pushes, and the PR's own CI as context - [Landing priority](/quest/m0/land-priority.md) - cheap checks wait a bounded time and the land daemon's check runs ahead of agent builds -- [Drop emptied Required headings](/quest/m0/required-heading.md) - a clean driver merge that empties a `Required` list passes `quest check` - [Current landing status](/quest/m0/status-landing.md) - `quest status` drops a stale `landing-stopped` line and ignores superseded cancelled checks - [Kix decision UI](/quest/m0/kix/README.md) - planning and implementation through prompts and summaries, run on the maintainer's own runners - [Codex cloud off](/quest/m0/codex-cloud-off.md) - automatic Codex cloud reviews stop once no open PR relies on them diff --git a/quest/m0/required-heading.md b/quest/m0/required-heading.md deleted file mode 100644 index 8e01ac3a..00000000 --- a/quest/m0/required-heading.md +++ /dev/null @@ -1,21 +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. diff --git a/src/core/merge.ts b/src/core/merge.ts index 17134b72..a6f8cbc0 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,85 @@ 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, 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))); + }); + 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; + 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) => 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 +236,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 fff79fb0..0d1e1395 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,52 @@ 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 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"])); + 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));