docs(oidc): reword consumeCode docblock to lead with the lock, not Cache::pull#3666
Merged
Conversation
…che::pull The docblock opened with 'Cache::pull is get()+forget()' even though the method uses Cache::lock — in the diff it reads as if the doc describes pull while the code locks. Reworded to lead with the lock-guarded read-and-delete (what the code actually does) and mention Cache::pull only as the rejected alternative. No code change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NtfhjPwUEEHJb4Jf7714eR
gloriafolaron
requested review from
marcelfolaron
and removed request for
a team
July 19, 2026 19:00
Contributor
There was a problem hiding this comment.
Pull request overview
Doc-only follow-up to #3663 that updates the OidcMobileCode::consumeCode() docblock so it describes the current lock-guarded “read-and-delete” behavior first, and mentions Cache::pull only as the rejected alternative (due to non-atomic get+forget semantics).
Changes:
- Reworded
consumeCode()PHPDoc to lead with the non-blocking cache-lock approach. - Moved the
Cache::pullrationale into a parenthetical explanation at the end of the docblock.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+86
to
+95
| * The read-and-delete is guarded by a non-blocking cache lock keyed on the | ||
| * code: of two concurrent exchanges that both peekCode()'d it, only the lock | ||
| * holder performs the get()+forget() and can return true — so at most one | ||
| * exchange mints from a single-use code. A caller that can't take the lock (a | ||
| * concurrent consume is in flight) gets false and must not mint; so does an | ||
| * already-consumed or unknown code. | ||
| * | ||
| * (A bare Cache::pull is NOT used precisely because it is get()+forget(), not | ||
| * a single atomic op on any driver — two callers could both read the code | ||
| * before either deletes it, and both mint. The lock closes that window.) |
Copilot nit: 'peekCode()'d' → 'called peekCode() on'; 'Cache::pull' → 'Cache::pull()' for method-style consistency. Comment-only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NtfhjPwUEEHJb4Jf7714eR
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.
Doc-only follow-up to the merged #3663 (lock-guarded
consumeCode).Marcel flagged a documentation mismatch. The
OidcMobileCode::consumeCode()docblock (now on master) leads with "Cache::pullis get()+forget()…" even though the method usesCache::lock. Reading the code, the doc appears to describeCache::pullwhile the implementation locks — a perceived mismatch.Reworded so the docblock leads with what the code does — the lock-guarded read-and-delete — and mentions
Cache::pullonly as the rejected alternative at the end (the reason a lock is needed). No behavior change; comment-only.🤖 Generated with Claude Code