♻️ Small auth improvements

[Friedinger]

This pull request refactors authentication logic to improve reliability and maintainability, especially around OAuth discovery and token management. The main changes include centralizing the discovery document loading logic, enhancing error handling, and optimizing React context value updates.

Authentication and OAuth Flow Improvements:

• Centralized the loading of the OAuth discovery document into a single loadDiscovery function, ensuring consistent and reliable fetching and error handling across all authentication actions. This also adds user-facing alerts when the discovery service is unavailable.
• Updated all authentication-related actions (login, refresh, logout, getValidAccessToken) to use the new loadDiscovery logic, reducing code duplication and preventing failures when the discovery document is missing. [1] [2] [3] [4] [5] [6] [7]

React Context and State Management:

• Replaced the direct object construction of the Auth context value with a useMemo hook, ensuring that consumers only re-render when relevant values actually change. [1] [2]
• Improved the type and initialization of the refreshTimeoutRef to support both browser and Node.js environments.
• Minor code cleanup, such as using optional chaining for safer property access and adding useMemo to imports. [1] [2]

These changes make the authentication flow more robust, easier to maintain, and less error-prone, especially in scenarios where the OAuth discovery document may not be immediately available.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a1c43f87-86c1-4b56-a845-509adacc5c73

📥 Commits

Reviewing files that changed from the base of the PR and between d5ba803 and f564523.

📒 Files selected for processing (2)

• ecoscan_app/context/AuthContext.tsx
• ecoscan_app/hooks/useOAuthFlow.ts

🚧 Files skipped from review as they are similar to previous changes (2)

• ecoscan_app/hooks/useOAuthFlow.ts
• ecoscan_app/context/AuthContext.tsx

---

Walkthrough

Auth discovery loading is shared across OAuth operations, with fallback loading and error handling. Auth context refreshes are deduplicated through an in-flight promise, successful refreshes trigger rescheduling, and provider values are memoized.

Changes

OAuth authentication flow

Layer / File(s)	Summary
Shared discovery resolution ecoscan_app/hooks/useOAuthFlow.ts	Discovery loading now manages state and errors, while login passes the resolved document to authorization and code exchange.
Refresh and logout discovery fallback ecoscan_app/hooks/useOAuthFlow.ts	Refresh, access-token validation, and logout load discovery when needed; logout clears tokens if loading fails.
Auth context refresh integration ecoscan_app/context/AuthContext.tsx	Refresh attempts are deduplicated, successful refreshes update tokenVersion, scheduling uses the active promise, and the provider value is memoized.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

• RubberDuckCrew/EcoScan#218: Overlaps with the token refresh, expiry decoding, and OAuth discovery handling changes.

Suggested labels: ✨ Enhancement

Poem

I’m a bunny who guards every token at night,
Loading discovery when paths aren’t in sight.
One refresh hops in, others wait in a row,
Memoized context keeps renders in flow.
Snug OAuth carrots, all ready to go!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title is related to auth changes, but it's too generic to tell the main change at a glance.	Use a more specific title that mentions the OAuth discovery loading or AuthContext memoization change.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description clearly matches the auth refactor and summarizes the main changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor/auth

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ecoscan_app/context/AuthContext.tsx`:
- Around line 128-150: Update refreshInternal in AuthContext to track the active
refresh operation with a ref holding the in-flight promise. Return that promise
when a refresh is already running, assign the promise when starting
refreshOAuth, and clear the ref in finally so concurrent callers await
completion instead of resolving immediately with stale tokens.
- Around line 138-139: Automatic token refresh stops after the first cycle
because updating tokenExpiresAtRef does not retrigger the scheduling effect. In
AuthContext’s successful refresh flow around refreshOAuth and the effect near
the authentication timeout logic, explicitly schedule the next refresh after
saveTokens completes, or make token expiry state reactive so the effect reruns;
ensure each successful refresh reliably establishes the following refresh timer.

In `@ecoscan_app/hooks/useOAuthFlow.ts`:
- Around line 80-83: Update the useCallback refresh function to throw an error
when discovery resolution fails instead of returning undefined. Ensure the
failure occurs after discovery || loadDiscovery() yields no discovery document,
allowing AuthContext.refreshInternal’s existing error path to clear expired
credentials.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9c5fcf67-1496-4d03-a6ae-677124a10705

📥 Commits

Reviewing files that changed from the base of the PR and between 8d1c265 and d5ba803.

📒 Files selected for processing (2)

• ecoscan_app/context/AuthContext.tsx
• ecoscan_app/hooks/useOAuthFlow.ts