diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 1b59f7c3c..ddfa571ac 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -6,11 +6,11 @@ ## Overview -| # | Phase | Goal | Requirements | Success Criteria | -|---|-------|------|--------------|------------------| -| 24 | Fix Recording Crash | Fix C++ client crash on Start Recording | CRASH-01, CRASH-02, CRASH-03, CRASH-04 | 4 | -| 25 | Verify Basic Controls | Verify Start/Stop/Pause work like desktop native app | CTRL-01, CTRL-02, CTRL-03 | 3 | -| 26 | Memory Leak Audit | Audit and fix memory leaks in recording modal | MEM-01, MEM-02, MEM-03 | 3 | +| # | Phase | Goal | Requirements | Plans | Success Criteria | +|---|-------|------|--------------|-------|------------------| +| 24 | Fix Recording Crash | Fix C++ client crash on Start Recording | CRASH-01, CRASH-02, CRASH-03, CRASH-04 | 1 | 4 | +| 25 | Verify Basic Controls | Verify Start/Stop/Pause work like desktop native app | CTRL-01, CTRL-02, CTRL-03 | TBD | 3 | +| 26 | Memory Leak Audit | Audit and fix memory leaks in recording modal | MEM-01, MEM-02, MEM-03 | TBD | 3 | **Total:** 3 phases | 10 requirements | 10 success criteria @@ -20,8 +20,13 @@ **Goal:** Fix C++ client crash when Start Recording is clicked +**Plans:** 1 plan + +Plans: +- [ ] 24-01-PLAN.md — Fix method names and parameters in useRecordingHelpers.js and JKSessionScreen.js + **Requirements:** -- CRASH-01: Call RegisterRecordingCallbacks before recording operations +- CRASH-01: Call RegisterRecordingCallbacks before recording operations (ALREADY DONE in JKSessionScreen.js line 500) - CRASH-02: Use correct method name StartRecording (not StartMediaRecording) - CRASH-03: Use correct method name StopRecording (not FrontStopRecording) - CRASH-04: Pass individual parameters to StartRecording (not settings object) @@ -36,8 +41,8 @@ **Key Files:** - `jam-ui/src/hooks/useRecordingHelpers.js` -- Recording modal component -- Session initialization code +- `jam-ui/src/components/client/JKSessionScreen.js` +- `jam-ui/src/components/client/JKSessionRecordingModal.js` --- @@ -45,6 +50,8 @@ **Goal:** Verify Start/Stop/Pause work like desktop native app +**Plans:** (created by /gsd:plan-phase) + **Requirements:** - CTRL-01: Start recording works - initiates recording like desktop native app - CTRL-02: Stop recording works - ends recording like desktop native app @@ -67,6 +74,8 @@ **Goal:** Audit and fix memory leaks in recording modal +**Plans:** (created by /gsd:plan-phase) + **Requirements:** - MEM-01: Audit recording modal useEffect cleanup functions - MEM-02: Audit timer/interval cleanup during recording lifecycle @@ -91,10 +100,11 @@ See: `.planning/research/RECORDING-ANALYSIS.md` Key findings: -- Missing `RegisterRecordingCallbacks` call is primary crash cause -- Wrong method names in jam-ui vs legacy -- Parameter format mismatch +- `RegisterRecordingCallbacks` is ALREADY called in JKSessionScreen.js line 500 (not a gap) +- Wrong method names in jam-ui vs legacy (`StartMediaRecording` vs `StartRecording`) +- Wrong method names (`FrontStopRecording` vs `StopRecording`) +- Parameter format mismatch (object vs unpacked params) --- *Roadmap created: 2026-02-19* -*Last updated: 2026-02-19 after initial creation* +*Last updated: 2026-02-19 after Phase 24 planning* diff --git a/.planning/phases/24-fix-recording-crash/24-01-PLAN.md b/.planning/phases/24-fix-recording-crash/24-01-PLAN.md new file mode 100644 index 000000000..8c04c7fa6 --- /dev/null +++ b/.planning/phases/24-fix-recording-crash/24-01-PLAN.md @@ -0,0 +1,185 @@ +--- +phase: 24-fix-recording-crash +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - jam-ui/src/hooks/useRecordingHelpers.js + - jam-ui/src/components/client/JKSessionScreen.js +autonomous: true + +must_haves: + truths: + - "Clicking Start Recording does not crash the C++ client" + - "Recording starts successfully after clicking Start Recording" + - "Recording stops successfully after clicking Stop Recording" + - "Recording callbacks are invoked correctly by C++ client" + artifacts: + - path: "jam-ui/src/hooks/useRecordingHelpers.js" + provides: "Recording start/stop with correct jamClient method calls" + contains: "StartRecording" + - path: "jam-ui/src/components/client/JKSessionScreen.js" + provides: "Session recording with correct method call" + contains: "StartRecording" + key_links: + - from: "jam-ui/src/hooks/useRecordingHelpers.js" + to: "jamClient.StartRecording" + via: "jamClient proxy call with unpacked parameters" + pattern: "jamClient\\.StartRecording\\(" + - from: "jam-ui/src/hooks/useRecordingHelpers.js" + to: "jamClient.StopRecording" + via: "jamClient proxy call" + pattern: "jamClient\\.StopRecording\\(" +--- + + +Fix C++ client crash when Start Recording is clicked by correcting method names and parameter format. + +Purpose: The recording feature crashes because jam-ui uses wrong method names (`StartMediaRecording` instead of `StartRecording`) and passes a settings object instead of unpacked individual parameters. This makes the C++ client unable to process the call correctly. + +Output: Working recording start/stop that matches the legacy implementation pattern. + + + +@/Users/nuwan/.claude/get-shit-done/workflows/execute-plan.md +@/Users/nuwan/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/research/RECORDING-ANALYSIS.md + +# Key source files +@jam-ui/src/hooks/useRecordingHelpers.js +@jam-ui/src/components/client/JKSessionScreen.js + +# Reference: Legacy implementation (working pattern) +@web/app/assets/javascripts/recordingModel.js (lines 99, 141) + + + + + + Task 1: Fix useRecordingHelpers.js method names and parameters + jam-ui/src/hooks/useRecordingHelpers.js + +Fix the recording method calls to match the legacy implementation: + +1. **Fix startRecording function (around line 101):** + - Change `jamClient.StartMediaRecording(recording.id, groupedTracks, recordSettings)` + - To: `jamClient.StartRecording(recording.id, groupedTracks, recordVideo, recordChat, recordFramerate)` + - Unpack `recordSettings` into individual parameters: + - `recordVideo` = `recordSettings.videoType || 0` (integer, 0 means no video) + - `recordChat` = `recordSettings.recordChat ? 1 : 0` (integer flag) + - `recordFramerate` = `0` (default, matches legacy) + - The `recordVideo` variable already exists at line 83: `const recordVideo = recordSettings.recordingType === RECORD_TYPE_BOTH;` + - This boolean needs to be converted to the videoType integer for the C++ call + +2. **Fix stopRecording function (around line 166):** + - Change `jamClient.FrontStopRecording(recording.id, groupedTracks)` + - To: `jamClient.StopRecording(recording.id, groupedTracks)` + +**Legacy signatures for reference:** +```javascript +// Start: recordingModel.js line 99 +jamClient.StartRecording(recording["id"], groupedTracks, recordVideo, recordChat, recordFramerate); + +// Stop: recordingModel.js line 141 +jamClient.StopRecording(recording.id, groupedTracks); +``` + +**Note:** Keep the existing error handling and state management logic unchanged. Only modify the jamClient method calls. + + +1. Grep for `StartMediaRecording` in useRecordingHelpers.js returns no results +2. Grep for `FrontStopRecording` in useRecordingHelpers.js returns no results +3. Grep for `StartRecording` in useRecordingHelpers.js returns the new call +4. Grep for `StopRecording` in useRecordingHelpers.js returns the new call +5. No TypeScript/ESLint errors when running `npm run lint` (if applicable) + + +- `useRecordingHelpers.js` calls `jamClient.StartRecording(id, tracks, videoType, chatFlag, framerate)` with unpacked parameters +- `useRecordingHelpers.js` calls `jamClient.StopRecording(id, tracks)` with correct method name +- No references to `StartMediaRecording` or `FrontStopRecording` in useRecordingHelpers.js + + + + + Task 2: Fix JKSessionScreen.js doStartRecording method + jam-ui/src/components/client/JKSessionScreen.js + +Fix the doStartRecording function (around line 880) to use the correct method name and parameters: + +1. **Current code (line 880):** + ```javascript + await jamClient.StartMediaRecording(currentRecordingId, groupedTracks, params); + ``` + +2. **Change to:** + ```javascript + const recordVideo = params.videoType || 0; + const recordChat = params.recordChat ? 1 : 0; + const recordFramerate = 0; + await jamClient.StartRecording(currentRecordingId, groupedTracks, recordVideo, recordChat, recordFramerate); + ``` + +**Note:** The `params` object coming into doStartRecording contains: +- `recordVideo` (boolean from modal) +- `recordChat` (boolean) +- `videoType` (integer for video source) + +Extract and convert these to match the legacy C++ client expectations. + + +1. Grep for `StartMediaRecording` in JKSessionScreen.js returns no results in doStartRecording +2. Grep for `StartRecording` in JKSessionScreen.js shows the corrected call +3. No syntax errors - file parses correctly + + +- `JKSessionScreen.js` `doStartRecording` function calls `jamClient.StartRecording` with correct parameters +- No references to `StartMediaRecording` in the recording-related code paths + + + + + + +After both tasks are complete: + +1. **Code verification:** + ```bash + # Should return 0 results for old method names in recording code + grep -n "StartMediaRecording\|FrontStopRecording" jam-ui/src/hooks/useRecordingHelpers.js + grep -n "StartMediaRecording" jam-ui/src/components/client/JKSessionScreen.js | grep doStartRecording + + # Should show new correct calls + grep -n "jamClient.StartRecording\|jamClient.StopRecording" jam-ui/src/hooks/useRecordingHelpers.js + ``` + +2. **Build verification:** + ```bash + cd jam-ui && npm run build + ``` + Build should complete without errors. + +3. **Manual testing (for human verification in Phase 25):** + - Start jam-ui with `npm run start` + - Join a session + - Open recording modal + - Click Start Recording + - Expected: No crash, recording starts + + + +1. Method names corrected: `StartRecording` and `StopRecording` used instead of `StartMediaRecording` and `FrontStopRecording` +2. Parameters unpacked: Individual params (id, tracks, videoType, chatFlag, framerate) passed instead of settings object +3. Build passes without errors +4. No regressions in existing code (state management, error handling unchanged) + + + +After completion, create `.planning/phases/24-fix-recording-crash/24-01-SUMMARY.md` +