From 0720eb59c05aedd0041d6b3c8a77a2926f1669fc Mon Sep 17 00:00:00 2001 From: jack Date: Mon, 20 Jul 2026 14:13:51 +0800 Subject: [PATCH 1/2] fix(sidebar): deleting the open conversation lands on welcome with a fresh session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deleting the currently-open conversation now transitions to the welcome page backed by a brand-new session, instead of clearing the UI while the backend stays stranded on the deleted task's engine. Frontend (Sidebar.tsx deleteItem): - After a successful delete of the active row, dispatch startNewChat(): it provisions a consistent new engine/task (and reclaims the stale one — its recorder was reset on delete). Merely clearing the UI left the deleted task's engine in the foreground, so the next message ran on it and every event was stamped with the deleted task id — dropped by the WS client filter, making the new conversation appear dead. - Remove the row via a new removeSession reducer that filters the LATEST state inside the reducer: the old render-scope whole-list filter could clobber updates that landed during the delete round-trip, and the setTasks/setSessions lazy-index guard resurrected the deleted row while it was still the current session. - Drop the active session's type-ahead queue BEFORE the delete round-trip: the cancelled run's agent_done reaches the client before the DELETE response and would otherwise drain the queue back into the deleted session (resurrecting its file + index entry on disk). - Re-check the current session after the await (via a ref): if the user navigated to another session while the DELETE was in flight, their navigation wins and the delete no longer yanks the foreground. Backend (sessions.go handleDeleteSession): - When the deleted active task's run hasn't drained within the bounded wait, no longer close the recorder under a live writer (which let the run resurrect the just-deleted JSONL + index entry). Defer the close until the run finishes, then re-run the deletion to sweep up anything the tail wrote — same discipline as automation-run teardown. Verified end-to-end in the browser: open a conversation → delete from the ⋯ menu → welcome page shows, the row is gone from the sidebar, the backend's active session is a fresh UUID, and the project's persisted last-activity time/order are unaffected. --- internal/web/sessions.go | 41 ++++++++++++++++++++++++++-------- web/src/app/store.ts | 8 +++++++ web/src/components/Sidebar.tsx | 37 ++++++++++++++++++++++++------ 3 files changed, 70 insertions(+), 16 deletions(-) diff --git a/internal/web/sessions.go b/internal/web/sessions.go index 234c496..1b2b9d2 100644 --- a/internal/web/sessions.go +++ b/internal/web/sessions.go @@ -227,19 +227,42 @@ func (s *Server) handleDeleteSession(w http.ResponseWriter, r *http.Request) { if eng != s.activeEngine() { s.deleteEngine(id) } else { - // Active task: wait for the cancelled run to drain so its final - // RecordAssistant/usage writes land before we close + reset the recorder - // (a post-close write would re-create and truncate the file). + // Active task: wait (bounded) for the cancelled run to drain so its + // final RecordAssistant/usage writes land before we close + reset + // the recorder (a post-close write would re-create and truncate the + // file). for i := 0; i < 200 && eng.running.Load(); i++ { time.Sleep(5 * time.Millisecond) } - eng.emu.Lock() - if eng.recorder != nil && eng.recorder.UUID() == id { - eng.recorder.Close() - eng.recorder = nil - eng.history = nil + if drained := !eng.running.Load(); drained { + eng.emu.Lock() + if eng.recorder != nil && eng.recorder.UUID() == id { + eng.recorder.Close() + eng.recorder = nil + eng.history = nil + } + eng.emu.Unlock() + } else { + // The run is still draining (a ctx-ignoring tool, a slow LLM + // tail). Closing the recorder now would let the live writer + // resurrect the session file this handler is about to delete. + // Defer the close until the run actually finishes, then re-run + // the deletion to sweep up anything the tail wrote in between + // (same discipline as the automation-run teardown). + go func() { + for i := 0; i < 6000 && eng.running.Load(); i++ { + time.Sleep(5 * time.Millisecond) + } + eng.emu.Lock() + if eng.recorder != nil && eng.recorder.UUID() == id { + eng.recorder.Close() + eng.recorder = nil + eng.history = nil + } + eng.emu.Unlock() + _, _ = session.DeleteSessionByUUID(id) + }() } - eng.emu.Unlock() } } diff --git a/web/src/app/store.ts b/web/src/app/store.ts index 2597b0c..b82ca1f 100644 --- a/web/src/app/store.ts +++ b/web/src/app/store.ts @@ -497,6 +497,14 @@ const sessionSlice = createSlice({ const { uuid: _uuid, ...rest } = a.payload Object.assign(t, rest) }, + /** Remove a deleted session from BOTH lists, inside the reducer (operates + * on the latest state — a component-side filter over render-scope copies + * would clobber any list update that landed during the delete round-trip, + * and unlike setTasks/setSessions this never re-adds anything). */ + removeSession(s, a: { payload: string }) { + s.sessions = s.sessions.filter((x) => x.uuid !== a.payload) + s.tasks = s.tasks.filter((x) => x.uuid !== a.payload) + }, /** Insert or merge a session for the active-project fallback list. */ upsertSession(s, a: { payload: SessionItem }) { const i = s.sessions.findIndex((x) => x.uuid === a.payload.uuid) diff --git a/web/src/components/Sidebar.tsx b/web/src/components/Sidebar.tsx index 2d231fe..72c78d0 100644 --- a/web/src/components/Sidebar.tsx +++ b/web/src/components/Sidebar.tsx @@ -92,6 +92,11 @@ export function Sidebar() { const tasks = useAppSelector((s) => s.session.tasks) const projectTimes = useAppSelector((s) => s.session.projectTimes) const currentSessionId = useAppSelector((s) => s.session.currentSessionId) + // Latest currentSessionId readable from async handlers (deleteItem) after an + // await, when the render-scope value is stale: if the user navigated away + // while the DELETE was in flight, the delete must not yank the foreground. + const currentSessionRef = useRef(currentSessionId) + currentSessionRef.current = currentSessionId const activePath = useAppSelector((s) => s.session.projectPath) const activeView = useAppSelector((s) => s.ui.activeView) @@ -376,19 +381,37 @@ export function Sidebar() { async function deleteItem(row: SessionRow) { const wasActive = row.uuid === currentSessionId + if (wasActive) { + // Drop queued type-ahead for this session BEFORE the delete round-trip: + // the cancelled run's agent_done reaches the client before the DELETE + // response, and would otherwise drain the queue back into the deleted + // session — resurrecting its file + index entry on disk. + dispatch(chatActions.dropSessionQueue(row.uuid)) + } try { await api.deleteSession(row.uuid) } catch { return } - dispatch(sessionActions.setSessions(sessions.filter((s) => s.uuid !== row.uuid))) - dispatch(sessionActions.setTasks(tasks.filter((t) => t.uuid !== row.uuid))) - dispatch(chatActions.dropSessionQueue(row.uuid)) - if (wasActive) { - dispatch(chatActions.clearChat()) - dispatch(sessionActions.setCurrentSession('')) - dispatch(chatActions.setRunning(false)) + // Filter inside the reducer: the render-scope sessions/tasks copies are + // stale after the await (a WS update or refresh may have landed since), + // and a whole-list setTasks would clobber it. + dispatch(sessionActions.removeSession(row.uuid)) + if (!wasActive) { + dispatch(chatActions.dropSessionQueue(row.uuid)) + return } + // The user may have navigated to another session while the DELETE was in + // flight (their click's loadSession wins) — only take over the foreground + // if we're still on the deleted session. + if (currentSessionRef.current !== row.uuid) return + // Land on the welcome page with a FRESH session. Merely clearing the UI + // would strand the backend on the deleted task's engine: the next + // message would run on that stale engine, whose events are stamped with + // the deleted task id and dropped by the WS filter (a conversation that + // appears dead). startNewChat provisions a consistent new engine/task — + // and reclaims the stale one (its recorder was reset on delete). + await dispatch(startNewChat()) } function openContext(e: React.MouseEvent, row: SessionRow) { From 7f7cc162bd43450be67c5c876cf6f9cefaab3ab6 Mon Sep 17 00:00:00 2001 From: jack Date: Mon, 20 Jul 2026 19:56:10 +0800 Subject: [PATCH 2/2] fix(sidebar): block deleting a conversation while the agent is running Refuse DELETE with 409 when the task engine is mid-run, disable the sidebar Delete action for running rows, and drop the cancel-and-drain path that was only needed for delete-while-running. --- internal/web/sessions.go | 58 ++++++++++----------------------- internal/web/tasks_test.go | 28 ++++++++++++++++ web/src/components/Sidebar.tsx | 31 ++++++++++++++---- web/src/i18n/locales/en.ts | 1 + web/src/i18n/locales/ja.ts | 1 + web/src/i18n/locales/ko.ts | 1 + web/src/i18n/locales/zh-Hans.ts | 1 + web/src/i18n/locales/zh-Hant.ts | 1 + 8 files changed, 76 insertions(+), 46 deletions(-) diff --git a/internal/web/sessions.go b/internal/web/sessions.go index 1b2b9d2..de84f80 100644 --- a/internal/web/sessions.go +++ b/internal/web/sessions.go @@ -4,7 +4,6 @@ import ( "encoding/json" "io" "net/http" - "time" "github.com/cloudwego/eino/schema" "github.com/cnjack/jcode/internal/config" @@ -213,10 +212,17 @@ func (s *Server) handleDeleteSession(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusBadRequest, map[string]string{"error": "session id is required"}) return } - // Tear down the live engine for this task (if any) so its run is cancelled and - // resources reclaimed. The active foreground engine is left in place — but its - // recorder is reset to a fresh session so post-delete writes don't land in the - // now-unlinked file (silent data loss). + // Refuse while the agent is mid-run. Cancelling + deleting a live task races + // the recorder (file/index resurrection) and is a bad UX for an intentional + // stop — the user should stop the run first, then delete. + if eng := s.resolveEngine(id); eng != nil && eng.running.Load() { + writeJSON(w, http.StatusConflict, map[string]string{"error": "agent is currently running"}) + return + } + // Tear down the live engine for this task (if any) so leftover cancel state + // is cleared and resources reclaimed. The active foreground engine is left + // in place — but its recorder is reset so post-delete writes don't land in + // the now-unlinked file (silent data loss). if eng := s.resolveEngine(id); eng != nil { eng.emu.Lock() cancel := eng.runCancel @@ -227,42 +233,14 @@ func (s *Server) handleDeleteSession(w http.ResponseWriter, r *http.Request) { if eng != s.activeEngine() { s.deleteEngine(id) } else { - // Active task: wait (bounded) for the cancelled run to drain so its - // final RecordAssistant/usage writes land before we close + reset - // the recorder (a post-close write would re-create and truncate the - // file). - for i := 0; i < 200 && eng.running.Load(); i++ { - time.Sleep(5 * time.Millisecond) - } - if drained := !eng.running.Load(); drained { - eng.emu.Lock() - if eng.recorder != nil && eng.recorder.UUID() == id { - eng.recorder.Close() - eng.recorder = nil - eng.history = nil - } - eng.emu.Unlock() - } else { - // The run is still draining (a ctx-ignoring tool, a slow LLM - // tail). Closing the recorder now would let the live writer - // resurrect the session file this handler is about to delete. - // Defer the close until the run actually finishes, then re-run - // the deletion to sweep up anything the tail wrote in between - // (same discipline as the automation-run teardown). - go func() { - for i := 0; i < 6000 && eng.running.Load(); i++ { - time.Sleep(5 * time.Millisecond) - } - eng.emu.Lock() - if eng.recorder != nil && eng.recorder.UUID() == id { - eng.recorder.Close() - eng.recorder = nil - eng.history = nil - } - eng.emu.Unlock() - _, _ = session.DeleteSessionByUUID(id) - }() + // Not running (guarded above): safe to close the recorder now. + eng.emu.Lock() + if eng.recorder != nil && eng.recorder.UUID() == id { + eng.recorder.Close() + eng.recorder = nil + eng.history = nil } + eng.emu.Unlock() } } diff --git a/internal/web/tasks_test.go b/internal/web/tasks_test.go index 004c68c..133ac44 100644 --- a/internal/web/tasks_test.go +++ b/internal/web/tasks_test.go @@ -196,6 +196,34 @@ func TestUpdateTaskMetaResponseShape(t *testing.T) { } } +// Running tasks must not be deleted: cancel+delete races the recorder and is +// the wrong product behaviour (stop first, then delete). +func TestDeleteSessionRejectsRunning(t *testing.T) { + seedIndex(t, map[string][]session.SessionMeta{ + "/work/p": {{UUID: "run-1", Project: "/work/p"}}, + }) + eng := &Engine{taskID: "run-1", pwd: "/work/p"} + eng.running.Store(true) + s := &Server{Engine: eng, tasks: map[string]*Engine{"run-1": eng}} + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodDelete, "/api/sessions/run-1", nil) + req.SetPathValue("id", "run-1") + s.handleDeleteSession(rec, req) + if rec.Code != http.StatusConflict { + t.Fatalf("delete while running: code=%d body=%q, want 409", rec.Code, rec.Body.String()) + } + all, err := session.ListAllSessions() + if err != nil { + t.Fatal(err) + } + if len(all["/work/p"]) != 1 || all["/work/p"][0].UUID != "run-1" { + t.Fatalf("running session must remain indexed, got %+v", all["/work/p"]) + } + if !eng.running.Load() { + t.Fatal("delete must not clear the running flag") + } +} + // Regression: deleting a task that belongs to a project OTHER than the active // one (s.pwd) must still remove it from the index — the sidebar tree can delete // across projects. Previously this silently no-op'd, leaving a ghost task. diff --git a/web/src/components/Sidebar.tsx b/web/src/components/Sidebar.tsx index 72c78d0..9eb8954 100644 --- a/web/src/components/Sidebar.tsx +++ b/web/src/components/Sidebar.tsx @@ -380,12 +380,14 @@ export function Sidebar() { } async function deleteItem(row: SessionRow) { + // Running tasks must not be deleted — backend also returns 409. Stop first. + if (row.running) return const wasActive = row.uuid === currentSessionId if (wasActive) { // Drop queued type-ahead for this session BEFORE the delete round-trip: - // the cancelled run's agent_done reaches the client before the DELETE - // response, and would otherwise drain the queue back into the deleted - // session — resurrecting its file + index entry on disk. + // a late agent_done (from a prior cancel/stop) can arrive before the + // DELETE response and would otherwise drain the queue back into the + // deleted session — resurrecting its file + index entry on disk. dispatch(chatActions.dropSessionQueue(row.uuid)) } try { @@ -480,6 +482,8 @@ export function Sidebar() { icon={TrashIcon} label={t('sidebar.actions.delete')} danger + disabled={row.running} + title={row.running ? t('sidebar.actions.deleteWhileRunning') : undefined} onClick={() => { void deleteItem(row); closeCtx() }} /> @@ -879,19 +883,34 @@ function CtxItem({ label, onClick, danger, + disabled, + title, }: { icon: React.ComponentType<{ className?: string }> label: string onClick: () => void danger?: boolean + disabled?: boolean + title?: string }) { return (