diff --git a/src/praisonai-desktop/engine/server.py b/src/praisonai-desktop/engine/server.py index 542e22cee..abe49c72a 100644 --- a/src/praisonai-desktop/engine/server.py +++ b/src/praisonai-desktop/engine/server.py @@ -1606,8 +1606,14 @@ def do_DELETE(self): return try: _chat_path(self.path.rsplit("/", 1)[-1]).unlink(missing_ok=True) - except (OSError, ValueError): - pass + except ValueError as exc: + self._json({"ok": False, "error": str(exc)}, 400) + return + except OSError as exc: + # Reporting a delete that did not happen is how a conversation + # closed on screen and was back in the sidebar on reopen. + self._json({"ok": False, "error": str(exc)}, 500) + return self._json({"ok": True}) def do_POST(self): diff --git a/src/praisonai-desktop/engine/test_train_routes.py b/src/praisonai-desktop/engine/test_train_routes.py index 85ae021e6..e58d26081 100644 --- a/src/praisonai-desktop/engine/test_train_routes.py +++ b/src/praisonai-desktop/engine/test_train_routes.py @@ -508,5 +508,53 @@ def test_quitting_the_engine_kills_the_running_trainer(self): f"the trainer (pid {pid}) outlived the engine quit") +class DeleteChats(unittest.TestCase): + """DELETE /chats/ must answer for the delete that actually happened.""" + + def setUp(self): + self.engine = EngineProcess(_python(SHORT_RUN)) + self.chats_dir = pathlib.Path(self.engine.home) / "chats" + self.chats_dir.mkdir(parents=True, exist_ok=True) + + def tearDown(self): + self.engine.close() + + def _write_chat(self, cid): + (self.chats_dir / f"{cid}.json").write_text( + json.dumps({"id": cid, "title": cid, "messages": []})) + + def _listed_ids(self): + _, body = self.engine.request("/chats") + return {c["id"] for c in body.get("chats", [])} + + def test_a_delete_that_succeeds_removes_the_chat(self): + self._write_chat("good1") + self.assertIn("good1", self._listed_ids()) + status, body = self.engine.request("/chats/good1", method="DELETE") + self.assertEqual(status, 200, body) + self.assertNotIn("good1", self._listed_ids()) + + def test_a_delete_that_cannot_happen_is_not_reported_as_done(self): + # The read-only data dir and the synced folder mid-conflict from the + # report both surface as an OSError from unlink(). A directory standing + # where the chat file would be reproduces that deterministically -- even + # for root, unlink() refuses it -- where a chmod'd dir does not, since + # root ignores the permission bit. Answering 200 here is how the + # conversation closed on screen and was back in the sidebar on reopen. + blocker = self.chats_dir / "stuck.json" + blocker.mkdir() + status, body = self.engine.request("/chats/stuck", method="DELETE") + self.assertNotEqual(status, 200, "a delete that did not happen was reported done") + self.assertFalse(body.get("ok", True)) + self.assertTrue(blocker.is_dir(), "the blocker vanished; the test proves nothing") + + def test_an_empty_id_is_refused_rather_than_silently_ok(self): + # An id that reduces to nothing (the report's `../..` after the route + # is stripped) raised a ValueError that used to be swallowed as 200. + status, body = self.engine.request("/chats/..", method="DELETE") + self.assertEqual(status, 400, body) + self.assertFalse(body.get("ok", True)) + + if __name__ == "__main__": unittest.main(verbosity=2) diff --git a/src/praisonai-desktop/frontend/tests/buttons.test.mjs b/src/praisonai-desktop/frontend/tests/buttons.test.mjs index 13efd254d..28f9f7953 100644 --- a/src/praisonai-desktop/frontend/tests/buttons.test.mjs +++ b/src/praisonai-desktop/frontend/tests/buttons.test.mjs @@ -332,3 +332,129 @@ test('Stop discards the queue rather than silently starting the next turn', asyn assert.equal(doc.querySelectorAll('.qrow').length, 0, 'stopping a turn left the queue armed, so the next prompt runs unasked'); }); + +// ---- deleting a conversation -------------------------------------------- +// A DELETE the engine could not perform (read-only or synced data dir) must +// not blank the open transcript: doing so is how a conversation closed on +// screen and reappeared in the sidebar on the next refresh. +async function bootWithDelete(deleteOk) { + const calls = []; + const dom = new JSDOM(HTML, { + runScripts: 'dangerously', resources: 'usable', url: ORIGIN + '/', + beforeParse(w) { + w.__TAURI__ = { core: { invoke: async () => ({ state: 'ready', port: PORT }) } }; + w.fetch = async (url, opts = {}) => { + const method = opts.method || 'GET'; + calls.push(`${method} ${String(url).replace(`http://127.0.0.1:${PORT}`, '')}`); + const u = String(url); + if (method === 'DELETE') { + return { ok: deleteOk, status: deleteOk ? 200 : 500, + json: async () => ({ ok: deleteOk }), text: async () => '' }; + } + const body = + u.includes('/chats/') ? { id: 'c1', title: 'Hi', messages: [ + { role: 'user', content: 'hi' }, + { role: 'assistant', content: '**hello**' }] } + : u.includes('/chats') ? { chats: [{ id: 'c1', title: 'Hi', updated: 1, count: 2, project: '' }] } + : { ok: true, model: 'gpt-4o-mini', theme: 'system' }; + return { ok: true, status: 200, json: async () => body, text: async () => '' }; + }; + w.requestAnimationFrame = (cb) => setTimeout(cb, 0); + w.navigator.clipboard = { writeText: async () => {} }; + w.confirm = () => true; w.prompt = () => ''; w.alert = () => {}; + Object.defineProperty(w.HTMLElement.prototype, 'scrollIntoView', { value() {} }); + }, + }); + const { window } = dom; + for (let i = 0; i < 60; i++) { + await new Promise((r) => setTimeout(r, 25)); + if (!window.document.getElementById('p').disabled) break; + } + // The confirm dialog is a custom overlay, not window.confirm; auto-accept it. + window.askConfirm = async () => true; + return { doc: window.document, window, calls }; +} + +test('a failed DELETE keeps the open transcript rather than blanking it', async () => { + const { doc, window } = await bootWithDelete(false); + await settle(); + // Open the conversation so the transcript has children and is the active id. + click(doc.querySelector('#chats .chat')); + await settle(); + assert.ok(doc.getElementById('turns').children.length > 0, 'transcript did not open'); + const before = doc.getElementById('turns').children.length; + click(doc.querySelector('#chats .chat .x')); + await settle(); + assert.equal(doc.getElementById('turns').children.length, before, + 'a delete the engine refused still blanked the open transcript'); +}); + +test('a successful DELETE clears the open transcript', async () => { + const { doc, window } = await bootWithDelete(true); + await settle(); + click(doc.querySelector('#chats .chat')); + await settle(); + assert.ok(doc.getElementById('turns').children.length > 0, 'transcript did not open'); + click(doc.querySelector('#chats .chat .x')); + await settle(); + assert.equal(doc.getElementById('turns').children.length, 0, + 'a successful delete left the deleted conversation on screen'); +}); + +// ---- Clear-all with a partial failure ------------------------------------ +// The open transcript must follow storage, not the batch result: if the active +// conversation was deleted but another one failed, the transcript must still be +// blanked -- otherwise it lingers on screen while gone on disk. +async function bootWithClear(failIds) { + const calls = []; + const dom = new JSDOM(HTML, { + runScripts: 'dangerously', resources: 'usable', url: ORIGIN + '/', + beforeParse(w) { + w.__TAURI__ = { core: { invoke: async () => ({ state: 'ready', port: PORT }) } }; + w.fetch = async (url, opts = {}) => { + const method = opts.method || 'GET'; + const path = String(url).replace(`http://127.0.0.1:${PORT}`, ''); + calls.push(`${method} ${path}`); + const u = String(url); + if (method === 'DELETE') { + const id = path.split('/').pop(); + const ok = !failIds.includes(id); + return { ok, status: ok ? 200 : 500, json: async () => ({ ok }), text: async () => '' }; + } + const body = + u.includes('/chats/') ? { id: u.split('/').pop(), title: 'Hi', messages: [ + { role: 'user', content: 'hi' }, + { role: 'assistant', content: '**hello**' }] } + : u.includes('/chats') ? { chats: [ + { id: 'c1', title: 'One', updated: 2, count: 2, project: '' }, + { id: 'c2', title: 'Two', updated: 1, count: 2, project: '' }] } + : { ok: true, model: 'gpt-4o-mini', theme: 'system' }; + return { ok: true, status: 200, json: async () => body, text: async () => '' }; + }; + w.requestAnimationFrame = (cb) => setTimeout(cb, 0); + w.navigator.clipboard = { writeText: async () => {} }; + w.confirm = () => true; w.prompt = () => ''; w.alert = () => {}; + Object.defineProperty(w.HTMLElement.prototype, 'scrollIntoView', { value() {} }); + }, + }); + const { window } = dom; + for (let i = 0; i < 60; i++) { + await new Promise((r) => setTimeout(r, 25)); + if (!window.document.getElementById('p').disabled) break; + } + window.askConfirm = async () => true; + return { doc: window.document, window, calls }; +} + +test('clear-all blanks the active transcript even when another delete fails', async () => { + const { doc, window } = await bootWithClear(['c2']); + await settle(); + // Open c1 so it is the active transcript; c2's delete will fail. + click(doc.querySelector('#chats .chat')); + await settle(); + assert.ok(doc.getElementById('turns').children.length > 0, 'transcript did not open'); + window.runAction({ action: 'clear' }); + await settle(); + assert.equal(doc.getElementById('turns').children.length, 0, + 'the active conversation was deleted from storage but its transcript stayed on screen'); +}); diff --git a/src/praisonai-desktop/ui/index.html b/src/praisonai-desktop/ui/index.html index a413a9fe3..b08acefe0 100644 --- a/src/praisonai-desktop/ui/index.html +++ b/src/praisonai-desktop/ui/index.html @@ -1043,7 +1043,9 @@

