fix(prd): keep a PRD it cannot read so its number is not reissued - #266
Merged
ralyodio merged 1 commit intoAug 3, 2026
Merged
Conversation
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.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
listPrdsdropped any PRD whosereadFileSyncthrew:That contradicts what the rest of the same loop does. A PRD whose front matter will not parse is deliberately kept, on the slug and
"?"fallbacks, and two existing tests assert exactly that ("listPrds does not take a title or status from a PRD with no front matter"). A file that throws on read was the one case that got discarded instead.Why it matters
The visible symptom is a missing row, but that is not the real cost.
nextIdtakes 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. Git stores symlinks, so this arrives from a plain clone rather than a corrupt disk.
readFileSyncthrowsENOENT.Reproduced on
mainbefore touching anything, with0001-first.mdand a dangling0002-search-ranking.md:And what the user actually sees from the real CLI, before:
One
0002listed, two0002files on disk, exit 0, no warning. After:The fix
Keep the entry on the same fallbacks the unparseable case already uses. One line of behaviour: the
continuebecomes a no-op catch. The id stays reserved and the index row keeps its link.Tests
Three regression tests and one control, all through the existing
createPrd/listPrds/regenerateIndexexports:listPrds keeps a PRD it cannot read instead of dropping itcreatePrd does not reissue a number an unreadable PRD already holds— the headlineregenerateIndex keeps a row for a PRD it cannot readlistPrds is unchanged when every PRD is readable— control, asserts the listing and the index row are byte-identical on a clean tree, so this cannot churn existing indexesStashing only
src/prd.mjsand rerunning: exactly the three bug tests fail, the control passes both ways.Full suite green on
fddd559(v0.18.0): 1141 tests, 928 pass, 0 fail, 213 skipped. Baseline before the change was 1137/924/0/213, so it is +4 tests and +4 passes with nothing else moving.Notes
npm install, and the generatedpackage-lock.jsonwas deleted rather than committed, since this is a pnpm repo./tmp/prdrepro) were cleaned up.