fix(core): drop ulid package; implement ULID via Web Crypto (#57)#59
Merged
Conversation
The previous fix (#58) was insufficient: importing the `ulid` package runs its module-level `export const ulid = factory()` → `detectPrng()` AT IMPORT TIME, which throws "secure crypto unusable, insecure Math.random not allowed" under crxjs MV3 service-worker bundling — before any of our code runs. So the SW kept crashing (registration failed, "Inactive") and the whole background never ran. Remove the `ulid` dependency entirely and implement newUlid() directly: 48-bit ms timestamp + 80 bits from crypto.getRandomValues, Crockford base32, 26 chars, time-sortable. crypto.getRandomValues is present in the SW, extension pages, the web UI, and Node 19+. Verified: detectPrng/detectRoot/etc. no longer appear in either shell's built bundle. docs: add a "Creating your GitHub token (PAT)" section to the root README (how to actually make the fine-grained token, not just revoke it); gitignore .env* (e2e throwaway-repo credentials); correct test counts (core 82->83, total 310->311). Closes #57 Co-Authored-By: Claude Opus 4.8 (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.
Closes #57. Supersedes the insufficient #58.
The real bug: importing the
ulidpackage runsexport const ulid = factory()at module load, which callsdetectPrng()and throws secure crypto unusable, insecure Math.random not allowed in the crxjs MV3 service-worker context — before any of our code runs. So #58 (binding a PRNG viafactory()) couldn't help: the throw fires on import. The SW stayed Inactive; the entire background (reconcile, native-tree sync, poll) never ran.Fix: remove the
uliddependency and implementnewUlid()directly — 48-bit ms timestamp + 80 bits fromcrypto.getRandomValues, Crockford base32, 26 chars, time-sortable. VerifieddetectPrng/detectRoot/etc. are gone from both shells' built bundles.Docs: added a "Creating your GitHub token (PAT)" section to the root README (how to make the fine-grained token, which was only documented in the Chrome package README); gitignored
.env*(e2e throwaway-repo creds); corrected test counts (core 82→83, total 310→311).Tests: core 83, extension-shared 119, web 109 — all pass; both shell builds clean.
🤖 Generated with Claude Code