diff --git a/assets/AGENTS.md b/assets/AGENTS.md index 50666ad9..9a0a3b5d 100644 --- a/assets/AGENTS.md +++ b/assets/AGENTS.md @@ -88,7 +88,7 @@ An epic's or milestone's `Required` also holds back every quest under it. A child that must precede a sibling is in that sibling's `Required`. Nothing else orders work within a milestone: `quest unblocked` ranks it by downstream impact, then path. When the section is empty, delete it; the quest is now unblocked. -`Required` must be acyclic. +`Required` must be acyclic, and never link to a later milestone than the document listing it: move one side with `quest mv` or remove the link. `quest check` enforces this structure. Run it after creating or updating quests. diff --git a/assets/skills/audit.md b/assets/skills/audit.md index 0d10cdb5..877e9180 100644 --- a/assets/skills/audit.md +++ b/assets/skills/audit.md @@ -10,7 +10,7 @@ The scope is every outstanding quest, or the quests and epics the argument names Look for: - Disagreements: quests whose goals or plans contradict each other, the code, or the repository's docs. - Conflicts: quests that would change the same code or interface incompatibly, or that must land in order without a `Required` link. -- Misaligned priorities: a quest placed in a milestone ahead of work it depends on, or behind work it blocks. +- Misaligned priorities: a quest whose milestone no longer matches its urgency, or that depends on later-milestone work without a `Required` link saying so (`quest check` already rejects a `Required` link to a later milestone). - Stale plans: work already done, blockers that have cleared, or references to code that no longer exists. - Obsolete quests: a problem that is no longer real, or a plan a later change or decision has made the wrong solution. diff --git a/quest/m0/milestone-order.md b/quest/m0/milestone-order.md deleted file mode 100644 index bf0abb63..00000000 --- a/quest/m0/milestone-order.md +++ /dev/null @@ -1,62 +0,0 @@ -# [S] Reject Required links to a later milestone - -## Goal - -`quest check` fails on a `Required` entry whose target sits in a later -milestone than the document listing it, such as an m1 quest requiring an m2 -one. The finding names both documents and their milestones, and says to move -the later quest or epic earlier with `quest mv`, or remove the link. A -milestone README cannot move, since milestones are permanent, so when either -side is one the finding offers only moving the quest or epic on the other -side, or removing the link. Quests, epic READMEs, and milestone READMEs are -all covered; the root README, which belongs to no milestone, and links to -other repositories are not. - -`quest unblocked` ranks a blocker by its own milestone, so an inverted link -leaves the earlier quest stalled behind all of the later milestone's work. On -2026-11-13, moq-dev/moq's main held six such links, moq.pro's one, and rind's -one. - -## Plan - -Decided 2026-11-13 (user, /quest-plan): - -- How is an inverted link prevented? - - ✅ `quest check` rejects it (recommended): the tree then states each - quest's real priority, and `/quest-audit` no longer has to spot it by eye - (rejected: ranking a blocker at its dependents' earliest milestone instead, - which hides the misplaced quest; both, which is redundant once check holds) -- Milestone? - - ✅ m0 (recommended): it fixes stalls in moq-dev's own repositories (rejected: - m1, m2) -- What about the links already on main in moq, moq.pro, and rind? - - ✅ Fix them now, in one PR per repository, before this rule ships - (rejected: list them here and leave them to each repository's pin bump): - [moq-dev/moq#8637](https://github.com/moq-dev/moq/pull/8637), the - pre-cut audit, which supersedes the closed moq-dev/moq#8645, and - [moq-dev/moq.pro#3992](https://github.com/moq-dev/moq.pro/pull/3992). -- Challenged the same day: moq splits m1 and m2 by kind, not priority, and - its 2026-11-12 audit kept additive m1 quests requiring breaking m2 work on - purpose. Should ranking instead give a blocker its dependents' earliest - milestone? - - ✅ Keep the check rule (user: "I definitely don't want earlier milestones - blocked on later milestones") (rejected: ranking inheritance instead; - both). moq moves those quests to m3. - rind's one link goes when - [kixel-ai/rind#606](https://github.com/kixel-ai/rind/pull/606) merges - and deletes the quest it points to. Before this rule ships, re-scan - each repository's main for links added since. - -Implementation guidance: - -- A document's milestone is the root child directory that holds it, ordered - as `quest unblocked` orders milestones, so `m10` follows `m9`. -- There is no `--fix`: which side moves is a priority decision for the - maintainer, as `/quest-audit` already treats moving a quest between - milestones. -- Every check finding also makes `quest unblocked` and `quest start` fail, so - a repository whose tree holds an inverted link cannot rank work until it - fixes it. That is intended; the fixes above clear moq-dev's trees first. -- Update the guide's `Required` rules in `assets/AGENTS.md` beside - "`Required` must be acyclic", and narrow `/quest-audit`'s "misaligned - priorities" item in `assets/skills/audit.md` to what check cannot see. diff --git a/src/core/rules.ts b/src/core/rules.ts index f66b2230..90034e1e 100644 --- a/src/core/rules.ts +++ b/src/core/rules.ts @@ -95,6 +95,7 @@ export function check(docs: Doc[], exists: (path: string) => boolean): Finding[] collisions(found, known, docs); membership(found, known, docs); milestones(found, docs); + inversions(found, docs); cycles(found, docs); return found.sort(compareFindings); @@ -339,6 +340,49 @@ export function finishedMilestone(docs: Doc[]): (doc: Doc, entry: Entry) => bool }; } +/** + * The milestone README of the root child directory holding `path`, or `null` + * for the root README and files directly under the root. + */ +function milestoneOf(path: string): string | null { + const parts = path.split("/"); + return parts.length >= 3 ? `${parts[0]}/${parts[1]}/README.md` : null; +} + +/** + * A `Required` link must not point to a later milestone. `quest unblocked` + * ranks a blocker by its own milestone, so an inverted link stalls the earlier + * quest behind all of the later milestone's work. Which side moves is the + * maintainer's priority call, so there is no fix. A milestone README cannot + * move, since milestones are permanent. + */ +function inversions(found: Finding[], docs: Doc[]) { + for (const doc of docs) { + const from = milestoneOf(doc.path); + if (from === null) continue; + for (const entry of entries(doc, "Required")) { + if (entry.target === null) continue; + const target = rooted(entry.target); + if (target === null) continue; + const to = milestoneOf(target); + if (to === null || comparePaths(to, from) <= 0) continue; + const [early, late] = [from.split("/")[1], to.split("/")[1]]; + const name = (path: string) => path.replace(/\/README\.md$/, ""); + const moves = [ + permanent(target) ? null : `move ${name(target)} to ${early}`, + permanent(doc.path) ? null : `move ${name(doc.path)} to ${late}`, + ].filter((move) => move !== null); + const fix = + moves.length > 0 ? `${moves.join(" or ")} with \`quest mv\`, or remove the link` : "remove the link"; + found.push({ + path: doc.path, + line: entry.line, + message: `in milestone ${early}, requires ${target} in later milestone ${late}; ${fix}`, + }); + } + } +} + /** * `Required` must be acyclic. A cycle is a set of quests none of which can ever * start, and walking the links to rule one out is exactly the manual step an diff --git a/test/tree.test.ts b/test/tree.test.ts index 290babf2..62f9e4a7 100644 --- a/test/tree.test.ts +++ b/test/tree.test.ts @@ -405,12 +405,18 @@ describe("finished milestones", () => { expect(t.ready()).toEqual([]); }); - // moq-dev/moq's m1 lists m0 quests: those blockers keep it open. + // A milestone README may list earlier quests: those blockers keep it open. test("a milestone with only blockers left is open", () => { - gated() - .append("quest/m0/README.md", "\n## Required\n\n- [Other](/quest/m1/other.md)\n") - .remove("quest/m0/epic") - .write("quest/m1/other.md", "# [S] Other\n\n## Goal\n\nOpen work m0 waits on.\n") + tree() + .write( + "quest/m1/README.md", + "# M1\n\n## Goal\n\nThe next milestone.\n\n## Required\n\n- [One](/quest/m0/epic/one.md)\n", + ) + .write("quest/m2/README.md", "# M2\n\n## Goal\n\nLater.\n") + .write( + "quest/m2/gate.md", + "# [S] Gate\n\n## Goal\n\nRuns once m1 finishes.\n\n## Required\n\n- [M1](/quest/m1/README.md)\n", + ) .accepts(); }); @@ -815,3 +821,66 @@ describe("scheduled", () => { expect(t.ready()).toEqual(["quest/m0/epic/one.md"]); }); }); + +// `quest unblocked` ranks a blocker by its own milestone, so a link to a later +// one stalls the earlier quest behind all of that milestone's work. +describe("Required links to a later milestone", () => { + const later = () => + tree() + .write("quest/m1/README.md", "# M1\n\n## Goal\n\nLater.\n") + .write("quest/m1/late.md", "# [S] Late\n\n## Goal\n\nLater work.\n"); + const REQ = "\n## Required\n\n- [Late](/quest/m1/late.md)\n"; + + test("a quest requiring a later quest", () => { + expect(later().append("quest/m0/epic/two.md", REQ).findings()).toEqual([ + "quest/m0/epic/two.md:13: in milestone m0, requires quest/m1/late.md in later milestone m1; move quest/m1/late.md to m0 or move quest/m0/epic/two.md to m1 with `quest mv`, or remove the link", + ]); + }); + + test("an epic README requiring a later quest", () => { + const findings = later().append("quest/m0/epic/README.md", REQ).findings(); + expect(findings).toHaveLength(1); + expect(findings[0]).toContain("move quest/m1/late.md to m0 or move quest/m0/epic to m1"); + }); + + test("a milestone README offers only moving the other side", () => { + const findings = later().append("quest/m0/README.md", REQ).findings(); + expect(findings).toHaveLength(1); + expect(findings[0]).toContain( + "in later milestone m1; move quest/m1/late.md to m0 with `quest mv`, or remove the link", + ); + }); + + test("a later milestone README offers only moving the requirer", () => { + const findings = later() + .append("quest/m0/epic/two.md", "\n## Required\n\n- [M1](/quest/m1/README.md)\n") + .findings(); + expect(findings[0]).toContain("move quest/m0/epic/two.md to m1 with `quest mv`, or remove the link"); + }); + + test("m10 follows m9", () => { + const t = tree() + .write("quest/m9/README.md", "# M9\n\n## Goal\n\nNine.\n") + .write("quest/m9/nine.md", "# [S] Nine\n\n## Goal\n\nNine.\n") + .write("quest/m10/README.md", "# M10\n\n## Goal\n\nTen.\n") + .write("quest/m10/a.md", "# [S] A\n\n## Goal\n\nA.\n\n## Required\n\n- [M9](/quest/m9/README.md)\n"); + t.accepts(); + t.write("quest/m10/a.md", "# [S] A\n\n## Goal\n\nA.\n").write( + "quest/m9/b.md", + "# [S] B\n\n## Goal\n\nB.\n\n## Required\n\n- [A](/quest/m10/a.md)\n", + ); + expect(t.findings()).toEqual([ + "quest/m9/b.md:9: in milestone m9, requires quest/m10/a.md in later milestone m10; move quest/m10/a.md to m9 or move quest/m9/b.md to m10 with `quest mv`, or remove the link", + ]); + }); + + test("earlier and same-milestone links pass", () => { + later() + .write("quest/m1/other.md", "# [S] Other\n\n## Goal\n\nSame milestone.\n") + .append( + "quest/m1/late.md", + "\n## Required\n\n- [One](/quest/m0/epic/one.md)\n- [Other](/quest/m1/other.md)\n", + ) + .accepts(); + }); +});