---
phase: 22-session-screen-fixes
plan: 01
type: execute
wave: 1
depends_on: []
files_modified:
  - jam-ui/src/components/client/JKSessionScreen.js
autonomous: true

must_haves:
  truths:
    - "Session screen cleanup runs on component unmount regardless of leave handler execution"
    - "Callback registration uses refs to ensure cleanup always has access to current callbacks"
    - "Session can be exited via any path (leave button, navigation, browser close) without leaking callbacks"
  artifacts:
    - path: "jam-ui/src/components/client/JKSessionScreen.js"
      provides: "Improved callback cleanup pattern with useRef"
      contains: "registeredCallbacksRef"
  key_links:
    - from: "JKSessionScreen cleanup useEffect"
      to: "unregisterMessageCallback"
      via: "registeredCallbacksRef.current"
      pattern: "registeredCallbacksRef\\.current\\.forEach"
---

<objective>
Harden session screen callback cleanup to ensure cleanup runs reliably on all exit paths

Purpose: Defensive improvement to SESS-01 finding - ensure callback unregistration works even if normal leave flow doesn't execute (browser crash, navigation away, etc.)

Output: JKSessionScreen.js with improved cleanup reliability using useRef pattern

**Note on SESS-02 and SESS-03:** Phase 19 audit verified these requirements already satisfied:
- SESS-02: useJamServer.js - All timers properly cleaned up (no issues found)
- SESS-03: useSessionStats.js, useSessionModel.js - All timers properly cleaned up (no issues found)

Only SESS-01 requires code changes. This plan addresses SESS-01 exclusively.
</objective>

<execution_context>
@/Users/nuwan/.claude/get-shit-done/workflows/execute-plan.md
@/Users/nuwan/.claude/get-shit-done/templates/summary.md
</execution_context>

<context>
@.planning/PROJECT.md
@.planning/ROADMAP.md
@.planning/phases/19-audit-and-discovery/19-01-SUMMARY.md

Source files:
@jam-ui/src/components/client/JKSessionScreen.js
</context>

<tasks>

<task type="auto">
  <name>Task 1: Improve callback cleanup reliability with useRef</name>
  <files>jam-ui/src/components/client/JKSessionScreen.js</files>
  <action>
The current implementation stores registered callbacks in useState, but the cleanup useEffect can have stale closure issues if the state was already cleared. The fix uses useRef to maintain a stable reference that cleanup can always access.

**Changes to make:**

1. Add a useRef to store callbacks (around line 293, after the useState declaration):
```javascript
const [registeredCallbacks, setRegisteredCallbacks] = useState([]);
const registeredCallbacksRef = useRef([]);
```

2. Update the setRegisteredCallbacks call (around line 629) to also update the ref:
```javascript
// Store registered callbacks for cleanup
setRegisteredCallbacks(callbacksToRegister);
registeredCallbacksRef.current = callbacksToRegister;
```

3. Update the unregisterMessageCallbacks function (around lines 296-301) to use the ref for cleanup reliability:
```javascript
const unregisterMessageCallbacks = useCallback(() => {
  // Use ref for cleanup to avoid stale closure issues
  const callbacks = registeredCallbacksRef.current;
  callbacks.forEach(({ type, callback }) => {
    unregisterMessageCallback(type, callback);
  });
  registeredCallbacksRef.current = [];
  setRegisteredCallbacks([]);
}, [unregisterMessageCallback]);
```

Note: Remove `registeredCallbacks` from the useCallback dependency array since we're now using the ref.

**Why this works:**
- useRef maintains a stable reference across renders
- The cleanup function in useEffect will always have access to the current callbacks via ref
- This ensures cleanup runs correctly even if the component unmounts unexpectedly
  </action>
  <verify>
1. Run `npm run lint` in jam-ui directory - should pass (or have no new lint errors)
2. Run `cd jam-ui && npm run build` - builds successfully
3. Verify the code changes match the pattern described:
   - registeredCallbacksRef declared alongside registeredCallbacks state
   - unregisterMessageCallbacks uses ref instead of state for cleanup
   - Callback registration stores to both state and ref
4. Run existing tests: `cd jam-ui && npm test -- --grep "session"` (if any session tests exist)
5. Trace through exit path logic to confirm cleanup works:
   - Normal leave flow (Leave button): handleLeaveSubmit → unregisterMessageCallbacks() → ref cleared, state cleared
   - Navigation away (browser back, route change): unmount → useEffect cleanup → unregisterMessageCallbacks() → ref has callbacks → callbacks unregistered
   - Browser close/crash: Connection closes → server handles orphaned connections (out of scope for frontend)
  </verify>
  <done>
- registeredCallbacksRef declared alongside registeredCallbacks state
- unregisterMessageCallbacks uses ref instead of state for cleanup
- Callback registration stores to both state and ref
- Cleanup will work reliably on all exit paths
- Code builds and passes lint
- Existing tests continue to pass
  </done>
</task>

</tasks>

<verification>
After completing the task:

1. **Lint check:** `cd jam-ui && npm run lint` - no new errors
2. **Build check:** `cd jam-ui && npm run build` - builds successfully
3. **Code review:** Verify useRef pattern is correctly implemented
4. **Test execution:** Run any existing session-related tests

Manual verification (optional, for UAT):
- Open session, then navigate away using browser back button
- Check console for cleanup logs (if debug logging is present)
- No errors should appear in console
</verification>

<success_criteria>
1. registeredCallbacksRef is used alongside registeredCallbacks state
2. unregisterMessageCallbacks reads from ref (not state) for cleanup
3. Callback registration stores to both state and ref
4. Code builds and passes lint
5. Existing tests continue to pass
</success_criteria>

<output>
After completion, create `.planning/phases/22-session-screen-fixes/22-01-SUMMARY.md`
</output>
