Destroy tiptap session on logout #185
Merged
Conversation
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
- Fix TiptapCollabProvider mock to return proper constructor function - Remove undefined mockProviders reference causing test failures - Simplify test suite while maintaining critical logout test coverage - Add logout cleanup logic to destroy collaboration providers on logout - Track login state changes to properly clean up resources - Use destroy() instead of disconnect() for complete cleanup 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Add eslint disable comment for useEffect dependency array. The clearCache function reference changing should not trigger re-clearing of the cache on login page render. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Implement three-layer functional composition (Top/Middle/Low) across all editor components - Replace type guards with simple boolean functions per TypeScript idioms - Consolidate useEffect hooks from 4 to 2 lifecycle-focused effects - Fix TypeScript ref typing consistency using RefObject<HTMLDivElement> with null assertion - Optimize editor state management with clear separation of concerns - Remove all 'any' types and implement proper TypeScript patterns 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
calebbourg
force-pushed
the
destroy_tiptap_session_on_logout
branch
from
d3e59de to
3bed072
Compare
Add a centralized registry pattern that allows components to register cleanup functions that execute during the logout sequence. This ensures reliable cleanup execution without relying on fragile state detection. Features: - Singleton registry with register/unregister capabilities - Promise-based cleanup execution with error handling - Graceful error isolation between cleanup functions 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Execute registered component cleanup functions synchronously during the logout process. This ensures all components can perform necessary cleanup (like TipTap provider destruction) before cache clearing. Changes: - Import and execute logoutCleanupRegistry.executeAll() - Add detailed logging for debugging cleanup execution - Remove async router.replace() to prevent timing issues 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Replace fragile logout state detection with direct cleanup registry integration. This fixes the race condition where user presence dots remained green after 401-triggered logout. Technical fixes: - Register TipTap cleanup function with logout registry - Clear awareness fields with null (standard TipTap pattern) - Use queueMicrotask() for proper async cleanup timing - Remove dependency on isLoggedIn state transitions - Eliminate wasLoggedInRef tracking The cleanup now executes reliably during logout, ensuring other users see presence indicators turn black immediately when someone is logged out. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Ensure SessionCleanupProvider is mounted in the component tree to register session cleanup handlers with the session guard interceptor. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Add comprehensive logging to help debug session management: - SessionCleanupProvider: Log handler registration/unregistration - SessionGuard: Add detailed 401 handling logs with context - AuthStore: Log logout state transitions These logs helped identify and fix the TipTap cleanup race condition by revealing when cleanup was being skipped due to state timing issues. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Mock SessionCleanupProvider in test setup to avoid router dependencies - Fix editor cache context test to properly simulate logout cleanup - Use act() wrapper for React state updates during testing - Update test expectations to match reset behavior 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
|
@calebbourg I encountered one more bug, but let's file it as an issue and move on. If you and I join a session, everything works as expected except when one of us uses the quick session selector component to change sessions. For the person that remains in the original session (the one who didn't switch), both presence indicators will remain green. |
jhodapp
requested changes
Sep 11, 2025
src/components/ui/coaching-sessions/coaching-notes/floating-toolbar.tsx
Outdated
Show resolved
Hide resolved
src/components/ui/coaching-sessions/coaching-notes/extensions.tsx
Outdated
Show resolved
Hide resolved
src/components/ui/coaching-sessions/coaching-notes/extensions.tsx
Outdated
Show resolved
Hide resolved
src/components/ui/coaching-sessions/coaching-notes/extensions.tsx
Outdated
Show resolved
Hide resolved
src/components/ui/coaching-sessions/coaching-notes/extensions.tsx
Outdated
Show resolved
Hide resolved
Return cleanup function from startProgressAnimation to prevent interval memory leak when loading state changes rapidly. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Combine 3 separate useEffect hooks into 1 cohesive provider lifecycle - Fix stale closure in logout cleanup by accessing current provider value - Remove type casting with any, use proper TypeScript intersection types - Reduce dependency array from 8 to 5 essential dependencies - Improve cleanup sequencing and session change handling 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Move document.title assignment directly to where title is computed - Remove onRender callback prop and handleTitleRender useCallback wrapper - Eliminate useEffect dependency on callback function - Remove unnecessary useCallback import This reduces indirection and makes the code more straightforward to follow. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Remove implementation detail leaks from console output - Streamline inline comments to focus on system architecture - Apply proper console semantics (error vs warn vs log) - Document extension composition and collaboration patterns - Maintain essential observability without exposing internals 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Document data flow through provider lifecycle and awareness system - Remove verbose logging that exposed implementation details - Fix useEffect dependency warning by including userRole - Streamline comments to focus on cache management patterns - Maintain collaboration presence tracking without debug noise 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Remove trace-level logging that exposed internal flow details - Streamline cleanup registry execution with error isolation - Apply proper error logging semantics without implementation leaks - Document cleanup orchestration patterns for maintainability - Ensure graceful component teardown during logout sequence 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Document session cleanup orchestration and provider coordination - Remove verbose session guard logging that exposed internal state - Streamline auth store operations without debug traces - Focus comments on data flow and dependency relationships - Maintain essential error reporting for production observability 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
|
@jhodapp thank you for the review! All excellent suggestions. I worked with claude-code to try to simplify the use of Here's what I used for improving docs and logging. Providing it here as I feel it worked very well. /improve --persona-architect --think-hard --c7 --focus observability, security review console logging in the coaching notes page and related code. (tiptap, etc.). We want to remove excess logging and inline comments.
requirements:
logging: we should only leave logging that allows for observability. We do not want to leak implementation details or anything that could allow people to infer implementation details. Apply proper use of semantics when logging (console.log, console.error, etc.)
inline comments: only leave comments that help readability and for understanding how data flows through the system. The comments should provide a solid description of how the various useEffects, caching, state management, and awareness/presence management works. A human should be able to read the inline documentation and reason about the system. AVOID VERBOSITY. Documentation comments should be succinct and provide just enough information to reason about the system |
- Add comprehensive Next.js navigation hook mocks (useRouter, useSearchParams, usePathname, useParams) - Mock SessionCleanupProvider from both old and new locations after provider refactoring - Import vitest lifecycle functions (beforeAll, afterEach, afterAll) to fix undefined references - Resolves 61 failing tests caused by 'invariant expected app router to be mounted' error All 105 tests now pass successfully. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
jhodapp
approved these changes
Sep 12, 2025
github-project-automation
bot
moved this from Review
to ✅ Done
in Refactor Coaching Platform
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.
Description
This PR intends to accomplish 2 things:
GitHub Issue: Fixes #192
Changes
Screenshots / Videos Showing UI Changes (if applicable)
Testing Strategy
Tested locally