Fix GitHub Verification Stuck in “Pending” State (#223) #224
Conversation
📝 WalkthroughWalkthroughAuth verification now uses timezone-aware UTC datetimes and calls token cleanup before creating sessions; the Discord cog checks existing verification token expirations (normalizing to UTC) and reuses pending messages if tokens remain valid, otherwise it creates new verification sessions. Changes
Sequence Diagram(s)mermaid mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-06-28T14:44:34.399ZApplied to files:
🧬 Code graph analysis (1)backend/app/services/auth/verification.py (2)
🔇 Additional comments (5)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @backend/integrations/discord/cogs.py:
- Around line 133-162: Update the code that sets verification_token_expires_at
to use an aware UTC timestamp: replace datetime.now() with
datetime.now(timezone.utc) when computing the expiry (the assignment in the
verification token creation code where verification_token_expires_at is set).
Keep storing the ISO string as-is so cogs.py can parse it; no changes needed to
the expiry check in the Discord cog.
🧹 Nitpick comments (1)
backend/app/services/auth/verification.py (1)
39-39: Consider performance impact of cleanup on every verification creation.Calling
cleanup_expired_tokens()before every verification session creation ensures immediate removal of expired tokens (fixing the stuck pending issue), but it executes a database query on each/verify_githubinvocation. Since a periodic cleanup task already runs every 5 minutes (indiscord/cogs.py), this could be redundant in most cases.The current approach prioritizes correctness over performance, which may be acceptable depending on verification request volume. However, if you experience high traffic, consider:
- Adding a simple in-memory timestamp check to skip cleanup if it ran recently (e.g., within last 30 seconds)
- Relying solely on the periodic task and accepting a small window where expired tokens exist
Details
♻️ Optional: Add rate-limiting for cleanup calls+_last_cleanup_time = None +CLEANUP_COOLDOWN_SECONDS = 30 + async def create_verification_session(discord_id: str) -> Optional[str]: """ Create a verification session with expiry and return session ID. """ supabase = get_supabase_client() + global _last_cleanup_time + now = datetime.now() + if _last_cleanup_time is None or (now - _last_cleanup_time).total_seconds() > CLEANUP_COOLDOWN_SECONDS: + await cleanup_expired_tokens() + _last_cleanup_time = now - await cleanup_expired_tokens() _cleanup_expired_sessions()
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/app/services/auth/verification.pybackend/integrations/discord/cogs.py
🧰 Additional context used
🧬 Code graph analysis (1)
backend/app/services/auth/verification.py (1)
backend/integrations/discord/cogs.py (1)
cleanup_expired_tokens(39-46)
🔇 Additional comments (1)
backend/integrations/discord/cogs.py (1)
6-6: LGTM: Necessary imports for timezone-aware datetime handling.The
datetimeandtimezoneimports are correctly added to support the expiry-aware verification logic below.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/app/services/auth/verification.py (1)
114-123: Inconsistent timezone usage:updated_atis missingtimezone.utc.Line 122 uses
datetime.now().isoformat()while line 119 usesdatetime.now(timezone.utc).isoformat(). This inconsistency within the sameupdate_datadict will causeverified_atto be stored as UTC-aware butupdated_atas local/naive time, leading to potential timestamp comparison issues in the database.🐛 Proposed fix
update_data = { "github_id": github_id, "github_username": github_username, "email": user_to_verify.get('email') or email, "is_verified": True, "verified_at": datetime.now(timezone.utc).isoformat(), "verification_token": None, "verification_token_expires_at": None, - "updated_at": datetime.now().isoformat() + "updated_at": datetime.now(timezone.utc).isoformat() }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/app/services/auth/verification.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-28T14:44:34.399Z
Learnt from: smokeyScraper
Repo: AOSSIE-Org/Devr.AI PR: 85
File: backend/app/services/auth/management.py:83-83
Timestamp: 2025-06-28T14:44:34.399Z
Learning: In the backend/app/services/auth/management.py file, the team prefers to use datetime.now() (local timezone/IST) during development and testing for easier debugging, with plans to change to UTC timezone-aware datetime (datetime.now(timezone.utc)) during deployment.
Applied to files:
backend/app/services/auth/verification.py
🧬 Code graph analysis (1)
backend/app/services/auth/verification.py (2)
backend/integrations/discord/cogs.py (1)
cleanup_expired_tokens(39-46)frontend/src/lib/supabaseClient.ts (1)
supabase(12-12)
🔇 Additional comments (5)
backend/app/services/auth/verification.py (5)
2-2: LGTM!The addition of
timezoneimport is correct for enabling UTC-aware datetime operations throughout the module.
15-31: LGTM!The UTC-aware timestamp comparison is correct and consistent with how
expiry_timeis set increate_verification_session.
33-62: Core fix implemented correctly.The addition of
await cleanup_expired_tokens()at line 39 before creating a new session ensures that expired verification tokens are cleared from the database, allowing users to re-initiate verification. This directly addresses the "stuck in pending state" issue described in the PR objectives.
138-155: LGTM!The cleanup function correctly uses UTC-aware timestamps for both the expiry comparison and the
updated_atfield update, maintaining consistency.
157-177: LGTM!UTC-aware timestamps are used consistently for expiry checks and time remaining calculations, which correctly compares against the UTC-aware
expiry_timestored in the session.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
1 participant
Closes # 223
📝 Description
This PR fixes an issue where users get stuck in a “Verification Pending” state when running /verify_github, even after the verification token has expired.
The fix ensures :
No existing verification logic or flow has been changed — only missing safety checks were added.
🔧 Changes Made
CHANGE 1 : Force DB cleanup before creating a verification session
File :
backend/app/services/auth/verification.py: Before creating a new verification session, we now explicitly clean expired tokens from Supabase usingawait cleanup_expired_tokens(), increate_verification_session()function.CHANGE 2 : Make
/verify_githubexpiry-awareFile :
backend/integrations/discord/cogs.py: Token expiry is checked before showing “Verification Pending”✅ Checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.