Fine-tune a model

ev.stopPropagation(); if(CFG.confirm_delete!==false && !await askConfirm( `Delete \u201c${c.title}\u201d? This cannot be undone.`)) return; - await fetch(`http://127.0.0.1:${PORT}/chats/${c.id}`,{method:'DELETE'}); + const r=await fetch(`http://127.0.0.1:${PORT}/chats/${c.id}`,{method:'DELETE'}) + .catch(()=>null); + if(!r||!r.ok){ toast('Could not delete that conversation.'); return; } if(c.id===chatId){ chatId=rid(); turns.innerHTML=''; syncEmpty(); } refreshChats(); }; @@ -1924,8 +1926,20 @@

Fine-tune a model

closeOverlay(); } else if(def.action==='clear'){ const {chats}=await (await fetch('http://127.0.0.1:'+PORT+'/chats')).json(); - for(const c of chats) await fetch('http://127.0.0.1:'+PORT+'/chats/'+c.id,{method:'DELETE'}); - chatId=rid(); turns.innerHTML=''; syncEmpty(); refreshChats(); closeOverlay(); + let failed=0, activeGone=false; + for(const c of chats){ + const r=await fetch('http://127.0.0.1:'+PORT+'/chats/'+c.id,{method:'DELETE'}) + .catch(()=>null); + if(!r||!r.ok) failed++; + else if(c.id===chatId) activeGone=true; + } + // The open transcript follows storage, not the batch result: if the active + // conversation was one of the deletes that succeeded, blank it even when a + // *different* one failed -- otherwise it lingers on screen while gone on disk. + if(activeGone){ chatId=rid(); turns.innerHTML=''; syncEmpty(); } + if(failed){ toast(`Could not delete ${failed} conversation(s).`); refreshChats(); closeOverlay(); return; } + if(!activeGone){ chatId=rid(); turns.innerHTML=''; syncEmpty(); } + refreshChats(); closeOverlay(); } else if(def.action==='reveal'){ const where=await actualDataFolder(); await navigator.clipboard.writeText(where);