diff --git a/CHANGELOG.md b/CHANGELOG.md index e52e037..cadb53c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Fix browser extension losing connection and feedback after MCP reconnect (`/mcp`) — session IDs are now derived deterministically from the project directory, so reconnecting preserves WebSocket connections and pending feedback (#43) +- Add process ID guard on session unregistration to prevent race conditions when old and new MCP processes overlap +- Add feedback migration when a new session registers with the same project directory as an existing session +- Browser extension now validates cached session IDs on page navigation and auto-refreshes stale sessions + ## [0.6.5] - 2026-04-21 ### Fixed diff --git a/extension/background.js b/extension/background.js index e6de1c9..549a7e4 100644 --- a/extension/background.js +++ b/extension/background.js @@ -105,6 +105,35 @@ async function resolveSessionForTab(tabUrl, serverUrl) { return null; } +// Validate that a cached session still exists on the server; re-resolve if stale +async function validateOrRefreshSession(tabId, cachedSessionId, serverUrl) { + if (!cachedSessionId) { + const tab = await chrome.tabs.get(tabId); + const resolved = await resolveSessionForTab(tab.url, serverUrl); + if (resolved) { + tabSessionMap.set(tabId, resolved); + persistActiveTabs(); + } + return resolved; + } + + const sessions = await fetchSessions(serverUrl); + if (sessions.length === 0) return cachedSessionId; // Server unreachable, keep cached + + const stillExists = sessions.some(s => s.sessionId === cachedSessionId); + if (stillExists) return cachedSessionId; + + // Session is stale — re-resolve by project URL + const tab = await chrome.tabs.get(tabId); + const resolved = await resolveSessionForTab(tab.url, serverUrl); + if (resolved) { + tabSessionMap.set(tabId, resolved); + persistActiveTabs(); + console.log(`[Feedback Ext] Session refreshed: ${cachedSessionId.slice(0, 8)} -> ${resolved.slice(0, 8)}`); + } + return resolved || cachedSessionId; +} + // Toggle widget on a specific tab async function toggleTab(tabId) { const serverUrl = await getServerUrl(); @@ -155,11 +184,13 @@ chrome.runtime.onMessage.addListener((message, sender, sendResponse) => { // Called by content script on page load const tabId = sender.tab?.id; if (tabId && activeTabs.has(tabId)) { - getServerUrl().then((serverUrl) => { + getServerUrl().then(async (serverUrl) => { + const cachedSession = tabSessionMap.get(tabId) || null; + const sessionId = await validateOrRefreshSession(tabId, cachedSession, serverUrl); sendResponse({ active: true, serverUrl, - sessionId: tabSessionMap.get(tabId) || null, + sessionId, }); }); return true; // async response @@ -218,7 +249,8 @@ chrome.runtime.onMessage.addListener((message, sender, sendResponse) => { chrome.tabs.onUpdated.addListener(async (tabId, changeInfo) => { if (changeInfo.status === 'complete' && activeTabs.has(tabId)) { const serverUrl = await getServerUrl(); - const sessionId = tabSessionMap.get(tabId) || null; + const cachedSession = tabSessionMap.get(tabId) || null; + const sessionId = await validateOrRefreshSession(tabId, cachedSession, serverUrl); updateBadge(tabId, true); await sendToTab(tabId, { action: 'activate', serverUrl, sessionId }); } diff --git a/src/server.js b/src/server.js index 4b4f0f3..8be4028 100644 --- a/src/server.js +++ b/src/server.js @@ -14,7 +14,7 @@ import crypto from "node:crypto"; import { fileURLToPath, pathToFileURL } from "url"; import { createRequire } from "module"; import { execFile } from "child_process"; -import { isValidSessionId, getPendingSummary, detectProjectUrl, formatFeedbackAsContent } from "./utils.js"; +import { deriveSessionId, isValidSessionId, getPendingSummary, detectProjectUrl, formatFeedbackAsContent } from "./utils.js"; const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); @@ -23,8 +23,9 @@ const PORT = parseInt(process.env.FEEDBACK_PORT || "9877"); const PKG_VERSION = JSON.parse(fs.readFileSync(path.join(__dirname, '..', 'package.json'), 'utf8')).version; // Session identity for this MCP server process -const SESSION_ID = crypto.randomUUID(); const PROJECT_DIR = process.cwd(); +const SESSION_ID = deriveSessionId(PROJECT_DIR); +const PROCESS_ID = crypto.randomUUID(); // Session registry (owner server only): sessionId -> metadata const sessionRegistry = new Map(); @@ -183,6 +184,7 @@ async function registerSessionViaHttp() { headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ sessionId: SESSION_ID, + processId: PROCESS_ID, projectDir: PROJECT_DIR, projectUrl: detected.url, detectedFrom: detected.detectedFrom, @@ -199,7 +201,7 @@ async function unregisterSessionViaHttp() { await fetch(`http://localhost:${PORT}/unregister-session`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ sessionId: SESSION_ID }), + body: JSON.stringify({ sessionId: SESSION_ID, processId: PROCESS_ID }), }); } catch (err) { // Ignore errors during shutdown @@ -371,8 +373,36 @@ const httpServer = http.createServer((req, res) => { res.end(JSON.stringify({ error: "Invalid session ID format" })); return; } + // Migrate feedback from any old session with the same projectDir but different sessionId + for (const [existingId, existingMeta] of sessionRegistry) { + if (existingId !== data.sessionId && existingMeta.projectDir === data.projectDir) { + const oldPending = pendingFeedbackBySession.get(existingId); + if (oldPending && oldPending.length > 0) { + getSessionPending(data.sessionId).push(...oldPending); + } + const oldReady = readyFeedbackBySession.get(existingId); + if (oldReady && oldReady.length > 0) { + getSessionReady(data.sessionId).push(...oldReady); + } + const oldClients = connectedClientsBySession.get(existingId); + if (oldClients && oldClients.size > 0) { + const newClients = getSessionClients(data.sessionId); + for (const client of oldClients) { + client._sessionId = data.sessionId; + newClients.add(client); + } + } + pendingFeedbackBySession.delete(existingId); + readyFeedbackBySession.delete(existingId); + feedbackResolversBySession.delete(existingId); + connectedClientsBySession.delete(existingId); + sessionRegistry.delete(existingId); + console.error(`[browser-feedback-mcp] Migrated session data: ${existingId} -> ${data.sessionId}`); + } + } sessionRegistry.set(data.sessionId, { sessionId: data.sessionId, + processId: data.processId || null, projectDir: data.projectDir, projectUrl: data.projectUrl || null, detectedFrom: data.detectedFrom || null, @@ -396,6 +426,14 @@ const httpServer = http.createServer((req, res) => { res.end(JSON.stringify({ error: "Invalid session ID format" })); return; } + // Guard: skip deletion if a different process has re-registered this session + const existing = sessionRegistry.get(data.sessionId); + if (existing && data.processId && existing.processId && existing.processId !== data.processId) { + console.error(`[browser-feedback-mcp] Skipping unregister: session ${data.sessionId} owned by different process`); + res.writeHead(200, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ success: true, skipped: true })); + return; + } sessionRegistry.delete(data.sessionId); // Clean up session data pendingFeedbackBySession.delete(data.sessionId); @@ -1719,8 +1757,11 @@ function shutdown(reason) { // Only close HTTP server if we own it if (isHttpServerOwner) { - // Remove own session from registry - sessionRegistry.delete(SESSION_ID); + // Remove own session from registry only if we still own it + const ownSession = sessionRegistry.get(SESSION_ID); + if (ownSession && ownSession.processId === PROCESS_ID) { + sessionRegistry.delete(SESSION_ID); + } // Close all WebSocket connections for (const client of connectedClients) { @@ -1831,6 +1872,7 @@ async function main() { // Owner registers directly sessionRegistry.set(SESSION_ID, { sessionId: SESSION_ID, + processId: PROCESS_ID, projectDir: PROJECT_DIR, projectUrl: detected.url, detectedFrom: detected.detectedFrom, diff --git a/src/utils.js b/src/utils.js index 8af0679..8e7841a 100644 --- a/src/utils.js +++ b/src/utils.js @@ -1,5 +1,19 @@ import fs from "fs"; import path from "path"; +import crypto from "node:crypto"; + +// Derive a deterministic session ID (UUID format) from the project directory. +// Ensures reconnecting the same project reuses the same session ID. +export function deriveSessionId(projectDir) { + const hash = crypto.createHash('sha256').update(projectDir).digest('hex'); + return [ + hash.slice(0, 8), + hash.slice(8, 12), + hash.slice(12, 16), + hash.slice(16, 20), + hash.slice(20, 32), + ].join('-'); +} // UUID format validation for session IDs const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; diff --git a/tests/http-endpoints.test.js b/tests/http-endpoints.test.js index b8a9154..da96c51 100644 --- a/tests/http-endpoints.test.js +++ b/tests/http-endpoints.test.js @@ -125,6 +125,47 @@ describe('session registration', () => { expect(found).toBeUndefined(); }); + it('POST /unregister-session skips deletion when processId does not match', async () => { + const sessionId = crypto.randomUUID(); + + // Register with processId 'proc-1' + await fetch(`${BASE_URL}/register-session`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ sessionId, processId: 'proc-1', projectDir: '/tmp/guard-test' }), + }); + + // Re-register with processId 'proc-2' (simulates new process taking over) + await fetch(`${BASE_URL}/register-session`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ sessionId, processId: 'proc-2', projectDir: '/tmp/guard-test' }), + }); + + // Old process tries to unregister with 'proc-1' — should be skipped + const unregResp = await fetch(`${BASE_URL}/unregister-session`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ sessionId, processId: 'proc-1' }), + }); + expect(unregResp.status).toBe(200); + const unregData = await unregResp.json(); + expect(unregData.skipped).toBe(true); + + // Session should still exist + const sessionsResp = await fetch(`${BASE_URL}/sessions`); + const sessionsData = await sessionsResp.json(); + const found = sessionsData.sessions.find(s => s.sessionId === sessionId); + expect(found).toBeDefined(); + + // Clean up with correct processId + await fetch(`${BASE_URL}/unregister-session`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ sessionId, processId: 'proc-2' }), + }); + }); + it('POST /unregister-session with invalid ID returns 400', async () => { const resp = await fetch(`${BASE_URL}/unregister-session`, { method: 'POST', @@ -252,6 +293,65 @@ describe('WebSocket session routing', () => { } }); + it('client reconnecting after session migration lands in correct bucket', async () => { + // Register session A with a projectDir, connect a WS client, submit feedback + const sessionA = crypto.randomUUID(); + const sessionB = crypto.randomUUID(); + const projectDir = '/tmp/migration-test-project'; + + await fetch(`${BASE_URL}/register-session`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ sessionId: sessionA, processId: 'proc-a', projectDir }), + }); + + // Connect WS client to session A and submit feedback + const { ws: wsA, msg: msgA } = await connectWs(sessionA); + expect(msgA.sessionId).toBe(sessionA); + + wsA.send(JSON.stringify({ + type: 'feedback', + payload: { id: 'fb-migrate-1', description: 'Test feedback for migration' }, + })); + + // Wait for feedback to be stored + await new Promise(r => setTimeout(r, 100)); + + // Verify feedback exists under session A + const respA = await fetch(`${BASE_URL}/status?session=${sessionA}`); + const dataA = await respA.json(); + expect(dataA.pendingFeedback).toBe(1); + + wsA.close(); + await new Promise(r => setTimeout(r, 100)); + + // Register session B with same projectDir — should migrate feedback + const regResp = await fetch(`${BASE_URL}/register-session`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ sessionId: sessionB, processId: 'proc-b', projectDir }), + }); + expect(regResp.status).toBe(200); + + // Verify feedback migrated to session B + const respB = await fetch(`${BASE_URL}/status?session=${sessionB}`); + const dataB = await respB.json(); + expect(dataB.pendingFeedback).toBe(1); + + // Verify session A no longer exists + const sessionsResp = await fetch(`${BASE_URL}/sessions`); + const sessionsData = await sessionsResp.json(); + const foundA = sessionsData.sessions.find(s => s.sessionId === sessionA); + expect(foundA).toBeUndefined(); + + // Clean up + await fetch(`${BASE_URL}/unregister-session`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ sessionId: sessionB, processId: 'proc-b' }), + }); + }); + it('second client on same session triggers duplicate warning', async () => { const sessionId = crypto.randomUUID(); const { ws: ws1, msg: msg1 } = await connectWs(sessionId); diff --git a/tests/utils.test.js b/tests/utils.test.js index 5b22fe0..dc8c5f6 100644 --- a/tests/utils.test.js +++ b/tests/utils.test.js @@ -3,12 +3,41 @@ import fs from 'fs'; import os from 'os'; import path from 'path'; import { + deriveSessionId, isValidSessionId, getPendingSummary, detectProjectUrl, formatFeedbackAsContent, } from '../src/utils.js'; +// ============================================ +// deriveSessionId +// ============================================ + +describe('deriveSessionId', () => { + it('returns a valid UUID-formatted session ID', () => { + const id = deriveSessionId('/home/user/project'); + expect(isValidSessionId(id)).toBe(true); + }); + + it('returns the same ID for the same project directory', () => { + const id1 = deriveSessionId('/home/user/project'); + const id2 = deriveSessionId('/home/user/project'); + expect(id1).toBe(id2); + }); + + it('returns different IDs for different project directories', () => { + const id1 = deriveSessionId('/home/user/project-a'); + const id2 = deriveSessionId('/home/user/project-b'); + expect(id1).not.toBe(id2); + }); + + it('returns lowercase hex characters', () => { + const id = deriveSessionId('/some/path'); + expect(id).toMatch(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/); + }); +}); + // ============================================ // isValidSessionId // ============================================