fix(magic-link): make consume work on node-redis and tolerate a null consumedAt - #30
Merged
Merged
Conversation
ExorTek
force-pushed
the
fix/magic-link-consume
branch
from
August 6, 2026 09:14
fa744ba to
4943e97
Compare
…consumedAt Two independent defects in the same script. `consume` and `revokeByEmail` passed their Lua arguments in the ioredis positional form for every client, so both failed against node-redis. The module header meanwhile claimed the store was verified against node-redis and @upstash/redis; that claim is corrected to describe what is actually covered. The script also guarded with `if record.consumedAt then`, and Redis's cjson decodes a JSON null to a truthy sentinel — so a record stored with an explicit `consumedAt: null` read as already consumed and could never be redeemed. The default `create()` path omits the field and was unaffected, but the field is part of the documented record shape. The guard now excludes cjson.null, the same way the jwt store's script already did. The fake client in the unit suite declares itself ioredis-shaped, since the store now keys on that rather than guessing.
ExorTek
force-pushed
the
fix/magic-link-consume
branch
from
August 6, 2026 09:17
4943e97 to
5a2dd7f
Compare
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.
Stacked on #29.
Two independent defects in the same script.
1. node-redis.
consumeandrevokeByEmailpassed their Lua arguments in the ioredis positional form for every client, so both failed there. The module header claimed the store was verified against node-redis and@upstash/redis; that claim is corrected to describe what is actually covered.2. Every client. The script guarded with
if record.consumedAt then, and Redis's cjson decodes a JSON null to a truthy sentinel. A record stored with an explicitconsumedAt: nulltherefore read as already consumed and could never be redeemed:create()omits the field, so the default flow was unaffected — butconsumedAtis part of the documented record shape, and normalising an absent field tonullis a common round-trip. The guard now excludescjson.null, as the jwt store's script already did.