diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 9ed0f248a..e38dc7473 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -279,7 +279,7 @@ Plans: 6. User joining session after upload sees attachment in chat history Plans: -- [ ] 15-01: WebSocket broadcast and attachment deduplication +- [ ] 15-01-PLAN.md — Verify real-time sync and create integration tests #### Phase 16: Attachment Finalization **Goal**: Complete attachment feature with comprehensive error handling, edge cases, and UAT validation diff --git a/.planning/phases/15-real-time-synchronization/15-01-PLAN.md b/.planning/phases/15-real-time-synchronization/15-01-PLAN.md new file mode 100644 index 000000000..ffba3566b --- /dev/null +++ b/.planning/phases/15-real-time-synchronization/15-01-PLAN.md @@ -0,0 +1,493 @@ +--- +phase: 15-real-time-synchronization +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - jam-ui/src/components/client/JKSessionScreen.js # Minor: remove debug console.log + - test/attachments/real-time-sync.spec.ts # New: integration tests +autonomous: false + +must_haves: + truths: + - "User uploads file -> WebSocket message broadcasts to all session participants" + - "Attachment appears immediately in all users' chat windows (no manual refresh)" + - "WebSocket message includes: session_id (added by frontend), file_name, user_name, timestamp" + - "Deduplication by msg_id prevents duplicate messages" + - "Multiple users in same session all see attachment in real-time" + - "User joining session after upload sees attachment in chat history" + artifacts: + - path: "jam-ui/src/components/client/JKSessionScreen.js" + provides: "WebSocket CHAT_MESSAGE handler with attachment field extraction" + contains: "attachmentId: payload.attachment_id" + - path: "jam-ui/src/store/features/sessionChatSlice.js" + provides: "Message deduplication by msg_id" + contains: "some(m => m.id === message.id)" + - path: "test/attachments/real-time-sync.spec.ts" + provides: "Integration tests for multi-user real-time sync" + min_lines: 50 + key_links: + - from: "JKSessionScreen.js handleChatMessage" + to: "sessionChatSlice addMessageFromWebSocket" + via: "dispatch(addMessageFromWebSocket(message))" + pattern: "dispatch\\(addMessageFromWebSocket" + - from: "sessionChatSlice addMessageFromWebSocket" + to: "messagesByChannel state" + via: "Deduplication check before push" + pattern: "some\\(m => m\\.id === message\\.id\\)" +--- + + +Verify and test real-time attachment synchronization across session participants + +Purpose: Confirm that the WebSocket broadcast infrastructure built in Phases 12-14 correctly delivers attachment messages to all session participants in real-time, with proper deduplication. + +Output: +- Integration tests validating multi-user real-time sync behavior +- Cleaned up debug logging from JKSessionScreen +- Verification that existing infrastructure meets Phase 15 success criteria + + + +@/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/phases/14-chat-integration-and-display/14-03-SUMMARY.md + +# Implementation files to verify +@jam-ui/src/components/client/JKSessionScreen.js +@jam-ui/src/store/features/sessionChatSlice.js +@jam-ui/src/components/client/chat/JKChatMessage.js + + + +## Infrastructure Analysis + +Phase 12-14 built the following real-time synchronization infrastructure: + +### Backend Broadcast (Already Working) +- `api_music_notations_controller.rb` line 52: Creates ChatMessage after upload +- `chat_message.rb` line 186: `server_publish_to_session()` broadcasts to all participants +- ChatMessage includes: sender_name, sender_id, msg, msg_id, created_at, channel, purpose, attachment_id, attachment_type, attachment_name + +### Frontend WebSocket Handler (Already Working) +- `JKSessionScreen.js` lines 575-596: handleChatMessage transforms payload +- Extracts: attachmentId, attachmentName, attachmentType, purpose, attachmentSize +- Adds: sessionId from currentSession.id (not in WebSocket payload) +- Dispatches: addMessageFromWebSocket(message) + +### Deduplication (Already Working) +- `sessionChatSlice.js` line 182: `const exists = state.messagesByChannel[channel].some(m => m.id === message.id)` +- Prevents duplicate messages when same msg_id arrives multiple times + +### REST API History (Already Working - Phase 14-03) +- `sessionChatSlice.js` lines 320-326: fetchChatHistory transforms music_notation to flat fields +- Users joining after upload see attachments in chat history + +## Success Criteria Analysis + +| Criteria | Status | Evidence | +|----------|--------|----------| +| WebSocket broadcasts to all participants | WORKING | server_publish_to_session() | +| Attachment appears immediately | WORKING | handleChatMessage + addMessageFromWebSocket | +| WebSocket includes required fields | PARTIAL | session_id added by frontend, size NOT in proto | +| Deduplication prevents duplicates | WORKING | some(m => m.id === message.id) | +| Multiple users see in real-time | WORKING | Broadcast to session | +| Chat history includes attachments | WORKING | Phase 14-03 fixed REST API | + +## Note on Missing Fields + +The success criteria mentions `file_url` and `size`: +- `file_url`: Intentionally NOT in WebSocket - URLs fetched on-demand via getMusicNotationUrl() because S3 signed URLs expire after 120 seconds +- `size`: NOT in Protocol Buffer definition - would require backend changes which are out of scope +- Frontend handles gracefully: formatFileSize() accepts null, displays "Unknown size" or omits + +This phase focuses on VERIFICATION and TESTING of existing infrastructure. + + + + + + Task 1: Verify WebSocket Handler Implementation + jam-ui/src/components/client/JKSessionScreen.js + +Verify the existing WebSocket handler correctly processes attachment messages: + +1. Read JKSessionScreen.js and locate handleChatMessage (around lines 575-596) +2. Confirm these fields are extracted from payload: + - attachmentId (from payload.attachment_id) + - attachmentName (from payload.attachment_name) + - attachmentType (from payload.attachment_type) + - purpose (from payload.purpose) + - attachmentSize (from payload.attachment_size - may be null/undefined) +3. Confirm sessionId is added from currentSession.id +4. Remove the debug console.log statements (lines 576, 594) - they were added during development +5. Verify the CHAT_MESSAGE callback is registered (around line 604) + +Changes to make: +- Remove: `console.log('[CHAT DEBUG] Received WebSocket message:', payload);` +- Remove: `console.log('[CHAT DEBUG] Dispatching to Redux:', message);` +- Keep: `console.log('[CHAT DEBUG] Registered CHAT_MESSAGE callback...');` only if helpful for debugging + +Keep the comment at line 610 but remove debug logging that clutters production. + + +Run: `grep -n "CHAT DEBUG" jam-ui/src/components/client/JKSessionScreen.js | wc -l` +Should show 1 or fewer debug logs (the registration log is acceptable) + + WebSocket handler verified, debug logging cleaned up, attachment field extraction confirmed + + + + Task 2: Verify Deduplication Logic + jam-ui/src/store/features/sessionChatSlice.js + +Verify the existing deduplication logic in sessionChatSlice.js: + +1. Read sessionChatSlice.js and locate addMessageFromWebSocket reducer (around line 172) +2. Confirm deduplication check exists: + ```javascript + const exists = state.messagesByChannel[channel].some(m => m.id === message.id); + if (exists) return; + ``` +3. Verify this prevents duplicates when: + - Same message arrives via WebSocket twice (network retry) + - User already has message from REST API (page refresh scenario) + +4. Read fetchChatHistory.fulfilled (around line 300) and confirm it also deduplicates: + ```javascript + const existingIds = new Set(state.messagesByChannel[channel].map(m => m.id)); + const newMessages = transformedMessages.filter(m => !existingIds.has(m.id)); + ``` + +5. Confirm both WebSocket and REST API paths use the same message.id field for deduplication + +No changes needed - this is verification only. + + +Run: `npm run test:unit -- sessionChatSlice --testNamePattern="deduplicate"` in jam-ui +Should show passing deduplication tests + + Deduplication logic verified for both WebSocket and REST API paths + + + + Task 3: Create Real-time Sync Integration Tests + jam-ui/test/attachments/real-time-sync.spec.ts + +Create integration tests that verify real-time attachment synchronization. + +Note: True multi-user testing requires two browser contexts. For now, create tests that: +1. Simulate WebSocket message receipt via Redux dispatch +2. Verify attachment appears in chat message list +3. Verify deduplication works (same message twice doesn't duplicate) +4. Verify REST API history integrates with WebSocket messages + +Create file: `jam-ui/test/attachments/real-time-sync.spec.ts` + +```typescript +import { test, expect, Page } from '@playwright/test'; + +/** + * Real-time Attachment Synchronization Tests + * + * These tests verify that attachment messages sync correctly across + * different message paths (WebSocket and REST API). + * + * True multi-user testing would require: + * - Two browser contexts with different user sessions + * - Or mock WebSocket server + * + * Current approach: Simulate WebSocket receipt via Redux dispatch + */ + +const TEST_SESSION_ID = process.env.TEST_SESSION_ID || 'test-session-id'; + +// Helper to access Redux store in test environment +async function getReduxStore(page: Page) { + return page.evaluate(() => (window as any).__REDUX_STORE__); +} + +// Helper to dispatch Redux action +async function dispatchAction(page: Page, action: any) { + return page.evaluate((actionData) => { + const store = (window as any).__REDUX_STORE__; + if (store) { + store.dispatch(actionData); + } + }, action); +} + +// Helper to get chat messages from Redux +async function getChatMessages(page: Page, channel: string) { + return page.evaluate((ch) => { + const store = (window as any).__REDUX_STORE__; + if (store) { + const state = store.getState(); + return state.sessionChat?.messagesByChannel?.[ch] || []; + } + return []; + }, channel); +} + +test.describe('Real-time Attachment Synchronization', () => { + + test.describe('WebSocket Message Receipt', () => { + + test('attachment message from WebSocket appears in chat', async ({ page }) => { + // This test requires being in an active session + // Skip if not in session context + test.skip(!process.env.TEST_SESSION_ACTIVE, 'Requires active session'); + + const attachmentMessage = { + type: 'sessionChat/addMessageFromWebSocket', + payload: { + id: 'test-msg-123', + senderId: 'user-456', + senderName: 'Test User', + message: '', + createdAt: new Date().toISOString(), + channel: 'session', + sessionId: TEST_SESSION_ID, + purpose: 'Notation File', + attachmentId: 'attachment-789', + attachmentType: 'notation', + attachmentName: 'test-file.pdf', + attachmentSize: 1024 + } + }; + + // Dispatch simulated WebSocket message + await dispatchAction(page, attachmentMessage); + + // Verify message appears in Redux state + const messages = await getChatMessages(page, TEST_SESSION_ID); + const found = messages.find((m: any) => m.id === 'test-msg-123'); + + expect(found).toBeDefined(); + expect(found.attachmentId).toBe('attachment-789'); + expect(found.attachmentName).toBe('test-file.pdf'); + }); + + test('duplicate WebSocket message is deduplicated', async ({ page }) => { + test.skip(!process.env.TEST_SESSION_ACTIVE, 'Requires active session'); + + const messageId = 'dedup-test-' + Date.now(); + const attachmentMessage = { + type: 'sessionChat/addMessageFromWebSocket', + payload: { + id: messageId, + senderId: 'user-456', + senderName: 'Test User', + message: '', + createdAt: new Date().toISOString(), + channel: 'session', + sessionId: TEST_SESSION_ID, + purpose: 'Audio File', + attachmentId: 'attachment-dedup', + attachmentType: 'audio', + attachmentName: 'test-audio.mp3', + attachmentSize: 2048 + } + }; + + // Dispatch same message twice (simulates network retry) + await dispatchAction(page, attachmentMessage); + await dispatchAction(page, attachmentMessage); + + // Verify message appears only once + const messages = await getChatMessages(page, TEST_SESSION_ID); + const matches = messages.filter((m: any) => m.id === messageId); + + expect(matches.length).toBe(1); + }); + + }); + + test.describe('Chat History Integration', () => { + + test('WebSocket message and REST API message with same ID deduplicate', async ({ page }) => { + test.skip(!process.env.TEST_SESSION_ACTIVE, 'Requires active session'); + + const sharedMessageId = 'shared-msg-' + Date.now(); + + // Simulate message already loaded from REST API + const restMessage = { + type: 'sessionChat/addMessageFromWebSocket', // Use same reducer for test + payload: { + id: sharedMessageId, + senderId: 'user-789', + senderName: 'History User', + message: '', + createdAt: new Date(Date.now() - 60000).toISOString(), // 1 min ago + channel: 'session', + sessionId: TEST_SESSION_ID, + purpose: 'Notation File', + attachmentId: 'att-shared', + attachmentType: 'notation', + attachmentName: 'shared-file.pdf', + attachmentSize: null // REST API doesn't have size + } + }; + + // Simulate WebSocket message with same ID (different source) + const wsMessage = { + type: 'sessionChat/addMessageFromWebSocket', + payload: { + ...restMessage.payload, + attachmentSize: 3072 // WebSocket has size + } + }; + + // Add REST message first + await dispatchAction(page, restMessage); + + // Then WebSocket message arrives (should be deduplicated) + await dispatchAction(page, wsMessage); + + // Verify only one message exists + const messages = await getChatMessages(page, TEST_SESSION_ID); + const matches = messages.filter((m: any) => m.id === sharedMessageId); + + expect(matches.length).toBe(1); + // First message wins (REST), so attachmentSize should be null + expect(matches[0].attachmentSize).toBeNull(); + }); + + }); + +}); + +test.describe('Attachment Message Display', () => { + + test('attachment message shows filename and metadata', async ({ page }) => { + // Navigate to a page with chat window + test.skip(!process.env.TEST_SESSION_ACTIVE, 'Requires active session'); + + // Dispatch test attachment message + const attachmentMessage = { + type: 'sessionChat/addMessageFromWebSocket', + payload: { + id: 'display-test-' + Date.now(), + senderId: 'user-display', + senderName: 'Display Tester', + message: '', + createdAt: new Date().toISOString(), + channel: 'session', + sessionId: TEST_SESSION_ID, + purpose: 'Notation File', + attachmentId: 'att-display', + attachmentType: 'notation', + attachmentName: 'my-music-sheet.pdf', + attachmentSize: 1536000 // ~1.5 MB + } + }; + + await dispatchAction(page, attachmentMessage); + + // Open chat window if not open + await page.click('[data-testid="chat-button"]').catch(() => { + // Button might not exist or chat might be open + }); + + // Look for attachment message in chat + const attachmentElement = page.locator('text=my-music-sheet.pdf'); + + // Note: This test may need adjustment based on actual test environment + // The key assertion is that the message was dispatched and would render + const messages = await getChatMessages(page, TEST_SESSION_ID); + const found = messages.find((m: any) => m.attachmentName === 'my-music-sheet.pdf'); + expect(found).toBeDefined(); + }); + +}); +``` + +The tests use Redux dispatch to simulate WebSocket messages, which is a pragmatic approach since true multi-browser testing requires complex setup. + + +Run: `ls -la jam-ui/test/attachments/` to confirm file exists +Run: `wc -l jam-ui/test/attachments/real-time-sync.spec.ts` should show ~150+ lines + + Integration test file created with WebSocket simulation, deduplication, and display tests + + + + +Real-time synchronization verification complete: +1. WebSocket handler verified and debug logging cleaned +2. Deduplication logic confirmed for both message paths +3. Integration tests created for sync behavior + +The infrastructure built in Phases 12-14 fully supports real-time attachment synchronization. + + +**Manual Multi-User Verification:** + +1. Open two browser windows (or use two different browsers) +2. Log in as different users in each window +3. Join the same music session in both windows +4. Open the chat window in both browsers + +**Test 1: Real-time Broadcast** +5. In Browser A: Click "Attach" button in session toolbar +6. Select a valid file (e.g., test.pdf, under 10MB) +7. Wait for upload to complete +8. In Browser B: Verify attachment message appears immediately (within 1-2 seconds) +9. Verify message shows: "[User A name] attached test.pdf" + +**Test 2: Deduplication (Uploader)** +10. In Browser A: Verify the same attachment message shows only ONCE +11. No duplicate messages should appear for the uploader + +**Test 3: Chat History Persistence** +12. In Browser B: Refresh the page +13. Rejoin the session +14. Open chat window +15. Verify the attachment message from step 5 is still visible + +**Expected Results:** +- Attachment broadcasts to all participants in real-time +- No duplicate messages for uploader or receivers +- Attachments persist in chat history across page refresh + + Type "verified" to confirm multi-user sync works, or describe any issues found + + + + + +## Code Verification + +1. WebSocket handler extracts all attachment fields correctly +2. Deduplication prevents duplicates from both WebSocket and REST API +3. Debug logging cleaned up from production code +4. Integration tests cover sync scenarios + +## Functional Verification (Manual) + +1. Multi-user broadcast works in real-time +2. Uploader sees single message (no duplicates) +3. Other participants see attachment immediately +4. Chat history includes attachments after refresh + + + +Phase 15 is complete when: +- [ ] WebSocket handler verified and debug logs cleaned +- [ ] Deduplication logic verified for both message paths +- [ ] Integration tests created and checked in +- [ ] Human verification confirms multi-user real-time sync works +- [ ] No duplicate messages for any user +- [ ] Attachments persist in chat history + + + +After completion, create `.planning/phases/15-real-time-synchronization/15-01-SUMMARY.md` +