Skip to content

Commit 55cc85e

Browse files
fix(prd): keep a PRD it cannot read so its number is not reissued
listPrds discarded any PRD whose readFileSync threw. That contradicts the rest of the same loop: a PRD whose front matter will not parse is kept on the slug and "?" fallbacks, which two existing tests assert. The consequence is not a missing row. nextId takes its max from this list, so dropping a PRD hands its number straight back out and two PRDs end up numbered the same, in a scheme where the number is the identity. A committed symlink whose target is not checked out is enough to trigger it: readFileSync throws ENOENT, so it arrives from a plain clone. Keep the entry on the same fallbacks the unparseable case already uses. The id stays reserved and the index row keeps its link.
1 parent fddd559 commit 55cc85e

2 files changed

Lines changed: 99 additions & 1 deletion

File tree

src/prd.mjs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,16 @@ export function listPrds(root = process.cwd()) {
229229
const t = l.match(/^title:\s*(.+)$/); if (t) title = unquoteYaml(t[1]);
230230
const s = l.match(/^status:\s*(.+)$/); if (s) status = unquoteYaml(s[1]);
231231
}
232-
} catch { continue; }
232+
// A file this cannot read is still a PRD, and its number is still taken.
233+
// Dropping it contradicts what the rest of this loop does: a PRD whose
234+
// front matter will not parse is deliberately kept on the slug/"?"
235+
// fallbacks above rather than discarded. Worse, `nextId` takes its max
236+
// from this list, so dropping one hands its number straight back out and
237+
// two PRDs end up numbered the same — in a scheme where the number is the
238+
// identity. A committed symlink whose target is not checked out is enough
239+
// to trigger it: `readFileSync` throws ENOENT. Keep the entry on the same
240+
// fallbacks; the id stays reserved and the index row keeps its link.
241+
} catch { /* unreadable: keep it with the file-name fallbacks */ }
233242
out.push({ id: m[1], slug: m[2], title, status, file: name, path: file });
234243
}
235244
return out.sort((a, b) => a.id.localeCompare(b.id));

test/prd.test.mjs

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,3 +317,92 @@ test("regenerateIndex leaves an ordinary slugified PRD link untouched", () => {
317317
fs.rmSync(root, { recursive: true, force: true });
318318
}
319319
});
320+
321+
// A PRD this cannot read is still a PRD, and its number is still taken. The
322+
// loop already keeps a PRD whose front matter will not parse, on the slug and
323+
// "?" fallbacks; a file that throws on read was the one case it discarded.
324+
// A committed symlink whose target is not checked out is enough to hit it —
325+
// git stores symlinks, so this arrives from a plain clone, not a corrupt disk.
326+
function dropDanglingPrd(root, name) {
327+
fs.symlinkSync("./target-not-checked-out.md", path.join(root, "prd", name));
328+
}
329+
330+
test("listPrds keeps a PRD it cannot read instead of dropping it", () => {
331+
const root = fs.mkdtempSync(path.join(os.tmpdir(), "moshcode-prd-"));
332+
try {
333+
createPrd("improve search ranking", root);
334+
dropDanglingPrd(root, "0002-search-ranking-v2.md");
335+
336+
const ids = listPrds(root).map((p) => p.id);
337+
assert.deepEqual(ids, ["0001", "0002"], "the unreadable PRD must still be listed");
338+
const prd = listPrds(root).find((p) => p.id === "0002");
339+
// Same fallbacks the no-front-matter case already uses.
340+
assert.equal(prd.title, "search-ranking-v2");
341+
assert.equal(prd.status, "?");
342+
} finally {
343+
fs.rmSync(root, { recursive: true, force: true });
344+
}
345+
});
346+
347+
test("createPrd does not reissue a number an unreadable PRD already holds", () => {
348+
const root = fs.mkdtempSync(path.join(os.tmpdir(), "moshcode-prd-"));
349+
try {
350+
createPrd("improve search ranking", root);
351+
dropDanglingPrd(root, "0002-search-ranking-v2.md");
352+
353+
createPrd("add a dark mode toggle", root);
354+
355+
const numbered = fs.readdirSync(path.join(root, "prd"))
356+
.filter((n) => /^\d{4}-/.test(n) && !n.startsWith("0000"))
357+
.map((n) => n.slice(0, 4))
358+
.sort();
359+
assert.deepEqual(
360+
numbered,
361+
["0001", "0002", "0003"],
362+
`two PRDs must never share a number: ${numbered.join(", ")}`,
363+
);
364+
} finally {
365+
fs.rmSync(root, { recursive: true, force: true });
366+
}
367+
});
368+
369+
test("regenerateIndex keeps a row for a PRD it cannot read", () => {
370+
const root = fs.mkdtempSync(path.join(os.tmpdir(), "moshcode-prd-"));
371+
try {
372+
createPrd("improve search ranking", root);
373+
dropDanglingPrd(root, "0002-search-ranking-v2.md");
374+
regenerateIndex(root);
375+
376+
assert.equal(
377+
indexRow(root, "0002"),
378+
"| [0002](0002-search-ranking-v2.md) | search-ranking-v2 | ? |",
379+
);
380+
} finally {
381+
fs.rmSync(root, { recursive: true, force: true });
382+
}
383+
});
384+
385+
// Control: a tree where every PRD reads cleanly must come out byte-identical,
386+
// so the fix cannot be read as churning existing indexes or listings.
387+
test("listPrds is unchanged when every PRD is readable", () => {
388+
const root = fs.mkdtempSync(path.join(os.tmpdir(), "moshcode-prd-"));
389+
try {
390+
createPrd("improve search ranking", root);
391+
dropPrd(root, "0002-import-notes.md", "Import notes");
392+
regenerateIndex(root);
393+
394+
assert.deepEqual(
395+
listPrds(root).map((p) => `${p.id}|${p.title}|${p.status}|${p.file}`),
396+
[
397+
"0001|Improve search ranking|Draft|0001-improve-search-ranking.md",
398+
"0002|Import notes|Review|0002-import-notes.md",
399+
],
400+
);
401+
assert.equal(
402+
indexRow(root, "Import notes"),
403+
"| [0002](0002-import-notes.md) | Import notes | Review |",
404+
);
405+
} finally {
406+
fs.rmSync(root, { recursive: true, force: true });
407+
}
408+
});

0 commit comments

Comments
 (0)