fix(sight): replace hand-rolled new_id with uuid v4#830
Open
jfeng18 wants to merge 1 commit into
Open
Conversation
The previous new_id() used nanosecond timestamp XOR stack address, providing zero actual entropy. The doc comment claimed "UUID v4 hex" but the implementation was not UUID v4. The uniqueness test never asserted id1 != id2. Replace with uuid::Uuid::new_v4() (already a transitive dep via moka/debugid/llm-tokenizer) and add assert_ne! to the test. Co-Authored-By: Claude Opus 4.6 (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.
Summary
new_id()(timestamp XOR stack address, zero entropy) withuuid::Uuid::new_v4()(122-bit CSPRNG)assert_ne!(id1, id2)totest_new_id_uniqueness(previously only checked format, never asserted uniqueness)rustfmtoninterruption/types.rsDetails
The previous
new_id()generated 32-char hex IDs using nanosecond timestamp XOR'd with a stack variable address. This provided zero actual entropy — the stack address is deterministic within the same thread/call depth, and the second half was the raw timestamp. The doc comment onInterruptionEvent.interruption_idalready claimed "UUID v4 hex, 32 chars" but the implementation was not UUID v4.uuidis already a transitive dependency (via moka, debugid, llm-tokenizer) so this adds zero compile cost — only activates thev4feature for CSPRNG-based ID generation.Test plan
test_new_id_uniquenessnow assertsid1 != id2rustfmtclean oninterruption/types.rs🤖 Generated with Claude Code