Fail on duplicate incident ids, and give contributors a way to allocate one - #124
Open
emmanuelgjr wants to merge 1 commit into
Open
emmanuelgjr wants to merge 1 commit into
emmanuelgjr wants to merge 1 commit into
Conversation
…te one INC-132 was allocated twice - by #117 and by #109 - because both read the end of data/incidents.json while the other was open, and the second one only found out when the merge conflicted. A duplicate id also silently breaks anything that resolves an incident by id: the webapp deep link, the evidence join, the reports. validate.js gains checkIncidentIds(), which fails on a duplicate and names it. scripts/next-incident-id.mjs prints a free id; --check-prs also accounts for ids claimed by open pull requests, which is what would have caught this one - it currently reports INC-136, because INC-135 is claimed by open PR #122. The duplicate case is deliberately not tested by mutating data/incidents.json: node --test runs suites in parallel, so writing to the shared corpus races the suites reading it, which is a bug this repository has already had. The guard is covered by a corpus-uniqueness test and a wiring test instead, and was negative-tested by hand: injecting a duplicate made validate.js exit 1 with "INC-006 is used by 2 records". Baseline before: 0 errors, 88 warnings, 327 passed; 85/85 tests. Baseline after : 0 errors, 88 warnings, 328 passed; 89/89 tests, twice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Item 4, second half. Do not merge yet.
Why
INC-132was allocated twice — by #117 and by #109 — because both read the end ofdata/incidents.jsonwhile the other was open. The second one found out when the merge conflicted. Beyond the conflict, a duplicate id silently breaks anything that resolves an incident by id: the webapp deep link, the evidence join, the reports.What
validate.js→checkIncidentIds()— fails on a duplicate and names it. Also reports unused numbers, since a gap plus a duplicate is how a renumbering goes wrong.scripts/next-incident-id.mjs— prints a free id.--check-prsalso accounts for ids claimed by open pull requests, which is exactly what would have prevented this collision:That is the id I have asked #109 to renumber to.
A deliberate testing choice
The duplicate case is not tested by mutating
data/incidents.json.node --testruns suites in parallel, so writing to the shared corpus races the suites reading it — a bug this repository has already had once. I wrote that test first, watched it take 5 unrelated suites down with it, and replaced it.Covered instead by: a corpus-uniqueness test, a wiring test (the guard exists, is called, and still fails on a duplicate), and a by-hand negative test when it was written — injecting a duplicate made
validate.jsexit 1 withINC-006 is used by 2 records.Verification
validate.jsnpm run test:scripts🤖 Generated with Claude Code