From c8ff15b46a99270b23b62ed4307b28f0c59cb5b5 Mon Sep 17 00:00:00 2001 From: Tryanks Date: Tue, 6 Oct 2026 21:09:27 +0800 Subject: [PATCH 1/5] fix: unarchiving a thread refreshes its activity time (#515) unarchive_session now stamps updated_at = now on every thread it restores (the root and the cascade members carrying its archive time), so the auto-archive sweep no longer takes an idle thread straight back. A thread that was read before it was archived has its last-visited mark advanced with it, so the restore does not light an unread dot. --- crates/runtime/src/app/sessions.rs | 15 ++++++- crates/runtime/src/app/tests.rs | 67 ++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/crates/runtime/src/app/sessions.rs b/crates/runtime/src/app/sessions.rs index 00f3b1814..183d3d574 100644 --- a/crates/runtime/src/app/sessions.rs +++ b/crates/runtime/src/app/sessions.rs @@ -777,6 +777,8 @@ impl AppState { } /// Restore an archived thread (Settings → Archived Threads → Unarchive). + /// Restoring is activity: without it the auto-archive sweep would take an + /// idle thread straight back. A thread that was read stays read. pub fn unarchive_session(&mut self, session_id: &str, cx: &mut HostCx) { let Some(archived_at) = self .sessions @@ -786,6 +788,8 @@ impl AppState { else { return; }; + let now = now_secs(); + let mut visited_changed = false; let ids = descendant_session_ids(&self.sessions, session_id); for id in ids { let Some(mut meta) = self @@ -796,10 +800,19 @@ impl AppState { else { continue; }; + if let Some(visited) = self.settings.last_visited.get_mut(&id) + && *visited >= meta.updated_at + { + *visited = now; + visited_changed = true; + } meta.archived_at = None; - let meta = meta.clone(); + meta.updated_at = now; self.persist_meta(&meta, cx); } + if visited_changed { + self.persist_settings(cx); + } } /// Sweep one project's visible sessions using the configured idle and diff --git a/crates/runtime/src/app/tests.rs b/crates/runtime/src/app/tests.rs index 2af36f2fe..c4c0c578d 100644 --- a/crates/runtime/src/app/tests.rs +++ b/crates/runtime/src/app/tests.rs @@ -1346,6 +1346,73 @@ fn archive_and_unarchive_apply_exact_timestamp_cascades() { ); } +#[test] +fn unarchived_thread_survives_the_next_auto_archive_sweep() { + let cx = &mut TestAppContext::default(); + let test_store = TestStore::new("tcode-unarchive-sweep-test"); + let root = test_store.root().clone(); + let store = (*test_store).clone(); + let idle_since = now_secs() - 30 * 86_400; + for (id, updated_at, archived_at) in [ + ("recent", now_secs(), None), + ("restored", idle_since, Some(idle_since + 1)), + ] { + let mut meta = SessionMeta::new(ProviderKind::Codex, root.clone(), None); + meta.id = id.into(); + meta.project_id = Some("project".into()); + meta.updated_at = updated_at; + meta.archived_at = archived_at; + store.upsert_meta(&meta).unwrap(); + } + let state = cx.new_entity(TestClientState::new(store.clone())); + state.update(cx, |state, _| { + state.settings.auto_archive_keep_count = 1; + state.settings.auto_archive_max_idle_days = 1; + state + .settings + .last_visited + .insert("restored".into(), idle_since); + }); + + state.dispatch_command( + cx, + 1, + Command::UnarchiveSession { + session_id: "restored".into(), + }, + ); + state.dispatch_command( + cx, + 2, + Command::AutoArchiveSweep { + project_id: "project".into(), + }, + ); + cx.run_until_parked(); + + assert!(cx.drain_outgoing().iter().any(|message| matches!( + message, + HostMessage::Ack { + id: 2, + result: Ok(CommandResponse::ArchivedCount(0)) + } + ))); + let restored = store + .load_index() + .unwrap() + .into_iter() + .find(|meta| meta.id == "restored") + .unwrap(); + assert_eq!(restored.archived_at, None); + assert!(restored.updated_at > idle_since); + state.read(|state| { + assert!( + !state.session_unread(&restored), + "a thread read before it was archived is still read" + ); + }); +} + #[test] fn title_session_uses_configured_model_with_low_effort() { let defaults = title_session_meta(&Settings::default(), PathBuf::from("/tmp/project")); From 90644dfbe533b25e113d0f68d8ad75441e75be6e Mon Sep 17 00:00:00 2001 From: Tryanks Date: Tue, 6 Oct 2026 21:17:20 +0800 Subject: [PATCH 2/5] fix: the orphan worktree sweep leaves worktrees the user kept (#516) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deleting a thread with its worktree kept (the "Keep" choice, Settings → Archived Threads → Delete all, deleting a project) now records the worktree in a kept_worktrees table, committed in the same transaction as the thread's removal. The startup sweep treats a kept worktree like one a thread works in and leaves it, however old and clean it is. The sweep now belongs to the store: cleanup_orphans(store) reads the store's threads (every cwd, so a fork living in a deleted source's worktree keeps it) and its kept list, and sweeps the store's own /worktrees. Worktrees were provisioned in and swept from ~/.tcode/worktrees whatever the data dir, so a TCODE_DATA_DIR or --data-dir profile swept the default profile's worktrees as orphans. For the default data dir (~/.tcode) the location is unchanged. TCODE_WORKTREES_DIR, which only a test used to isolate itself, is gone; that test now checks the worktree lands in its store's data dir. The table is created on open when missing, without a schema version change: an older build ignores it and loses nothing. --- Cargo.lock | 1 + crates/runtime/Cargo.toml | 1 + crates/runtime/src/app/orchestrate.rs | 6 +- crates/runtime/src/app/sessions.rs | 37 ++++----- crates/runtime/src/app/store_write.rs | 19 ++++- crates/runtime/src/app/tests.rs | 106 +++++++++++++++++++------- crates/services/src/store/db.rs | 6 ++ crates/services/src/store/mod.rs | 37 ++++++++- crates/services/src/worktree.rs | 76 +++++++++++------- 9 files changed, 206 insertions(+), 83 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index eb0b37c5e..e0a3dd8d4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10937,6 +10937,7 @@ dependencies = [ "async-channel", "base64 0.23.1", "computer-use-mcp", + "filetime", "log", "mcp-host", "orchestrate-mcp", diff --git a/crates/runtime/Cargo.toml b/crates/runtime/Cargo.toml index d8c39519a..ef8d8c361 100644 --- a/crates/runtime/Cargo.toml +++ b/crates/runtime/Cargo.toml @@ -25,6 +25,7 @@ term = { path = "../term" } uuid = { version = "1", features = ["v4"] } [dev-dependencies] +filetime = "0.2" mcp-host = { path = "../mcp-host" } tcode-services = { path = "../services", features = ["test-support"] } tcode-traverse = { path = "../traverse", default-features = false } diff --git a/crates/runtime/src/app/orchestrate.rs b/crates/runtime/src/app/orchestrate.rs index b11c81c9f..52d0f4fe0 100644 --- a/crates/runtime/src/app/orchestrate.rs +++ b/crates/runtime/src/app/orchestrate.rs @@ -561,6 +561,7 @@ impl AppState { ); meta.title = title; let child_id = meta.id.clone(); + let data_dir = self.store.root().clone(); let host_cx = cx.clone(); HostCx::spawn_detached(cx, async move { let resolved_cwd = host_cx @@ -572,7 +573,7 @@ impl AppState { return Err(format!("invalid cwd: {}", canonical.display())); } if isolate { - Ok(resolve_child_worktree(canonical, &child_id)) + Ok(resolve_child_worktree(canonical, &child_id, &data_dir)) } else { Ok((canonical, None, None)) } @@ -1641,8 +1642,9 @@ pub(super) fn provider_name(provider: ProviderKind) -> &'static str { fn resolve_child_worktree( cwd: PathBuf, child_id: &str, + data_dir: &Path, ) -> (PathBuf, Option, Option) { - match provision(&cwd, child_id) { + match provision(&cwd, child_id, data_dir) { Ok(created) => { let info = WorktreeInfo { root_project_path: cwd, diff --git a/crates/runtime/src/app/sessions.rs b/crates/runtime/src/app/sessions.rs index 183d3d574..bbfa8e532 100644 --- a/crates/runtime/src/app/sessions.rs +++ b/crates/runtime/src/app/sessions.rs @@ -1001,7 +1001,8 @@ impl AppState { /// Permanently delete a thread: stop the provider, close its terminal, /// delete meta + event log, and (when `remove_worktree`) remove the git worktree - /// it was the last user of. + /// it was the last user of. A worktree it owns that is not removed is + /// recorded as kept, so the startup sweep leaves it. pub fn delete_session(&mut self, session_id: &str, remove_worktree: bool, cx: &mut HostCx) { self.clear_approvals(session_id); let meta = self.sessions.iter().find(|m| m.id == session_id).cloned(); @@ -1033,7 +1034,17 @@ impl AppState { self.settings .collapsed_threads .retain(|id| id != session_id); - self.enqueue_store_write(StoreWrite::RemoveSession(session_id.to_string()), cx); + let kept_worktree = meta + .as_ref() + .filter(|meta| meta.worktree.is_some() && !remove_worktree) + .map(|meta| meta.cwd.clone()); + self.enqueue_store_write( + StoreWrite::RemoveSession { + id: session_id.to_string(), + kept_worktree, + }, + cx, + ); // Persist the pruned last-visited map (ignore save errors — cosmetic). self.persist_settings(cx); if meta @@ -1157,24 +1168,13 @@ impl AppState { .is_some_and(|&visited| meta.updated_at > visited) } - /// Remove app-owned worktrees that no session in the loaded store owns or - /// works in. + /// Remove app-owned worktrees that no thread in the store works in and the + /// user did not keep. pub(crate) fn recover_orphaned_worktrees(&self, cx: &mut HostCx) { - let known_ids = self - .sessions - .iter() - .map(|session| session.id.clone()) - .collect(); - let cwds: Vec<_> = self - .sessions - .iter() - .map(|session| session.cwd.clone()) - .collect(); + let store = self.store.clone(); let host_cx = cx.clone(); HostCx::spawn_detached(cx, async move { - let summary = host_cx - .unblock(move || cleanup_orphans(&known_ids, &cwds)) - .await; + let summary = host_cx.unblock(move || cleanup_orphans(&store)).await; if !summary.removed.is_empty() || !summary.skipped.is_empty() { log::info!( "worktree orphan recovery removed {}, left {}", @@ -1212,12 +1212,13 @@ impl AppState { let root = active.meta.cwd.clone(); let root_for_task = root.clone(); + let data_dir = self.store.root().clone(); let target_id = target_id.to_string(); let delivery_key = cx.delivery_key.clone(); let host_cx = cx.clone(); HostCx::spawn_detached(cx, async move { let result = host_cx - .unblock(move || provision(&root_for_task, &session_id_for_task)) + .unblock(move || provision(&root_for_task, &session_id_for_task, &data_dir)) .await; host_cx.enqueue(move |state, cx| { if state diff --git a/crates/runtime/src/app/store_write.rs b/crates/runtime/src/app/store_write.rs index c61148a72..3d42a4ba9 100644 --- a/crates/runtime/src/app/store_write.rs +++ b/crates/runtime/src/app/store_write.rs @@ -18,7 +18,12 @@ pub(super) enum StoreWrite { initial: bool, }, UpsertProject(Project), - RemoveSession(String), + /// A thread's meta and event log, committed together with the record of + /// the worktree it leaves behind. + RemoveSession { + id: String, + kept_worktree: Option, + }, RemoveProject(String), /// The source's event log copied to a new thread, committed together with /// the new thread's meta. @@ -82,7 +87,11 @@ impl StoreWrite { } StoreWrite::UpsertMeta { meta, .. } => vec![Mutation::upsert_meta((**meta).clone())], StoreWrite::UpsertProject(project) => vec![Mutation::upsert_project(project.clone())], - StoreWrite::RemoveSession(id) => vec![Mutation::remove_session(id)], + StoreWrite::RemoveSession { id, kept_worktree } => { + let mut mutations = vec![Mutation::remove_session(id)]; + mutations.extend(kept_worktree.as_deref().map(Mutation::keep_worktree)); + mutations + } StoreWrite::RemoveProject(id) => vec![Mutation::remove_project(id)], StoreWrite::Fork { src, meta, .. } => vec![ Mutation::clone_events(src, &meta.id), @@ -102,7 +111,9 @@ impl StoreWrite { (|error| RuntimeError::PersistSessionIndex { error }, None) } StoreWrite::UpsertProject(_) => (|error| RuntimeError::PersistProject { error }, None), - StoreWrite::RemoveSession(_) => (|error| RuntimeError::DeleteSession { error }, None), + StoreWrite::RemoveSession { .. } => { + (|error| RuntimeError::DeleteSession { error }, None) + } StoreWrite::RemoveProject(_) => (|error| RuntimeError::DeleteProject { error }, None), StoreWrite::Fork { completion, .. } => ( |error| RuntimeError::PersistSession { error }, @@ -314,7 +325,7 @@ impl StoreWriter { StoreWrite::AppendEvent { .. } | StoreWrite::UpsertMeta { .. } | StoreWrite::UpsertProject(_) - | StoreWrite::RemoveSession(_) + | StoreWrite::RemoveSession { .. } | StoreWrite::RemoveProject(_) | StoreWrite::Fork { .. } => unreachable!("database writes are batched"), } diff --git a/crates/runtime/src/app/tests.rs b/crates/runtime/src/app/tests.rs index c4c0c578d..bb602ac28 100644 --- a/crates/runtime/src/app/tests.rs +++ b/crates/runtime/src/app/tests.rs @@ -7282,34 +7282,9 @@ fn orchestrate_dispatch_resolves_cwd_before_reply() { #[test] fn orchestrate_worktree_dispatch_resolves_child_cwd_to_worktree() { - let Some(root) = std::env::var_os("TCODE_TEST_WORKTREE_DISPATCH_ROOT") else { - let data = TestStore::new("tcode-dispatch-worktree"); - // The override must be inherited at process creation: other tests and - // their background workers can read it without participating in a lock. - let output = tcode_services::process::command(std::env::current_exe().unwrap()) - .args([ - "--exact", - "app::tests::orchestrate_worktree_dispatch_resolves_child_cwd_to_worktree", - "--nocapture", - ]) - .env("TCODE_TEST_WORKTREE_DISPATCH_ROOT", data.root()) - .env( - "TCODE_WORKTREES_DIR", - data.root().join("tcode-owned-worktrees"), - ) - .output() - .unwrap(); - assert!( - output.status.success(), - "child test failed:\n{}\n{}", - String::from_utf8_lossy(&output.stdout), - String::from_utf8_lossy(&output.stderr) - ); - return; - }; - let root = PathBuf::from(root); - let isolated_worktrees = root.join("tcode-owned-worktrees"); let cx = &mut TestAppContext::default(); + let root = + std::env::temp_dir().join(format!("tcode-dispatch-worktree-{}", uuid::Uuid::new_v4())); std::fs::create_dir_all(&root).unwrap(); run_git(&root, &["init", "-b", "main"]).unwrap(); run_git(&root, &["config", "commit.gpgsign", "false"]).unwrap(); @@ -7322,6 +7297,7 @@ fn orchestrate_worktree_dispatch_resolves_child_cwd_to_worktree() { run_git(&root, &["commit", "-m", "initial"]).unwrap(); let test_store = TestStore::new("tcode-dispatch-worktree-data"); + let data_worktrees = test_store.root().join("worktrees"); let scripted = scripted_provider(ProviderKind::Codex); let state = cx.new_entity({ let mut state = TestClientState::new((*test_store).clone()); @@ -7358,7 +7334,7 @@ fn orchestrate_worktree_dispatch_resolves_child_cwd_to_worktree() { let response = recv_dispatch_reply(cx, &response).unwrap(); let child_id = response["thread_id"].as_str().unwrap().to_string(); let expected_branch = format!("tcode/{child_id}"); - let expected_path = isolated_worktrees.join(&child_id); + let expected_path = data_worktrees.join(&child_id); assert_eq!( response["worktree_path"], expected_path.display().to_string(), @@ -7380,6 +7356,80 @@ fn orchestrate_worktree_dispatch_resolves_child_cwd_to_worktree() { let _ = std::fs::remove_dir_all(&root); } +#[test] +fn orphan_sweep_leaves_worktrees_a_thread_works_in_or_the_user_kept() { + let cx = &mut TestAppContext::default(); + let test_store = TestStore::new("tcode-orphan-sweep"); + let store = (*test_store).clone(); + let repo = test_store.root().join("repo"); + std::fs::create_dir_all(&repo).unwrap(); + for args in [ + &["init", "-b", "main"][..], + &["config", "commit.gpgsign", "false"], + &["config", "core.hooksPath", ".no-hooks"], + &["config", "user.name", "tcode"], + &["config", "user.email", "tcode@localhost"], + ] { + run_git(&repo, args).unwrap(); + } + std::fs::write(repo.join("tracked.txt"), "initial\n").unwrap(); + run_git(&repo, &["add", "tracked.txt"]).unwrap(); + run_git(&repo, &["commit", "-m", "initial"]).unwrap(); + let owner = |id: &str| { + let created = provision(&repo, id, store.root()).unwrap(); + let mut meta = SessionMeta::new(ProviderKind::Codex, created.path, None); + meta.id = id.into(); + meta.worktree = Some(WorktreeInfo { + root_project_path: repo.clone(), + base: created.base, + branch: created.branch, + }); + meta + }; + let source = owner("source"); + let kept = owner("kept"); + let orphan = owner("orphan").cwd; + let mut fork = SessionMeta::new(ProviderKind::Codex, source.cwd.clone(), None); + fork.id = "fork".into(); + for meta in [&source, &kept, &fork] { + store.upsert_meta(meta).unwrap(); + } + let state = cx.new_entity(TestClientState::new(store.clone())); + + state.dispatch_command( + cx, + 1, + Command::DeleteSession { + session_id: "source".into(), + remove_worktree: true, + }, + ); + state.dispatch_command( + cx, + 2, + Command::DeleteSession { + session_id: "kept".into(), + remove_worktree: false, + }, + ); + cx.run_until_parked(); + assert!(source.cwd.exists(), "the fork still works in it"); + // The next start is past the age that marks a worktree as abandoned. + let abandoned = filetime::FileTime::from_system_time( + std::time::SystemTime::now() - Duration::from_secs(2 * 60 * 60), + ); + for path in [&source.cwd, &kept.cwd, &orphan] { + filetime::set_file_mtime(path, abandoned).unwrap(); + } + + let summary = cleanup_orphans(&store); + + assert_eq!(summary.removed, std::slice::from_ref(&orphan)); + assert!(!orphan.exists()); + assert!(source.cwd.join("tracked.txt").exists()); + assert!(kept.cwd.join("tracked.txt").exists()); +} + #[test] fn mux_clients_target_independent_drafts_and_receive_only_their_session_tail() { use crate::pipe::{HostServices, spawn_host}; diff --git a/crates/services/src/store/db.rs b/crates/services/src/store/db.rs index a50c38da8..7e9761236 100644 --- a/crates/services/src/store/db.rs +++ b/crates/services/src/store/db.rs @@ -28,6 +28,12 @@ CREATE TABLE events ( ); "; +/// Worktrees whose thread was deleted with "keep": the orphan sweep leaves +/// them. Added to version 1 without a version change, since an older build +/// that ignores the table loses nothing, so every open creates it if missing. +pub(super) const KEPT_WORKTREES: &str = + "CREATE TABLE IF NOT EXISTS kept_worktrees (path TEXT PRIMARY KEY)"; + /// A failure after which the connection or database can no longer be /// trusted; the store stops serving every handle once it sees one. #[derive(Debug)] diff --git a/crates/services/src/store/mod.rs b/crates/services/src/store/mod.rs index 60b5cd586..cf7a0cf37 100644 --- a/crates/services/src/store/mod.rs +++ b/crates/services/src/store/mod.rs @@ -7,6 +7,8 @@ //! * `events` holds each thread's log as raw byte segments, one per line //! including its `\n`, densely numbered from 0: the bytes of a `{ ts, event }` //! record, or whatever a migrated or imported log contained. +//! * `kept_worktrees` holds the path of every worktree whose thread was +//! deleted with the worktree kept. //! //! Model and command caches stay as JSON files beside it. //! @@ -36,7 +38,7 @@ use serde::{Deserialize, Serialize}; use tcode_core::project::{IndexFile, Project, SessionMeta}; use tcode_core::session::StoredEvent; -use db::{Broken, Db, SCHEMA_VERSION, blob, integer, is_broken}; +use db::{Broken, Db, KEPT_WORKTREES, SCHEMA_VERSION, blob, integer, is_broken}; const DATA_DIR_ENV: &str = "TCODE_DATA_DIR"; const DB_FILE: &str = "tcode.db"; @@ -202,6 +204,7 @@ enum Op { UpsertProject(Box), RemoveSession(String), RemoveProject(String), + KeepWorktree(PathBuf), } impl Mutation { @@ -255,6 +258,11 @@ impl Mutation { Self(Op::RemoveProject(id.to_owned())) } + /// Record that the worktree at `path` outlives its deleted thread. + pub fn keep_worktree(path: &Path) -> Self { + Self(Op::KeepWorktree(path.to_owned())) + } + /// Approximate bytes this change writes, for bounding a batch. pub fn payload_len(&self) -> usize { match &self.0 { @@ -272,7 +280,10 @@ impl Mutation { } Op::CloneEvents { dst, .. } => Some(dst), Op::RemoveSession(id) => Some(id), - Op::UpsertMeta(_) | Op::UpsertProject(_) | Op::RemoveProject(_) => None, + Op::UpsertMeta(_) + | Op::UpsertProject(_) + | Op::RemoveProject(_) + | Op::KeepWorktree(_) => None, } } } @@ -838,6 +849,20 @@ impl SessionStore { pub fn remove_session(&self, id: &str) -> io::Result<()> { self.apply(&[Mutation::remove_session(id)]) } + + /// Every worktree recorded by [`Mutation::keep_worktree`]. + pub fn kept_worktrees(&self) -> io::Result> { + self.run("read kept worktrees", |db| { + db.read(|db, connection| { + let mut paths = Vec::new(); + db.query(connection, "SELECT path FROM kept_worktrees", (), |row| { + paths.push(PathBuf::from(db::text(row, 0)?)); + Ok(()) + })?; + Ok(paths) + }) + }) + } } impl Shared { @@ -961,6 +986,7 @@ fn open_live(root: &Path, previous: Option<&Path>, ownership: File) -> io::Resul )); } } + db.write(|db, connection| db.execute(connection, KEPT_WORKTREES, ()))?; migrate::warn_about_stray_sources(root); migrate::remove_legacy(root); Ok(Live { @@ -1180,6 +1206,13 @@ fn apply_op( (id.as_str(),), )?; } + Op::KeepWorktree(path) => { + db.execute( + connection, + "INSERT INTO kept_worktrees (path) VALUES (?1) ON CONFLICT (path) DO NOTHING", + (path.to_string_lossy().as_ref(),), + )?; + } } Ok(()) } diff --git a/crates/services/src/worktree.rs b/crates/services/src/worktree.rs index 2722c68ee..dc2d0c6f7 100644 --- a/crates/services/src/worktree.rs +++ b/crates/services/src/worktree.rs @@ -9,9 +9,9 @@ use std::path::{Component, Path, PathBuf}; use std::time::{Duration, SystemTime}; use crate::git::run_git; +use crate::store::SessionStore; const WORKTREE_SEED_LIMIT_BYTES: u64 = 512 * 1024 * 1024; -const WORKTREES_DIR_ENV: &str = "TCODE_WORKTREES_DIR"; /// A fresh directory may belong to another running tcode process or store. /// One hour is deliberately conservative: startup recovery favors preserving @@ -114,12 +114,17 @@ impl std::fmt::Display for MergeBackError { impl std::error::Error for MergeBackError {} -/// Provision `session_id` from the branch currently checked out at `root`. +/// Provision `session_id` from the branch currently checked out at `root`, in +/// the worktrees directory of the data dir `data_dir`. /// /// `root` must itself be the canonical main repository root. The target path, /// requested branch, collision suffix, base revision, seeding, and rollback are /// all owned by this module. -pub fn provision(root: &Path, session_id: &str) -> Result { +pub fn provision( + root: &Path, + session_id: &str, + data_dir: &Path, +) -> Result { let canonical_root = root .canonicalize() .map_err(|error| ProvisionError::Git(error.to_string()))?; @@ -146,7 +151,7 @@ pub fn provision(root: &Path, session_id: &str) -> Result Result { .map_err(MergeBackError::Git) } -fn worktrees_root() -> PathBuf { - std::env::var_os(WORKTREES_DIR_ENV) - .filter(|path| !path.is_empty()) - .map(PathBuf::from) - .unwrap_or_else(|| { - dirs::home_dir() - .unwrap_or_else(std::env::temp_dir) - .join(".tcode") - .join("worktrees") - }) +/// A data dir's worktrees live inside it: its sweep can only tell which ones +/// its own threads use or keep. +fn worktrees_root(data_dir: &Path) -> PathBuf { + data_dir.join("worktrees") } -/// Remove old, clean app-owned worktrees that no known session owns or works in. +/// Remove old, clean app-owned worktrees in `store`'s data dir that none of +/// its threads works in and that were not kept when their thread was deleted. /// -/// Tests and isolated processes may set `TCODE_WORKTREES_DIR`; production falls -/// back to `~/.tcode/worktrees`. Fresh unknown entries are presumed live and -/// preserved for at least [`ORPHAN_MIN_AGE`]. Removal is never forced: a -/// worktree the user kept on delete may hold uncommitted work. -pub fn cleanup_orphans( - known_session_ids: &HashSet, - session_cwds: &[PathBuf], -) -> CleanupSummary { +/// Fresh unknown entries are presumed live and preserved for at least +/// [`ORPHAN_MIN_AGE`]. Removal is never forced: a worktree may hold +/// uncommitted work. Nothing is removed when the store cannot be read. +pub fn cleanup_orphans(store: &SessionStore) -> CleanupSummary { + let sessions = match store.read_file() { + Ok(file) => file.sessions, + Err(error) => { + log::warn!("skipping orphaned worktree recovery: {error}"); + return CleanupSummary::default(); + } + }; + let kept = match store.kept_worktrees() { + Ok(kept) => kept, + Err(error) => { + log::warn!("skipping orphaned worktree recovery: {error}"); + return CleanupSummary::default(); + } + }; + let known_ids = sessions.iter().map(|meta| meta.id.clone()).collect(); + let in_use: Vec<_> = sessions + .into_iter() + .map(|meta| meta.cwd) + .chain(kept) + .collect(); cleanup_orphans_at( - &worktrees_root(), - known_session_ids, - session_cwds, + &worktrees_root(store.root()), + &known_ids, + &in_use, SystemTime::now(), ORPHAN_MIN_AGE, ) } +/// `in_use` holds paths that keep the worktree containing them: thread cwds +/// and kept worktrees. fn cleanup_orphans_at( worktrees: &Path, known_session_ids: &HashSet, - session_cwds: &[PathBuf], + in_use: &[PathBuf], now: SystemTime, minimum_age: Duration, ) -> CleanupSummary { @@ -443,7 +461,7 @@ fn cleanup_orphans_at( .file_type() .is_ok_and(|kind| kind.is_dir() && !kind.is_symlink()); if known_session_ids.contains(&session_id) - || session_cwds.iter().any(|cwd| cwd.starts_with(&path)) + || in_use.iter().any(|used| used.starts_with(&path)) || !is_directory { continue; @@ -1000,7 +1018,7 @@ mod tests { let kept = provision_for_test(&root, "kept", &worktrees).path; // A deleted source whose fork still runs in its worktree. let forked = provision_for_test(&root, "deleted-source", &worktrees).path; - // Deleted with "Keep worktree" while holding uncommitted work. + // Unknown, but holding uncommitted work: removal is never forced. let dirty = provision_for_test(&root, "kept-on-delete", &worktrees).path; std::fs::write(dirty.join("notes.txt"), "uncommitted\n").unwrap(); let known = HashSet::from(["kept".to_string()]); From 930294c078e00c964f2880cc660a973d77402647 Mon Sep 17 00:00:00 2001 From: Tryanks Date: Tue, 6 Oct 2026 21:30:58 +0800 Subject: [PATCH 3/5] fix: deleting a thread deletes every thread under it (#518) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit delete_session now deletes the thread and its whole descendant tree (the tree archive and unarchive act on) in one command: every member's provider, background work, terminals, approvals and MCP registrations are closed, and all of their metas and event logs are removed in one store transaction (StoreWrite::RemoveSessions), together with the kept-worktree records. Worktrees owned in the tree follow the same remove_worktree choice; one is removed only when no thread outside the tree works in it. Deleting a project deletes the union of its threads' trees the same way. descendant_session_ids moves to tcode_core::project so the host's cascade and the client's count are the same tree. The confirmation (sidebar and Settings → Archived Threads) states how many threads go when there are descendants (sidebar.delete_tree_description, en and zh-CN). The client counts from its index replica plus the archived threads, which it holds on the Archived page and otherwise fetches once with the existing ArchivedSessions query, since orchestrated children are often archived and absent from the index. The wire is unchanged. --- crates/core/src/project.rs | 32 ++++++ crates/runtime/src/app/lifecycle.rs | 1 - crates/runtime/src/app/mod.rs | 2 +- crates/runtime/src/app/sessions.rs | 155 ++++++++++++++------------ crates/runtime/src/app/store_write.rs | 31 ++++-- crates/runtime/src/app/subagents.rs | 1 - crates/runtime/src/app/tests.rs | 64 +++++++++++ crates/ui/src/settings_page.rs | 35 ++---- crates/ui/src/sidebar.rs | 104 ++++++++++++----- crates/ui/src/store/mod.rs | 121 +++++++++++++++++++- locales/en.yml | 1 + locales/zh-CN.yml | 1 + 12 files changed, 406 insertions(+), 142 deletions(-) diff --git a/crates/core/src/project.rs b/crates/core/src/project.rs index 74ef128ee..044810d50 100644 --- a/crates/core/src/project.rs +++ b/crates/core/src/project.rs @@ -263,6 +263,38 @@ pub struct AutoArchiveExemptions { pub active: HashSet, } +/// `root_id` and every thread under it, each parent before its children: +/// the threads archive, unarchive and delete act on together. Empty when +/// `root_id` is not among `sessions`. +pub fn descendant_session_ids<'a>( + sessions: impl IntoIterator, + root_id: &str, +) -> Vec { + let mut children: HashMap<&str, Vec<&str>> = HashMap::new(); + let mut found = false; + for meta in sessions { + found |= meta.id == root_id; + if let Some(parent) = meta.parent_session_id.as_deref() { + children.entry(parent).or_default().push(&meta.id); + } + } + if !found { + return Vec::new(); + } + let mut visited = HashSet::from([root_id]); + let mut output = vec![root_id.to_owned()]; + let mut next = 0; + while let Some(id) = output.get(next).cloned() { + for &child in children.get(id.as_str()).into_iter().flatten() { + if visited.insert(child) { + output.push(child.to_owned()); + } + } + next += 1; + } + output +} + /// Return the cascade-closed ids eligible for auto-archive in one project. /// `sessions` may contain only non-archived entries; archived entries are also /// defensively ignored here so they cannot consume ranking slots. diff --git a/crates/runtime/src/app/lifecycle.rs b/crates/runtime/src/app/lifecycle.rs index 7355890d6..66d44d46e 100644 --- a/crates/runtime/src/app/lifecycle.rs +++ b/crates/runtime/src/app/lifecycle.rs @@ -1,4 +1,3 @@ -use super::sessions::descendant_session_ids; use super::*; impl AppState { diff --git a/crates/runtime/src/app/mod.rs b/crates/runtime/src/app/mod.rs index 5f27b2dc1..dc7faf37b 100644 --- a/crates/runtime/src/app/mod.rs +++ b/crates/runtime/src/app/mod.rs @@ -27,7 +27,7 @@ use tcode_core::attachments::mime_from_path; use tcode_core::git::{GitAction, GitStatus, build_commit_prompt, sanitize_commit_message}; use tcode_core::project::{ AutoArchiveConfig, AutoArchiveExemptions, Project, SessionMeta, WorktreeInfo, WorktreeSharing, - auto_archive_candidates, + auto_archive_candidates, descendant_session_ids, }; use tcode_core::provider_status::ProviderSnapshot; use tcode_core::relay::{ diff --git a/crates/runtime/src/app/sessions.rs b/crates/runtime/src/app/sessions.rs index bbfa8e532..db16868b8 100644 --- a/crates/runtime/src/app/sessions.rs +++ b/crates/runtime/src/app/sessions.rs @@ -999,63 +999,93 @@ impl AppState { Some(fork_id) } - /// Permanently delete a thread: stop the provider, close its terminal, - /// delete meta + event log, and (when `remove_worktree`) remove the git worktree - /// it was the last user of. A worktree it owns that is not removed is - /// recorded as kept, so the startup sweep leaves it. + /// Permanently delete a thread and every thread under it (the tree + /// archive acts on): stop their providers, close their terminals, delete + /// their metas and event logs in one transaction, and (when + /// `remove_worktree`) remove each git worktree they own that no remaining + /// thread works in. A worktree they own that is not removed is recorded as + /// kept, so the startup sweep leaves it. pub fn delete_session(&mut self, session_id: &str, remove_worktree: bool, cx: &mut HostCx) { - self.clear_approvals(session_id); - let meta = self.sessions.iter().find(|m| m.id == session_id).cloned(); - if self.residents.live.contains_key(session_id) { - // shutdown_active drops the ActiveSession (and its terminal PTY). - self.shutdown_active(session_id, cx); + let mut ids = descendant_session_ids(&self.sessions, session_id); + if ids.is_empty() { + ids.push(session_id.to_string()); + } + self.delete_session_ids(&ids, remove_worktree, cx); + } + + fn delete_session_ids(&mut self, ids: &[String], remove_worktree: bool, cx: &mut HostCx) { + if ids.is_empty() { + return; + } + let deleted: HashSet<&str> = ids.iter().map(String::as_str).collect(); + let metas: Vec<_> = self + .sessions + .iter() + .filter(|meta| deleted.contains(meta.id.as_str())) + .cloned() + .collect(); + let mut terminal_preferences_changed = false; + for id in ids { + self.clear_approvals(id); + if self.residents.live.contains_key(id) { + // shutdown_active drops the ActiveSession (and its terminal PTY). + self.shutdown_active(id, cx); + } + // Deleting a thread that is working in the background kills it for real. + self.drop_background(id, cx); + self.terminal_workspaces + .remove(&ConversationDestination::Thread(id.clone())); + terminal_preferences_changed |= self.terminal_preferences.remove(id).is_some(); + self.revoke_orchestrate_child_registration(id); + self.close_orchestrator_children(id, cx); + self.settings.last_visited.remove(id); } - // Deleting a thread that is working in the background kills it for real. - self.drop_background(session_id, cx); - self.terminal_workspaces - .remove(&ConversationDestination::Thread(session_id.to_string())); - if self.terminal_preferences.remove(session_id).is_some() { + if terminal_preferences_changed { self.write_terminal_preferences(cx); } - self.close_orchestrator_children(session_id, cx); - let worktree_remove = meta.as_ref().and_then(|meta| { - let worktree = meta.worktree.as_ref().filter(|_| remove_worktree)?; - let shared = self.worktree_sharing().is_shared(meta); - if shared { + self.settings + .collapsed_threads + .retain(|id| !deleted.contains(id.as_str())); + if metas + .iter() + .any(|meta| meta.archived_at.is_some() || self.archived_sharing_affected(meta)) + { + self.archived_revision += 1; + } + self.sessions + .retain(|meta| !deleted.contains(meta.id.as_str())); + let mut kept_worktrees = Vec::new(); + let mut worktree_removals = Vec::new(); + let sharing = self.worktree_sharing(); + for meta in &metas { + let Some(worktree) = &meta.worktree else { + continue; + }; + if !remove_worktree { + kept_worktrees.push(meta.cwd.clone()); + } else if sharing.is_shared(meta) { log::info!( "keeping worktree {} in use by another thread", meta.cwd.display() ); - return None; + } else { + worktree_removals.push(( + meta.id.clone(), + worktree.root_project_path.clone(), + meta.cwd.clone(), + )); } - Some((worktree.root_project_path.clone(), meta.cwd.clone())) - }); - self.settings.last_visited.remove(session_id); - self.settings - .collapsed_threads - .retain(|id| id != session_id); - let kept_worktree = meta - .as_ref() - .filter(|meta| meta.worktree.is_some() && !remove_worktree) - .map(|meta| meta.cwd.clone()); + } self.enqueue_store_write( - StoreWrite::RemoveSession { - id: session_id.to_string(), - kept_worktree, + StoreWrite::RemoveSessions { + ids: ids.to_vec(), + kept_worktrees, }, cx, ); // Persist the pruned last-visited map (ignore save errors — cosmetic). self.persist_settings(cx); - if meta - .as_ref() - .is_some_and(|meta| meta.archived_at.is_some() || self.archived_sharing_affected(meta)) - { - self.archived_revision += 1; - } - self.sessions.retain(|meta| meta.id != session_id); - if let Some((root, cwd)) = worktree_remove { - let deleted_id = session_id.to_string(); + for (deleted_id, root, cwd) in worktree_removals { let host_cx = cx.clone(); HostCx::spawn_detached(cx, async move { let result = host_cx @@ -1107,9 +1137,19 @@ impl AppState { { self.write_terminal_preferences(cx); } + let mut ids = Vec::new(); + let mut seen = HashSet::new(); for session_id in session_ids { - self.delete_session(&session_id, false, cx); + if seen.contains(&session_id) { + continue; + } + for id in descendant_session_ids(&self.sessions, &session_id) { + if seen.insert(id.clone()) { + ids.push(id); + } + } } + self.delete_session_ids(&ids, false, cx); self.enqueue_store_write(StoreWrite::RemoveProject(project_id.to_string()), cx); self.settings .collapsed_projects @@ -1666,32 +1706,3 @@ impl AppState { self.refresh_git_status(&session_id, cx); } } - -pub(super) fn descendant_session_ids(sessions: &[SessionMeta], root_id: &str) -> Vec { - fn append( - sessions: &[SessionMeta], - session_id: &str, - visited: &mut HashSet, - output: &mut Vec, - ) { - if !visited.insert(session_id.to_string()) { - return; - } - output.push(session_id.to_string()); - let children: Vec<_> = sessions - .iter() - .filter(|meta| meta.parent_session_id.as_deref() == Some(session_id)) - .map(|meta| meta.id.clone()) - .collect(); - for child in children { - append(sessions, &child, visited, output); - } - } - - if !sessions.iter().any(|meta| meta.id == root_id) { - return Vec::new(); - } - let mut output = Vec::new(); - append(sessions, root_id, &mut HashSet::new(), &mut output); - output -} diff --git a/crates/runtime/src/app/store_write.rs b/crates/runtime/src/app/store_write.rs index 3d42a4ba9..c7e3d0d4e 100644 --- a/crates/runtime/src/app/store_write.rs +++ b/crates/runtime/src/app/store_write.rs @@ -18,11 +18,11 @@ pub(super) enum StoreWrite { initial: bool, }, UpsertProject(Project), - /// A thread's meta and event log, committed together with the record of - /// the worktree it leaves behind. - RemoveSession { - id: String, - kept_worktree: Option, + /// Threads' metas and event logs, removed together and with the record of + /// the worktrees they leave behind. + RemoveSessions { + ids: Vec, + kept_worktrees: Vec, }, RemoveProject(String), /// The source's event log copied to a new thread, committed together with @@ -87,11 +87,18 @@ impl StoreWrite { } StoreWrite::UpsertMeta { meta, .. } => vec![Mutation::upsert_meta((**meta).clone())], StoreWrite::UpsertProject(project) => vec![Mutation::upsert_project(project.clone())], - StoreWrite::RemoveSession { id, kept_worktree } => { - let mut mutations = vec![Mutation::remove_session(id)]; - mutations.extend(kept_worktree.as_deref().map(Mutation::keep_worktree)); - mutations - } + StoreWrite::RemoveSessions { + ids, + kept_worktrees, + } => ids + .iter() + .map(|id| Mutation::remove_session(id)) + .chain( + kept_worktrees + .iter() + .map(|path| Mutation::keep_worktree(path)), + ) + .collect(), StoreWrite::RemoveProject(id) => vec![Mutation::remove_project(id)], StoreWrite::Fork { src, meta, .. } => vec![ Mutation::clone_events(src, &meta.id), @@ -111,7 +118,7 @@ impl StoreWrite { (|error| RuntimeError::PersistSessionIndex { error }, None) } StoreWrite::UpsertProject(_) => (|error| RuntimeError::PersistProject { error }, None), - StoreWrite::RemoveSession { .. } => { + StoreWrite::RemoveSessions { .. } => { (|error| RuntimeError::DeleteSession { error }, None) } StoreWrite::RemoveProject(_) => (|error| RuntimeError::DeleteProject { error }, None), @@ -325,7 +332,7 @@ impl StoreWriter { StoreWrite::AppendEvent { .. } | StoreWrite::UpsertMeta { .. } | StoreWrite::UpsertProject(_) - | StoreWrite::RemoveSession { .. } + | StoreWrite::RemoveSessions { .. } | StoreWrite::RemoveProject(_) | StoreWrite::Fork { .. } => unreachable!("database writes are batched"), } diff --git a/crates/runtime/src/app/subagents.rs b/crates/runtime/src/app/subagents.rs index 841aaab68..c56f592e0 100644 --- a/crates/runtime/src/app/subagents.rs +++ b/crates/runtime/src/app/subagents.rs @@ -1,4 +1,3 @@ -use super::sessions::descendant_session_ids; use super::*; /// A Subagent item recorded inside a mirror: where it was recorded and its diff --git a/crates/runtime/src/app/tests.rs b/crates/runtime/src/app/tests.rs index bb602ac28..f55910cee 100644 --- a/crates/runtime/src/app/tests.rs +++ b/crates/runtime/src/app/tests.rs @@ -1346,6 +1346,70 @@ fn archive_and_unarchive_apply_exact_timestamp_cascades() { ); } +#[test] +fn deleting_a_thread_deletes_every_descendant_in_one_command() { + let cx = &mut TestAppContext::default(); + let test_store = TestStore::new("tcode-delete-cascade-test"); + let root = test_store.root().clone(); + let store = (*test_store).clone(); + for (id, parent) in [ + ("parent", None), + ("child", Some("parent")), + ("grandchild", Some("child")), + ("other", None), + ] { + let mut meta = SessionMeta::new(ProviderKind::Codex, root.clone(), None); + meta.id = id.into(); + meta.parent_session_id = parent.map(str::to_string); + if id == "child" { + meta.archived_at = Some(1); + } + store.upsert_meta(&meta).unwrap(); + store + .append_event(id, 1, &AgentEvent::Warning { message: id.into() }) + .unwrap(); + } + let state = cx.new_entity(TestClientState::new(store.clone())); + let (commands, background) = smol::channel::unbounded(); + state.update(cx, |state, _| { + let mut grandchild = live_session(ProviderKind::Codex, commands); + grandchild.meta = state.find_meta("grandchild").unwrap(); + state + .residents + .parked + .insert("grandchild".into(), grandchild); + }); + + state.dispatch_command( + cx, + 1, + Command::DeleteSession { + session_id: "parent".into(), + remove_worktree: false, + }, + ); + cx.run_until_parked(); + + assert!(matches!( + background.try_recv(), + Ok(SessionCommand::Shutdown) + )); + state.update(cx, |state, _| state.close_store().unwrap()); + let reopened = SessionStore::open_at(root).unwrap(); + let remaining: Vec<_> = reopened + .load_index() + .unwrap() + .into_iter() + .map(|meta| meta.id) + .collect(); + assert_eq!(remaining, ["other"]); + for id in ["parent", "child", "grandchild"] { + assert!(reopened.read_event_log(id).unwrap().is_empty(), "{id}"); + } + assert!(!reopened.read_event_log("other").unwrap().is_empty()); + reopened.close().unwrap(); +} + #[test] fn unarchived_thread_survives_the_next_auto_archive_sweep() { let cx = &mut TestAppContext::default(); diff --git a/crates/ui/src/settings_page.rs b/crates/ui/src/settings_page.rs index 577370d92..5e955e1f9 100644 --- a/crates/ui/src/settings_page.rs +++ b/crates/ui/src/settings_page.rs @@ -2350,33 +2350,14 @@ impl SettingsPage { window: &mut Window, cx: &mut Context, ) { - let store = self.store.clone(); - let session_id = session_id.to_string(); - let title = title.to_string(); - window.open_alert_dialog(cx, move |alert, _, cx| { - let alert = alert.bg(cx.theme().popover); - let store = store.clone(); - let session_id = session_id.clone(); - alert - .title(crate::tr!("sidebar.delete_title", title = title.clone())) - .description(crate::tr!("sidebar.delete_description")) - .button_props( - DialogButtons::default() - .ok_variant(ButtonVariant::Danger) - .ok_text(crate::tr!("settings.delete_permanently")) - .cancel_text(crate::tr!("settings.cancel")) - .show_cancel(true), - ) - .on_ok(move |_, window, cx| { - let store = store.clone(); - let session_id = session_id.clone(); - // The alert closes after this callback; open the next prompt afterwards. - window.defer(cx, move |window, cx| { - crate::sidebar::proceed_delete(store, session_id, window, cx); - }); - true - }) - }); + crate::sidebar::confirm_delete( + self.store.clone(), + session_id.to_string(), + title.to_string(), + crate::tr!("settings.delete_permanently").into(), + window, + cx, + ); } /// Bulk permanent delete. Always confirms, whatever diff --git a/crates/ui/src/sidebar.rs b/crates/ui/src/sidebar.rs index e1457f557..b07a81dd2 100644 --- a/crates/ui/src/sidebar.rs +++ b/crates/ui/src/sidebar.rs @@ -1559,8 +1559,9 @@ impl SessionsSidebar { }); } - /// Permanently delete a thread: an optional confirm, then (when it orphans a - /// worktree) a second "remove the worktree too?" prompt. + /// Permanently delete a thread and every thread under it: an optional + /// confirm, then (when it orphans a worktree) a second "remove the worktree + /// too?" prompt. fn delete_thread( &mut self, session_id: &str, @@ -1575,31 +1576,14 @@ impl SessionsSidebar { proceed_delete(store, session_id, window, cx); return; } - let title = title.to_string(); - window.open_alert_dialog(cx, move |alert, _, cx| { - let alert = alert.bg(cx.theme().popover); - let store = store.clone(); - let session_id = session_id.clone(); - alert - .title(crate::tr!("sidebar.delete_title", title = title.clone())) - .description(crate::tr!("sidebar.delete_description")) - .button_props( - DialogButtons::default() - .ok_variant(ButtonVariant::Danger) - .ok_text(crate::tr!("sidebar.delete_action")) - .cancel_text(crate::tr!("settings.cancel")) - .show_cancel(true), - ) - .on_ok(move |_, window, cx| { - let store = store.clone(); - let session_id = session_id.clone(); - // The alert closes after this callback; open the next prompt afterwards. - window.defer(cx, move |window, cx| { - proceed_delete(store, session_id, window, cx); - }); - true - }) - }); + confirm_delete( + store, + session_id, + title.to_string(), + crate::tr!("sidebar.delete_action").into(), + window, + cx, + ); } fn render_app_row(&self, window: &mut Window, cx: &mut Context) -> impl IntoElement { @@ -2911,6 +2895,72 @@ impl SessionsSidebar { } } +/// Ask before deleting `session_id`, stating how many threads go with it, then +/// continue with [`proceed_delete`]. The count needs the archived threads, so +/// the dialog waits for them when they are not held. +pub(crate) fn confirm_delete( + store: Entity, + session_id: String, + title: String, + ok_text: SharedString, + window: &mut Window, + cx: &mut gpui::App, +) { + if let Some(count) = store.read(cx).held_deletion_count(&session_id) { + open_delete_confirmation(store, session_id, title, ok_text, count, window, cx); + return; + } + let count = store.update(cx, |store, cx| store.fetch_deletion_count(&session_id, cx)); + window + .spawn(cx, async move |cx| { + let count = count.await; + let _ = cx.update(|window, cx| { + open_delete_confirmation(store, session_id, title, ok_text, count, window, cx) + }); + }) + .detach(); +} + +fn open_delete_confirmation( + store: Entity, + session_id: String, + title: String, + ok_text: SharedString, + count: usize, + window: &mut Window, + cx: &mut gpui::App, +) { + window.open_alert_dialog(cx, move |alert, _, cx| { + let alert = alert.bg(cx.theme().popover); + let store = store.clone(); + let session_id = session_id.clone(); + let description = if count > 1 { + crate::tr!("sidebar.delete_tree_description", count = count) + } else { + crate::tr!("sidebar.delete_description") + }; + alert + .title(crate::tr!("sidebar.delete_title", title = title.clone())) + .description(description) + .button_props( + DialogButtons::default() + .ok_variant(ButtonVariant::Danger) + .ok_text(ok_text.clone()) + .cancel_text(crate::tr!("settings.cancel")) + .show_cancel(true), + ) + .on_ok(move |_, window, cx| { + let store = store.clone(); + let session_id = session_id.clone(); + // The alert closes after this callback; open the next prompt afterwards. + window.defer(cx, move |window, cx| { + proceed_delete(store, session_id, window, cx); + }); + true + }) + }); +} + /// Delete `session_id`, first asking whether to also remove an orphaned worktree. pub(crate) fn proceed_delete( store: Entity, diff --git a/crates/ui/src/store/mod.rs b/crates/ui/src/store/mod.rs index 6d03f5e8e..b126db66c 100644 --- a/crates/ui/src/store/mod.rs +++ b/crates/ui/src/store/mod.rs @@ -10,7 +10,7 @@ use tcode_client::{ use tcode_core::{ git::{GitFileEntry, MenuItem, QuickAction, menu_items, quick_action}, project::{ - Project, ProjectGroup, SessionMeta, WorktreeInfo, group_sessions, + Project, ProjectGroup, SessionMeta, WorktreeInfo, descendant_session_ids, group_sessions, order_sessions_with_children, }, provider_models::{ResolvedModel, picker_models, resolve_models}, @@ -2972,6 +2972,41 @@ impl WorkspaceStore { self.session_status_replica.as_ref() } + /// How many threads deleting `session_id` removes (the host deletes every + /// thread under it too), while the archived threads are held: archived + /// descendants are not in the index. + pub fn held_deletion_count(&self, session_id: &str) -> Option { + let archived = self.archived_replica.as_ref()?; + Some(self.deletion_count(session_id, &archived.sessions)) + } + + /// [`Self::held_deletion_count`] over the archived threads, fetched once. + pub fn fetch_deletion_count(&self, session_id: &str, cx: &mut Context) -> Task { + let host = self.host.clone(); + let session_id = session_id.to_string(); + cx.spawn(async move |this, cx| { + let archived = match host.query(Query::ArchivedSessions).await { + Ok(QueryResponse::ArchivedSessions(archived)) => archived.sessions, + Ok(other) => { + log::warn!("unexpected archived-sessions response: {other:?}"); + Vec::new() + } + Err(error) => { + log::warn!("archived sessions failed: {}", error.message); + Vec::new() + } + }; + this.read_with(cx, |store, _| store.deletion_count(&session_id, &archived)) + .unwrap_or(1) + }) + } + + fn deletion_count(&self, session_id: &str, archived: &[SessionMeta]) -> usize { + descendant_session_ids(self.index_replica.0.iter().chain(archived), session_id) + .len() + .max(1) + } + pub fn worktree_orphaned_by_delete(&self, session_id: &str) -> Option { let (meta, shared) = if let Some(meta) = self .index_replica @@ -4587,6 +4622,90 @@ mod tests { }) } + /// Deleting a thread deletes every thread under it, archived ones too, + /// though the index carries no archived thread: the confirmation counts + /// them all. + #[gpui::test] + fn delete_confirmation_counts_archived_descendants(cx: &mut TestAppContext) { + let (to_host, requests) = async_channel::unbounded(); + let (replies, from_host) = async_channel::unbounded(); + let link = tcode_client::HostLink::new(to_host, from_host); + let store = cx.new(|cx| { + WorkspaceStore::new_attached( + link.clone(), + WorkspaceAttachment::Local, + None, + None, + false, + cx, + ) + }); + let mut pump = std::pin::pin!(link.pump()); + let mut task_cx = std::task::Context::from_waker(std::task::Waker::noop()); + let root = std::path::Path::new("/project"); + let archived = |id: &str, parent: &str| { + let mut meta = thread(root, id, "p", Some(parent)); + meta.archived_at = Some(1); + meta + }; + store.update(cx, |store, cx| { + store.apply_domain_event( + &EventEnvelope { + request_id: None, + topic: Topic::Index, + event: ServerEvent::IndexSnapshot(tcode_protocol::IndexSnapshot { + summary: Default::default(), + sessions: vec![ + thread(root, "parent", "p", None), + thread(root, "other", "p", None), + ], + projects: Vec::new(), + }), + }, + cx, + ); + }); + + let count = std::rc::Rc::new(std::cell::Cell::new(None)); + let result = count.clone(); + store.update(cx, |store, cx| { + let task = store.fetch_deletion_count("parent", cx); + cx.spawn(async move |_, _| result.set(Some(task.await))) + .detach(); + }); + cx.run_until_parked(); + let query = std::iter::from_fn(|| requests.try_recv().ok()) + .map(|line| tcode_protocol::decode_client_line(&line).unwrap()) + .find(|message| { + message.payload + == tcode_protocol::ClientPayload::Query(tcode_protocol::Query::ArchivedSessions) + }) + .expect("the count fetches the archived threads"); + replies + .try_send( + tcode_protocol::encode_line(&tcode_protocol::HostMessage::QueryResult { + id: query.id, + result: Ok(tcode_protocol::QueryResponse::ArchivedSessions( + tcode_protocol::ArchivedSessions { + sessions: vec![ + archived("child", "parent"), + archived("grandchild", "child"), + ], + worktree_shared: Default::default(), + revision: 0, + }, + )), + }) + .unwrap(), + ) + .unwrap(); + assert!(std::future::Future::poll(pump.as_mut(), &mut task_cx).is_pending()); + cx.run_until_parked(); + + assert_eq!(count.get(), Some(3)); + link.close(); + } + /// An Orchestrate child auto-archived on completion hands the workspace to /// its parent, keeping the parent's client-side records; archiving a thread /// the user is not viewing must not move them at all. diff --git a/locales/en.yml b/locales/en.yml index dcc71816d..f6c25d563 100644 --- a/locales/en.yml +++ b/locales/en.yml @@ -207,6 +207,7 @@ sidebar: ctx_delete: "Delete" delete_title: 'Delete thread "%{title}"?' delete_description: "This permanently clears conversation history for this thread." + delete_tree_description: "This permanently clears conversation history for this thread and every thread under it: %{count} threads in all." delete_action: "Delete" worktree_cleanup_title: "Remove worktree?" worktree_cleanup_description: "This thread is the only one linked to the worktree at %{path}. Delete the worktree too?" diff --git a/locales/zh-CN.yml b/locales/zh-CN.yml index 8f57ddeb8..7ed7b237a 100644 --- a/locales/zh-CN.yml +++ b/locales/zh-CN.yml @@ -207,6 +207,7 @@ sidebar: ctx_delete: "删除" delete_title: "删除对话“%{title}”?" delete_description: "这将永久清除该对话的历史记录。" + delete_tree_description: "这将永久清除该对话及其下所有对话的历史记录,共 %{count} 个对话。" delete_action: "删除" worktree_cleanup_title: "移除工作树?" worktree_cleanup_description: "此对话是唯一关联到位于 %{path} 的工作树的对话。要一并删除该工作树吗?" From 134254c0b48005b25f82cdba4c45135ed94b6e4e Mon Sep 17 00:00:00 2001 From: Tryanks Date: Tue, 6 Oct 2026 22:54:27 +0800 Subject: [PATCH 4/5] fix(ui): Delete all archived threads states the unarchived threads it takes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deleting a thread deletes every thread under it, so Settings → Archived Threads → Delete all also removes threads that were unarchived under an archived parent, while the confirmation only named the archived ones. WorkspaceStore::archived_deletion computes, from the index replica and the held archived threads (descendant_session_ids, as the single-thread confirmation does), how many listed archived threads go and how many unarchived threads under them go with them. When any unarchived thread is taken, the dialog says so and gives the total (settings.archived_delete_all_tree_title/_description, en and zh-CN); otherwise the existing text stays. delete_archived sends one DeleteSession per archived thread with no archived ancestor, so a thread that is both archived and under another archived thread is neither counted nor deleted twice. delete_all_archived_counts_unarchived_descendants_once drives the store as the Archived page does and asserts the counts and the DeleteSession commands on the wire; without the dedup it sees ["child", "parent"]. --- crates/ui/src/settings_page.rs | 54 ++++++----- crates/ui/src/store/intents.rs | 9 +- crates/ui/src/store/mod.rs | 165 +++++++++++++++++++++++++++++++++ locales/en.yml | 2 + locales/zh-CN.yml | 2 + 5 files changed, 208 insertions(+), 24 deletions(-) diff --git a/crates/ui/src/settings_page.rs b/crates/ui/src/settings_page.rs index 5e955e1f9..a185ab8a2 100644 --- a/crates/ui/src/settings_page.rs +++ b/crates/ui/src/settings_page.rs @@ -2268,7 +2268,7 @@ impl SettingsPage { .small() .label(crate::tr!("settings.archived_delete_all")) .on_click(cx.listener(move |this, _, window, cx| { - this.confirm_delete_all_archived(total, window, cx); + this.confirm_delete_all_archived(window, cx); })), ), ) @@ -2362,22 +2362,39 @@ impl SettingsPage { /// Bulk permanent delete. Always confirms, whatever /// `skip_delete_confirmation` says — this destroys every archived thread. - fn confirm_delete_all_archived( - &self, - total: usize, - window: &mut Window, - cx: &mut Context, - ) { + fn confirm_delete_all_archived(&self, window: &mut Window, cx: &mut Context) { + let Some(deletion) = self.store.read(cx).archived_deletion() else { + return; + }; + let (title, description) = if deletion.unarchived == 0 { + ( + crate::tr!( + "settings.archived_delete_all_title", + count = deletion.archived + ), + crate::tr!("settings.archived_delete_all_description"), + ) + } else { + ( + crate::tr!( + "settings.archived_delete_all_tree_title", + count = deletion.archived, + unarchived = deletion.unarchived + ), + crate::tr!( + "settings.archived_delete_all_tree_description", + total = deletion.archived + deletion.unarchived + ), + ) + }; let store = self.store.clone(); window.open_alert_dialog(cx, move |alert, _, cx| { let alert = alert.bg(cx.theme().popover); let store = store.clone(); + let deletion = deletion.clone(); alert - .title(crate::tr!( - "settings.archived_delete_all_title", - count = total - )) - .description(crate::tr!("settings.archived_delete_all_description")) + .title(title.clone()) + .description(description.clone()) .button_props( DialogButtons::default() .ok_variant(ButtonVariant::Danger) @@ -2386,17 +2403,8 @@ impl SettingsPage { .show_cancel(true), ) .on_ok(move |_, _, cx| { - store.update(cx, |store, _cx| { - let ids: Vec = store - .archived_groups() - .into_iter() - .flat_map(|group| group.sessions.into_iter().map(|meta| meta.id)) - .collect(); - // ponytail: one DeleteSession command per thread; add a bulk command if thousands feel slow. - for id in ids { - store.delete_session(id, false); - } - }); + let deletion = deletion.clone(); + store.update(cx, |store, _cx| store.delete_archived(deletion)); true }) }); diff --git a/crates/ui/src/store/intents.rs b/crates/ui/src/store/intents.rs index 0f31ed9be..77db7064a 100644 --- a/crates/ui/src/store/intents.rs +++ b/crates/ui/src/store/intents.rs @@ -15,7 +15,7 @@ use tcode_core::{ }; use tcode_protocol::{Command, CommandResponse, ProtocolError, RuntimeOperationId, SettingsPatch}; -use super::{StoreChange, TopicKind, WorkspaceStore}; +use super::{ArchivedDeletion, StoreChange, TopicKind, WorkspaceStore}; impl WorkspaceStore { pub(super) fn dispatch(&mut self, command: Command) { @@ -219,6 +219,13 @@ impl WorkspaceStore { remove_worktree, }); } + /// Deletes what [`WorkspaceStore::archived_deletion`] counted. + pub fn delete_archived(&mut self, deletion: ArchivedDeletion) { + // ponytail: one DeleteSession command per tree; add a bulk command if thousands feel slow. + for session_id in deletion.roots { + self.delete_session(session_id, false); + } + } pub fn mark_session_unread(&mut self, session_id: String) { self.dispatch(Command::MarkSessionUnread { session_id }); } diff --git a/crates/ui/src/store/mod.rs b/crates/ui/src/store/mod.rs index b126db66c..4a8f62829 100644 --- a/crates/ui/src/store/mod.rs +++ b/crates/ui/src/store/mod.rs @@ -243,6 +243,16 @@ pub struct ThreadExportArtifact { pub mime: String, } +/// Settings → Archived Threads → Delete all: the listed archived threads, the +/// threads under them that are not archived and go with them, and the threads +/// whose deletion removes exactly that set. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ArchivedDeletion { + pub archived: usize, + pub unarchived: usize, + roots: Vec, +} + pub(crate) struct DiffActiveState { pub session: String, pub cwd: PathBuf, @@ -3001,6 +3011,52 @@ impl WorkspaceStore { }) } + /// What deleting every listed archived thread removes, while the archived + /// threads are held. Only the archived threads with no archived ancestor + /// are sent: the host deletes each one's tree, so a thread under one is + /// neither sent nor counted twice. + pub fn archived_deletion(&self) -> Option { + let held = self.archived_replica.as_ref()?; + let listed: HashSet = self + .archived_groups() + .into_iter() + .flat_map(|group| group.sessions.into_iter().map(|meta| meta.id)) + .collect(); + let all = || self.index_replica.0.iter().chain(&held.sessions); + let parents: HashMap<&str, &str> = all() + .filter_map(|meta| Some((meta.id.as_str(), meta.parent_session_id.as_deref()?))) + .collect(); + let under_archived = |id: &str| { + let mut seen = HashSet::from([id]); + let mut current = id; + while let Some(&parent) = parents.get(current) { + if !seen.insert(parent) { + return false; + } + if listed.contains(parent) { + return true; + } + current = parent; + } + false + }; + let roots: Vec = held + .sessions + .iter() + .filter(|meta| listed.contains(&meta.id) && !under_archived(&meta.id)) + .map(|meta| meta.id.clone()) + .collect(); + let deleted: HashSet = roots + .iter() + .flat_map(|root| descendant_session_ids(all(), root)) + .collect(); + Some(ArchivedDeletion { + archived: listed.len(), + unarchived: deleted.iter().filter(|id| !listed.contains(*id)).count(), + roots, + }) + } + fn deletion_count(&self, session_id: &str, archived: &[SessionMeta]) -> usize { descendant_session_ids(self.index_replica.0.iter().chain(archived), session_id) .len() @@ -4706,6 +4762,115 @@ mod tests { link.close(); } + /// Delete all deletes each archived thread's tree: the confirmation counts + /// the unarchived threads under them, and a thread under another archived + /// thread is neither counted nor deleted twice. + #[gpui::test] + fn delete_all_archived_counts_unarchived_descendants_once(cx: &mut TestAppContext) { + let root = std::path::Path::new("/project"); + let archived = |id: &str, parent: Option<&str>| { + let mut meta = thread(root, id, "p", parent); + meta.archived_at = Some(1); + meta + }; + let delete_all = |cx: &mut TestAppContext, index, archived| { + let (to_host, requests) = async_channel::unbounded(); + let (replies, from_host) = async_channel::unbounded(); + let link = tcode_client::HostLink::new(to_host, from_host); + let store = cx.new(|cx| { + WorkspaceStore::new_attached( + link.clone(), + WorkspaceAttachment::Local, + None, + None, + false, + cx, + ) + }); + let mut pump = std::pin::pin!(link.pump()); + let mut task_cx = std::task::Context::from_waker(std::task::Waker::noop()); + // A thread on screen, so the empty-workspace draft does not open. + store.update(cx, |store, cx| { + store.select_session("other".into()); + store.apply_domain_event( + &EventEnvelope { + request_id: None, + topic: Topic::Index, + event: ServerEvent::IndexSnapshot(tcode_protocol::IndexSnapshot { + summary: Default::default(), + sessions: index, + projects: vec![project_at("p", root)], + }), + }, + cx, + ); + store.load_archived_sessions(cx); + }); + cx.run_until_parked(); + let query = std::iter::from_fn(|| requests.try_recv().ok()) + .map(|line| tcode_protocol::decode_client_line(&line).unwrap()) + .find(|message| { + message.payload + == tcode_protocol::ClientPayload::Query( + tcode_protocol::Query::ArchivedSessions, + ) + }) + .expect("the Archived page fetches the archived threads"); + replies + .try_send( + tcode_protocol::encode_line(&tcode_protocol::HostMessage::QueryResult { + id: query.id, + result: Ok(tcode_protocol::QueryResponse::ArchivedSessions( + tcode_protocol::ArchivedSessions { + sessions: archived, + worktree_shared: Default::default(), + revision: 0, + }, + )), + }) + .unwrap(), + ) + .unwrap(); + assert!(std::future::Future::poll(pump.as_mut(), &mut task_cx).is_pending()); + cx.run_until_parked(); + let deletion = store + .read_with(cx, |store, _| store.archived_deletion()) + .expect("the archived threads are held"); + let counts = (deletion.archived, deletion.unarchived); + store.update(cx, |store, _| store.delete_archived(deletion)); + cx.run_until_parked(); + let deleted: Vec = std::iter::from_fn(|| requests.try_recv().ok()) + .filter_map(|line| { + match tcode_protocol::decode_client_line(&line).unwrap().payload { + tcode_protocol::ClientPayload::Command(Command::DeleteSession { + session_id, + .. + }) => Some(session_id), + _ => None, + } + }) + .collect(); + link.close(); + (counts, deleted) + }; + + let (counts, deleted) = delete_all( + cx, + vec![ + thread(root, "child", "p", Some("parent")), + thread(root, "other", "p", None), + ], + vec![archived("parent", None)], + ); + assert_eq!((counts, deleted), ((1, 1), vec!["parent".to_string()])); + let (counts, deleted) = delete_all( + cx, + vec![thread(root, "other", "p", None)], + vec![archived("child", Some("parent")), archived("parent", None)], + ); + assert_eq!((counts, deleted), ((2, 0), vec!["parent".to_string()])); + } + /// An Orchestrate child auto-archived on completion hands the workspace to /// its parent, keeping the parent's client-side records; archiving a thread /// the user is not viewing must not move them at all. diff --git a/locales/en.yml b/locales/en.yml index f6c25d563..1b9e72924 100644 --- a/locales/en.yml +++ b/locales/en.yml @@ -271,6 +271,8 @@ settings: archived_delete_all: "Delete all" archived_delete_all_title: "Delete all %{count} archived threads?" archived_delete_all_description: "This permanently deletes every archived thread and its conversation history. This cannot be undone." + archived_delete_all_tree_title: "Delete all %{count} archived threads and %{unarchived} unarchived threads under them?" + archived_delete_all_tree_description: "Every thread under an archived thread is deleted with it, archived or not. This permanently deletes %{total} threads and their conversation history. This cannot be undone." back: "Back" restore: "Restore defaults" restore_title: "Restore default settings?" diff --git a/locales/zh-CN.yml b/locales/zh-CN.yml index 7ed7b237a..91f597715 100644 --- a/locales/zh-CN.yml +++ b/locales/zh-CN.yml @@ -271,6 +271,8 @@ settings: archived_delete_all: "全部删除" archived_delete_all_title: "永久删除全部 %{count} 个已归档对话?" archived_delete_all_description: "这将永久删除所有已归档对话及其历史记录,且无法撤销。" + archived_delete_all_tree_title: "永久删除 %{count} 个已归档对话及其下 %{unarchived} 个未归档对话?" + archived_delete_all_tree_description: "已归档对话下的所有对话,无论是否归档,都会随之删除。这将永久删除共 %{total} 个对话及其历史记录,且无法撤销。" back: "返回" restore: "恢复默认设置" restore_title: "恢复默认设置?" From e9783bdaf433f226315d7f3487a1f0941b2121bf Mon Sep 17 00:00:00 2001 From: Tryanks Date: Tue, 6 Oct 2026 23:58:47 +0800 Subject: [PATCH 5/5] test(ui): wait for the selected session's baseline before driving live events classifier_stop_and_review_preserve_diagnostics_until_the_next_turn waited only for the status replica, so the injected TurnStarted could reach the host before the SessionEvents subscription existed and was dropped. The app applies live session events only after that baseline (baseline_ready gates Connected), so the driver now waits for it. --- crates/ui/src/store/mod.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/crates/ui/src/store/mod.rs b/crates/ui/src/store/mod.rs index 4a8f62829..065d47878 100644 --- a/crates/ui/src/store/mod.rs +++ b/crates/ui/src/store/mod.rs @@ -5645,13 +5645,8 @@ mod tests { let host = test_host(session_store); let workspace = cx.new(|cx| WorkspaceStore::new(host.link(), cx)); workspace.update(cx, |store, _| store.select_session(meta.id.clone())); - wait_until(cx, &workspace, "selected session", |cx| { - workspace.read_with(cx, |store, _| { - store - .session_status_replica - .as_ref() - .is_some_and(|status| status.session_id == meta.id) - }) + wait_until(cx, &workspace, "selected session baseline", |cx| { + workspace.read_with(cx, |store, _| store.baseline_ready()) }); let session_id = meta.id.clone();