Skip to content

refactor: effect cleanup for timeouts#1823

Open
mfortman11 wants to merge 1 commit into
mainfrom
effect-cleanups
Open

refactor: effect cleanup for timeouts#1823
mfortman11 wants to merge 1 commit into
mainfrom
effect-cleanups

Conversation

@mfortman11

@mfortman11 mfortman11 commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

setTimeout /listeners in useEffect with no cleanup → memory leaks and late callbacks. chat/page.tsx (×2), auth/callback/page.tsx, onboarding, agent-settings-section.tsx.

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Improved timeout handling in authentication, chat, onboarding, and settings to prevent stale redirects and ensure delayed operations complete correctly and reliably.

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c496df33-9fae-4fe7-bc4f-1a37b36ec4f7

📥 Commits

Reviewing files that changed from the base of the PR and between 8530ab0 and 2c73c0e.

📒 Files selected for processing (5)
  • frontend/app/auth/callback/page.tsx
  • frontend/app/chat/page.tsx
  • frontend/app/onboarding/_components/onboarding-card.tsx
  • frontend/app/onboarding/_components/onboarding-upload.tsx
  • frontend/app/settings/_components/agent-settings-section.tsx

Walkthrough

Five frontend React components add proper setTimeout lifecycle management via useEffect cleanup functions. The OAuth callback, chat input focus, onboarding card completion, onboarding upload completion, and agent settings LLM selector effects now store timeout IDs and clear them on unmount or dependency change to prevent stale callbacks.

Changes

React setTimeout cleanup lifecycle management

Layer / File(s) Summary
OAuth callback redirect timeout cleanup
frontend/app/auth/callback/page.tsx
OAuth callback page introduces redirectTimeoutId, assigns both the app-auth and connector-auth redirect setTimeout calls to it, and returns a cleanup function that clears the timeout on effect unmount.
Chat input focus timeout cleanup
frontend/app/chat/page.tsx
Chat page updates the conversation-loading and new-conversation useEffect blocks to track focusTimeoutId and return cleanup functions that clear the focus timeout when the effects re-run or the component unmounts.
Onboarding card completion timeout cleanup
frontend/app/onboarding/_components/onboarding-card.tsx
OnboardingCard's task-monitoring useEffect stores the completion setTimeout in completeTimeoutId and clears it in the cleanup return to prevent stale completion callbacks.
Onboarding upload completion timeout cleanup
frontend/app/onboarding/_components/onboarding-upload.tsx
OnboardingUpload's upload-completion effect introduces completeTimeoutId, assigns the setTimeout in both the filter-creation and no-filter paths, and clears the timeout via cleanup to prevent onComplete from firing after dependency changes.
Agent settings LLM selector timeout cleanup
frontend/app/settings/_components/agent-settings-section.tsx
AgentSettingsSection's focusLlmModel effect captures the setTimeout id and returns a cleanup function to clear the timeout, preventing stale state updates on effect re-run or unmount.

🎯 2 (Simple) | ⏱️ ~12 minutes

refactor

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'refactor: effect cleanup for timeouts' clearly and concisely summarizes the main change across all modified files, which is adding proper cleanup functions for setTimeout calls in useEffect hooks.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 effect-cleanups

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


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

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant