Repository navigation
fix(state/redis): round sub-millisecond ExtendLock TTL up to 1 ms - #130
Conversation
ExtendLock with a positive TTL under 1 ms deletes the lock and still returns true. This test fails until the fix lands. Refs #125.
ExtendLock sent ttl.Milliseconds() to PEXPIRE. A positive TTL under 1 ms became 0, Redis deleted the lock, and ExtendLock still returned true. Round it up to 1 ms, as go-redis does on its SetNX path, and update the README contract. MarkEvent and AcquireLock already get this rounding from go-redis SetNX.
|
@codex review |
|
@codex security review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6f8c6829c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
A round-to-nearest fix passes the 500 µs case but still sends 0 for 1 ns. Write the extend ARGV in the README as two values.
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
Why
The operator goal is one small, reviewed PR for every open follow-up issue in coder/chat. This one fixes the Redis state:
ExtendLockwith a positive TTL under 1 ms sentPEXPIRE 0, Redis deleted the lock, andExtendLockstill returned true. The fix rounds that TTL up to 1 ms, as go-redis already does forSetNX.Closes #125
What changed
ExtendLocksends at least 1 ms to the extend script.AcquireLockfails.state/redis/README.mdnow describes the rounding instead of the bug.Scope
MarkEventandAcquireLockneed no change. They call go-redisSetNX, which rounds a positive TTL under 1 ms up to 1 ms.Blast Radius
Only callers that pass a TTL under 1 ms see a change. They now keep the lock for 1 ms instead of losing it.
Verification
redis:7.2-alpine(testcontainers, throwaway test),ExtendLockby 500 µs leftEXISTS=0before the fix. After the fix it leftEXISTS=1andPTTL=1ms, and a second token could not take the lock.go vetandgo testpass in every module,go test -racepasses instate/redis, andgofmt -lprints nothing.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high