Merge pull request #3809 from rudycelekli/fix/web-message-mutation-ack-20261021

fix(web): preserve chat history when deletion fails

channel/web/static/js/chat/message-actions.js

@@ -121,6 +121,26 @@ function copyToClipboard(text) {
     });
 }
 
+// Keep visible history in sync with the store before editing, regenerating,
+// or deleting a persisted turn. Callers keep their existing deletion options.
+async function deletePersistedMessage(userSeq, options = {}) {
+    try {
+        const resp = await fetch('/api/messages/delete', {
+            method: 'POST',
+            headers: { 'Content-Type': 'application/json' },
+            body: JSON.stringify({ session_id: sessionId, user_seq: parseInt(userSeq), ...options })
+        });
+        if (!resp.ok) throw new Error(`HTTP ${resp.status}`);
+        const data = await resp.json();
+        if (data.status !== 'success') throw new Error(data.message || 'Delete failed');
+        return true;
+    } catch (err) {
+        console.error('Failed to delete messages:', err);
+        alert(t('records_delete_failed'));
+        return false;
+    }
+}
+
 // Edit user message: extract content, remove this and subsequent messages, fill input
 async function editUserMessage(msgEl) {
     if (isCurrentSessionConversationActive()) return;
@@ -130,24 +150,7 @@ async function editUserMessage(msgEl) {
     // Delete this message and ALL subsequent messages from database (cascade)
     // Must await to ensure delete completes before user sends a new message
     const userSeq = msgEl.dataset.seq;
-    if (userSeq) {
-        try {
-            const resp = await fetch('/api/messages/delete', {
-                method: 'POST',
-                headers: { 'Content-Type': 'application/json' },
-                body: JSON.stringify({ 
-                    session_id: sessionId, 
-                    user_seq: parseInt(userSeq),
-                    delete_user: true,
-                    cascade: true
-                })
-            });
-            const data = await resp.json();
-            if (data.status === 'success') console.log(`Deleted ${data.deleted} old messages`);
-        } catch (err) {
-            console.error('Failed to delete old messages:', err);
-        }
-    }
+    if (userSeq && !await deletePersistedMessage(userSeq, { delete_user: true, cascade: true })) return;
 
     // Remove this message bubble and every later bubble that belongs to
     // this or a subsequent turn. We mirror the backend cascade contract:

channel/web/static/js/chat/send.js

@@ -24,23 +24,7 @@ async function regenerateResponse(botMsgEl) {
     // (because /message will create a fresh user message + new bot reply)
     // Must await to ensure delete completes before /message is sent
     const userSeq = prevEl.dataset.seq;
-    if (userSeq) {
-        try {
-            const resp = await fetch('/api/messages/delete', {
-                method: 'POST',
-                headers: { 'Content-Type': 'application/json' },
-                body: JSON.stringify({ 
-                    session_id: sessionId, 
-                    user_seq: parseInt(userSeq),
-                    delete_user: true
-                })
-            });
-            const data = await resp.json();
-            if (data.status === 'success') console.log(`Deleted ${data.deleted} old messages`);
-        } catch (err) {
-            console.error('Failed to delete old messages:', err);
-        }
-    }
+    if (userSeq && !await deletePersistedMessage(userSeq, { delete_user: true })) return;
 
     // Remove both the old user message and bot message from DOM
     if (prevEl.parentNode) prevEl.parentNode.removeChild(prevEl);

channel/web/static/js/chat/state.js

@@ -604,7 +604,7 @@ messagesDiv.addEventListener('click', (e) => {
         const userMsgEl = deleteBtn.closest('.user-message-group');
         if (!userMsgEl) return;
 
-        showConfirmModal(t('delete_message_title'), t('delete_message_confirm'), () => {
+        showConfirmModal(t('delete_message_title'), t('delete_message_confirm'), async () => {
             // Find the next bot reply for this turn (skip non-message nodes).
             let botReplyEl = null;
             let sibling = userMsgEl.nextElementSibling;
@@ -615,21 +615,13 @@ messagesDiv.addEventListener('click', (e) => {
                 }
                 sibling = sibling.nextElementSibling;
             }
+            const userSeq = userMsgEl.dataset.seq;
+            if (userSeq && !await deletePersistedMessage(userSeq)) return;
+
             userMsgEl.remove();
             if (botReplyEl) botReplyEl.remove();
-
-            const userSeq = userMsgEl.dataset.seq;
-            if (userSeq) {
-                fetch('/api/messages/delete', {
-                    method: 'POST',
-                    headers: { 'Content-Type': 'application/json' },
-                    body: JSON.stringify({ session_id: sessionId, user_seq: parseInt(userSeq) })
-                }).then(r => r.json()).then(data => {
-                    if (data.status === 'success') console.log(`Deleted ${data.deleted} messages`);
-                    // Drop the removed question from the navigation rail.
-                    if (typeof refreshTimeline === 'function') refreshTimeline();
-                }).catch(err => console.error('Failed to delete:', err));
-            }
+            // Drop the removed question from the navigation rail.
+            if (userSeq && typeof refreshTimeline === 'function') refreshTimeline();
         });
         return;
     }

tests/web_message_mutation.test.cjs

@@ -0,0 +1,141 @@
+const { test } = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('node:fs');
+const vm = require('node:vm');
+const path = require('node:path');
+const ROOT = path.join(__dirname, '..');
+
+// Follow web_scroll.test.cjs: exercise the actual classic-script consumers
+// with controlled browser collaborators and deferred persistence responses.
+function setup({ active = false, persisted = true } = {}) {
+    const events = [], requests = [], alerts = [], nodes = [];
+    let click, confirm, resolveDelete, rejectDelete;
+    const deletion = new Promise((resolve, reject) => { resolveDelete = resolve; rejectDelete = reject; });
+    const box = {
+        removeChild(item) { events.push(`remove:${item.dataset.seq || item.role}`); nodes.splice(nodes.indexOf(item), 1); },
+        addEventListener(name, callback) { if (name === 'click') click = callback; },
+    };
+    function node(role, seq, content) {
+        const item = {
+            role, dataset: { rawContent: content },
+            classList: { contains(name) { return name === `${role}-message-group`; } },
+            get parentNode() { return nodes.includes(item) ? box : null; },
+            get nextElementSibling() { return nodes[nodes.indexOf(item) + 1] || null; },
+            get previousElementSibling() { return nodes[nodes.indexOf(item) - 1] || null; },
+            remove() { if (item.parentNode) box.removeChild(item); },
+        };
+        if (seq != null) item.dataset.seq = String(seq);
+        nodes.push(item);
+        return item;
+    }
+    const first = node('user', persisted ? 1 : null, 'First question');
+    const answer = node('bot', persisted ? 2 : null, 'First answer');
+    node('user', 3, 'Later question'); node('bot', 4, 'Later answer');
+    const input = { value: 'Unsent draft', dispatchEvent() {}, focus() {} };
+    const context = vm.createContext({
+        messagesDiv: box, chatInput: input, sessionId: 'owned-session', currentLang: 'en',
+        isCurrentSessionConversationActive: () => active,
+        t: key => key, alert: message => alerts.push(message),
+        console: { log() {}, warn() {}, error() {} }, Event: class {},
+        fetch(url, options) {
+            requests.push({ url, body: JSON.parse(options.body) });
+            if (url === '/api/messages/delete') {
+                events.push('delete:request');
+                return deletion.then(response => { events.push('delete:response'); return response; });
+            }
+            assert.equal(url, '/message'); events.push('regenerate:send');
+            return Promise.resolve({ json: async () => ({ status: 'success', inline_reply: 'Fresh answer' }) });
+        },
+        showConfirmModal(_title, _message, callback) { confirm = callback; },
+        refreshTimeline() { events.push('timeline:refresh'); }, scrollChatToBottom() {},
+        addUserMessage(content) { events.push('user:add'); node('user', null, content); },
+        addBotMessage() { events.push('bot:add'); }, addLoadingIndicator() { return { remove() {} }; },
+        addressedAgentId: () => '', rememberLiveSpeaker() {}, setLoadingSpeaker() {},
+        loadingContainers: {}, Date, setTimeout,
+    });
+    const source = name => fs.readFileSync(path.join(ROOT, 'channel/web/static/js/chat', name), 'utf8');
+    vm.runInContext(source('message-actions.js'), context);
+    vm.runInContext(source('send.js'), context);
+    const state = source('state.js');
+    vm.runInContext(state.slice(state.indexOf('// Intercept internal navigation links'), state.indexOf('const attachmentPreview')), context);
+    function deleteClick() {
+        const button = { closest(selector) { return selector === '.user-message-group' ? first : null; } };
+        click({ preventDefault() {}, target: { closest(selector) { return selector === '.delete-msg-btn' ? button : null; } } });
+    }
+    return {
+        nodes, requests, alerts, events, input,
+        ids: () => nodes.filter(item => item.dataset.seq).map(item => item.dataset.seq),
+        start(action) {
+            if (action === 'edit') return context.editUserMessage(first);
+            if (action === 'regenerate') return context.regenerateResponse(answer);
+            deleteClick(); return confirm ? confirm() : Promise.resolve();
+        },
+        cancelDelete() { deleteClick(); },
+        respond(body = { status: 'success', deleted: 2 }, ok = true) {
+            resolveDelete({ ok, status: ok ? 200 : 500, json: async () => body });
+        },
+        invalidJson() { resolveDelete({ ok: true, json: async () => { throw new SyntaxError('Invalid JSON'); } }); },
+        networkError() { rejectDelete(new Error('Connection lost')); },
+    };
+}
+const settle = () => new Promise(resolve => setImmediate(resolve));
+
+for (const action of ['edit', 'regenerate', 'delete']) {
+    for (const failure of ['api-error', 'network-error', 'invalid-json', 'http-error']) {
+        test(`${action}: ${failure} preserves history and draft without a new send`, async () => {
+            const s = setup(), operation = s.start(action);
+            await settle();
+            assert.deepEqual(s.ids(), ['1', '2', '3', '4'], 'wait for persistence before removing a turn');
+            assert.equal(s.input.value, 'Unsent draft');
+            assert.equal(s.requests.filter(r => r.url === '/message').length, 0);
+            if (failure === 'network-error') s.networkError();
+            else if (failure === 'invalid-json') s.invalidJson();
+            else if (failure === 'http-error') s.respond({ status: 'success' }, false);
+            else s.respond({ status: 'error', message: 'Store failed' });
+            await operation; await settle();
+            assert.deepEqual(s.ids(), ['1', '2', '3', '4']);
+            assert.equal(s.input.value, 'Unsent draft');
+            assert.deepEqual(s.alerts, ['records_delete_failed']);
+            assert.equal(s.requests.filter(r => r.url === '/message').length, 0);
+            assert.equal(s.events.includes('timeline:refresh'), false);
+        });
+    }
+    test(`${action}: acknowledgment preserves the existing successful mutation`, async () => {
+        const s = setup(), operation = s.start(action);
+        await settle(); assert.deepEqual(s.ids(), ['1', '2', '3', '4']);
+        s.respond(); await operation; await settle();
+        const expected = { session_id: 'owned-session', user_seq: 1 };
+        if (action !== 'delete') expected.delete_user = true;
+        if (action === 'edit') expected.cascade = true;
+        assert.deepEqual(s.requests[0], { url: '/api/messages/delete', body: expected });
+        assert.deepEqual(s.ids(), action === 'edit' ? [] : ['3', '4']);
+        const removed = s.events.findIndex(event => event.startsWith('remove:'));
+        assert.ok(removed > s.events.indexOf('delete:response'));
+        assert.deepEqual(s.alerts, []);
+        assert.equal(s.input.value, action === 'edit' ? 'First question' : 'Unsent draft');
+        const sends = s.requests.filter(r => r.url === '/message');
+        assert.equal(sends.length, action === 'regenerate' ? 1 : 0);
+        if (sends.length) assert.equal(sends[0].body.message, 'First question');
+        if (action === 'delete') assert.ok(s.events.indexOf('timeline:refresh') > removed);
+    });
+}
+
+test('canceling direct Delete does not submit or change the conversation', () => {
+    const s = setup(); s.cancelDelete();
+    assert.deepEqual(s.requests, []); assert.deepEqual(s.ids(), ['1', '2', '3', '4']);
+    assert.equal(s.input.value, 'Unsent draft');
+});
+test('active reply still blocks Edit and direct Delete', async () => {
+    for (const action of ['edit', 'delete']) {
+        const s = setup({ active: true }); await s.start(action);
+        assert.deepEqual(s.requests, []); assert.deepEqual(s.ids(), ['1', '2', '3', '4']);
+    }
+});
+for (const action of ['edit', 'regenerate', 'delete']) {
+    test(`${action}: nonpersisted turns still work without a deletion request`, async () => {
+        const s = setup({ persisted: false }); await s.start(action); await settle();
+        assert.equal(s.requests.filter(r => r.url === '/api/messages/delete').length, 0);
+        assert.deepEqual(s.ids(), action === 'edit' ? [] : ['3', '4']); assert.deepEqual(s.alerts, []);
+        if (action === 'delete') assert.equal(s.events.includes('timeline:refresh'), false);
+    });
+}