diff --git a/package.json b/package.json index 85f122536..ed1e5bfd7 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "codeg", "private": true, - "version": "0.23.0", + "version": "0.23.1", "packageManager": "pnpm@11.9.0", "scripts": { "dev": "next dev --turbopack", diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock index 2db7beb06..d8d49e5ac 100644 --- a/src-tauri/Cargo.lock +++ b/src-tauri/Cargo.lock @@ -1014,7 +1014,7 @@ checksum = "c3e64b0cc0439b12df2fa678eae89a1c56a529fd067a9115f7827f1fffd22b32" [[package]] name = "codeg" -version = "0.23.0" +version = "0.23.1" dependencies = [ "aes-gcm", "agent-client-protocol-schema", diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index d482bf36a..930d1fe32 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "codeg" -version = "0.23.0" +version = "0.23.1" description = "Agent Code Generation App" authors = ["feitao"] edition = "2021" diff --git a/src-tauri/src/commands/conversations.rs b/src-tauri/src/commands/conversations.rs index e37a690cc..53ca1f5a9 100644 --- a/src-tauri/src/commands/conversations.rs +++ b/src-tauri/src/commands/conversations.rs @@ -600,6 +600,17 @@ fn build_scan_result( /// Scan every local agent's sessions and reconcile them against the DB for the /// import-picker window. Emits [`IMPORT_SCAN_PROGRESS_EVENT`] once per parser /// while the walk runs. +/// +/// The scan also refreshes the conversations that are ALREADY imported, from +/// the same parse it just did: a title generated after the first import, and +/// the transcript's own last-activity time when the user kept working on the +/// session in the agent's own CLI (see +/// [`import_service::sync_imported_sessions`]). Without this, a re-scan can +/// only ever offer the *new* sessions — the picker does not let you re-select +/// an imported one — so an already-imported conversation would keep the +/// `updated_at` it had at import time forever, and sit in the wrong place in a +/// recency-sorted sidebar. Each refreshed row is broadcast so every window and +/// web client re-sorts live. pub async fn scan_importable_sessions_core( conn: &sea_orm::DatabaseConnection, emitter: &EventEmitter, @@ -629,17 +640,23 @@ pub async fn scan_importable_sessions_core( .map_err(crate::db::error::DbError::from) .map_err(AppCommandError::from)?; let mut imported_index: HashMap<(String, String), bool> = HashMap::new(); - for row in conv_rows { - let Some(external_id) = row.external_id else { + for row in &conv_rows { + let Some(external_id) = row.external_id.clone() else { continue; }; let live = row.deleted_at.is_none(); let entry = imported_index - .entry((row.agent_type, external_id)) + .entry((row.agent_type.clone(), external_id)) .or_insert(live); *entry = *entry || live; } + // Refresh the already-imported rows in place before answering, then + // broadcast each one so open sidebars re-sort without a refetch. + for id in import_service::sync_imported_sessions(conn, &conv_rows, &summaries).await { + emit_conversation_upsert(emitter, conn, id).await; + } + let folder_rows = load_folder_rows(conn).await?; Ok(build_scan_result(summaries, &imported_index, &folder_rows)) } diff --git a/src-tauri/src/commands/folder_links.rs b/src-tauri/src/commands/folder_links.rs new file mode 100644 index 000000000..0cb4a1245 --- /dev/null +++ b/src-tauri/src/commands/folder_links.rs @@ -0,0 +1,1424 @@ +//! Multi-folder workspaces: symlink other directories into a workspace folder +//! so one root can host several projects. +//! +//! A plain UI-level merge would be invisible to the agent CLIs, which run with +//! `cwd` set to the workspace root — real symlinks are what let them `ls` and +//! edit the linked projects. Every link is recorded in `folder_link`, which +//! doubles as the authorization record the workspace path guard consults (see +//! [`crate::folder_links`]). + +use std::collections::HashSet; +use std::path::{Path, PathBuf}; + +use serde::{Deserialize, Serialize}; + +use crate::app_error::AppCommandError; +use crate::db::service::{folder_link_service, folder_service}; +use crate::db::AppDatabase; +use crate::web::event_bridge::{emit_event, EventEmitter}; + +/// Emitted whenever a folder's link set changes, so every open window (desktop +/// windows, web clients, remote sessions) refreshes its list and file tree. +pub const FOLDER_LINKS_CHANGED_EVENT: &str = "folder://links-changed"; + +#[derive(Debug, Clone, Serialize)] +pub struct FolderLinksChanged { + pub folder_id: i32, +} + +/// Longest link name we will derive. Long enough for any real project +/// directory, short enough to stay clear of per-component limits (255 bytes on +/// most filesystems) once a `-NN` disambiguator is appended. +const MAX_LINK_NAME_LEN: usize = 96; + +/// How many `-2`, `-3`, … variants to probe before giving up on a name. +const MAX_NAME_PROBES: u32 = 99; + +const FALLBACK_LINK_NAME: &str = "linked-folder"; + +/// Names Windows refuses on any filesystem, with or without an extension. +const WINDOWS_RESERVED: &[&str] = &[ + "con", "prn", "aux", "nul", "com1", "com2", "com3", "com4", "com5", "com6", "com7", "com8", + "com9", "lpt1", "lpt2", "lpt3", "lpt4", "lpt5", "lpt6", "lpt7", "lpt8", "lpt9", +]; + +// --------------------------------------------------------------------------- +// Wire types +// --------------------------------------------------------------------------- + +/// Live state of a link, recomputed from disk on every list so the UI can offer +/// a repair instead of silently showing a link that no longer works. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum FolderLinkStatus { + /// The symlink exists and resolves to the recorded target. + Ok, + /// Nothing at `/` — the user (or a tool) deleted the link. + Missing, + /// Something is at `/`, but it is not a link to the target + /// (a real directory, or a link pointing somewhere else). + Conflicted, + /// The link is there but its target no longer resolves. + Broken, +} + +#[derive(Debug, Clone, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct FolderLinkDetail { + pub id: i32, + pub folder_id: i32, + pub name: String, + pub target_path: String, + pub status: FolderLinkStatus, +} + +/// Why a picked directory cannot be linked. Rendered by the frontend, so these +/// are stable identifiers rather than prose. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum FolderLinkRejection { + /// The path does not exist or cannot be resolved. + NotFound, + /// The path exists but is not a directory. + NotADirectory, + /// The picked directory *is* the workspace root. + SameAsRoot, + /// The picked directory contains the workspace root — linking it would nest + /// the workspace inside itself. + AncestorOfRoot, + /// The picked directory is already inside the workspace root; it is + /// reachable without a link. + InsideRoot, + /// This directory is already linked into this workspace. + AlreadyLinked, + /// Every candidate name (`api`, `api-2`, … `api-99`) is already taken. + NameUnavailable, +} + +/// What [`preview_folder_links_core`] would do with one picked directory: the +/// name it would get, whether that name had to be disambiguated, and why it +/// would be skipped. +#[derive(Debug, Clone, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct FolderLinkPlan { + pub target_path: String, + /// Name derived straight from the directory, before disambiguation. + pub base_name: String, + /// Name that would actually be created; empty when `rejection` is set. + pub name: String, + /// True when `name` had to differ from `base_name` because something else + /// already occupies it. + pub renamed: bool, + /// Set when the collision was with a real file/directory already in the + /// root, rather than with another link or another entry in this batch — + /// worth surfacing louder in the UI. + pub collides_with_existing_entry: bool, + pub rejection: Option, + /// Name of the existing link, when `rejection` is `AlreadyLinked`. + pub existing_link_name: Option, +} + +#[derive(Debug, Clone, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct FolderLinkRequest { + pub path: String, + /// Explicit name chosen by the user. When absent (or blank) the name is + /// derived from the directory. A name that is already taken is still + /// disambiguated — the create call never clobbers an existing entry. + #[serde(default)] + pub name: Option, +} + +// --------------------------------------------------------------------------- +// Name derivation (pure — unit-tested on every platform) +// --------------------------------------------------------------------------- + +/// Strip everything that would make a path component invalid or ambiguous on +/// any supported platform. Never returns a name that needs further escaping. +pub(crate) fn sanitize_link_name(raw: &str) -> String { + let cleaned: String = raw + .chars() + .filter(|c| !c.is_control()) + // Path separators and the characters Windows rejects in a file name. + .map(|c| match c { + '/' | '\\' | ':' | '*' | '?' | '"' | '<' | '>' | '|' | '\0' => '-', + other => other, + }) + .collect(); + + // Windows silently strips trailing dots and spaces, which would make the + // name we recorded differ from the one on disk. + let trimmed = cleaned.trim().trim_end_matches(['.', ' ']).trim(); + if trimmed.is_empty() || trimmed == "." || trimmed == ".." { + return FALLBACK_LINK_NAME.to_string(); + } + + // Truncate on a char boundary, then re-trim in case the cut exposed a dot. + let limited: String = trimmed.chars().take(MAX_LINK_NAME_LEN).collect(); + let limited = limited.trim_end_matches(['.', ' ']).trim().to_string(); + if limited.is_empty() { + return FALLBACK_LINK_NAME.to_string(); + } + + // `CON`, `con.txt`, … are reserved device names on Windows regardless of + // extension. Suffix rather than replace so the name stays recognizable. + let stem = limited + .split('.') + .next() + .unwrap_or(&limited) + .to_ascii_lowercase(); + if WINDOWS_RESERVED.contains(&stem.as_str()) { + return format!("{limited}-dir"); + } + + limited +} + +/// Name a linked directory should get, derived from its own final component. +/// Falls back to the whole path (e.g. a drive or filesystem root, which has no +/// final component) and finally to a fixed placeholder. +pub(crate) fn derive_link_name(target: &Path) -> String { + if let Some(name) = target.file_name().and_then(|s| s.to_str()) { + let sanitized = sanitize_link_name(name); + if sanitized != FALLBACK_LINK_NAME { + return sanitized; + } + } + // No usable final component: `/`, `C:\`, `\\server\share`. + let whole = target.to_string_lossy(); + let collapsed = whole + .split(['/', '\\']) + .filter(|s| !s.is_empty() && *s != "." && *s != "..") + .collect::>() + .join("-"); + let sanitized = sanitize_link_name(&collapsed); + if sanitized.is_empty() { + FALLBACK_LINK_NAME.to_string() + } else { + sanitized + } +} + +/// Case-insensitive key for collision checks. macOS and Windows filesystems are +/// case-insensitive by default, so `API` and `api` are the *same* entry there — +/// comparing case-sensitively would let us "successfully" plan two links that +/// then clobber each other on disk. +pub(crate) fn name_key(name: &str) -> String { + name.to_lowercase() +} + +/// First free variant of `base`: `base`, then `base-2` … `base-99`. +/// `taken` holds [`name_key`]s. Returns `None` when every probe is occupied. +pub(crate) fn unique_link_name(base: &str, taken: &HashSet) -> Option { + if !taken.contains(&name_key(base)) { + return Some(base.to_string()); + } + for n in 2..=MAX_NAME_PROBES { + let candidate = format!("{base}-{n}"); + if !taken.contains(&name_key(&candidate)) { + return Some(candidate); + } + } + None +} + +/// Every name already spoken for directly inside `root`. +/// +/// `read_dir` is used rather than `Path::exists`, which follows symlinks and +/// reports a *dangling* link as absent — creating over one of those fails with +/// `AlreadyExists` and would surface as a confusing error instead of a rename. +fn occupied_names(root: &Path) -> HashSet { + let mut names = HashSet::new(); + let Ok(entries) = std::fs::read_dir(root) else { + return names; + }; + for entry in entries.flatten() { + names.insert(name_key(&entry.file_name().to_string_lossy())); + } + names +} + +// --------------------------------------------------------------------------- +// Target validation +// --------------------------------------------------------------------------- + +struct ValidatedTarget { + canonical: PathBuf, +} + +fn validate_target( + canonical_root: &Path, + raw_target: &str, +) -> Result { + let trimmed = raw_target.trim(); + if trimmed.is_empty() { + return Err(FolderLinkRejection::NotFound); + } + let canonical = + std::fs::canonicalize(trimmed).map_err(|_| FolderLinkRejection::NotFound)?; + if !canonical.is_dir() { + return Err(FolderLinkRejection::NotADirectory); + } + if canonical == canonical_root { + return Err(FolderLinkRejection::SameAsRoot); + } + // The root lives inside the pick: linking it would make the workspace + // contain itself, and the tree walker would recurse forever. + if canonical_root.starts_with(&canonical) { + return Err(FolderLinkRejection::AncestorOfRoot); + } + // Already reachable by walking the workspace — a link would just duplicate + // it (and create a cycle). + if canonical.starts_with(canonical_root) { + return Err(FolderLinkRejection::InsideRoot); + } + Ok(ValidatedTarget { canonical }) +} + +// --------------------------------------------------------------------------- +// Symlink creation / removal +// --------------------------------------------------------------------------- + +#[cfg(unix)] +fn create_dir_symlink(target: &Path, link: &Path) -> Result<(), AppCommandError> { + std::os::unix::fs::symlink(target, link).map_err(|e| { + AppCommandError::io_error("Failed to create the folder link") + .with_detail(format!("{} -> {}: {e}", link.display(), target.display())) + }) +} + +#[cfg(windows)] +fn create_dir_symlink(target: &Path, link: &Path) -> Result<(), AppCommandError> { + // A real symlink needs SeCreateSymbolicLinkPrivilege, which an unelevated + // process only has with Developer Mode on. Fall back to a directory + // junction, which needs no privilege and behaves the same for local + // absolute paths (which is all we ever link). + match std::os::windows::fs::symlink_dir(target, link) { + Ok(()) => Ok(()), + Err(symlink_err) => { + let output = crate::process::std_command("cmd") + .args(["/C", "mklink", "/J"]) + .arg(link) + .arg(target) + .output(); + match output { + Ok(out) if out.status.success() => Ok(()), + Ok(out) => Err(AppCommandError::io_error( + "Failed to create the folder link. Enable Developer Mode in Windows settings, \ + or run the app as administrator.", + ) + .with_detail(format!( + "symlink_dir: {symlink_err}; mklink /J: {}", + String::from_utf8_lossy(&out.stderr).trim() + ))), + Err(spawn_err) => Err(AppCommandError::io_error( + "Failed to create the folder link. Enable Developer Mode in Windows settings, \ + or run the app as administrator.", + ) + .with_detail(format!( + "symlink_dir: {symlink_err}; mklink /J: {spawn_err}" + ))), + } + } + } +} + +#[cfg(not(any(unix, windows)))] +fn create_dir_symlink(_target: &Path, _link: &Path) -> Result<(), AppCommandError> { + Err(AppCommandError::io_error( + "Folder links are not supported on this platform", + )) +} + +/// Delete the link entry itself — never its contents. +/// +/// `remove_dir_all` is deliberately not used anywhere in this module: on +/// Windows it would recurse *through* a junction and delete the user's real +/// project. `remove_file` handles unix symlinks; `remove_dir` handles Windows +/// directory symlinks and junctions (both are empty reparse points to the OS). +/// What [`remove_link_entry`] found at the path. Distinguishes "there was +/// nothing of ours to remove" — which still lets the caller drop the record — +/// from a genuine removal failure, which must not be reported as success. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum LinkRemoval { + /// The symlink was removed. + Removed, + /// Nothing was there: already the desired end state. + Absent, + /// A real file/directory holds the name. Removing it would destroy user + /// data, so it is left alone. + NotALink, +} + +fn remove_link_entry(link: &Path) -> std::io::Result { + let metadata = match std::fs::symlink_metadata(link) { + Ok(m) => m, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(LinkRemoval::Absent), + Err(e) => return Err(e), + }; + if !metadata.file_type().is_symlink() { + return Ok(LinkRemoval::NotALink); + } + match std::fs::remove_file(link) { + Ok(()) => Ok(LinkRemoval::Removed), + // Windows directory symlinks / junctions must go through remove_dir. + Err(first) => match std::fs::remove_dir(link) { + Ok(()) => Ok(LinkRemoval::Removed), + // Report the original error: on unix `remove_dir` on a symlink + // fails with a misleading ENOTDIR that hides the real cause. + Err(_) => Err(first), + }, + } +} + +/// Read where a link points, or `None` when the path is not a link. +fn read_link_target(link: &Path) -> Option { + let metadata = std::fs::symlink_metadata(link).ok()?; + if !metadata.file_type().is_symlink() { + return None; + } + std::fs::canonicalize(link).ok() +} + +// --------------------------------------------------------------------------- +// git exclude +// --------------------------------------------------------------------------- + +/// Ignore rule that hides the link from `git status`, anchored at the +/// repository root. +/// +/// `info/exclude` patterns are relative to the *work tree top level*, not to +/// the workspace folder — so when the folder is a subdirectory of the repo the +/// rule has to carry that prefix, or it would exclude an unrelated +/// `/` instead. A leading `/` anchors it so only that one entry is +/// matched. +fn git_exclude_line(toplevel: &Path, root: &Path, name: &str) -> String { + let rel = std::fs::canonicalize(root) + .ok() + .and_then(|canonical_root| { + std::fs::canonicalize(toplevel) + .ok() + .and_then(|top| canonical_root.strip_prefix(&top).map(Path::to_path_buf).ok()) + }) + .unwrap_or_default(); + let prefix = rel.to_string_lossy().replace('\\', "/"); + if prefix.is_empty() { + format!("/{name}") + } else { + format!("/{}/{}", prefix.trim_matches('/'), name) + } +} + +/// Append an ignore rule for the link to the repository's `info/exclude` so it +/// doesn't show up as an untracked file in the user's own project. +/// +/// `info/exclude`, not `.gitignore`: it is local-only and never committed, so +/// we never modify a file the user would have to review or push. Best-effort — +/// a failure only means a noisier `git status`. +async fn add_to_git_exclude(root: &Path, name: &str) { + let Some(git_dir) = resolve_git_common_dir(root).await else { + return; + }; + let Some(toplevel) = resolve_git_toplevel(root).await else { + return; + }; + let info_dir = git_dir.join("info"); + if std::fs::create_dir_all(&info_dir).is_err() { + return; + } + let exclude_path = info_dir.join("exclude"); + let existing = std::fs::read_to_string(&exclude_path).unwrap_or_default(); + let line = git_exclude_line(&toplevel, root, name); + if existing.lines().any(|l| l.trim() == line) { + return; + } + + let mut next = existing; + if !next.is_empty() && !next.ends_with('\n') { + next.push('\n'); + } + if !next.contains("# codeg workspace links") { + next.push_str("# codeg workspace links\n"); + } + next.push_str(&line); + next.push('\n'); + if let Err(e) = std::fs::write(&exclude_path, next) { + tracing::debug!("[folder-link] could not update {}: {e}", exclude_path.display()); + } +} + +async fn git_rev_parse_path(root: &Path, arg: &str) -> Option { + let output = crate::process::tokio_command("git") + .args(["rev-parse", arg]) + .current_dir(root) + .output() + .await + .ok()?; + if !output.status.success() { + return None; + } + let raw = String::from_utf8_lossy(&output.stdout).trim().to_string(); + if raw.is_empty() { + return None; + } + let path = PathBuf::from(&raw); + Some(if path.is_absolute() { + path + } else { + root.join(path) + }) +} + +/// Absolute path of the repository's common git dir (shared by every worktree, +/// which is where `info/exclude` lives). `None` when `root` is not a repo. +async fn resolve_git_common_dir(root: &Path) -> Option { + git_rev_parse_path(root, "--git-common-dir").await +} + +/// Absolute path of the work tree's top level — the directory `info/exclude` +/// patterns are resolved against. +async fn resolve_git_toplevel(root: &Path) -> Option { + git_rev_parse_path(root, "--show-toplevel").await +} + +// --------------------------------------------------------------------------- +// Core operations +// --------------------------------------------------------------------------- + +async fn load_root(db: &AppDatabase, folder_id: i32) -> Result { + let folder = folder_service::get_folder_by_id(&db.conn, folder_id) + .await + .map_err(AppCommandError::from)? + .ok_or_else(|| AppCommandError::not_found(format!("Folder {folder_id} not found")))?; + Ok(PathBuf::from(folder.path)) +} + +fn link_status(root: &Path, name: &str, target: &str) -> FolderLinkStatus { + let link_path = root.join(name); + let Ok(metadata) = std::fs::symlink_metadata(&link_path) else { + return FolderLinkStatus::Missing; + }; + if !metadata.file_type().is_symlink() { + return FolderLinkStatus::Conflicted; + } + let Ok(resolved) = std::fs::canonicalize(&link_path) else { + return FolderLinkStatus::Broken; + }; + match std::fs::canonicalize(target) { + Ok(expected) if expected == resolved => FolderLinkStatus::Ok, + Ok(_) => FolderLinkStatus::Conflicted, + Err(_) => FolderLinkStatus::Broken, + } +} + +pub async fn list_folder_links_core( + db: &AppDatabase, + folder_id: i32, +) -> Result, AppCommandError> { + let root = load_root(db, folder_id).await?; + let rows = folder_link_service::list_by_folder(&db.conn, folder_id) + .await + .map_err(AppCommandError::from)?; + Ok(rows + .into_iter() + .map(|row| FolderLinkDetail { + status: link_status(&root, &row.name, &row.target_path), + id: row.id, + folder_id: row.folder_id, + name: row.name, + target_path: row.target_path, + }) + .collect()) +} + +/// Dry run: resolve names and conflicts for a batch of picked directories +/// without touching the filesystem. The dialog calls this on every change so +/// the user sees the final names (and any rejections) before committing. +pub async fn preview_folder_links_core( + db: &AppDatabase, + folder_id: i32, + paths: Vec, +) -> Result, AppCommandError> { + let root = load_root(db, folder_id).await?; + let canonical_root = std::fs::canonicalize(&root) + .map_err(|_| AppCommandError::not_found("Workspace folder does not exist"))?; + let existing = folder_link_service::list_by_folder(&db.conn, folder_id) + .await + .map_err(AppCommandError::from)?; + + let on_disk = occupied_names(&root); + let mut taken: HashSet = on_disk.clone(); + for link in &existing { + taken.insert(name_key(&link.name)); + } + + let mut plans = Vec::with_capacity(paths.len()); + for raw in paths { + let validated = match validate_target(&canonical_root, &raw) { + Ok(v) => v, + Err(rejection) => { + plans.push(FolderLinkPlan { + base_name: derive_link_name(Path::new(raw.trim())), + target_path: raw, + name: String::new(), + renamed: false, + collides_with_existing_entry: false, + rejection: Some(rejection), + existing_link_name: None, + }); + continue; + } + }; + + // Same directory already linked here (compared canonically, so `~/a` + // and a symlinked route to it count as one). + let duplicate = existing.iter().find(|link| { + std::fs::canonicalize(&link.target_path) + .map(|p| p == validated.canonical) + .unwrap_or(false) + }); + if let Some(dup) = duplicate { + plans.push(FolderLinkPlan { + base_name: derive_link_name(&validated.canonical), + target_path: raw, + name: String::new(), + renamed: false, + collides_with_existing_entry: false, + rejection: Some(FolderLinkRejection::AlreadyLinked), + existing_link_name: Some(dup.name.clone()), + }); + continue; + } + + let base = derive_link_name(&validated.canonical); + let Some(name) = unique_link_name(&base, &taken) else { + plans.push(FolderLinkPlan { + base_name: base, + target_path: raw, + name: String::new(), + renamed: false, + collides_with_existing_entry: true, + rejection: Some(FolderLinkRejection::NameUnavailable), + existing_link_name: None, + }); + continue; + }; + let renamed = name != base; + taken.insert(name_key(&name)); + plans.push(FolderLinkPlan { + target_path: raw, + collides_with_existing_entry: renamed && on_disk.contains(&name_key(&base)), + base_name: base, + name, + renamed, + rejection: None, + existing_link_name: None, + }); + } + + Ok(plans) +} + +/// Create the links. Rejected entries are skipped rather than failing the whole +/// batch — the returned list is what actually landed, and the caller compares +/// it against what it asked for. +pub async fn create_folder_links_core( + emitter: &EventEmitter, + db: &AppDatabase, + folder_id: i32, + items: Vec, + git_exclude: bool, +) -> Result, AppCommandError> { + let root = load_root(db, folder_id).await?; + let canonical_root = std::fs::canonicalize(&root) + .map_err(|_| AppCommandError::not_found("Workspace folder does not exist"))?; + let existing = folder_link_service::list_by_folder(&db.conn, folder_id) + .await + .map_err(AppCommandError::from)?; + + let mut taken: HashSet = occupied_names(&root); + for link in &existing { + taken.insert(name_key(&link.name)); + } + + // Targets already linked here, canonicalized so `~/a` and a symlinked route + // to it count as one. Grows as the batch lands, so the same directory + // submitted twice in one call links once instead of becoming `api` + `api-2`. + let mut linked_targets: Vec = existing + .iter() + .filter_map(|link| std::fs::canonicalize(&link.target_path).ok()) + .collect(); + + let mut created = Vec::new(); + for item in items { + let Ok(validated) = validate_target(&canonical_root, &item.path) else { + continue; + }; + if linked_targets.contains(&validated.canonical) { + continue; + } + + // An explicit name still goes through sanitization and disambiguation: + // the client's view of the directory can be stale, and we must never + // overwrite something that is already there. + let base = match item.name.as_deref().map(str::trim) { + Some(name) if !name.is_empty() => sanitize_link_name(name), + _ => derive_link_name(&validated.canonical), + }; + let Some(name) = unique_link_name(&base, &taken) else { + return Err(AppCommandError::already_exists( + "Could not find a free name for the linked folder", + ) + .with_detail(base)); + }; + + let link_path = root.join(&name); + create_dir_symlink(&validated.canonical, &link_path)?; + + // Persist only after the link exists, so a failed create never leaves a + // row granting access to a path with nothing behind it. + let row = match folder_link_service::insert( + &db.conn, + folder_id, + &name, + &validated.canonical.to_string_lossy(), + ) + .await + { + Ok(row) => row, + Err(e) => { + let _ = remove_link_entry(&link_path); + return Err(AppCommandError::from(e)); + } + }; + + crate::folder_links::register(&root, &validated.canonical); + taken.insert(name_key(&name)); + linked_targets.push(validated.canonical.clone()); + if git_exclude { + add_to_git_exclude(&root, &name).await; + } + + created.push(FolderLinkDetail { + status: link_status(&root, &row.name, &row.target_path), + id: row.id, + folder_id: row.folder_id, + name: row.name, + target_path: row.target_path, + }); + } + + if !created.is_empty() { + emit_event( + emitter, + FOLDER_LINKS_CHANGED_EVENT, + FolderLinksChanged { folder_id }, + ); + } + Ok(created) +} + +/// Rename a link: move the symlink on disk, then update the row. The target is +/// untouched. +pub async fn rename_folder_link_core( + emitter: &EventEmitter, + db: &AppDatabase, + link_id: i32, + new_name: String, +) -> Result { + let row = folder_link_service::get_by_id(&db.conn, link_id) + .await + .map_err(AppCommandError::from)? + .ok_or_else(|| AppCommandError::not_found("Folder link not found"))?; + let root = load_root(db, row.folder_id).await?; + + let sanitized = sanitize_link_name(&new_name); + // Compared byte-for-byte, NOT case-folded: on a case-sensitive filesystem + // `api` -> `API` is a real move, and skipping it would leave the row naming + // an entry that isn't there (reported `missing`, "repaired" into a second + // link, with the original left behind and unmanageable). + if sanitized == row.name { + return Ok(FolderLinkDetail { + status: link_status(&root, &row.name, &row.target_path), + id: row.id, + folder_id: row.folder_id, + name: row.name, + target_path: row.target_path, + }); + } + + let mut taken = occupied_names(&root); + // The link's own current name is about to be freed. + taken.remove(&name_key(&row.name)); + for other in folder_link_service::list_by_folder(&db.conn, row.folder_id) + .await + .map_err(AppCommandError::from)? + { + if other.id != link_id { + taken.insert(name_key(&other.name)); + } + } + if taken.contains(&name_key(&sanitized)) { + return Err( + AppCommandError::already_exists("That name is already used in this folder") + .with_detail(sanitized), + ); + } + + let from = root.join(&row.name); + let to = root.join(&sanitized); + // Only move something that is actually a link — if a real directory took + // over the name, renaming it would move the user's data. + if read_link_target(&from).is_some() { + std::fs::rename(&from, &to).map_err(|e| { + AppCommandError::io_error("Failed to rename the folder link").with_detail(e.to_string()) + })?; + } else if std::fs::symlink_metadata(&from).is_ok() { + return Err(AppCommandError::invalid_input( + "That entry is no longer a folder link", + )); + } else { + // The link is missing on disk; recreate it under the new name so the + // rename doubles as a repair. + create_dir_symlink(Path::new(&row.target_path), &to)?; + } + + let updated = folder_link_service::rename(&db.conn, link_id, &sanitized) + .await + .map_err(AppCommandError::from)? + .ok_or_else(|| AppCommandError::not_found("Folder link not found"))?; + + emit_event( + emitter, + FOLDER_LINKS_CHANGED_EVENT, + FolderLinksChanged { + folder_id: row.folder_id, + }, + ); + Ok(FolderLinkDetail { + status: link_status(&root, &updated.name, &updated.target_path), + id: updated.id, + folder_id: updated.folder_id, + name: updated.name, + target_path: updated.target_path, + }) +} + +/// Recreate the symlink for a link whose on-disk entry went missing. +pub async fn repair_folder_link_core( + emitter: &EventEmitter, + db: &AppDatabase, + link_id: i32, +) -> Result { + let row = folder_link_service::get_by_id(&db.conn, link_id) + .await + .map_err(AppCommandError::from)? + .ok_or_else(|| AppCommandError::not_found("Folder link not found"))?; + let root = load_root(db, row.folder_id).await?; + let link_path = root.join(&row.name); + + let target = std::fs::canonicalize(&row.target_path).map_err(|_| { + AppCommandError::not_found("The linked folder no longer exists") + .with_detail(row.target_path.clone()) + })?; + + match std::fs::symlink_metadata(&link_path) { + Ok(md) if md.file_type().is_symlink() => { + // Dangling or pointing elsewhere — replace it. + remove_link_entry(&link_path).map_err(|e| { + AppCommandError::io_error("Failed to replace the folder link") + .with_detail(e.to_string()) + })?; + } + Ok(_) => { + return Err(AppCommandError::already_exists( + "Something else already occupies that name", + ) + .with_detail(row.name.clone())); + } + Err(_) => {} + } + + create_dir_symlink(&target, &link_path)?; + crate::folder_links::register(&root, &target); + + emit_event( + emitter, + FOLDER_LINKS_CHANGED_EVENT, + FolderLinksChanged { + folder_id: row.folder_id, + }, + ); + Ok(FolderLinkDetail { + status: link_status(&root, &row.name, &row.target_path), + id: row.id, + folder_id: row.folder_id, + name: row.name, + target_path: row.target_path, + }) +} + +/// Drop a link. With `delete_link` the symlink itself is removed from the root; +/// the directory it pointed at is never touched. +/// +/// The filesystem goes first and its failure is fatal. Dropping the row first +/// would report success while leaving a symlink the UI no longer lists — still +/// fully traversable by an agent CLI whose cwd is the workspace root, and no +/// longer removable from the app. +pub async fn remove_folder_link_core( + emitter: &EventEmitter, + db: &AppDatabase, + link_id: i32, + delete_link: bool, +) -> Result<(), AppCommandError> { + let Some(row) = folder_link_service::get_by_id(&db.conn, link_id) + .await + .map_err(AppCommandError::from)? + else { + return Ok(()); + }; + + let root = load_root(db, row.folder_id).await.ok(); + + if delete_link { + if let Some(root) = root.as_ref() { + let link_path = root.join(&row.name); + remove_link_entry(&link_path).map_err(|e| { + AppCommandError::io_error("Failed to remove the folder link") + .with_detail(format!("{}: {e}", link_path.display())) + })?; + } + } + + folder_link_service::delete(&db.conn, link_id) + .await + .map_err(AppCommandError::from)?; + + if let Some(root) = root.as_ref() { + crate::folder_links::unregister(root, Path::new(&row.target_path)); + } + + emit_event( + emitter, + FOLDER_LINKS_CHANGED_EVENT, + FolderLinksChanged { + folder_id: row.folder_id, + }, + ); + Ok(()) +} + +// --------------------------------------------------------------------------- +// Tauri command wrappers +// --------------------------------------------------------------------------- + +#[cfg(feature = "tauri-runtime")] +#[cfg_attr(feature = "tauri-runtime", tauri::command)] +pub async fn list_folder_links( + db: tauri::State<'_, AppDatabase>, + folder_id: i32, +) -> Result, AppCommandError> { + list_folder_links_core(&db, folder_id).await +} + +#[cfg(feature = "tauri-runtime")] +#[cfg_attr(feature = "tauri-runtime", tauri::command)] +pub async fn preview_folder_links( + db: tauri::State<'_, AppDatabase>, + folder_id: i32, + paths: Vec, +) -> Result, AppCommandError> { + preview_folder_links_core(&db, folder_id, paths).await +} + +#[cfg(feature = "tauri-runtime")] +#[cfg_attr(feature = "tauri-runtime", tauri::command)] +pub async fn create_folder_links( + app: tauri::AppHandle, + db: tauri::State<'_, AppDatabase>, + folder_id: i32, + items: Vec, + git_exclude: Option, +) -> Result, AppCommandError> { + create_folder_links_core( + &EventEmitter::Tauri(app), + &db, + folder_id, + items, + git_exclude.unwrap_or(true), + ) + .await +} + +#[cfg(feature = "tauri-runtime")] +#[cfg_attr(feature = "tauri-runtime", tauri::command)] +pub async fn rename_folder_link( + app: tauri::AppHandle, + db: tauri::State<'_, AppDatabase>, + link_id: i32, + new_name: String, +) -> Result { + rename_folder_link_core(&EventEmitter::Tauri(app), &db, link_id, new_name).await +} + +#[cfg(feature = "tauri-runtime")] +#[cfg_attr(feature = "tauri-runtime", tauri::command)] +pub async fn repair_folder_link( + app: tauri::AppHandle, + db: tauri::State<'_, AppDatabase>, + link_id: i32, +) -> Result { + repair_folder_link_core(&EventEmitter::Tauri(app), &db, link_id).await +} + +#[cfg(feature = "tauri-runtime")] +#[cfg_attr(feature = "tauri-runtime", tauri::command)] +pub async fn remove_folder_link( + app: tauri::AppHandle, + db: tauri::State<'_, AppDatabase>, + link_id: i32, + delete_link: Option, +) -> Result<(), AppCommandError> { + remove_folder_link_core( + &EventEmitter::Tauri(app), + &db, + link_id, + delete_link.unwrap_or(true), + ) + .await +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn sanitize_replaces_separators_and_illegal_characters() { + assert_eq!(sanitize_link_name("a/b"), "a-b"); + assert_eq!(sanitize_link_name("a\\b"), "a-b"); + assert_eq!(sanitize_link_name("a:b*c?d\"eg|h"), "a-b-c-d-e-f-g-h"); + } + + #[test] + fn sanitize_trims_trailing_dots_and_spaces() { + // Windows silently strips these, which would desync the stored name + // from what actually lands on disk. + assert_eq!(sanitize_link_name("api. "), "api"); + assert_eq!(sanitize_link_name(" api "), "api"); + } + + #[test] + fn sanitize_falls_back_for_degenerate_input() { + assert_eq!(sanitize_link_name(""), FALLBACK_LINK_NAME); + assert_eq!(sanitize_link_name(" "), FALLBACK_LINK_NAME); + assert_eq!(sanitize_link_name("."), FALLBACK_LINK_NAME); + assert_eq!(sanitize_link_name(".."), FALLBACK_LINK_NAME); + } + + #[test] + fn sanitize_defuses_windows_reserved_names() { + assert_eq!(sanitize_link_name("con"), "con-dir"); + assert_eq!(sanitize_link_name("CON"), "CON-dir"); + // Reserved with or without an extension. + assert_eq!(sanitize_link_name("nul.txt"), "nul.txt-dir"); + // Not reserved — only the exact device names are. + assert_eq!(sanitize_link_name("console"), "console"); + } + + #[test] + fn sanitize_truncates_long_names_on_a_char_boundary() { + let long = "é".repeat(200); + let out = sanitize_link_name(&long); + assert_eq!(out.chars().count(), MAX_LINK_NAME_LEN); + } + + #[test] + fn derive_uses_the_final_component() { + assert_eq!(derive_link_name(Path::new("/Users/me/work/api")), "api"); + } + + #[test] + fn derive_falls_back_when_there_is_no_final_component() { + // A filesystem root has no final component; the name must still be + // usable rather than empty. + let name = derive_link_name(Path::new("/")); + assert!(!name.is_empty()); + assert!(!name.contains('/')); + } + + #[test] + fn unique_name_disambiguates_case_insensitively() { + // macOS and Windows filesystems are case-insensitive by default, so + // `API` already occupies `api`. + let taken: HashSet = ["api".to_string()].into_iter().collect(); + assert_eq!(unique_link_name("API", &taken).as_deref(), Some("API-2")); + } + + #[test] + fn unique_name_walks_up_the_suffixes() { + let taken: HashSet = ["api", "api-2", "api-3"] + .into_iter() + .map(str::to_string) + .collect(); + assert_eq!(unique_link_name("api", &taken).as_deref(), Some("api-4")); + } + + #[test] + fn unique_name_gives_up_when_every_probe_is_taken() { + let mut taken: HashSet = HashSet::new(); + taken.insert("api".to_string()); + for n in 2..=MAX_NAME_PROBES { + taken.insert(format!("api-{n}")); + } + assert!(unique_link_name("api", &taken).is_none()); + } + + #[test] + fn git_exclude_line_anchors_at_the_repository_root() { + // `info/exclude` patterns resolve against the work tree top level, so a + // workspace folder nested inside the repo must carry its prefix — + // otherwise the rule would hide an unrelated `/api`. + let top = std::env::temp_dir().join("codeg-exclude-test-repo"); + let nested = top.join("packages/app"); + std::fs::create_dir_all(&nested).expect("mkdir"); + + assert_eq!(git_exclude_line(&top, &top, "api"), "/api"); + assert_eq!( + git_exclude_line(&top, &nested, "api"), + "/packages/app/api" + ); + + let _ = std::fs::remove_dir_all(&top); + } + + #[test] + fn git_exclude_line_degrades_to_the_bare_name_off_tree() { + // Unresolvable paths must not produce a `..`-style pattern; anchoring at + // the root is the safe fallback. + let line = git_exclude_line( + Path::new("/definitely/not/here"), + Path::new("/also/not/here"), + "api", + ); + assert_eq!(line, "/api"); + } + + #[test] + fn unique_name_passes_a_free_name_through() { + assert_eq!( + unique_link_name("api", &HashSet::new()).as_deref(), + Some("api") + ); + } +} + +#[cfg(all(test, unix))] +mod unix_tests { + use super::*; + use std::os::unix::fs::symlink; + + #[test] + fn occupied_names_counts_a_dangling_symlink() { + let root = tempfile::tempdir().expect("root"); + // `Path::exists()` follows the link and reports false for a dangling + // one — creating over it would then fail with AlreadyExists instead of + // being disambiguated. + symlink(root.path().join("gone"), root.path().join("api")).expect("symlink"); + let names = occupied_names(root.path()); + assert!(names.contains("api"), "dangling link occupies its name"); + } + + #[test] + fn validate_rejects_self_ancestor_and_inside() { + let parent = tempfile::tempdir().expect("parent"); + let root = parent.path().join("root"); + std::fs::create_dir(&root).expect("mkdir root"); + let inner = root.join("inner"); + std::fs::create_dir(&inner).expect("mkdir inner"); + let canonical_root = std::fs::canonicalize(&root).expect("canon"); + + assert_eq!( + validate_target(&canonical_root, &root.to_string_lossy()).err(), + Some(FolderLinkRejection::SameAsRoot) + ); + assert_eq!( + validate_target(&canonical_root, &parent.path().to_string_lossy()).err(), + Some(FolderLinkRejection::AncestorOfRoot) + ); + assert_eq!( + validate_target(&canonical_root, &inner.to_string_lossy()).err(), + Some(FolderLinkRejection::InsideRoot) + ); + } + + #[test] + fn validate_rejects_missing_and_non_directories() { + let root = tempfile::tempdir().expect("root"); + let canonical_root = std::fs::canonicalize(root.path()).expect("canon"); + let file = root.path().join("../a-file.txt"); + std::fs::write(&file, b"x").expect("write"); + + assert_eq!( + validate_target(&canonical_root, "/definitely/not/here").err(), + Some(FolderLinkRejection::NotFound) + ); + assert_eq!( + validate_target(&canonical_root, "").err(), + Some(FolderLinkRejection::NotFound) + ); + assert_eq!( + validate_target(&canonical_root, &file.to_string_lossy()).err(), + Some(FolderLinkRejection::NotADirectory) + ); + let _ = std::fs::remove_file(&file); + } + + #[test] + fn remove_link_entry_never_touches_the_target_contents() { + let root = tempfile::tempdir().expect("root"); + let linked = tempfile::tempdir().expect("linked"); + let payload = linked.path().join("important.txt"); + std::fs::write(&payload, b"do not delete").expect("write"); + let link = root.path().join("api"); + symlink(linked.path(), &link).expect("symlink"); + + remove_link_entry(&link).expect("remove link"); + + assert!( + std::fs::symlink_metadata(&link).is_err(), + "the link itself is gone" + ); + assert!( + payload.exists(), + "the linked project's files must survive unlinking" + ); + } + + #[test] + fn remove_link_entry_reports_a_real_directory_without_touching_it() { + let root = tempfile::tempdir().expect("root"); + let real = root.path().join("api"); + std::fs::create_dir(&real).expect("mkdir"); + std::fs::write(real.join("file.txt"), b"x").expect("write"); + + assert_eq!( + remove_link_entry(&real).expect("classified, not failed"), + LinkRemoval::NotALink, + "a real directory in the link's place must be reported, not removed" + ); + assert!(real.join("file.txt").exists(), "contents untouched"); + } + + #[test] + fn remove_link_entry_is_idempotent() { + let root = tempfile::tempdir().expect("root"); + assert_eq!( + remove_link_entry(&root.path().join("nope")).expect("absent is fine"), + LinkRemoval::Absent + ); + } + + #[test] + fn remove_link_entry_surfaces_a_real_failure() { + // A read-only parent makes unlink fail with EACCES. That must be an + // error, not a silent success — the caller drops the DB row on success + // and would otherwise strand a symlink the UI can no longer manage. + use std::os::unix::fs::PermissionsExt; + let root = tempfile::tempdir().expect("root"); + let linked = tempfile::tempdir().expect("linked"); + let link = root.path().join("api"); + symlink(linked.path(), &link).expect("symlink"); + + let original = std::fs::metadata(root.path()).expect("meta").permissions(); + let mut readonly = original.clone(); + readonly.set_mode(0o500); + std::fs::set_permissions(root.path(), readonly).expect("chmod"); + + let result = remove_link_entry(&link); + + // Restore before asserting so the tempdir can always clean itself up. + std::fs::set_permissions(root.path(), original).expect("restore"); + assert!( + result.is_err(), + "an unlink that could not happen must not report success" + ); + assert!( + std::fs::symlink_metadata(&link).is_ok(), + "the link is still there, which is exactly why this must error" + ); + } +} + +/// End-to-end coverage of the create/rename/remove lifecycle against a real +/// filesystem and a real (in-memory) database. +#[cfg(all(test, unix))] +mod lifecycle_tests { + use super::*; + use crate::db::test_helpers::fresh_in_memory_db; + use crate::web::event_bridge::EventEmitter; + + /// These tests assert on filesystem and database state, not on broadcasts. + fn emitter() -> EventEmitter { + EventEmitter::Noop + } + + /// Symlink children of `root`, by name. + fn link_names(root: &Path) -> Vec { + let mut names: Vec = std::fs::read_dir(root) + .expect("read_dir") + .flatten() + .filter(|e| { + e.file_type() + .map(|ft| ft.is_symlink()) + .unwrap_or(false) + }) + .map(|e| e.file_name().to_string_lossy().to_string()) + .collect(); + names.sort(); + names + } + + async fn setup( + root: &Path, + targets: &[&Path], + ) -> (crate::db::AppDatabase, i32, Vec) { + let db = fresh_in_memory_db().await; + let folder = crate::commands::folders::open_folder_core( + &db, + root.to_string_lossy().into_owned(), + ) + .await + .expect("open folder"); + let items = targets + .iter() + .map(|t| FolderLinkRequest { + path: t.to_string_lossy().into_owned(), + name: None, + }) + .collect(); + // `git_exclude: false` keeps the test off `git` entirely. + let created = create_folder_links_core(&emitter(), &db, folder.id, items, false) + .await + .expect("create links"); + (db, folder.id, created) + } + + #[tokio::test] + async fn rename_moves_the_link_even_when_only_the_case_changes() { + let root = tempfile::tempdir().expect("root"); + let linked = tempfile::tempdir().expect("linked"); + let (db, folder_id, created) = setup(root.path(), &[linked.path()]).await; + let link_id = created[0].id; + let original = created[0].name.clone(); + + let renamed = rename_folder_link_core( + &emitter(), + &db, + link_id, + original.to_uppercase(), + ) + .await + .expect("case-only rename"); + + assert_eq!(renamed.name, original.to_uppercase()); + // The row must describe something that is actually there. Skipping the + // filesystem move (because the names fold to the same key) would leave + // the old entry behind and report the link as `missing`. + assert_eq!( + renamed.status, + FolderLinkStatus::Ok, + "the renamed link must resolve on disk" + ); + // Exactly one link, whatever the filesystem's case sensitivity. + assert_eq!(link_names(root.path()).len(), 1); + + let listed = list_folder_links_core(&db, folder_id) + .await + .expect("list"); + assert_eq!(listed.len(), 1); + assert_eq!(listed[0].status, FolderLinkStatus::Ok); + } + + #[tokio::test] + async fn removing_a_link_keeps_the_target_and_drops_the_record() { + let root = tempfile::tempdir().expect("root"); + let linked = tempfile::tempdir().expect("linked"); + std::fs::write(linked.path().join("keep.txt"), b"x").expect("write"); + let (db, folder_id, created) = setup(root.path(), &[linked.path()]).await; + + remove_folder_link_core(&emitter(), &db, created[0].id, true) + .await + .expect("remove"); + + assert!(link_names(root.path()).is_empty(), "symlink is gone"); + assert!( + linked.path().join("keep.txt").exists(), + "the linked project's files survive" + ); + assert!(list_folder_links_core(&db, folder_id) + .await + .expect("list") + .is_empty()); + } + + #[tokio::test] + async fn a_failed_unlink_keeps_the_record_instead_of_reporting_success() { + use std::os::unix::fs::PermissionsExt; + + let root = tempfile::tempdir().expect("root"); + let linked = tempfile::tempdir().expect("linked"); + let (db, folder_id, created) = setup(root.path(), &[linked.path()]).await; + + let original = std::fs::metadata(root.path()).expect("meta").permissions(); + let mut readonly = original.clone(); + readonly.set_mode(0o500); + std::fs::set_permissions(root.path(), readonly).expect("chmod"); + + let result = remove_folder_link_core(&emitter(), &db, created[0].id, true).await; + + std::fs::set_permissions(root.path(), original).expect("restore"); + + assert!(result.is_err(), "the unlink could not happen"); + // The row must survive so the link stays listed and retryable — an + // agent CLI rooted at the workspace can still traverse the symlink. + assert_eq!( + list_folder_links_core(&db, folder_id) + .await + .expect("list") + .len(), + 1 + ); + } + + #[tokio::test] + async fn the_same_directory_submitted_twice_links_once() { + let root = tempfile::tempdir().expect("root"); + let linked = tempfile::tempdir().expect("linked"); + let (_db, _folder_id, created) = + setup(root.path(), &[linked.path(), linked.path()]).await; + + assert_eq!(created.len(), 1, "no `api` + `api-2` pair for one directory"); + assert_eq!(link_names(root.path()).len(), 1); + } + + #[tokio::test] + async fn a_second_workspace_folder_cannot_reach_another_ones_link() { + let root_a = tempfile::tempdir().expect("root a"); + let root_b = tempfile::tempdir().expect("root b"); + let linked = tempfile::tempdir().expect("linked"); + std::fs::write(linked.path().join("secret.txt"), b"x").expect("write"); + let (_db, _folder_id, created) = setup(root_a.path(), &[linked.path()]).await; + let name = created[0].name.clone(); + + // B has an identically-named symlink to the same directory, but never + // authorized it — authorization is per (root, target), not per target. + std::os::unix::fs::symlink(linked.path(), root_b.path().join(&name)).expect("symlink"); + + let canonical_b = std::fs::canonicalize(root_b.path()).expect("canon b"); + let canonical_target = std::fs::canonicalize(linked.path()).expect("canon target"); + assert!( + !crate::folder_links::is_allowed(&canonical_b, &canonical_target.join("secret.txt")), + "another workspace's link must not grant access here" + ); + } +} diff --git a/src-tauri/src/commands/folders.rs b/src-tauri/src/commands/folders.rs index 27b983daf..e22de8486 100644 --- a/src-tauri/src/commands/folders.rs +++ b/src-tauri/src/commands/folders.rs @@ -2945,10 +2945,22 @@ fn compute_etag(content: &[u8], metadata: &std::fs::Metadata) -> String { format!("{:016x}", hasher.finish()) } +/// Whether `canonical_target` may be touched by a workspace operation rooted at +/// `canonical_root`: either it is inside the root, or it is inside a directory +/// the user explicitly linked into that root (see [`crate::folder_links`]). +/// +/// Everything else stays rejected — in particular a symlink that merely happens +/// to sit in the tree, which is what keeps a cloned repo's `secrets -> ~/.ssh` +/// out of reach of the HTML preview's sub-resource inlining. +pub(crate) fn is_within_workspace(canonical_root: &Path, canonical_target: &Path) -> bool { + canonical_target.starts_with(canonical_root) + || crate::folder_links::is_allowed(canonical_root, canonical_target) +} + fn ensure_path_in_workspace(root: &Path, target: &Path) -> Result<(), AppCommandError> { let canonical_root = std::fs::canonicalize(root).map_err(AppCommandError::io)?; let canonical_target = std::fs::canonicalize(target).map_err(AppCommandError::io)?; - if !canonical_target.starts_with(&canonical_root) { + if !is_within_workspace(&canonical_root, &canonical_target) { return Err(AppCommandError::invalid_input( "Path is outside workspace root", )); @@ -3264,6 +3276,58 @@ pub async fn list_directory_with_files( Ok(items) } +/// Directories the user linked into `root`, as `(link name, canonical target)` +/// pairs, restricted to entries that are actually a symlink on disk right now. +/// +/// Only *authorized* links are returned (see [`crate::folder_links`]): a +/// symlink that merely happens to sit in the tree — the `secrets -> ~/.ssh` a +/// cloned repo might ship — is not one of them and stays unfollowed. +fn authorized_links_in(root: &Path) -> Vec<(String, PathBuf)> { + let Ok(canonical_root) = std::fs::canonicalize(root) else { + return Vec::new(); + }; + let targets = crate::folder_links::canonical_targets_for(&canonical_root); + if targets.is_empty() { + return Vec::new(); + } + let Ok(entries) = std::fs::read_dir(root) else { + return Vec::new(); + }; + + let mut links = Vec::new(); + for entry in entries.flatten() { + let Ok(file_type) = entry.file_type() else { + continue; + }; + if !file_type.is_symlink() { + continue; + } + let Ok(resolved) = std::fs::canonicalize(entry.path()) else { + continue; + }; + if !resolved.is_dir() || !targets.contains(&resolved) { + continue; + } + links.push((entry.file_name().to_string_lossy().to_string(), resolved)); + } + links.sort_by_key(|a| a.0.to_lowercase()); + links +} + +/// Rewrite every `path` in `nodes` (recursively) to `/`. Used when +/// a subtree built against a different root is grafted into this one. +fn prefix_tree_paths(nodes: &mut [FileTreeNode], prefix: &str) { + for node in nodes { + match node { + FileTreeNode::File { path, .. } => *path = format!("{prefix}/{path}"), + FileTreeNode::Dir { path, children, .. } => { + *path = format!("{prefix}/{path}"); + prefix_tree_paths(children, prefix); + } + } + } +} + #[cfg_attr(feature = "tauri-runtime", tauri::command)] pub async fn get_file_tree( path: String, @@ -3271,18 +3335,48 @@ pub async fn get_file_tree( ) -> Result, AppCommandError> { let root = PathBuf::from(&path); let depth = max_depth.unwrap_or(usize::MAX); + let mut visited = HashSet::new(); + build_file_tree(&root, depth, &mut visited) +} + +/// Build the tree under `root`, grafting in the subtree of every directory the +/// user linked into it. +/// +/// `visited` holds the canonical roots already expanded on this path, so a +/// workspace that links a folder which links back can't recurse forever; a link +/// that would revisit one is left as an ordinary (unexpanded) entry. +fn build_file_tree( + root: &Path, + depth: usize, + visited: &mut HashSet, +) -> Result, AppCommandError> { + if let Ok(canonical_root) = std::fs::canonicalize(root) { + visited.insert(canonical_root); + } + + // Linked directories are handled separately below: `WalkDir` reports a + // symlink's type as "symlink", not "dir", so left in the main walk they + // would surface as leaf *files* with no way to expand them. + let linked: Vec<(String, PathBuf)> = authorized_links_in(root) + .into_iter() + .filter(|(_, target)| !visited.contains(target)) + .collect(); + let linked_names: HashSet = linked.iter().map(|(name, _)| name.clone()).collect(); // Collect all entries, skipping ignored directories let mut dir_children: HashMap> = HashMap::new(); let mut dir_order: Vec = Vec::new(); let mut dir_paths_by_rel: HashMap = HashMap::new(); - for entry in WalkDir::new(&root) + for entry in WalkDir::new(root) .max_depth(depth) .sort_by_file_name() .into_iter() .filter_entry(|e| { let name = e.file_name().to_string_lossy(); + if e.depth() == 1 && linked_names.contains(name.as_ref()) { + return false; + } if e.file_type().is_dir() { !FILE_TREE_IGNORED_DIRS.contains(&name.as_ref()) } else { @@ -3290,22 +3384,23 @@ pub async fn get_file_tree( } }) { - let entry = entry.map_err(|e| { - AppCommandError::io_error("Failed to walk file tree").with_detail(e.to_string()) - })?; + // Skip unreadable entries (permission errors, races, a symlink loop + // somewhere in the tree) instead of failing the whole tree: one bad + // directory should not blank out the file panel. + let Ok(entry) = entry else { continue }; let entry_path = entry.path().to_path_buf(); // Skip the root itself if entry_path == root { - dir_children.entry(root.clone()).or_default(); - dir_order.push(root.clone()); + dir_children.entry(root.to_path_buf()).or_default(); + dir_order.push(root.to_path_buf()); continue; } - let parent = entry_path.parent().unwrap_or(&root).to_path_buf(); + let parent = entry_path.parent().unwrap_or(root).to_path_buf(); let name = entry.file_name().to_string_lossy().to_string(); let rel_path = entry_path - .strip_prefix(&root) + .strip_prefix(root) .unwrap_or(&entry_path) .to_string_lossy() .replace('\\', "/"); @@ -3397,7 +3492,56 @@ pub async fn get_file_tree( dir_children.insert(dir_path.clone(), sorted); } - Ok(dir_children.remove(&root).unwrap_or_default()) + let mut nodes = dir_children.remove(root).unwrap_or_default(); + + if linked.is_empty() { + return Ok(nodes); + } + + // Graft each linked directory in as a real `Dir`, with its own subtree + // built through the link. `max_depth == 1` (the panel's lazy load) yields + // an empty child list, which the frontend already treats as "not loaded + // yet" and fills in when the row is expanded. + let child_depth = depth.saturating_sub(1); + let mut linked_nodes: Vec = Vec::with_capacity(linked.len()); + for (name, _) in linked { + let link_path = root.join(&name); + // The recursive call is rooted at the link, so its paths come back + // relative to the *linked* directory. Re-anchor them to the workspace + // root, or the panel would resolve `inside.txt` against the root + // instead of `api/inside.txt`. + let prefix = name.replace('\\', "/"); + let children = if child_depth == 0 { + Vec::new() + } else { + let mut sub = build_file_tree(&link_path, child_depth, visited).unwrap_or_default(); + prefix_tree_paths(&mut sub, &prefix); + sub + }; + linked_nodes.push(FileTreeNode::Dir { + name, + path: prefix, + children, + }); + } + + // Re-establish the "directories first, then files, each alphabetical" + // ordering the walk produced before the graft. + let split = nodes + .iter() + .position(|n| matches!(n, FileTreeNode::File { .. })) + .unwrap_or(nodes.len()); + let files = nodes.split_off(split); + nodes.extend(linked_nodes); + nodes.sort_by(|a, b| { + let key = |n: &FileTreeNode| match n { + FileTreeNode::Dir { name, .. } | FileTreeNode::File { name, .. } => name.to_lowercase(), + }; + key(a).cmp(&key(b)) + }); + nodes.extend(files); + + Ok(nodes) } /// Flat, gitignore-aware listing of every file and directory under `path`, for @@ -3411,6 +3555,11 @@ pub async fn list_workspace_files( path: String, ) -> Result, AppCommandError> { let root = PathBuf::from(&path); + // Linked directories are walked separately below, rooted at the link so + // their own `.gitignore` applies. Excluded from the main pass because the + // walker reports a symlink as a leaf file. + let linked = authorized_links_in(&root); + let linked_names: HashSet = linked.iter().map(|(name, _)| name.clone()).collect(); // Conservative gitignore parity with the previous client-side pass: respect // in-tree `.gitignore`/`.ignore`/`.git/info/exclude`, but not the global @@ -3427,8 +3576,11 @@ pub async fn list_workspace_files( .git_global(false) .require_git(false) .sort_by_file_name(|a, b| a.cmp(b)) - .filter_entry(|e| { + .filter_entry(move |e| { let name = e.file_name().to_string_lossy(); + if e.depth() == 1 && linked_names.contains(name.as_ref()) { + return false; + } if e.file_type().map(|t| t.is_dir()).unwrap_or(false) { !FILE_TREE_IGNORED_DIRS.contains(&name.as_ref()) } else { @@ -3468,9 +3620,68 @@ pub async fn list_workspace_files( }); } + for (link_name, target) in linked { + entries.push(WorkspaceFileEntry { + name: link_name.clone(), + path: link_name.clone(), + kind: WorkspaceEntryKind::Dir, + }); + // Rooted at the resolved target so the linked project's own ignore + // files apply, then re-prefixed with the link name so every path stays + // relative to the workspace root and resolves back through the link. + entries.extend(list_files_under(&target, &link_name)); + } + Ok(entries) } +/// Flat listing of `root`, with every path prefixed by `prefix/`. Shares the +/// ignore configuration of [`list_workspace_files`]; nested symlinks are not +/// followed, so this cannot recurse. +fn list_files_under(root: &Path, prefix: &str) -> Vec { + let walker = WalkBuilder::new(root) + .hidden(false) + .parents(false) + .ignore(true) + .git_ignore(true) + .git_exclude(true) + .git_global(false) + .require_git(false) + .sort_by_file_name(|a, b| a.cmp(b)) + .filter_entry(|e| { + let name = e.file_name().to_string_lossy(); + if e.file_type().map(|t| t.is_dir()).unwrap_or(false) { + !FILE_TREE_IGNORED_DIRS.contains(&name.as_ref()) + } else { + name != ".DS_Store" + } + }) + .build(); + + let mut entries = Vec::new(); + for result in walker { + let Ok(entry) = result else { continue }; + let entry_path = entry.path(); + if entry_path == root { + continue; + } + let Ok(rel) = entry_path.strip_prefix(root) else { + continue; + }; + let is_dir = entry.file_type().map(|t| t.is_dir()).unwrap_or(false); + entries.push(WorkspaceFileEntry { + name: entry.file_name().to_string_lossy().to_string(), + path: format!("{prefix}/{}", rel.to_string_lossy().replace('\\', "/")), + kind: if is_dir { + WorkspaceEntryKind::Dir + } else { + WorkspaceEntryKind::File + }, + }); + } + entries +} + #[cfg_attr(feature = "tauri-runtime", tauri::command)] pub async fn read_file_base64( path: String, @@ -3582,7 +3793,7 @@ pub async fn read_workspace_file_base64( std::fs::canonicalize(&root).map_err(AppCommandError::io)?; let canonical_target = std::fs::canonicalize(&target).map_err(AppCommandError::io)?; - if !canonical_target.starts_with(&canonical_root) { + if !is_within_workspace(&canonical_root, &canonical_target) { return Err(AppCommandError::invalid_input( "Path is outside workspace root", )); @@ -5880,4 +6091,124 @@ mod workspace_confinement_tests { assert!(meta.file_type().is_symlink(), "entry stays a symlink"); assert!(!root.path().join("link.txt").exists(), "old link gone"); } + #[tokio::test] + async fn file_tree_walks_into_an_authorized_link_only() { + let root = tempfile::tempdir().expect("root"); + let linked = tempfile::tempdir().expect("linked"); + let stray = tempfile::tempdir().expect("stray"); + std::fs::write(root.path().join("own.txt"), b"x").expect("write"); + std::fs::write(linked.path().join("inside.txt"), b"x").expect("write"); + std::fs::create_dir(linked.path().join("src")).expect("mkdir"); + std::fs::write(linked.path().join("src/deep.ts"), b"x").expect("write"); + std::fs::write(stray.path().join("secret.txt"), b"x").expect("write"); + symlink(linked.path(), root.path().join("api")).expect("symlink"); + symlink(stray.path(), root.path().join("evil")).expect("symlink"); + + // Only `api` is registered, mimicking a user-created link; `evil` is the + // kind of symlink a cloned repo could ship. + let canonical_target = std::fs::canonicalize(linked.path()).expect("canon"); + crate::folder_links::register(root.path(), &canonical_target); + + let nodes = get_file_tree(root.path().to_string_lossy().into_owned(), None) + .await + .expect("tree"); + + let api = nodes + .iter() + .find(|n| matches!(n, FileTreeNode::Dir { name, .. } if name == "api")) + .expect("authorized link renders as a directory"); + match api { + FileTreeNode::Dir { children, path, .. } => { + assert_eq!(path, "api"); + assert!( + children.iter().any( + |c| matches!(c, FileTreeNode::File { path, .. } if path == "api/inside.txt") + ), + "the linked directory's contents are grafted in: {children:?}" + ); + // Nested paths must be re-anchored too, or the panel would + // resolve them against the workspace root instead of the link. + let src = children + .iter() + .find(|c| matches!(c, FileTreeNode::Dir { name, .. } if name == "src")) + .expect("nested directory is present"); + match src { + FileTreeNode::Dir { path, children, .. } => { + assert_eq!(path, "api/src"); + assert!( + children.iter().any(|c| matches!( + c, + FileTreeNode::File { path, .. } if path == "api/src/deep.ts" + )), + "deep paths are prefixed once: {children:?}" + ); + } + _ => unreachable!(), + } + } + _ => unreachable!(), + } + + // The unregistered symlink stays a leaf: it is not followed. + assert!( + nodes + .iter() + .any(|n| matches!(n, FileTreeNode::File { name, .. } if name == "evil")), + "an unauthorized symlink must not be walked into: {nodes:?}" + ); + + crate::folder_links::unregister(root.path(), &canonical_target); + } + + #[tokio::test] + async fn workspace_search_lists_files_inside_an_authorized_link() { + let root = tempfile::tempdir().expect("root"); + let linked = tempfile::tempdir().expect("linked"); + std::fs::create_dir(linked.path().join("src")).expect("mkdir"); + std::fs::write(linked.path().join("src/deep.ts"), b"x").expect("write"); + symlink(linked.path(), root.path().join("api")).expect("symlink"); + let canonical_target = std::fs::canonicalize(linked.path()).expect("canon"); + crate::folder_links::register(root.path(), &canonical_target); + + let entries = list_workspace_files(root.path().to_string_lossy().into_owned()) + .await + .expect("list"); + let paths: Vec<&str> = entries.iter().map(|e| e.path.as_str()).collect(); + assert!(paths.contains(&"api"), "link itself is listed: {paths:?}"); + assert!( + paths.contains(&"api/src/deep.ts"), + "linked content is reachable by fuzzy search: {paths:?}" + ); + + crate::folder_links::unregister(root.path(), &canonical_target); + } + + #[test] + fn workspace_guard_allows_a_registered_link_but_not_a_stray_symlink() { + let root = tempfile::tempdir().expect("root"); + let linked = tempfile::tempdir().expect("linked"); + let stray = tempfile::tempdir().expect("stray"); + std::fs::write(linked.path().join("ok.txt"), b"x").expect("write"); + std::fs::write(stray.path().join("secret.txt"), b"x").expect("write"); + symlink(linked.path(), root.path().join("api")).expect("symlink"); + symlink(stray.path(), root.path().join("evil")).expect("symlink"); + + let canonical_target = std::fs::canonicalize(linked.path()).expect("canon"); + crate::folder_links::register(root.path(), &canonical_target); + + assert!( + ensure_path_in_workspace(root.path(), &root.path().join("api/ok.txt")).is_ok(), + "a file inside a user-authorized link is in the workspace" + ); + assert!( + ensure_path_in_workspace(root.path(), &root.path().join("evil/secret.txt")).is_err(), + "an unregistered symlink still cannot escape the root" + ); + + crate::folder_links::unregister(root.path(), &canonical_target); + assert!( + ensure_path_in_workspace(root.path(), &root.path().join("api/ok.txt")).is_err(), + "revoking the link revokes access" + ); + } } diff --git a/src-tauri/src/commands/mod.rs b/src-tauri/src/commands/mod.rs index 4d96c845c..ee9d94fc8 100644 --- a/src-tauri/src/commands/mod.rs +++ b/src-tauri/src/commands/mod.rs @@ -14,6 +14,7 @@ pub mod feedback; #[cfg(feature = "tauri-runtime")] pub mod file_io; pub mod folder_commands; +pub mod folder_links; pub mod folders; pub mod logging; pub mod mcp; diff --git a/src-tauri/src/commands/work_task.rs b/src-tauri/src/commands/work_task.rs index 541caba23..a9e40b7fa 100644 --- a/src-tauri/src/commands/work_task.rs +++ b/src-tauri/src/commands/work_task.rs @@ -107,7 +107,10 @@ pub async fn work_task_delete_core( } if matches!( task.status, - WorkTaskStatus::Queued | WorkTaskStatus::Running | WorkTaskStatus::AwaitingInput + WorkTaskStatus::Queued + | WorkTaskStatus::Preparing + | WorkTaskStatus::Running + | WorkTaskStatus::AwaitingInput ) { engine()?.cancel(id).await.map_err(DbError::Validation)?; } diff --git a/src-tauri/src/db/entities/folder_link.rs b/src-tauri/src/db/entities/folder_link.rs new file mode 100644 index 000000000..e92cdcc4c --- /dev/null +++ b/src-tauri/src/db/entities/folder_link.rs @@ -0,0 +1,41 @@ +use sea_orm::entity::prelude::*; + +/// A directory the user symlinked into a workspace folder to build a +/// multi-folder workspace. `name` is the symlink's file name inside the root, +/// `target_path` the absolute path it points at (stored verbatim, resolved at +/// use time so a moved/remounted target surfaces as broken instead of silently +/// resolving somewhere else). +/// +/// Doubles as the authorization record consulted by the workspace path guard — +/// see `crate::folder_links`. +#[derive(Clone, Debug, PartialEq, DeriveEntityModel)] +#[sea_orm(table_name = "folder_link")] +pub struct Model { + #[sea_orm(primary_key)] + pub id: i32, + pub folder_id: i32, + #[sea_orm(column_type = "Text")] + pub name: String, + #[sea_orm(column_type = "Text")] + pub target_path: String, + pub created_at: DateTimeUtc, + pub updated_at: DateTimeUtc, +} + +#[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] +pub enum Relation { + #[sea_orm( + belongs_to = "super::folder::Entity", + from = "Column::FolderId", + to = "super::folder::Column::Id" + )] + Folder, +} + +impl Related for Entity { + fn to() -> RelationDef { + Relation::Folder.def() + } +} + +impl ActiveModelBehavior for ActiveModel {} diff --git a/src-tauri/src/db/entities/mod.rs b/src-tauri/src/db/entities/mod.rs index a74d9c185..e3b1e54e9 100644 --- a/src-tauri/src/db/entities/mod.rs +++ b/src-tauri/src/db/entities/mod.rs @@ -10,6 +10,7 @@ pub mod conversation; pub mod custom_agent; pub mod folder; pub mod folder_command; +pub mod folder_link; pub mod model_provider; pub mod opened_tab; pub mod prelude; diff --git a/src-tauri/src/db/entities/prelude.rs b/src-tauri/src/db/entities/prelude.rs index abf8b9f23..53ea3c89c 100644 --- a/src-tauri/src/db/entities/prelude.rs +++ b/src-tauri/src/db/entities/prelude.rs @@ -12,6 +12,7 @@ pub use super::conversation::Entity as Conversation; pub use super::custom_agent::Entity as CustomAgent; pub use super::folder::Entity as Folder; pub use super::folder_command::Entity as FolderCommand; +pub use super::folder_link::Entity as FolderLink; pub use super::model_provider::Entity as ModelProvider; pub use super::opened_tab::Entity as OpenedTab; pub use super::quick_message::Entity as QuickMessage; diff --git a/src-tauri/src/db/entities/work_task.rs b/src-tauri/src/db/entities/work_task.rs index ddb64393b..425673ef9 100644 --- a/src-tauri/src/db/entities/work_task.rs +++ b/src-tauri/src/db/entities/work_task.rs @@ -2,8 +2,8 @@ use sea_orm::entity::prelude::*; use serde::{Deserialize, Serialize}; /// Lifecycle of a work task. The pipeline is -/// `todo → queued → running ⇄ awaiting_input → review → merging → done`, with -/// `failed` / `canceled` as side paths. Two hard invariants: +/// `todo → queued → preparing → running ⇄ awaiting_input → review → merging → +/// done`, with `failed` / `canceled` as side paths. Two hard invariants: /// - `done` ⟺ merged: only the merge landing (or its crash recovery) writes /// `done`, and `done` never rolls back. /// - Every transition is a conditional UPDATE (CAS) guarded by the expected @@ -14,9 +14,15 @@ use serde::{Deserialize, Serialize}; pub enum WorkTaskStatus { #[sea_orm(string_value = "todo")] Todo, - /// Claimed for execution; waiting for (or undergoing) launch. + /// Claimed for execution; waiting for a concurrency slot. #[sea_orm(string_value = "queued")] Queued, + /// Out of the queue and setting up: worktree creation, the folder's init + /// command, then spawning the agent CLI. No agent turn has started yet — + /// the task holds a slot, can be canceled, and a restart treats it as + /// interrupted exactly like `queued`. + #[sea_orm(string_value = "preparing")] + Preparing, #[sea_orm(string_value = "running")] Running, /// The agent is blocked on a question / permission / plan approval. diff --git a/src-tauri/src/db/migration/m20260803_000001_folder_link.rs b/src-tauri/src/db/migration/m20260803_000001_folder_link.rs new file mode 100644 index 000000000..a810befc0 --- /dev/null +++ b/src-tauri/src/db/migration/m20260803_000001_folder_link.rs @@ -0,0 +1,96 @@ +use sea_orm_migration::prelude::*; + +#[derive(DeriveMigrationName)] +pub struct Migration; + +#[async_trait::async_trait] +impl MigrationTrait for Migration { + async fn up(&self, manager: &SchemaManager) -> Result<(), DbErr> { + // folder_link: a directory the user explicitly symlinked into a + // workspace folder, turning one root into a multi-folder workspace the + // agent CLI (whose cwd is that root) can actually traverse. + // + // This table is also the *authorization* record: the workspace path + // guard only follows a symlink out of the root when a row here says the + // user asked for it, so a checked-in `secrets -> ~/.ssh` in some cloned + // repo stays unreadable. Hard-deleted — nothing references a link, and a + // tombstone would keep granting access after the user unlinked. + manager + .create_table( + Table::create() + .table(FolderLink::Table) + .if_not_exists() + .col( + ColumnDef::new(FolderLink::Id) + .integer() + .not_null() + .auto_increment() + .primary_key(), + ) + .col(ColumnDef::new(FolderLink::FolderId).integer().not_null()) + // Name of the symlink inside the root folder. + .col(ColumnDef::new(FolderLink::Name).text().not_null()) + // Absolute path of the linked directory, stored verbatim so + // the UI can show what the user picked. Resolved (and + // re-resolved) at use time rather than canonicalized here. + .col(ColumnDef::new(FolderLink::TargetPath).text().not_null()) + .col( + ColumnDef::new(FolderLink::CreatedAt) + .timestamp_with_time_zone() + .not_null(), + ) + .col( + ColumnDef::new(FolderLink::UpdatedAt) + .timestamp_with_time_zone() + .not_null(), + ) + .to_owned(), + ) + .await?; + + manager + .create_index( + Index::create() + .if_not_exists() + .name("idx_folder_link_folder_id") + .table(FolderLink::Table) + .col(FolderLink::FolderId) + .to_owned(), + ) + .await?; + + // One name per root: the symlink itself is a filesystem entry, so two + // rows claiming the same name could not both exist on disk anyway. + manager + .create_index( + Index::create() + .if_not_exists() + .name("idx_folder_link_folder_id_name") + .table(FolderLink::Table) + .col(FolderLink::FolderId) + .col(FolderLink::Name) + .unique() + .to_owned(), + ) + .await?; + + Ok(()) + } + + async fn down(&self, manager: &SchemaManager) -> Result<(), DbErr> { + manager + .drop_table(Table::drop().table(FolderLink::Table).to_owned()) + .await + } +} + +#[derive(DeriveIden)] +enum FolderLink { + Table, + Id, + FolderId, + Name, + TargetPath, + CreatedAt, + UpdatedAt, +} diff --git a/src-tauri/src/db/migration/mod.rs b/src-tauri/src/db/migration/mod.rs index 0a0b9755c..a7c6d9c35 100644 --- a/src-tauri/src/db/migration/mod.rs +++ b/src-tauri/src/db/migration/mod.rs @@ -33,6 +33,7 @@ mod m20260728_000002_custom_agent_source; mod m20260801_000001_work_task; mod m20260801_000002_work_task_p2; mod m20260801_000003_work_task_template; +mod m20260803_000001_folder_link; pub struct Migrator; #[async_trait::async_trait] @@ -72,6 +73,7 @@ impl MigratorTrait for Migrator { Box::new(m20260801_000001_work_task::Migration), Box::new(m20260801_000002_work_task_p2::Migration), Box::new(m20260801_000003_work_task_template::Migration), + Box::new(m20260803_000001_folder_link::Migration), ] } } diff --git a/src-tauri/src/db/mod.rs b/src-tauri/src/db/mod.rs index 836aa409d..7a327352c 100644 --- a/src-tauri/src/db/mod.rs +++ b/src-tauri/src/db/mod.rs @@ -96,6 +96,18 @@ pub async fn init_database( tracing::warn!("[custom-agent] failed to hydrate custom agent registry: {e}"); } + // Load user-authorized workspace links before any file command can run, so + // the workspace path guard follows exactly the symlinks the user created + // and nothing else. A failure here fails *closed* (registry stays empty: + // linked subtrees look unreadable) rather than blocking startup. + match crate::folder_links::hydrate(&conn).await { + Ok(count) if count > 0 => { + tracing::info!("[folder-link] hydrated {count} workspace link(s)"); + } + Ok(_) => {} + Err(e) => tracing::warn!("[folder-link] failed to hydrate workspace links: {e}"), + } + Ok(AppDatabase { conn }) } diff --git a/src-tauri/src/db/service/conversation_service.rs b/src-tauri/src/db/service/conversation_service.rs index 4fc86efb5..4ea3dcfd1 100644 --- a/src-tauri/src/db/service/conversation_service.rs +++ b/src-tauri/src/db/service/conversation_service.rs @@ -215,6 +215,52 @@ pub async fn refresh_auto_title( Ok(res.rows_affected > 0) } +/// Adopt an imported conversation's newest activity from its agent-side +/// transcript: stamp `updated_at` with the session file's own last-activity +/// time (never `now()` — the scan is not the activity) and re-sync +/// `message_count`. Returns `true` when a row was written, so the caller can +/// broadcast a sidebar upsert. +/// +/// This is the counterpart to [`refresh_auto_title`] for the OTHER half of a +/// re-import: a session the user kept working on in the agent's own CLI after +/// importing it into codeg. Its title may be unchanged while its activity is +/// hours newer, and `updated_at` is what the sidebar's "recently updated" +/// ordering (and the relative timestamp on each row) reads. +/// +/// One conditional UPDATE, guarded so it can never do harm: +/// * `updated_at < activity_at` — strictly forward. A re-import can never move +/// a conversation backwards or re-order an unchanged one, re-running is a +/// no-op, and a turn running live in codeg (which stamps `updated_at = +/// now()`) wins over a transcript tail parsed moments earlier. +/// * `deleted_at IS NULL` — a soft-deleted conversation stays deleted; agent +/// activity must not half-resurrect an invisible row. +/// * `parent_id IS NULL` — delegation children are not sidebar rows and are +/// maintained by the delegation flow. +/// +/// Everything else the user owns is left alone: `created_at`, `title` (and its +/// lock), `pinned_at`, `status`, and folder placement. +pub async fn refresh_external_activity( + conn: &DatabaseConnection, + conversation_id: i32, + activity_at: chrono::DateTime, + message_count: u32, +) -> Result { + use sea_orm::sea_query::Expr; + let res = conversation::Entity::update_many() + .col_expr(conversation::Column::UpdatedAt, Expr::value(activity_at)) + .col_expr( + conversation::Column::MessageCount, + Expr::value(message_count as i32), + ) + .filter(conversation::Column::Id.eq(conversation_id)) + .filter(conversation::Column::DeletedAt.is_null()) + .filter(conversation::Column::ParentId.is_null()) + .filter(conversation::Column::UpdatedAt.lt(activity_at)) + .exec(conn) + .await?; + Ok(res.rows_affected > 0) +} + /// Pin or unpin a conversation. Sets `pinned_at = now()` when pinning, `NULL` /// when unpinning. Only the `pinned_at` column is written — `updated_at` is /// deliberately left untouched (SeaORM updates only the `Set` field), because @@ -1036,6 +1082,106 @@ mod tests { ); } + #[tokio::test] + async fn refresh_external_activity_moves_forward_only() { + let db = fresh_in_memory_db().await; + let folder = seed_folder(&db, "/tmp/codeg-activity").await; + let row = create( + &db.conn, + folder, + AgentType::ClaudeCode, + Some("kept".into()), + None, + ) + .await + .expect("create"); + let created_at = row.created_at; + let before = row.updated_at; + + // The session kept running in the agent's own CLI after import. + let later = before + chrono::Duration::hours(2); + assert!( + refresh_external_activity(&db.conn, row.id, later, 7) + .await + .expect("newer"), + "newer transcript activity must be adopted" + ); + let summary = get_by_id(&db.conn, row.id).await.expect("get"); + assert_eq!(summary.updated_at, later); + assert_eq!(summary.message_count, 7); + assert_eq!( + summary.created_at, created_at, + "created_at is the import/creation time and must not move" + ); + assert_eq!(summary.title.as_deref(), Some("kept")); + + // Re-scanning the same (or an older) transcript must not move the row + // back or re-order a sidebar sorted by recency. + for (at, label) in [(later, "identical"), (before, "older")] { + assert!( + !refresh_external_activity(&db.conn, row.id, at, 1) + .await + .expect("no-op"), + "{label} activity must be a no-op" + ); + } + let summary = get_by_id(&db.conn, row.id).await.expect("get"); + assert_eq!(summary.updated_at, later); + assert_eq!(summary.message_count, 7, "a no-op must not resync counts"); + } + + #[tokio::test] + async fn refresh_external_activity_skips_deleted_and_child_rows() { + let db = fresh_in_memory_db().await; + let folder = seed_folder(&db, "/tmp/codeg-activity-guards").await; + let parent = create(&db.conn, folder, AgentType::ClaudeCode, None, None) + .await + .expect("parent"); + let child = create_with_delegation( + &db.conn, + folder, + AgentType::ClaudeCode, + None, + None, + Some(DelegationLink { + parent_conversation_id: parent.id, + parent_tool_use_id: "tu-activity".into(), + delegation_call_id: "call-activity".into(), + }), + ) + .await + .expect("child"); + let deleted = create(&db.conn, folder, AgentType::ClaudeCode, None, None) + .await + .expect("deleted"); + soft_delete(&db.conn, deleted.id) + .await + .expect("soft delete"); + + let later = Utc::now() + chrono::Duration::hours(1); + assert!( + !refresh_external_activity(&db.conn, child.id, later, 9) + .await + .expect("child"), + "a delegation child is not a sidebar row" + ); + assert!( + !refresh_external_activity(&db.conn, deleted.id, later, 9) + .await + .expect("deleted"), + "a soft-deleted conversation must never be half-resurrected" + ); + + for id in [child.id, deleted.id] { + let raw = conversation::Entity::find_by_id(id) + .one(&db.conn) + .await + .expect("query") + .expect("row present"); + assert_eq!(raw.message_count, 0, "row {id} untouched"); + } + } + #[tokio::test] async fn create_paths_write_expected_kinds() { let db = fresh_in_memory_db().await; diff --git a/src-tauri/src/db/service/folder_link_service.rs b/src-tauri/src/db/service/folder_link_service.rs new file mode 100644 index 000000000..a1bcfdbd7 --- /dev/null +++ b/src-tauri/src/db/service/folder_link_service.rs @@ -0,0 +1,100 @@ +use chrono::Utc; +use sea_orm::{ + ActiveModelTrait, ActiveValue::NotSet, ColumnTrait, DatabaseConnection, EntityTrait, + QueryFilter, QueryOrder, Set, +}; + +use crate::db::entities::{folder, folder_link}; +use crate::db::error::DbError; + +pub async fn list_by_folder( + conn: &DatabaseConnection, + folder_id: i32, +) -> Result, DbError> { + Ok(folder_link::Entity::find() + .filter(folder_link::Column::FolderId.eq(folder_id)) + .order_by_asc(folder_link::Column::Id) + .all(conn) + .await?) +} + +pub async fn get_by_id( + conn: &DatabaseConnection, + link_id: i32, +) -> Result, DbError> { + Ok(folder_link::Entity::find_by_id(link_id).one(conn).await?) +} + +/// Every link paired with the path of the folder it belongs to. Feeds the +/// process-global authorization registry at startup; links whose folder row is +/// gone (soft-deleted or hard-removed) are dropped rather than granting access +/// against a root nothing can reach. +pub async fn list_all_with_root( + conn: &DatabaseConnection, +) -> Result, DbError> { + let links = folder_link::Entity::find().all(conn).await?; + if links.is_empty() { + return Ok(vec![]); + } + + let folders = folder::Entity::find() + .filter(folder::Column::DeletedAt.is_null()) + .all(conn) + .await?; + let paths: std::collections::HashMap = + folders.into_iter().map(|f| (f.id, f.path)).collect(); + + Ok(links + .into_iter() + .filter_map(|l| { + paths + .get(&l.folder_id) + .map(|root| (root.clone(), l.target_path)) + }) + .collect()) +} + +pub async fn insert( + conn: &DatabaseConnection, + folder_id: i32, + name: &str, + target_path: &str, +) -> Result { + let now = Utc::now(); + let model = folder_link::ActiveModel { + id: NotSet, + folder_id: Set(folder_id), + name: Set(name.to_string()), + target_path: Set(target_path.to_string()), + created_at: Set(now), + updated_at: Set(now), + }; + Ok(model.insert(conn).await?) +} + +pub async fn rename( + conn: &DatabaseConnection, + link_id: i32, + new_name: &str, +) -> Result, DbError> { + let Some(existing) = folder_link::Entity::find_by_id(link_id).one(conn).await? else { + return Ok(None); + }; + let mut active: folder_link::ActiveModel = existing.into(); + active.name = Set(new_name.to_string()); + active.updated_at = Set(Utc::now()); + Ok(Some(active.update(conn).await?)) +} + +/// Hard-delete a link row. Returns the removed row so the caller can drop the +/// on-disk symlink and revoke the matching authorization. +pub async fn delete( + conn: &DatabaseConnection, + link_id: i32, +) -> Result, DbError> { + let Some(existing) = folder_link::Entity::find_by_id(link_id).one(conn).await? else { + return Ok(None); + }; + folder_link::Entity::delete_by_id(link_id).exec(conn).await?; + Ok(Some(existing)) +} diff --git a/src-tauri/src/db/service/import_service.rs b/src-tauri/src/db/service/import_service.rs index 3fad26230..c01e5578d 100644 --- a/src-tauri/src/db/service/import_service.rs +++ b/src-tauri/src/db/service/import_service.rs @@ -229,31 +229,147 @@ pub async fn import_local_conversations( enum ImportOutcome { /// A new conversation row was inserted. Imported, - /// An already-imported conversation had its auto-title refreshed; carries - /// the row id so the caller can broadcast a sidebar upsert. + /// An already-imported conversation was refreshed in place (title and/or + /// transcript activity); carries the row id so the caller can broadcast a + /// sidebar upsert. Updated(i32), - /// Already imported, title left unchanged (locked, identical, or the parse - /// produced no title). + /// Already imported and nothing changed — or the row is one the sidebar + /// never shows (soft-deleted, delegation child). Skipped, } -/// Insert a brand-new conversation, or — when it already exists — refresh its -/// title from the freshly parsed session file so an AI-generated title that did -/// not exist at first import is adopted. `refresh_auto_title` is a single -/// conditional UPDATE that skips locked or unchanged rows and never bumps -/// `updated_at`, so a re-import neither clobbers a manual rename nor reorders a -/// recency-sorted sidebar. A missing/empty parsed title leaves the existing -/// title intact rather than nulling it. +/// The `conversation.agent_type` column's string form. +fn agent_type_db_str(agent_type: &AgentType) -> String { + serde_json::to_value(agent_type) + .ok() + .and_then(|v| v.as_str().map(String::from)) + .unwrap_or_default() +} + +/// Reconcile ONE already-imported conversation against a fresh parse of its +/// agent-side session file. Returns `true` when a row was written, so the +/// caller can count it and broadcast a sidebar upsert. Never inserts, never +/// moves the conversation between folders. +/// +/// Two independent refreshes, because a re-scan can find either kind of drift +/// (or both) — a session titled after the fact, and a session the user kept +/// working on in the agent's own CLI: +/// +/// * [`conversation_service::refresh_auto_title`] adopts a title that did not +/// exist at first import. A missing/empty parsed title leaves the existing +/// one intact rather than nulling it, a locked title is never clobbered, and +/// it deliberately does not bump `updated_at` (a title is metadata, not +/// activity). +/// * [`conversation_service::refresh_external_activity`] adopts the transcript's +/// own last-activity time into `updated_at` (plus the fresh `message_count`) +/// when it is strictly newer, so a conversation continued outside codeg sorts +/// and reads correctly in a recency-ordered sidebar. +/// +/// Both are single conditional UPDATEs whose guards are re-evaluated by the +/// database at write time, so a concurrent manual rename or a live turn wins. +/// The `if` conditions here only mirror those guards in Rust to skip the +/// round-trip when nothing drifted — a converged conversation (the common case, +/// and every row on a re-scan) costs ZERO statements, which is what keeps a +/// whole-machine scan over thousands of imported sessions cheap. +async fn refresh_existing( + conn: &DatabaseConnection, + existing: &conversation::Model, + summary: &ConversationSummary, +) -> Result { + // Rows the sidebar never shows are left completely alone: a soft-deleted + // conversation must stay deleted (never resurrected or rewritten), and a + // delegation child is not a sidebar row (the upsert broadcast suppresses it + // too, which would also desync the `updated` count). + if existing.parent_id.is_some() || existing.deleted_at.is_some() { + return Ok(false); + } + + let mut wrote = false; + + if !existing.title_locked { + if let Some(title) = summary + .title + .as_deref() + .map(str::trim) + .filter(|t| !t.is_empty() && existing.title.as_deref() != Some(*t)) + { + wrote |= conversation_service::refresh_auto_title(conn, existing.id, title.to_string()) + .await?; + } + } + + if let Some(activity_at) = summary.ended_at.filter(|at| *at > existing.updated_at) { + wrote |= conversation_service::refresh_external_activity( + conn, + existing.id, + activity_at, + summary.message_count, + ) + .await?; + } + + Ok(wrote) +} + +/// Refresh every already-imported session in `items` in place — see +/// [`refresh_existing`]. Returns the ids actually written so the caller can +/// broadcast sidebar upserts. +/// +/// Unlike [`import_one`] this NEVER inserts: a session the user has not +/// imported yet stays untouched (and unimported) until they pick it. It rides +/// along with the import-picker scan, which already has to load `rows` — every +/// conversation carrying an `external_id` — to mark which sessions exist, so +/// the sync costs one index build plus one UPDATE per row that genuinely +/// drifted, with no per-session SELECT. +/// +/// `rows` is matched, not `items`, so historical duplicate rows sharing an +/// `(agent_type, external_id)` (the pair has no unique index) all converge. +/// A row error is logged and skipped: a best-effort refresh must never fail the +/// scan it rides along with. +pub(crate) async fn sync_imported_sessions( + conn: &DatabaseConnection, + rows: &[conversation::Model], + items: &[(AgentType, ConversationSummary)], +) -> Vec { + let parsed: std::collections::HashMap<(String, &str), &ConversationSummary> = items + .iter() + .map(|(at, s)| ((agent_type_db_str(at), s.id.as_str()), s)) + .collect(); + + let mut refreshed = Vec::new(); + for row in rows { + if row.parent_id.is_some() || row.deleted_at.is_some() { + continue; + } + let Some(external_id) = row.external_id.as_deref() else { + continue; + }; + let Some(summary) = parsed.get(&(row.agent_type.clone(), external_id)) else { + continue; + }; + match refresh_existing(conn, row, summary).await { + Ok(true) => refreshed.push(row.id), + Ok(false) => {} + Err(e) => tracing::error!( + "Failed to refresh imported session {} ({}): {}", + external_id, + row.agent_type, + e + ), + } + } + refreshed +} + +/// Insert a brand-new conversation, or — when it already exists — refresh it in +/// place from the freshly parsed session file (see [`refresh_existing`]). async fn import_one( conn: &DatabaseConnection, folder_id: i32, agent_type: &AgentType, summary: &ConversationSummary, ) -> Result { - let at_str = serde_json::to_value(agent_type) - .ok() - .and_then(|v| v.as_str().map(String::from)) - .unwrap_or_default(); + let at_str = agent_type_db_str(agent_type); let exists = conversation::Entity::find() .filter(conversation::Column::ExternalId.eq(&summary.id)) @@ -262,26 +378,11 @@ async fn import_one( .await?; if let Some(existing) = exists { - // Preserve the original skip for rows the sidebar never shows: a - // soft-deleted conversation must stay deleted (never resurrected or - // rewritten), and a delegation child is not a sidebar row (the upsert - // broadcast suppresses it too, which would also desync the `updated` - // count). Only a visible root conversation gets its title refreshed. - if existing.parent_id.is_some() || existing.deleted_at.is_some() { - return Ok(ImportOutcome::Skipped); - } - if let Some(title) = summary - .title - .as_deref() - .map(str::trim) - .filter(|t| !t.is_empty()) - { - if conversation_service::refresh_auto_title(conn, existing.id, title.to_string()).await? - { - return Ok(ImportOutcome::Updated(existing.id)); - } - } - return Ok(ImportOutcome::Skipped); + return Ok(if refresh_existing(conn, &existing, summary).await? { + ImportOutcome::Updated(existing.id) + } else { + ImportOutcome::Skipped + }); } let created_at = summary.started_at; @@ -320,7 +421,7 @@ async fn import_one( mod tests { use super::*; use crate::db::test_helpers::{fresh_in_memory_db, seed_folder}; - use chrono::Utc; + use chrono::{DateTime, Duration, Utc}; fn summary(id: &str, title: Option<&str>) -> ConversationSummary { ConversationSummary { @@ -340,6 +441,41 @@ mod tests { } } + /// A parse of a session whose transcript ends at `ended_at` — what a + /// re-scan sees after the user kept working on it in the agent's own CLI. + fn timed_summary( + id: &str, + title: Option<&str>, + ended_at: DateTime, + message_count: u32, + ) -> ConversationSummary { + ConversationSummary { + started_at: ended_at - Duration::hours(1), + ended_at: Some(ended_at), + message_count, + ..summary(id, title) + } + } + + async fn find_row(conn: &DatabaseConnection, ext: &str) -> conversation::Model { + conversation::Entity::find() + .filter(conversation::Column::ExternalId.eq(ext)) + .one(conn) + .await + .expect("query") + .expect("row exists") + } + + /// Every conversation carrying an `external_id` — the row set the import + /// scan loads and hands to [`sync_imported_sessions`]. + async fn external_rows(conn: &DatabaseConnection) -> Vec { + conversation::Entity::find() + .filter(conversation::Column::ExternalId.is_not_null()) + .all(conn) + .await + .expect("query") + } + async fn find_id(conn: &DatabaseConnection, ext: &str) -> i32 { conversation::Entity::find() .filter(conversation::Column::ExternalId.eq(ext)) @@ -508,6 +644,235 @@ mod tests { assert!(row.deleted_at.is_some(), "must stay soft-deleted"); } + #[tokio::test] + async fn reimport_adopts_newer_transcript_activity() { + let db = fresh_in_memory_db().await; + let folder = seed_folder(&db, "/tmp/codeg-import-activity").await; + let at = AgentType::ClaudeCode; + + let first_end = Utc::now() - Duration::hours(3); + import_one( + &db.conn, + folder, + &at, + &timed_summary("ext-1", Some("kept"), first_end, 2), + ) + .await + .expect("import"); + let created_at = find_row(&db.conn, "ext-1").await.created_at; + let id = find_id(&db.conn, "ext-1").await; + + // The user resumed the session in the agent's own CLI and sent more + // messages; a re-scan must move it to the front of a recency-sorted + // sidebar and show the transcript's time, not the scan's. + let later = first_end + Duration::hours(1); + assert_eq!( + import_one( + &db.conn, + folder, + &at, + &timed_summary("ext-1", Some("kept"), later, 5) + ) + .await + .expect("re-import"), + ImportOutcome::Updated(id) + ); + + let row = find_row(&db.conn, "ext-1").await; + assert_eq!(row.updated_at, later, "updated_at follows the transcript"); + assert_eq!(row.message_count, 5); + assert_eq!( + row.created_at, created_at, + "created_at is the original session start and must not move" + ); + assert_eq!(row.title.as_deref(), Some("kept")); + } + + #[tokio::test] + async fn reimport_never_moves_updated_at_backwards() { + let db = fresh_in_memory_db().await; + let folder = seed_folder(&db, "/tmp/codeg-import-monotonic").await; + let at = AgentType::ClaudeCode; + + let end = Utc::now() - Duration::hours(1); + import_one( + &db.conn, + folder, + &at, + &timed_summary("ext-1", Some("kept"), end, 4), + ) + .await + .expect("import"); + + // Re-scanning the same transcript, or one that somehow parses older + // (clock skew, a truncated tail), must change nothing at all. + for (ended_at, label) in [(end, "identical"), (end - Duration::hours(2), "older")] { + assert_eq!( + import_one( + &db.conn, + folder, + &at, + &timed_summary("ext-1", Some("kept"), ended_at, 99) + ) + .await + .expect("re-import"), + ImportOutcome::Skipped, + "{label} activity must be a no-op" + ); + } + + let row = find_row(&db.conn, "ext-1").await; + assert_eq!(row.updated_at, end); + assert_eq!(row.message_count, 4); + } + + #[tokio::test] + async fn activity_refresh_preserves_pin_status_and_locked_title() { + let db = fresh_in_memory_db().await; + let folder = seed_folder(&db, "/tmp/codeg-import-preserve").await; + let at = AgentType::ClaudeCode; + + import_one( + &db.conn, + folder, + &at, + &timed_summary("ext-1", Some("first prompt"), Utc::now(), 2), + ) + .await + .expect("import"); + let id = find_id(&db.conn, "ext-1").await; + conversation_service::update_title(&db.conn, id, "User Pick".into()) + .await + .expect("rename"); + conversation_service::update_pin(&db.conn, id, true) + .await + .expect("pin"); + conversation_service::update_status( + &db.conn, + id, + conversation::ConversationStatus::Completed, + ) + .await + .expect("status"); + + let later = Utc::now() + Duration::hours(1); + assert_eq!( + import_one( + &db.conn, + folder, + &at, + &timed_summary("ext-1", Some("AI Summary"), later, 6) + ) + .await + .expect("re-import"), + ImportOutcome::Updated(id), + "activity alone is enough to report an update" + ); + + let row = find_row(&db.conn, "ext-1").await; + assert_eq!(row.updated_at, later); + assert_eq!(row.message_count, 6); + assert_eq!(row.title.as_deref(), Some("User Pick"), "rename survives"); + assert!(row.title_locked); + assert!(row.pinned_at.is_some(), "pin survives"); + assert_eq!( + row.status, + conversation::ConversationStatus::Completed, + "codeg's own status survives" + ); + } + + #[tokio::test] + async fn sync_imported_sessions_refreshes_in_place_and_never_inserts() { + let db = fresh_in_memory_db().await; + let folder = seed_folder(&db, "/tmp/codeg-sync-scan").await; + let at = AgentType::ClaudeCode; + + let end = Utc::now() - Duration::hours(2); + import_one( + &db.conn, + folder, + &at, + &timed_summary("ext-live", Some("first prompt"), end, 2), + ) + .await + .expect("import"); + let live_id = find_id(&db.conn, "ext-live").await; + + // What a re-scan sees: the imported session grew AND got a title, plus + // a session the user has never imported. + let later = end + Duration::hours(1); + let items = vec![ + (at, timed_summary("ext-live", Some("AI Summary"), later, 5)), + (at, timed_summary("ext-never", Some("untouched"), later, 3)), + ]; + let rows = external_rows(&db.conn).await; + assert_eq!( + sync_imported_sessions(&db.conn, &rows, &items).await, + vec![live_id] + ); + + let row = find_row(&db.conn, "ext-live").await; + assert_eq!(row.updated_at, later); + assert_eq!(row.message_count, 5); + assert_eq!(row.title.as_deref(), Some("AI Summary")); + + assert!( + conversation::Entity::find() + .filter(conversation::Column::ExternalId.eq("ext-never")) + .one(&db.conn) + .await + .expect("query") + .is_none(), + "a session the user never imported must stay unimported" + ); + + // Idempotent: a second scan over the same transcripts writes nothing. + let rows = external_rows(&db.conn).await; + assert!(sync_imported_sessions(&db.conn, &rows, &items) + .await + .is_empty()); + } + + #[tokio::test] + async fn sync_imported_sessions_skips_deleted_rows() { + let db = fresh_in_memory_db().await; + let folder = seed_folder(&db, "/tmp/codeg-sync-deleted").await; + let at = AgentType::ClaudeCode; + + let end = Utc::now() - Duration::hours(2); + import_one( + &db.conn, + folder, + &at, + &timed_summary("ext-1", Some("original"), end, 2), + ) + .await + .expect("import"); + let id = find_id(&db.conn, "ext-1").await; + conversation_service::soft_delete(&db.conn, id) + .await + .expect("soft delete"); + + let items = vec![( + at, + timed_summary("ext-1", Some("AI Summary"), end + Duration::hours(1), 9), + )]; + let rows = external_rows(&db.conn).await; + assert!( + sync_imported_sessions(&db.conn, &rows, &items) + .await + .is_empty(), + "a deleted conversation must never be half-resurrected by a scan" + ); + + let row = find_row(&db.conn, "ext-1").await; + assert_eq!(row.updated_at, end, "activity untouched"); + assert_eq!(row.message_count, 2); + assert_eq!(row.title.as_deref(), Some("original")); + assert!(row.deleted_at.is_some()); + } + #[tokio::test] async fn reimport_skips_a_delegation_child() { let db = fresh_in_memory_db().await; diff --git a/src-tauri/src/db/service/mod.rs b/src-tauri/src/db/service/mod.rs index 8bf89cbf3..1d4a29cdf 100644 --- a/src-tauri/src/db/service/mod.rs +++ b/src-tauri/src/db/service/mod.rs @@ -6,6 +6,7 @@ pub mod chat_channel_service; pub mod conversation_service; pub mod custom_agent_service; pub mod folder_command_service; +pub mod folder_link_service; pub mod folder_service; pub mod import_service; pub mod model_provider_service; diff --git a/src-tauri/src/db/service/work_task_service.rs b/src-tauri/src/db/service/work_task_service.rs index 47814c906..f112dc8a7 100644 --- a/src-tauri/src/db/service/work_task_service.rs +++ b/src-tauri/src/db/service/work_task_service.rs @@ -35,6 +35,7 @@ pub fn status_str(s: WorkTaskStatus) -> &'static str { match s { WorkTaskStatus::Todo => "todo", WorkTaskStatus::Queued => "queued", + WorkTaskStatus::Preparing => "preparing", WorkTaskStatus::Running => "running", WorkTaskStatus::AwaitingInput => "awaiting_input", WorkTaskStatus::Review => "review", @@ -657,6 +658,7 @@ pub async fn auto_claim_next( .filter(work_task::Column::FolderId.eq(folder_id)) .filter(work_task::Column::Status.is_in([ WorkTaskStatus::Queued, + WorkTaskStatus::Preparing, WorkTaskStatus::Running, WorkTaskStatus::AwaitingInput, WorkTaskStatus::Merging, @@ -756,7 +758,86 @@ pub async fn attach_worktree( Ok(()) } -/// queued → running for the given generation; binds conversation + connection. +/// queued → preparing for the given generation: the task leaves the queue and +/// starts its setup (worktree, init command, agent spawn). Does NOT bump +/// `run_seq` — it is the same generation, just past the wait for a slot. +/// Losing the CAS means a concurrent cancel/claim moved the task on. +pub async fn begin_setup( + conn: &DatabaseConnection, + id: i32, + run_seq: i32, +) -> Result { + let txn = conn.begin().await?; + let res = work_task::Entity::update_many() + .col_expr( + work_task::Column::Status, + Expr::value(status_str(WorkTaskStatus::Preparing)), + ) + .col_expr(work_task::Column::UpdatedAt, Expr::value(Utc::now())) + .filter(work_task::Column::Id.eq(id)) + .filter(work_task::Column::Status.eq(WorkTaskStatus::Queued)) + .filter(work_task::Column::RunSeq.eq(run_seq)) + .filter(work_task::Column::DeletedAt.is_null()) + .exec(&txn) + .await?; + if res.rows_affected != 1 { + txn.rollback().await?; + return Ok(false); + } + status_changed_event( + &txn, + id, + "engine", + Some(WorkTaskStatus::Queued), + WorkTaskStatus::Preparing, + None, + ) + .await?; + txn.commit().await?; + Ok(true) +} + +/// preparing → queued: the reconcile sweep's escape hatch for a setup that no +/// process owns any more (its launch task died without failing the row). Back +/// in the queue, the pump simply relaunches it — the same self-healing a stuck +/// `queued` row gets today. +pub async fn abandon_setup( + conn: &DatabaseConnection, + id: i32, + run_seq: i32, +) -> Result { + let txn = conn.begin().await?; + let res = work_task::Entity::update_many() + .col_expr( + work_task::Column::Status, + Expr::value(status_str(WorkTaskStatus::Queued)), + ) + .col_expr(work_task::Column::UpdatedAt, Expr::value(Utc::now())) + .filter(work_task::Column::Id.eq(id)) + .filter(work_task::Column::Status.eq(WorkTaskStatus::Preparing)) + .filter(work_task::Column::RunSeq.eq(run_seq)) + .filter(work_task::Column::DeletedAt.is_null()) + .exec(&txn) + .await?; + if res.rows_affected != 1 { + txn.rollback().await?; + return Ok(false); + } + status_changed_event( + &txn, + id, + "engine", + Some(WorkTaskStatus::Preparing), + WorkTaskStatus::Queued, + None, + ) + .await?; + txn.commit().await?; + Ok(true) +} + +/// preparing → running for the given generation; binds conversation + +/// connection. pub async fn mark_running( conn: &DatabaseConnection, id: i32, @@ -782,7 +863,7 @@ pub async fn mark_running( .col_expr(work_task::Column::StartedAt, Expr::value(Some(now))) .col_expr(work_task::Column::UpdatedAt, Expr::value(now)) .filter(work_task::Column::Id.eq(id)) - .filter(work_task::Column::Status.eq(WorkTaskStatus::Queued)) + .filter(work_task::Column::Status.eq(WorkTaskStatus::Preparing)) .filter(work_task::Column::RunSeq.eq(run_seq)) .filter(work_task::Column::DeletedAt.is_null()) .exec(&txn) @@ -795,7 +876,7 @@ pub async fn mark_running( &txn, id, "engine", - Some(WorkTaskStatus::Queued), + Some(WorkTaskStatus::Preparing), WorkTaskStatus::Running, None, ) @@ -1300,6 +1381,7 @@ pub async fn cancel(conn: &DatabaseConnection, id: i32) -> Result .filter(work_task::Column::Status.is_in([ WorkTaskStatus::Todo, WorkTaskStatus::Queued, + WorkTaskStatus::Preparing, WorkTaskStatus::Running, WorkTaskStatus::AwaitingInput, WorkTaskStatus::Review, @@ -1373,6 +1455,7 @@ pub async fn boot_reconcile_interrupted(conn: &DatabaseConnection) -> Result Result Result { + assert!(begin_setup(conn, id, run_seq).await?); + mark_running(conn, id, run_seq, conversation_id, connection_id).await + } + /// Drive a task to review and return its current run_seq. async fn to_review(db: &crate::db::AppDatabase, id: i32) -> i32 { let seq = claim_for_run(&db.conn, id, WorkTaskStatus::Todo, "user") .await .unwrap() .unwrap(); - assert!(mark_running(&db.conn, id, seq, 1, "c").await.unwrap()); + assert!(start_running(&db.conn, id, seq, 1, "c").await.unwrap()); assert!(settle_review(&db.conn, id, seq, None, None).await.unwrap()); seq } @@ -2056,7 +2249,7 @@ mod tests { .unwrap() .unwrap(); assert!(get_model(&db.conn, t.id).await.unwrap().preflight.is_none()); - assert!(mark_running(&db.conn, t.id, seq2, 1, "c2").await.unwrap()); + assert!(start_running(&db.conn, t.id, seq2, 1, "c2").await.unwrap()); assert!(settle_review(&db.conn, t.id, seq2, None, None).await.unwrap()); assert!(!set_preflight(&db.conn, t.id, seq, &light).await.unwrap()); assert!(get_model(&db.conn, t.id).await.unwrap().preflight.is_none()); @@ -2076,7 +2269,7 @@ mod tests { .await .unwrap() .unwrap(); - assert!(mark_running(&db.conn, t.id, seq, 1, "c").await.unwrap()); + assert!(start_running(&db.conn, t.id, seq, 1, "c").await.unwrap()); assert!(fail( &db.conn, t.id, diff --git a/src-tauri/src/folder_links.rs b/src-tauri/src/folder_links.rs new file mode 100644 index 000000000..427a1c7d2 --- /dev/null +++ b/src-tauri/src/folder_links.rs @@ -0,0 +1,170 @@ +//! Process-global registry of user-authorized workspace links. +//! +//! A workspace folder can have other directories symlinked in as subdirectories +//! (see `commands::folder_links`). Every file operation in the workspace is +//! confined by canonicalizing the target and requiring it to stay under the +//! canonical root — deliberately, so a cloned repo shipping +//! `secrets -> ~/.ssh` can't be read through the HTML preview's sub-resource +//! inlining. Following the *user's own* links therefore needs an explicit +//! allowlist, and that allowlist is this registry. +//! +//! Hydrated from `folder_link` rows at startup (mirroring +//! `custom_agent_service::hydrate_registry`) and kept in sync as links are +//! created / renamed / removed. Empty by default, so an install that never +//! links a folder behaves exactly as before. +//! +//! Paths are stored verbatim and canonicalized at check time rather than at +//! registration time: a target on a slow/absent mount that fails to resolve +//! during startup still works once it's back, and a target replaced on disk +//! never keeps its old authorization. + +use std::path::{Path, PathBuf}; +use std::sync::{LazyLock, RwLock}; + +#[derive(Debug, Clone, PartialEq, Eq)] +struct LinkEntry { + /// Workspace folder path, as stored in `folder.path`. + root: PathBuf, + /// Linked directory, as the user picked it. + target: PathBuf, +} + +static LINKS: LazyLock>> = LazyLock::new(|| RwLock::new(Vec::new())); + +fn snapshot() -> Vec { + LINKS + .read() + .map(|guard| guard.clone()) + .unwrap_or_else(|poisoned| poisoned.into_inner().clone()) +} + +fn with_links(f: impl FnOnce(&mut Vec) -> R) -> R { + match LINKS.write() { + Ok(mut guard) => f(&mut guard), + Err(poisoned) => f(&mut poisoned.into_inner()), + } +} + +/// Load every persisted link into the registry, replacing whatever was there. +/// Called once per process right after migrations, on the single chokepoint +/// both runtimes go through. +pub async fn hydrate(conn: &sea_orm::DatabaseConnection) -> Result { + let rows = crate::db::service::folder_link_service::list_all_with_root(conn).await?; + let entries: Vec = rows + .into_iter() + .map(|(root, target)| LinkEntry { + root: PathBuf::from(root), + target: PathBuf::from(target), + }) + .collect(); + let count = entries.len(); + with_links(|links| *links = entries); + Ok(count) +} + +/// Authorize `target` as a link of `root`. Idempotent. +pub fn register(root: &Path, target: &Path) { + let entry = LinkEntry { + root: root.to_path_buf(), + target: target.to_path_buf(), + }; + with_links(|links| { + if !links.contains(&entry) { + links.push(entry); + } + }); +} + +/// Revoke a single link. A no-op when it was never registered. +pub fn unregister(root: &Path, target: &Path) { + with_links(|links| { + links.retain(|entry| !(entry.root == root && entry.target == target)); + }); +} + +/// Whether `canonical_target` lies inside a directory the user linked into +/// `canonical_root`. +/// +/// Ordered target-first: this only runs after the plain "inside the root" check +/// already failed, which for a workspace with no links means the registry is +/// empty and the call costs nothing. +pub fn is_allowed(canonical_root: &Path, canonical_target: &Path) -> bool { + let entries = snapshot(); + if entries.is_empty() { + return false; + } + for entry in entries { + let Ok(link_target) = std::fs::canonicalize(&entry.target) else { + continue; + }; + if !canonical_target.starts_with(&link_target) { + continue; + } + let Ok(link_root) = std::fs::canonicalize(&entry.root) else { + continue; + }; + if link_root == canonical_root { + return true; + } + } + false +} + +/// Canonicalized targets linked into `root`. Used by the file-tree and +/// workspace-search walkers to decide which symlinks to descend into; a target +/// that no longer resolves is simply absent. +pub fn canonical_targets_for(canonical_root: &Path) -> Vec { + let entries = snapshot(); + let mut out = Vec::new(); + for entry in entries { + let Ok(link_root) = std::fs::canonicalize(&entry.root) else { + continue; + }; + if link_root != canonical_root { + continue; + } + if let Ok(link_target) = std::fs::canonicalize(&entry.target) { + if !out.contains(&link_target) { + out.push(link_target); + } + } + } + out +} + +#[cfg(test)] +mod tests { + use super::*; + + // Real symlink behaviour is exercised in `commands::folder_links`; these + // cover the registry bookkeeping itself, which is platform-independent. + // The registry is process-global and tests share a process, so every case + // uses paths unique to itself and never asserts on the global size. + + fn count(root: &Path, target: &Path) -> usize { + snapshot() + .iter() + .filter(|e| e.root == root && e.target == target) + .count() + } + + #[test] + fn register_is_idempotent_and_unregister_removes() { + let root = Path::new("/tmp/codeg-registry-a/root"); + let target = Path::new("/tmp/codeg-registry-a/target"); + register(root, target); + register(root, target); + assert_eq!(count(root, target), 1); + unregister(root, target); + assert_eq!(count(root, target), 0); + } + + #[test] + fn unknown_paths_are_denied() { + // Neither path exists, so canonicalization fails and no entry can match. + assert!(!is_allowed( + Path::new("/tmp/codeg-registry-b/root"), + Path::new("/tmp/codeg-registry-b/elsewhere/file.txt") + )); + } +} diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index cc6704295..44909d43e 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -20,6 +20,7 @@ pub mod backgrounds; pub mod chat_channel; pub mod commands; pub mod db; +pub mod folder_links; pub mod git_credential; pub mod git_repo; pub mod intern; @@ -65,7 +66,7 @@ mod tauri_app { chat_channel as chat_channel_commands, conversations, custom_skills as custom_skills_commands, delegation as delegation_commands, experts as experts_commands, feedback as feedback_commands, file_io, folder_commands, - office_tools as office_tools_commands, + folder_links, office_tools as office_tools_commands, folders, logging as logging_commands, mcp as mcp_commands, model_provider as model_provider_commands, notification, pet as pet_commands, project_boot, question as question_commands, quick_messages as quick_messages_commands, @@ -940,6 +941,12 @@ mod tauri_app { folders::update_folder_color, folders::update_folder_alias, folders::update_folder_default_agent, + folder_links::list_folder_links, + folder_links::preview_folder_links, + folder_links::create_folder_links, + folder_links::rename_folder_link, + folder_links::repair_folder_link, + folder_links::remove_folder_link, folders::add_folder_to_history, folders::remove_folder_from_history, folders::create_folder_directory, diff --git a/src-tauri/src/models/mod.rs b/src-tauri/src/models/mod.rs index c619b1e4f..2752ece2d 100644 --- a/src-tauri/src/models/mod.rs +++ b/src-tauri/src/models/mod.rs @@ -38,7 +38,7 @@ pub use remote_workspace_connection::RemoteWorkspaceConnectionInfo; pub use work_task::{ WorkTaskChangedFile, WorkTaskConfig, WorkTaskDraft, WorkTaskEventInfo, WorkTaskFolderSettings, WorkTaskInfo, WorkTaskMergeState, WorkTaskPreflight, WorkTaskStatus, - WorkTaskTemplateDraft, WorkTaskTemplateInfo, + WorkTaskTemplateDraft, WorkTaskTemplateInfo, STAGE_PROMPT_ALL, }; #[cfg(feature = "tauri-runtime")] pub use system::SystemRenderingSettings; diff --git a/src-tauri/src/models/work_task.rs b/src-tauri/src/models/work_task.rs index a7edd316b..a9d111169 100644 --- a/src-tauri/src/models/work_task.rs +++ b/src-tauri/src/models/work_task.rs @@ -149,6 +149,14 @@ pub struct WorkTaskFolderSettings { /// starts (deps install, env seeding). Not re-run on reused worktrees. #[serde(default)] pub init_command: Option, + /// User-authored instructions appended *after* the built-in prompt of a + /// launch stage — project conventions or personal preferences the standard + /// wording can't cover. Keys are the stage identifiers the engine already + /// stamps on its `round` events (`work` / `retry` / `return` / `merge`), + /// plus the reserved key `all`, which is appended to every stage. Unknown + /// keys are ignored, so a future stage needs no schema change. + #[serde(default)] + pub stage_prompts: std::collections::BTreeMap, } impl Default for WorkTaskFolderSettings { @@ -165,10 +173,14 @@ impl Default for WorkTaskFolderSettings { preflight_command_id: None, preflight_command: None, init_command: None, + stage_prompts: Default::default(), } } } +/// Reserved `stage_prompts` key whose text is appended to every launch stage. +pub const STAGE_PROMPT_ALL: &str = "all"; + fn default_max_concurrent() -> i32 { 2 } @@ -225,3 +237,32 @@ pub struct WorkTaskChangedFile { pub additions: i32, pub deletions: i32, } + +#[cfg(test)] +mod tests { + use super::*; + + /// Settings rows written before stage prompts existed must keep decoding — + /// the column is one JSON blob, so a missing key is the migration. + #[test] + fn legacy_settings_json_decodes_without_stage_prompts() { + let legacy = r#"{ + "default_agent_type": "claude_code", + "mode_id": null, + "config_values": {}, + "auto_process": true, + "max_concurrent": 3, + "merge_strategy": "merge", + "delete_worktree_default": false, + "init_command": "pnpm install" + }"#; + let settings: WorkTaskFolderSettings = + serde_json::from_str(legacy).expect("legacy settings decode"); + assert!(settings.stage_prompts.is_empty()); + assert_eq!(settings.max_concurrent, 3); + assert_eq!(settings.merge_strategy, "merge"); + assert!(settings.auto_process); + assert!(!settings.delete_worktree_default); + assert_eq!(settings.init_command.as_deref(), Some("pnpm install")); + } +} diff --git a/src-tauri/src/parsers/claude.rs b/src-tauri/src/parsers/claude.rs index 5b0469aac..8f46ccd5b 100644 --- a/src-tauri/src/parsers/claude.rs +++ b/src-tauri/src/parsers/claude.rs @@ -238,6 +238,40 @@ pub(crate) fn is_meta_message(value: &serde_json::Value) -> bool { .unwrap_or(false) } +/// Capture Claude Code's two dedicated title records into their slots. +/// +/// * `{"type":"custom-title","customTitle":…}` — the name the USER set, via +/// `/rename`, `claude -n `, `Ctrl+R` in the `/resume` picker, or a +/// `/branch`/fork (which stamps `" (fork)"`). +/// * `{"type":"ai-title","aiTitle":…}` — the summary Claude Code generates in +/// the background when the session has no user-set name. +/// +/// Both records are appended (never rewritten) and can repeat over a session's +/// life, so the newest non-empty value of each wins. Claude Code resolves the +/// pair the same way — its session picker folds these records into per-session +/// maps (last write wins) and renders `customTitle ?? aiTitle` — so the caller +/// must prefer `custom_title` over `ai_title`. Empty/whitespace values are +/// ignored: Claude Code refuses to set a blank name, and it emits an empty +/// `aiTitle` for trivial sessions. +fn capture_title_record( + value: &serde_json::Value, + msg_type: &str, + custom_title: &mut Option, + ai_title: &mut Option, +) { + let (field, slot) = match msg_type { + "custom-title" => ("customTitle", custom_title), + "ai-title" => ("aiTitle", ai_title), + _ => return, + }; + if let Some(t) = value.get(field).and_then(|v| v.as_str()) { + let t = t.trim(); + if !t.is_empty() { + *slot = Some(truncate_str(t, 100)); + } + } +} + /// Check if an assistant message is a synthetic placeholder (e.g. generated by /// Claude Code for local commands like `/context` or `/model`). /// These carry `model: ""` and all-zero usage, so they should be @@ -426,6 +460,7 @@ impl ClaudeParser { let mut model: Option = None; let mut title: Option = None; let mut ai_title: Option = None; + let mut custom_title: Option = None; let mut first_timestamp: Option> = None; let mut last_timestamp: Option> = None; let mut message_count: u32 = 0; @@ -456,17 +491,10 @@ impl ClaudeParser { continue; } - // Claude Code records its own AI-generated title as a dedicated - // `{"type":"ai-title","aiTitle":...}` entry. Prefer it over the first - // user message; the newest non-empty value wins. - if msg_type == "ai-title" { - if let Some(t) = value.get("aiTitle").and_then(|v| v.as_str()) { - let t = t.trim(); - if !t.is_empty() { - ai_title = Some(truncate_str(t, 100)); - } - } - } + // Claude Code records the user-set name (`/rename`) and its own + // generated title as dedicated entries — prefer both over the first + // user message. See `capture_title_record`. + capture_title_record(&value, msg_type, &mut custom_title, &mut ai_title); if conversation_id.is_none() { conversation_id = value @@ -538,8 +566,9 @@ impl ClaudeParser { let folder_path = cwd.clone(); let folder_name = folder_path.as_ref().map(|p| folder_name_from_path(p)); - // Prefer Claude Code's own AI-generated title when present. - let title = ai_title.or(title); + // The user's own `/rename` first, then Claude Code's generated title, + // then the first user message. + let title = custom_title.or(ai_title).or(title); Ok(Some(ConversationSummary { id, @@ -677,6 +706,7 @@ pub(crate) struct ClaudeRecordAccumulator { pub(crate) model: Option, pub(crate) title: Option, pub(crate) ai_title: Option, + pub(crate) custom_title: Option, pub(crate) first_timestamp: Option>, pub(crate) last_timestamp: Option>, /// A prompt-expanding slash command is buffered (with its promptId) until @@ -703,6 +733,7 @@ impl ClaudeRecordAccumulator { model: None, title: None, ai_title: None, + custom_title: None, first_timestamp: None, last_timestamp: None, pending_command: None, @@ -734,6 +765,7 @@ impl ClaudeRecordAccumulator { model, title, ai_title, + custom_title, first_timestamp, last_timestamp, pending_command, @@ -761,17 +793,10 @@ impl ClaudeRecordAccumulator { return; } - // Claude Code records its own AI-generated title as a dedicated - // `{"type":"ai-title","aiTitle":...}` entry. Prefer it over the first - // user message; the newest non-empty value wins. - if msg_type == "ai-title" { - if let Some(t) = value.get("aiTitle").and_then(|v| v.as_str()) { - let t = t.trim(); - if !t.is_empty() { - *ai_title = Some(truncate_str(t, 100)); - } - } - } + // Claude Code records the user-set name (`/rename`) and its own + // generated title as dedicated entries — prefer both over the first + // user message. See `capture_title_record`. + capture_title_record(&value, msg_type, custom_title, ai_title); if cwd.is_none() { *cwd = value @@ -1286,6 +1311,7 @@ impl ClaudeParser { model, title, ai_title, + custom_title, first_timestamp, last_timestamp, .. @@ -1307,8 +1333,9 @@ impl ClaudeParser { context_window_max_tokens, ); - // Prefer Claude Code's own AI-generated title when present. - let title = ai_title.or(title); + // Same precedence as `parse_jsonl_summary` — the two paths MUST agree, + // or the auto-title backfill would oscillate between them. + let title = custom_title.or(ai_title).or(title); let summary = ConversationSummary { id: conversation_id.to_string(), @@ -2220,6 +2247,162 @@ mod tests { assert_eq!(summary.title.as_deref(), Some("fallback prompt")); } + /// Write a session whose entries are `lines`, parse it through BOTH title + /// paths, and return `(detail_title, summary_title)`. The two must always + /// agree — a divergence makes the auto-title backfill oscillate. + fn parse_both_titles( + tag: &str, + lines: &[serde_json::Value], + ) -> (Option, Option) { + let path = + std::env::temp_dir().join(format!("codeg-claude-{tag}-{}.jsonl", uuid::Uuid::new_v4())); + let mut file = fs::File::create(&path).expect("create temp jsonl"); + for line in lines { + writeln!(file, "{line}").expect("write line"); + } + drop(file); + + let parser = ClaudeParser { + base_dir: PathBuf::new(), + }; + let detail = parser + .parse_conversation_detail(&path, tag) + .expect("parse conversation detail"); + let summary = parser + .parse_jsonl_summary(&path) + .expect("parse summary") + .expect("summary present"); + fs::remove_file(&path).expect("cleanup temp jsonl"); + + (detail.summary.title, summary.title) + } + + fn user_line(session: &str, text: &str) -> serde_json::Value { + serde_json::json!({ + "type": "user", + "sessionId": session, + "timestamp": "2026-03-01T10:00:00Z", + "uuid": "u1", + "message": { "content": [{"type": "text", "text": text}] } + }) + } + + #[test] + fn parse_prefers_rename_custom_title_over_ai_title() { + // `/rename auth-refactor` appends a `custom-title` entry. Claude Code's + // own picker renders `customTitle ?? aiTitle`, so the user's name must + // beat the generated one no matter which came last in the file. + let (detail, summary) = parse_both_titles( + "customtitle", + &[ + user_line("custom-title-test", "first user prompt"), + serde_json::json!({ + "type": "custom-title", + "customTitle": "auth-refactor", + "sessionId": "custom-title-test", + "timestamp": "2026-03-01T10:05:00Z" + }), + serde_json::json!({ + "type": "ai-title", + "aiTitle": "Concise AI Summary", + "sessionId": "custom-title-test" + }), + ], + ); + assert_eq!(detail.as_deref(), Some("auth-refactor")); + assert_eq!(summary.as_deref(), Some("auth-refactor")); + } + + #[test] + fn parse_takes_the_last_non_empty_custom_title() { + // Renaming twice appends twice — the newest name wins, and a blank + // value (which Claude Code itself refuses to write) never clears one. + let (detail, summary) = parse_both_titles( + "customtitle-last", + &[ + user_line("custom-title-last", "first user prompt"), + serde_json::json!({ + "type": "custom-title", + "customTitle": "old-name", + "sessionId": "custom-title-last" + }), + serde_json::json!({ + "type": "custom-title", + "customTitle": "new-name", + "sessionId": "custom-title-last" + }), + serde_json::json!({ + "type": "custom-title", + "customTitle": " ", + "sessionId": "custom-title-last" + }), + ], + ); + assert_eq!(detail.as_deref(), Some("new-name")); + assert_eq!(summary.as_deref(), Some("new-name")); + } + + #[test] + fn parse_falls_back_to_ai_title_when_custom_title_blank() { + let (detail, summary) = parse_both_titles( + "customtitle-blank", + &[ + user_line("custom-title-blank", "first user prompt"), + serde_json::json!({ + "type": "custom-title", + "customTitle": "", + "sessionId": "custom-title-blank" + }), + serde_json::json!({ + "type": "ai-title", + "aiTitle": "Concise AI Summary", + "sessionId": "custom-title-blank" + }), + ], + ); + assert_eq!(detail.as_deref(), Some("Concise AI Summary")); + assert_eq!(summary.as_deref(), Some("Concise AI Summary")); + } + + #[test] + fn custom_title_entry_is_not_rendered_as_a_turn() { + // The record carries a timestamp and a sessionId, so it must stay a + // metadata line — never a visible turn or a counted message. + let path = std::env::temp_dir().join(format!( + "codeg-claude-customtitle-turn-{}.jsonl", + uuid::Uuid::new_v4() + )); + let mut file = fs::File::create(&path).expect("create temp jsonl"); + writeln!(file, "{}", user_line("custom-title-turn", "hello")).expect("write user"); + writeln!( + file, + "{}", + serde_json::json!({ + "type": "custom-title", + "customTitle": "named", + "sessionId": "custom-title-turn", + "timestamp": "2026-03-01T10:05:00Z" + }) + ) + .expect("write custom-title"); + drop(file); + + let parser = ClaudeParser { + base_dir: PathBuf::new(), + }; + let detail = parser + .parse_conversation_detail(&path, "custom-title-turn") + .expect("parse conversation detail"); + let summary = parser + .parse_jsonl_summary(&path) + .expect("parse summary") + .expect("summary present"); + fs::remove_file(&path).expect("cleanup temp jsonl"); + + assert_eq!(detail.turns.len(), 1, "only the user turn is rendered"); + assert_eq!(summary.message_count, 1); + } + #[test] fn parse_detail_completion_time_uses_event_log_timestamp_not_added_duration() { // Regression: turn_duration encodes the *entire* turn span, so diff --git a/src-tauri/src/parsers/codebuddy.rs b/src-tauri/src/parsers/codebuddy.rs index 262ce554f..c8f31c54a 100644 --- a/src-tauri/src/parsers/codebuddy.rs +++ b/src-tauri/src/parsers/codebuddy.rs @@ -65,7 +65,7 @@ impl CodeBuddyParser { let mut first_ts: Option> = None; let mut last_ts: Option> = None; - let mut ai_title: Option = None; + let mut titles = CodeBuddyTitles::default(); let mut first_user_text: Option = None; let mut model: Option = None; let mut cwd: Option = None; @@ -102,16 +102,7 @@ impl CodeBuddyParser { } match record_type { - "ai-title" => { - if ai_title.is_none() { - ai_title = value - .get("aiTitle") - .and_then(|t| t.as_str()) - .map(str::trim) - .filter(|s| !s.is_empty()) - .map(String::from); - } - } + "custom-title" | "ai-title" | "topic" => titles.feed(record_type, &value), "message" => match value.get("role").and_then(|r| r.as_str()).unwrap_or("") { "user" => { message_count += 1; @@ -143,7 +134,7 @@ impl CodeBuddyParser { agent_type: AgentType::CodeBuddy, folder_path: cwd, folder_name, - title: ai_title.or(first_user_text), + title: titles.resolve(first_user_text), started_at, ended_at: last_ts, message_count, @@ -165,7 +156,7 @@ impl CodeBuddyParser { let mut messages: Vec = Vec::new(); let mut first_ts: Option> = None; let mut last_ts: Option> = None; - let mut ai_title: Option = None; + let mut titles = CodeBuddyTitles::default(); let mut first_user_text: Option = None; let mut model: Option = None; let mut cwd: Option = None; @@ -206,16 +197,7 @@ impl CodeBuddyParser { } match record_type { - "ai-title" => { - if ai_title.is_none() { - ai_title = value - .get("aiTitle") - .and_then(|t| t.as_str()) - .map(str::trim) - .filter(|s| !s.is_empty()) - .map(String::from); - } - } + "custom-title" | "ai-title" | "topic" => titles.feed(record_type, &value), "message" => match value.get("role").and_then(|r| r.as_str()).unwrap_or("") { "user" => { message_count += 1; @@ -338,7 +320,9 @@ impl CodeBuddyParser { agent_type: AgentType::CodeBuddy, folder_path: cwd, folder_name, - title: ai_title.or(first_user_text), + // Same precedence as `parse_summary` — the two paths MUST agree, or + // the auto-title backfill would oscillate between them. + title: titles.resolve(first_user_text), started_at: first_ts.unwrap_or_else(Utc::now), ended_at: last_ts, message_count, @@ -463,6 +447,55 @@ fn record_cwd(value: &Value) -> Option { .map(String::from) } +/// CodeBuddy's session title, resolved exactly like its own +/// `getEffectiveSessionTitle`: the newest non-empty `custom-title` (what +/// `/rename` writes), else the newest usable `ai-title`, else the newest usable +/// `topic`, else the first real user message. All three records are appended +/// (never rewritten) and repeat over a session's life — CodeBuddy scans them +/// back-to-front — so the LAST value of each kind wins. +#[derive(Default)] +struct CodeBuddyTitles { + custom: Option, + ai: Option, + topic: Option, +} + +impl CodeBuddyTitles { + /// Feed one record. Non-title records are ignored, so callers can route the + /// three types through a single match arm. + fn feed(&mut self, record_type: &str, value: &Value) { + let (field, slot) = match record_type { + "custom-title" => ("customTitle", &mut self.custom), + "ai-title" => ("aiTitle", &mut self.ai), + "topic" => ("topic", &mut self.topic), + _ => return, + }; + let Some(text) = value.get(field).and_then(|t| t.as_str()).map(str::trim) else { + return; + }; + // A `custom-title` is user-typed and taken verbatim; only the generated + // kinds can carry CodeBuddy's placeholders, which it skips rather than + // showing. + if text.is_empty() || (record_type != "custom-title" && is_placeholder_title(text)) { + return; + } + *slot = Some(truncate_str(text, 100)); + } + + fn resolve(self, first_user_text: Option) -> Option { + self.custom.or(self.ai).or(self.topic).or(first_user_text) + } +} + +/// CodeBuddy's `isGeneratedPlaceholderTitle`: what its titler emits when there +/// was nothing to summarize. Showing one of these as the session name is worse +/// than falling through to the first user message. +fn is_placeholder_title(text: &str) -> bool { + text == "(No content)" + || text == "/compact" + || (text.starts_with("") && text.ends_with("")) +} + /// Record types that carry actual conversation content, as opposed to the /// `ai-title` / `summary` / `file-history-snapshot` metadata records (which also /// carry timestamps). Only content records define the session's @@ -1069,6 +1102,93 @@ mod tests { std::fs::remove_dir_all(&root).ok(); } + /// Parse one session through BOTH title paths (list + detail) and assert + /// they agree on `expected` — a divergence makes the auto-title backfill + /// oscillate between them. + fn assert_title(tag: &str, records: &[Value], expected: Option<&str>) { + let root = std::env::temp_dir().join(format!("codeg-cb-{tag}-{}", uuid::Uuid::new_v4())); + let sid = "sess-title"; + write_session(&root, "Users-demo-app", sid, records); + + let parser = CodeBuddyParser::with_base_dir(root.clone()); + let summaries = parser.list_conversations().expect("list"); + assert_eq!(summaries.len(), 1, "{tag}: one session expected"); + let listed = summaries[0].title.clone(); + let detail = parser.get_conversation(sid).expect("detail"); + std::fs::remove_dir_all(&root).ok(); + + assert_eq!(listed.as_deref(), expected, "{tag}: list title"); + assert_eq!( + detail.summary.title.as_deref(), + expected, + "{tag}: detail title" + ); + } + + fn cb_user(text: &str) -> Value { + json!({"type":"message","role":"user","timestamp":1781821844178i64,"cwd":"/Users/demo/app", + "sessionId":"sess-title","content":[{"type":"input_text","text":text}]}) + } + + #[test] + fn title_prefers_rename_over_generated_titles() { + // CodeBuddy's `/rename` appends `{"type":"custom-title","customTitle":…}` + // and its own `getEffectiveSessionTitle` scans custom → ai → topic, so a + // generated title written afterwards never displaces the user's name. + assert_title( + "title-custom", + &[ + cb_user("first prompt"), + json!({"type":"ai-title","timestamp":1781821846000i64,"aiTitle":"stale title","sessionId":"sess-title"}), + json!({"type":"custom-title","timestamp":1781821847000i64,"customTitle":"auth-refactor","sessionId":"sess-title"}), + json!({"type":"ai-title","timestamp":1781821848000i64,"aiTitle":"fresh title","sessionId":"sess-title"}), + ], + Some("auth-refactor"), + ); + } + + #[test] + fn title_takes_the_newest_generated_value() { + // CodeBuddy re-titles a session as it grows, appending a new record each + // time. The NEWEST value wins (it used to keep the first one, pinning a + // long session to the title of its opening exchange). + assert_title( + "title-newest", + &[ + cb_user("first prompt"), + json!({"type":"ai-title","timestamp":1781821846000i64,"aiTitle":"stale title","sessionId":"sess-title"}), + json!({"type":"ai-title","timestamp":1781821848000i64,"aiTitle":"fresh title","sessionId":"sess-title"}), + ], + Some("fresh title"), + ); + } + + #[test] + fn title_skips_placeholders_and_falls_through_to_topic() { + assert_title( + "title-topic", + &[ + cb_user("first prompt"), + json!({"type":"ai-title","timestamp":1781821846000i64,"aiTitle":"(No content)","sessionId":"sess-title"}), + json!({"type":"topic","timestamp":1781821847000i64,"topic":"重构鉴权","sessionId":"sess-title"}), + ], + Some("重构鉴权"), + ); + } + + #[test] + fn title_falls_back_to_first_prompt_when_every_generated_value_is_a_placeholder() { + assert_title( + "title-fallback", + &[ + cb_user("first prompt"), + json!({"type":"ai-title","timestamp":1781821846000i64,"aiTitle":"/compact","sessionId":"sess-title"}), + json!({"type":"topic","timestamp":1781821847000i64,"topic":" ","sessionId":"sess-title"}), + ], + Some("first prompt"), + ); + } + #[test] fn parses_tool_calls_with_error_detection() { let root = std::env::temp_dir().join(format!("codeg-cb-tool-{}", uuid::Uuid::new_v4())); diff --git a/src-tauri/src/web/handlers/folder_links.rs b/src-tauri/src/web/handlers/folder_links.rs new file mode 100644 index 000000000..6b80de73f --- /dev/null +++ b/src-tauri/src/web/handlers/folder_links.rs @@ -0,0 +1,124 @@ +use std::sync::Arc; + +use axum::{extract::Extension, Json}; +use serde::Deserialize; + +use crate::app_error::AppCommandError; +use crate::app_state::AppState; +use crate::commands::folder_links as link_commands; +use crate::commands::folder_links::{FolderLinkDetail, FolderLinkPlan, FolderLinkRequest}; + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct FolderIdParams { + pub folder_id: i32, +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct PreviewParams { + pub folder_id: i32, + pub paths: Vec, +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct CreateParams { + pub folder_id: i32, + pub items: Vec, + #[serde(default)] + pub git_exclude: Option, +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct RenameParams { + pub link_id: i32, + pub new_name: String, +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct LinkIdParams { + pub link_id: i32, +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct RemoveParams { + pub link_id: i32, + #[serde(default)] + pub delete_link: Option, +} + +pub async fn list_folder_links( + Extension(state): Extension>, + Json(params): Json, +) -> Result>, AppCommandError> { + Ok(Json( + link_commands::list_folder_links_core(&state.db, params.folder_id).await?, + )) +} + +pub async fn preview_folder_links( + Extension(state): Extension>, + Json(params): Json, +) -> Result>, AppCommandError> { + Ok(Json( + link_commands::preview_folder_links_core(&state.db, params.folder_id, params.paths).await?, + )) +} + +pub async fn create_folder_links( + Extension(state): Extension>, + Json(params): Json, +) -> Result>, AppCommandError> { + Ok(Json( + link_commands::create_folder_links_core( + &state.emitter, + &state.db, + params.folder_id, + params.items, + params.git_exclude.unwrap_or(true), + ) + .await?, + )) +} + +pub async fn rename_folder_link( + Extension(state): Extension>, + Json(params): Json, +) -> Result, AppCommandError> { + Ok(Json( + link_commands::rename_folder_link_core( + &state.emitter, + &state.db, + params.link_id, + params.new_name, + ) + .await?, + )) +} + +pub async fn repair_folder_link( + Extension(state): Extension>, + Json(params): Json, +) -> Result, AppCommandError> { + Ok(Json( + link_commands::repair_folder_link_core(&state.emitter, &state.db, params.link_id).await?, + )) +} + +pub async fn remove_folder_link( + Extension(state): Extension>, + Json(params): Json, +) -> Result, AppCommandError> { + link_commands::remove_folder_link_core( + &state.emitter, + &state.db, + params.link_id, + params.delete_link.unwrap_or(true), + ) + .await?; + Ok(Json(())) +} diff --git a/src-tauri/src/web/handlers/mod.rs b/src-tauri/src/web/handlers/mod.rs index 20b68fcba..9fc57f833 100644 --- a/src-tauri/src/web/handlers/mod.rs +++ b/src-tauri/src/web/handlers/mod.rs @@ -13,6 +13,7 @@ pub mod experts; pub mod feedback; pub mod files; pub mod folder_commands; +pub mod folder_links; pub mod folders; pub mod git; pub mod logging; diff --git a/src-tauri/src/web/handlers/workspace_files.rs b/src-tauri/src/web/handlers/workspace_files.rs index 5ff167987..819e9293d 100644 --- a/src-tauri/src/web/handlers/workspace_files.rs +++ b/src-tauri/src/web/handlers/workspace_files.rs @@ -91,7 +91,7 @@ fn resolve_relative_path(root: &Path, rel: &str) -> Result Result<(), AppCommandError> { let canonical_root = std::fs::canonicalize(root).map_err(AppCommandError::io)?; let canonical_target = std::fs::canonicalize(target).map_err(AppCommandError::io)?; - if !canonical_target.starts_with(&canonical_root) { + if !crate::commands::folders::is_within_workspace(&canonical_root, &canonical_target) { return Err(AppCommandError::invalid_input( "Resolved path escapes workspace root", )); @@ -108,11 +108,27 @@ fn ensure_inside_root(root: &Path, target: &Path) -> Result<(), AppCommandError> /// workspace. The earlier post-hoc `canonicalize` check caught the /// escape but the side-effect (empty dir at the symlink target) was /// already on disk. -fn ensure_no_symlink_in_chain(root: &Path, target: &Path) -> Result<(), AppCommandError> { +/// +/// The one symlink that *is* followed is a directory the user explicitly linked +/// into this root (see [`crate::folder_links`]) — uploading into a linked +/// project is the point of a multi-folder workspace. The walk continues from +/// the *resolved* target, so a stray symlink inside the linked project is still +/// rejected. +/// +/// Returns the link-free equivalent of `target`. Callers must use that path for +/// `create_dir_all` and the commit rather than the one they passed in: re-walking +/// the original would resolve the link a second time, and a link swapped in +/// between the two walks would place directories somewhere this check never saw. +fn resolve_upload_chain(root: &Path, target: &Path) -> Result { let rel = target .strip_prefix(root) .map_err(|_| AppCommandError::invalid_input("Target path is not under workspace root"))?; + let canonical_root = std::fs::canonicalize(root).map_err(AppCommandError::io)?; let mut current = root.to_path_buf(); + // Once a component is missing, everything below it is missing too — there is + // nothing left for `create_dir_all` to follow into, so the remaining + // segments are appended without stat'ing them. + let mut reached_missing = false; for component in rel.components() { let segment = match component { Component::Normal(s) => s, @@ -124,23 +140,30 @@ fn ensure_no_symlink_in_chain(root: &Path, target: &Path) -> Result<(), AppComma } }; current.push(segment); + if reached_missing { + continue; + } match std::fs::symlink_metadata(¤t) { Ok(md) => { if md.file_type().is_symlink() { - return Err(AppCommandError::invalid_input( - "Upload path traverses a symlink; refuse to follow it", - )); + let resolved = std::fs::canonicalize(¤t).map_err(AppCommandError::io)?; + if !crate::folder_links::is_allowed(&canonical_root, &resolved) { + return Err(AppCommandError::invalid_input( + "Upload path traverses a symlink; refuse to follow it", + )); + } + // Continue from the real directory so the rest of the chain + // is validated against it rather than through the link. + current = resolved; } } Err(e) if e.kind() == std::io::ErrorKind::NotFound => { - // The remainder of the path doesn't exist yet — nothing - // for create_dir_all to follow into, so we're safe. - return Ok(()); + reached_missing = true; } Err(e) => return Err(AppCommandError::io(e)), } } - Ok(()) + Ok(current) } /// Strip cross-platform-hostile characters from a single path segment. @@ -294,26 +317,37 @@ pub async fn upload_workspace_file( relative_path.as_deref().unwrap_or(""), &file_name_hint, )?; - let final_abs = resolve_relative_path(&root, &final_rel)?; + let mut final_abs = resolve_relative_path(&root, &final_rel)?; if let Some(parent) = final_abs.parent() { - // Reject *before* touching the filesystem if any - // existing component along the path is a symlink — - // otherwise `create_dir_all` would follow the link - // and create directories outside the workspace - // before the canonical check below could fire. - ensure_no_symlink_in_chain(&root, parent)?; - tokio::fs::create_dir_all(parent).await.map_err(|e| { - AppCommandError::io_error("Failed to create upload directory") - .with_detail(e.to_string()) - })?; + // Reject *before* touching the filesystem if any existing + // component along the path is a symlink the user did not + // authorize — otherwise `create_dir_all` would follow the + // link and create directories outside the workspace before + // the canonical check below could fire. The resolved parent + // replaces the original: everything downstream then operates + // on real directories, so a link swapped in afterwards can't + // redirect the write. + let resolved_parent = resolve_upload_chain(&root, parent)?; + tokio::fs::create_dir_all(&resolved_parent) + .await + .map_err(|e| { + AppCommandError::io_error("Failed to create upload directory") + .with_detail(e.to_string()) + })?; let canonical_parent = - std::fs::canonicalize(parent).map_err(AppCommandError::io)?; - if !canonical_parent.starts_with(&canonical_root) { + std::fs::canonicalize(&resolved_parent).map_err(AppCommandError::io)?; + if !crate::commands::folders::is_within_workspace( + &canonical_root, + &canonical_parent, + ) { return Err(AppCommandError::invalid_input( "Resolved path escapes workspace root", )); } + if let Some(file_name) = final_abs.file_name() { + final_abs = canonical_parent.join(file_name); + } } if final_abs.is_dir() { @@ -1027,7 +1061,7 @@ mod tests { // `link` component is a symlink that would carry create_dir_all // out of the root. let target = root.path().join("link").join("sub"); - let err = ensure_no_symlink_in_chain(root.path(), &target) + let err = resolve_upload_chain(root.path(), &target) .expect_err("should reject symlink in chain"); assert!( err.message.contains("symlink"), @@ -1035,9 +1069,42 @@ mod tests { err.message ); - // Sanity: no symlink in chain → ok. + // Sanity: no symlink in chain → ok, and the path comes back unchanged. fs::create_dir(root.path().join("real")).expect("real dir"); let ok_target = root.path().join("real").join("nested").join("file.txt"); - assert!(ensure_no_symlink_in_chain(root.path(), &ok_target).is_ok()); + assert_eq!( + resolve_upload_chain(root.path(), &ok_target).expect("no symlinks"), + ok_target + ); + } + + #[cfg(unix)] + #[test] + fn resolve_upload_chain_returns_the_link_free_path_for_an_authorized_link() { + use std::os::unix::fs::symlink; + + let root = tempfile::tempdir().expect("tempdir root"); + let linked = tempfile::tempdir().expect("tempdir linked"); + symlink(linked.path(), root.path().join("api")).expect("symlink"); + + let canonical_root = std::fs::canonicalize(root.path()).expect("canon root"); + let canonical_target = std::fs::canonicalize(linked.path()).expect("canon target"); + crate::folder_links::register(root.path(), &canonical_target); + + // The caller must receive the resolved path: re-walking the original + // would resolve `api` a second time, so a link swapped in between the + // check and `create_dir_all` could redirect the write. + let resolved = resolve_upload_chain(root.path(), &root.path().join("api").join("sub")) + .expect("authorized link is followed"); + assert_eq!(resolved, canonical_target.join("sub")); + assert!(!resolved.starts_with(&canonical_root)); + + crate::folder_links::unregister(root.path(), &canonical_target); + + // Revoked: the very same path is rejected again. + assert!( + resolve_upload_chain(root.path(), &root.path().join("api").join("sub")).is_err(), + "unlinking must revoke upload access too" + ); } } diff --git a/src-tauri/src/web/router.rs b/src-tauri/src/web/router.rs index 05c9b01d7..9f2578500 100644 --- a/src-tauri/src/web/router.rs +++ b/src-tauri/src/web/router.rs @@ -203,6 +203,30 @@ pub fn build_router( "/update_folder_default_agent", post(handlers::folders::update_folder_default_agent), ) + .route( + "/list_folder_links", + post(handlers::folder_links::list_folder_links), + ) + .route( + "/preview_folder_links", + post(handlers::folder_links::preview_folder_links), + ) + .route( + "/create_folder_links", + post(handlers::folder_links::create_folder_links), + ) + .route( + "/rename_folder_link", + post(handlers::folder_links::rename_folder_link), + ) + .route( + "/repair_folder_link", + post(handlers::folder_links::repair_folder_link), + ) + .route( + "/remove_folder_link", + post(handlers::folder_links::remove_folder_link), + ) .route( "/add_folder_to_history", post(handlers::folders::add_folder_to_history), diff --git a/src-tauri/src/work_task/engine.rs b/src-tauri/src/work_task/engine.rs index 02fc0f75a..b3375b487 100644 --- a/src-tauri/src/work_task/engine.rs +++ b/src-tauri/src/work_task/engine.rs @@ -23,7 +23,7 @@ use std::time::Duration; use sea_orm::{ActiveModelTrait, EntityTrait, IntoActiveModel, Set}; use tokio::sync::broadcast::error::RecvError; -use tokio::sync::Mutex; +use tokio::sync::{Mutex, Notify}; use tokio::time::MissedTickBehavior; use crate::acp::manager::ConnectionManager; @@ -44,7 +44,7 @@ use crate::db::AppDatabase; use crate::logging::throttle::{LagLogThrottle, LAG_LOG_WINDOW}; use crate::models::{ AgentType, WorkTaskConfig, WorkTaskFolderSettings, WorkTaskMergeState, - WorkTaskPreflight, + WorkTaskPreflight, STAGE_PROMPT_ALL, }; use crate::web::event_bridge::{ emit_event, EventEmitter, WorkTaskChange, WORK_TASK_CHANGED_EVENT, @@ -79,11 +79,20 @@ pub struct TaskEngine { /// `"a:"` — namespaced so the three id spaces can't collide). Non-empty /// set ⇔ awaiting_input. awaiting: Arc>>>, - /// Tasks currently being launched (queued in DB but owned by an in-flight - /// launch), mapped to their folder so the pump's concurrency accounting - /// stays per-folder. Keeps the pump from double-launching and reconcile - /// from re-claiming them. - launching: Arc>>, + /// Tasks currently being launched (`queued`, then `preparing` in DB, but + /// owned by an in-flight launch), mapped to their folder so the pump's + /// concurrency accounting stays per-folder. Keeps the pump from + /// double-launching, and is the reconcile sweep's ownership test for + /// `preparing` rows. Entries carry an ownership token: a slow launch that + /// is still unwinding must not drop the entry a newer launch of the same + /// task now depends on. + launching: Arc>>, + /// Source of `LaunchOwner` tokens. + launch_token: Arc, + /// Live setup (init command) child processes, by task. A cancel kills the + /// process TREE so a long `pnpm install` stops with the task instead of + /// running to completion in the background. + setup_children: Arc>>, /// Tasks whose merge/cleanup is executing in THIS process — the reconcile /// tick must not run crash recovery against them. merging: Arc>>, @@ -138,6 +147,8 @@ pub fn build_task_engine( index: Arc::new(Mutex::new(HashMap::new())), awaiting: Arc::new(Mutex::new(HashMap::new())), launching: Arc::new(Mutex::new(HashMap::new())), + launch_token: Arc::new(std::sync::atomic::AtomicU64::new(0)), + setup_children: Arc::new(Mutex::new(HashMap::new())), merging: Arc::new(Mutex::new(HashSet::new())), task_locks: Arc::new(Mutex::new(HashMap::new())), folder_locks: Arc::new(Mutex::new(HashMap::new())), @@ -188,10 +199,12 @@ fn acquire_engine_ownership(data_dir: &Path) -> Ownership { /// Long-running driver: boot recovery, then a select loop over the event bus + /// the reconcile tick. Spawn once per process in each boot path. pub async fn run_task_engine(engine: Arc) { - // Boot recovery: no connections survive a restart, so queued / running / - // awaiting_input are interruptions → failed(interrupted); retry is - // idempotent (the worktree is reused). merging is exempt — it recovers - // from git truth below, never from connection liveness. + // Boot recovery: no connections (and no setup child processes) survive a + // restart, so queued / preparing / running / awaiting_input are + // interruptions → failed(interrupted); retry is idempotent (the worktree is + // reused, and an init command that never finished re-runs — see the setup + // marker). merging is exempt — it recovers from git truth below, never from + // connection liveness. match work_task_service::boot_reconcile_interrupted(&engine.db.conn).await { Ok(n) if n > 0 => { tracing::info!("[work_task] boot reconcile failed {n} interrupted task(s)"); @@ -263,7 +276,7 @@ enum LaunchMode { } impl LaunchMode { - /// The task status a launch of this mode expects to find (and keep). + /// The task status a launch of this mode expects to find when it starts. fn expected_status(&self) -> WorkTaskStatus { match self { LaunchMode::Merge { .. } => WorkTaskStatus::Merging, @@ -271,6 +284,17 @@ impl LaunchMode { } } + /// The status the task holds for the REST of the launch, once setup has + /// begun — what the cancel gates re-check. A merge generation stays + /// `merging` throughout; every other mode moves `queued → preparing` before + /// it touches the worktree. + fn in_flight_status(&self) -> WorkTaskStatus { + match self { + LaunchMode::Merge { .. } => WorkTaskStatus::Merging, + _ => WorkTaskStatus::Preparing, + } + } + /// Timeline `round` label for the prompt this mode composes. fn round_kind(&self) -> &'static str { match self { @@ -413,6 +437,15 @@ impl TaskEngine { } self.emit_upsert(task_id); + // Kill a running init command BEFORE waiting on the task lock: the + // launch holds that lock for its whole setup, so waiting first would + // mean waiting out the very `pnpm install` we are trying to stop. The + // run_seq we just canceled scopes the kill to this generation (cancel + // does not bump it, so the row still carries it). + if let Ok(task) = work_task_service::get_model(&self.db.conn, task_id).await { + self.kill_setup_child(task_id, task.run_seq).await; + } + // Serialize the teardown with a possibly in-flight launch: the launch // holds the task lock across spawn → prompt, and its status gates // re-read after each step — so we tear down either before the prompt @@ -498,7 +531,7 @@ impl TaskEngine { .lock() .await .iter() - .filter(|(_, fid)| **fid == folder_id) + .filter(|(_, owner)| owner.folder_id == folder_id) .map(|(tid, _)| *tid) .collect(); let active = match work_task_service::active_launched_count(&self.db.conn, folder_id) @@ -523,46 +556,107 @@ impl TaskEngine { return; } }; - self.launching.lock().await.insert(next.id, folder_id); - self.spawn_launch(next.id, folder_id, launch_mode_for(&next)); + // Claimed synchronously: this loop iterates immediately, and the + // task must already read as in-flight when it does. + let token = self.claim_launch_slot(next.id, folder_id).await; + self.spawn_launch_owned(next.id, folder_id, launch_mode_for(&next), Some(token)); + } + } + + /// Mark `task_id` as owned by a new launch and return the ownership token. + async fn claim_launch_slot(&self, task_id: i32, folder_id: i32) -> u64 { + let token = self + .launch_token + .fetch_add(1, std::sync::atomic::Ordering::Relaxed); + self.launching + .lock() + .await + .insert(task_id, LaunchOwner { folder_id, token }); + token + } + + /// Give up ownership — but only if the entry is still ours. A launch that + /// is slowly unwinding must not drop the slot a NEWER launch of the same + /// task now holds, or the reconcile sweep would requeue a live setup and + /// the folder's accounting would lose a slot. + async fn release_launch_slot(&self, task_id: i32, token: u64) { + let mut launching = self.launching.lock().await; + if launching.get(&task_id).is_some_and(|o| o.token == token) { + launching.remove(&task_id); } } fn spawn_launch(self: &Arc, task_id: i32, folder_id: i32, mode: LaunchMode) { + self.spawn_launch_owned(task_id, folder_id, mode, None); + } + + /// `token: Some(_)` when the caller already claimed the slot (the pump, + /// whose loop must see the task as in-flight the moment it continues); + /// `None` to claim it here. + fn spawn_launch_owned( + self: &Arc, + task_id: i32, + folder_id: i32, + mode: LaunchMode, + token: Option, + ) { let engine = self.clone(); tokio::spawn(async move { - engine.launching.lock().await.insert(task_id, folder_id); - let result = engine.launch(task_id, mode).await; - engine.launching.lock().await.remove(&task_id); + let token = match token { + Some(token) => token, + None => engine.claim_launch_slot(task_id, folder_id).await, + }; + // A merge generation stays `merging` and is NEVER failed: it + // recovers from git truth (`recover_merging`, driven by the + // reconcile tick) so a half-merge can go back to review. + let is_merge = matches!(mode, LaunchMode::Merge { .. }); + // The generation the launch actually operated on, published as soon + // as it has read the row. A setup failure must be attributed to THAT + // generation: a cancel + requeue (which bumps run_seq) can land + // while a slow setup is still unwinding, and failing the row by its + // *current* sequence would kill the fresh run instead. + let launched_seq = LaunchSeq::default(); + let result = engine.launch(task_id, mode, &launched_seq).await; + let errored = result.is_err(); if let Err(e) = result { tracing::info!("[work_task] launch {task_id}: {e}"); - let task = work_task_service::get_model(&engine.db.conn, task_id).await.ok(); - let seq = task.as_ref().map(|t| t.run_seq); - let failed = work_task_service::fail( - &engine.db.conn, - task_id, - &[WorkTaskStatus::Queued, WorkTaskStatus::Running], - seq, - "setup_error", - Some(e), - ) - .await - .unwrap_or(false); - if failed { - engine.emit_upsert(task_id); + if let (false, Some(seq)) = (is_merge, launched_seq.get()) { + let failed = work_task_service::fail( + &engine.db.conn, + task_id, + &[WorkTaskStatus::Queued, WorkTaskStatus::Preparing], + Some(seq), + "setup_error", + Some(e), + ) + .await + .unwrap_or(false); + if failed { + engine.emit_upsert(task_id); + } } + } + // Released only after the failure is settled: while the entries are + // held, the reconcile sweep cannot mistake this row for an orphaned + // `preparing`, and a cancel still has a kill slot to write to. + engine.release_setup_slot(task_id, launched_seq.get()).await; + engine.release_launch_slot(task_id, token).await; + if errored { // A slot may have opened up (or this task left the queue) — // keep draining. - if let Some(t) = task { - engine.pump_folder(t.folder_id).await; - } + engine.pump_folder(folder_id).await; } }); } // ── launch ────────────────────────────────────────────────────────────── - async fn launch(self: &Arc, task_id: i32, mode: LaunchMode) -> Result<(), String> { + async fn launch( + self: &Arc, + task_id: i32, + mode: LaunchMode, + launched_seq: &LaunchSeq, + ) -> Result<(), String> { let lock = self.task_lock(task_id).await; let _guard = lock.lock().await; @@ -573,6 +667,7 @@ impl TaskEngine { return Ok(()); // canceled (or otherwise moved on) before we got here } let run_seq = task.run_seq; + launched_seq.set(run_seq); let root = get_folder_core(&self.db, task.folder_id) .await .map_err(|e| e.to_string())?; @@ -597,6 +692,34 @@ impl TaskEngine { return Err("prompt is empty".to_string()); } + // Out of the queue: from here the task holds its slot and does real + // work (worktree, init command, agent spawn), so the board must stop + // calling it "queued". Losing the CAS means a concurrent cancel or a + // newer generation took over. A merge generation stays `merging`. + if !matches!(mode, LaunchMode::Merge { .. }) { + // Reserve the kill slot BEFORE the row can read as `preparing`, and + // hold it for the whole phase. A cancel cannot wait for the task + // lock (this launch holds it across the entire init command), so it + // must always find somewhere to record itself — including during + // `ensure_worktree`, which takes seconds on a big repo. + // `spawn_launch_owned` releases the slot, including when the CAS + // below loses. + self.reserve_setup_slot(task_id, run_seq).await; + if !work_task_service::begin_setup(&self.db.conn, task_id, run_seq) + .await + .map_err(|e| e.to_string())? + { + return Ok(()); + } + self.emit_upsert(task_id); + } + + // Cancel gate before the expensive part of setup, so a cancel that + // landed while we were reading config never reaches `git worktree add`. + if !still_expected(&self.db.conn, task_id, run_seq, mode.in_flight_status()).await { + return Ok(()); + } + // Worktree: reuse the recorded one when it still exists (retry/return), // else mint a fresh one pinned to the base recorded FIRST (no drift // window between reading the branch and creating the worktree). A merge @@ -606,17 +729,22 @@ impl TaskEngine { self.existing_worktree(&task).await? } else { let wt = self.ensure_worktree(&task, &root).await?; - // Freshly created tree → run the folder's init command (deps - // install etc.) before the agent ever sees it. A failure is a - // setup error: the task must not start half-initialized. - if wt.created { - let init = settings - .init_command - .as_deref() - .map(str::trim) - .filter(|c| !c.is_empty()); - if let Some(command) = init { - self.run_init_command(task_id, command, &wt.path).await?; + // Run the folder's init command (deps install etc.) before the + // agent ever sees the tree. Gated on the setup marker, NOT on "the + // tree was just created": an init that was killed (cancel) or cut + // short (restart) leaves a half-installed tree that must initialize + // again. A failure is a setup error — the task must not start + // half-initialized. + if let Some(command) = settings + .init_command + .as_deref() + .map(str::trim) + .filter(|c| !c.is_empty()) + { + if !setup_marker_present(&wt.path).await { + self.run_init_command(task_id, run_seq, command, &wt.path) + .await?; + write_setup_marker(&wt.path).await; } } wt @@ -666,7 +794,7 @@ impl TaskEngine { .map_err(|e| e.to_string())?; // Cancel gate before spawning the CLI. - if !still_expected(&self.db.conn, task_id, run_seq, mode.expected_status()).await { + if !still_expected(&self.db.conn, task_id, run_seq, mode.in_flight_status()).await { return Ok(()); } @@ -735,7 +863,7 @@ impl TaskEngine { }; emit_conversation_upsert(&self.emitter, &self.db.conn, conversation_id).await; - let blocks = compose_prompt(&cfg, &task, &mode, resumed, &self.db.conn).await?; + let blocks = compose_prompt(&cfg, &task, &mode, &settings, resumed, &self.db.conn).await?; // Register for completion correlation BEFORE prompting so a fast // TurnComplete can't race ahead of the index entry. @@ -834,7 +962,6 @@ impl TaskEngine { return Ok(WorktreeRef { folder_id: detail.id, path: detail.path, - created: false, }); } } @@ -892,7 +1019,6 @@ impl TaskEngine { Ok(WorktreeRef { folder_id: wt.id, path: wt.path, - created: true, }) } @@ -911,15 +1037,20 @@ impl TaskEngine { Ok(WorktreeRef { folder_id: detail.id, path: detail.path, - created: false, }) } - /// Run the folder's worktree init command in a freshly created worktree. + /// Run the folder's worktree init command in an uninitialized worktree. /// The outcome is always recorded on the timeline; a failure aborts the /// launch (setup error) so the agent never starts half-initialized. - async fn run_init_command(&self, task_id: i32, command: &str, cwd: &str) -> Result<(), String> { - let run = run_shell_capture(command, cwd).await; + async fn run_init_command( + &self, + task_id: i32, + run_seq: i32, + command: &str, + cwd: &str, + ) -> Result<(), String> { + let run = self.run_setup_shell(task_id, run_seq, command, cwd).await; let (exit_code, tail) = match &run { Ok((code, tail)) => (*code, tail.clone()), Err(e) => (None, e.clone()), @@ -945,6 +1076,111 @@ impl TaskEngine { } } + /// Open the kill slot for a generation entering setup. Held for the whole + /// preparing phase so a cancel always has somewhere to record itself, even + /// before (or after) a child process exists. + async fn reserve_setup_slot(&self, task_id: i32, run_seq: i32) { + self.setup_children.lock().await.insert( + task_id, + SetupChild { + kill_requested: false, + run_seq, + wake: Arc::new(Notify::new()), + }, + ); + } + + /// Close the slot once the launch is over — but only if it is still this + /// generation's, so a slow unwinding launch cannot drop the slot a newer + /// one is relying on. + async fn release_setup_slot(&self, task_id: i32, run_seq: Option) { + let Some(run_seq) = run_seq else { return }; + let mut children = self.setup_children.lock().await; + if children.get(&task_id).is_some_and(|s| s.run_seq == run_seq) { + children.remove(&task_id); + } + } + + /// `run_shell_capture` for the setup phase: the child is bound to the + /// task's kill slot so a cancel kills its process TREE instead of letting a + /// multi-minute install run to completion after the user gave up. + /// + /// Every cancel window is covered, and the kill always happens HERE, in the + /// task that owns the child: + /// - before the spawn → the slot already says `kill_requested`, so no child + /// is ever started; + /// - during the spawn → `notify_one` leaves a permit, so the wait loop's + /// first `notified()` returns immediately and kills; + /// - while waiting → the wake fires and we kill, then keep awaiting the + /// child so it is still reaped normally; + /// - after the child was reaped → nobody holds its pid any more, so a + /// recycled pid can never be signalled. + async fn run_setup_shell( + &self, + task_id: i32, + run_seq: i32, + line: &str, + cwd: &str, + ) -> Result<(Option, String), String> { + let wake = { + // The slot is opened by `launch` before setup begins; re-assert it + // if it somehow went missing, so the child is never unkillable. + let mut children = self.setup_children.lock().await; + let slot = children.entry(task_id).or_insert_with(|| SetupChild { + kill_requested: false, + run_seq, + wake: Arc::new(Notify::new()), + }); + if slot.kill_requested { + return Err("canceled before the init command started".to_string()); + } + slot.run_seq = run_seq; + slot.wake.clone() + }; + let child = shell_command(line, cwd) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .map_err(|e| e.to_string())?; + let pid = child.id(); + + let wait = child.wait_with_output(); + tokio::pin!(wait); + let mut killed = false; + let out = loop { + tokio::select! { + out = &mut wait => break out, + // Kill once, then go back to awaiting the child: the process + // is only reaped by the `wait` branch, so `pid` is still ours. + _ = wake.notified(), if !killed => { + killed = true; + kill_process_tree(pid).await; + } + } + }; + let out = out.map_err(|e| e.to_string())?; + Ok(combine_capture(&out)) + } + + /// Stop the setup of `task_id`, but only when the slot belongs to + /// generation `run_seq` — a stale cancel must not stop a task that was + /// already requeued and relaunched. Records the request and wakes the + /// runner; the actual kill happens in `run_setup_shell`, which is the only + /// place that knows the child is still alive. + async fn kill_setup_child(&self, task_id: i32, run_seq: i32) { + let wake = { + let mut children = self.setup_children.lock().await; + match children.get_mut(&task_id) { + Some(slot) if slot.run_seq == run_seq => { + slot.kill_requested = true; + slot.wake.clone() + } + _ => return, + } + }; + wake.notify_one(); + } + /// Start preflight: the target folder must exist, be live, and be a /// project root (not a worktree). async fn preflight_folder(&self, folder_id: i32) -> Result<(), String> { @@ -1375,6 +1611,10 @@ impl TaskEngine { Ok(None) => Err("task left review before the merge began".to_string()), Ok(Some(_run_seq)) => { self.emit_upsert(task_id); + // A merge generation never transitions out of `merging` here, + // so the sequence sink stays unused — the failure is handled by + // the match below (residue cleanup + back to review). + let merge_seq = LaunchSeq::default(); match self .launch( task_id, @@ -1385,6 +1625,7 @@ impl TaskEngine { strategy, message, }, + &merge_seq, ) .await { @@ -1643,7 +1884,10 @@ impl TaskEngine { .map_err(|e| e.to_string())?; if matches!( task.status, - WorkTaskStatus::Queued | WorkTaskStatus::Running | WorkTaskStatus::AwaitingInput + WorkTaskStatus::Queued + | WorkTaskStatus::Preparing + | WorkTaskStatus::Running + | WorkTaskStatus::AwaitingInput | WorkTaskStatus::Merging ) { return Err("cancel or finish the task before removing its worktree".to_string()); @@ -1844,6 +2088,36 @@ impl TaskEngine { } } + // preparing rows that no in-flight launch owns → back to the queue. + // `launching` is authoritative here: this process holds the exclusive + // engine lock, and the entry outlives the whole launch (including its + // failure handling). Without this sweep an orphan would sit in + // `preparing` forever — `next_queued` only picks `queued`, so it would + // lose the self-healing a stuck `queued` row gets today. + let preparing = + work_task_service::list_by_status(&self.db.conn, &[WorkTaskStatus::Preparing]) + .await + .unwrap_or_default(); + if !preparing.is_empty() { + let launching: HashSet = self.launching.lock().await.keys().copied().collect(); + for task in preparing { + if launching.contains(&task.id) { + continue; + } + match work_task_service::abandon_setup(&self.db.conn, task.id, task.run_seq).await { + Ok(true) => { + tracing::info!( + "[work_task] requeued orphaned setup of task {}", + task.id + ); + self.emit_upsert(task.id); + } + Ok(false) => {} + Err(e) => tracing::warn!("[work_task] abandon setup error: {e}"), + } + } + } + // merging not owned by this process's in-flight merges → git truth. // Spawned off-thread: recovery waits on the per-folder git lock, and an // in-flight merge on that folder must not stall the event loop here. @@ -1928,8 +2202,6 @@ impl TaskEngine { struct WorktreeRef { folder_id: i32, path: String, - /// The worktree was created by this call (→ run the init command). - created: bool, } /// Pick the launch mode for a pump-driven launch from the task's history: a @@ -1979,11 +2251,13 @@ fn effective_agent_config( /// Compose the prompt for a launch mode. Fresh runs replay the task's blocks. /// Retry/return compose against the session we actually got: a resumed session /// already carries the task context, while a fresh fallback session needs the -/// full original description again. Every prompt ends with the worktree guard. +/// full original description again. Every prompt ends with the worktree guard, +/// then with whatever the folder's settings add for this stage. async fn compose_prompt( cfg: &WorkTaskConfig, task: &crate::db::entities::work_task::Model, mode: &LaunchMode, + settings: &WorkTaskFolderSettings, resumed: bool, conn: &sea_orm::DatabaseConnection, ) -> Result, String> { @@ -2115,9 +2389,34 @@ async fn compose_prompt( ), }); } + + // Whatever the user added in task settings, always last: it refines the + // built-in wording above it, and staying at the end keeps `prompt_head` + // (the transcript's round marker) on the prompt's own opening text. + blocks.extend(stage_prompt_block(settings, mode.round_kind())); Ok(blocks) } +/// The user's own instructions for a stage: the `all` text (every stage) then +/// the stage's own, as one trailing block. Empty when neither is configured. +fn stage_prompt_block( + settings: &WorkTaskFolderSettings, + stage: &str, +) -> Option { + let extras: Vec<&str> = [STAGE_PROMPT_ALL, stage] + .into_iter() + .filter_map(|key| settings.stage_prompts.get(key)) + .map(|text| text.trim()) + .filter(|text| !text.is_empty()) + .collect(); + if extras.is_empty() { + return None; + } + Some(PromptInputBlock::Text { + text: format!("—— Additional instructions ——\n{}", extras.join("\n\n")), + }) +} + /// The feedback text of the most recent "return" user action, if any. async fn latest_return_feedback( conn: &sea_orm::DatabaseConnection, @@ -2137,6 +2436,82 @@ async fn latest_return_feedback( }) } +/// One-shot sink for the generation a launch actually operated on. The launch +/// fills it as soon as it has read the row; `spawn_launch` reads it afterwards +/// so a setup failure is attributed to the right generation. +#[derive(Default)] +struct LaunchSeq(std::sync::Mutex>); + +impl LaunchSeq { + fn set(&self, run_seq: i32) { + *self.0.lock().expect("launch seq mutex") = Some(run_seq); + } + + fn get(&self) -> Option { + *self.0.lock().expect("launch seq mutex") + } +} + +/// Ownership of a task's in-flight launch slot: which folder it counts +/// against, plus a token so only the launch that took the slot can release it. +struct LaunchOwner { + folder_id: i32, + token: u64, +} + +/// A task's setup (init command) kill slot, open for the whole preparing phase. +/// +/// The slot deliberately does NOT hold the child's pid. Only the task that +/// spawned the child ever kills it, so a pid can never be signalled after it +/// has been reaped (and possibly recycled by the OS onto an unrelated +/// process). A canceller just raises `kill_requested` and rings `wake`. +struct SetupChild { + /// A cancel arrived: refuse to start an init command, and kill a live one. + kill_requested: bool, + /// Generation that owns the slot — a stale cancel must not stop a newer run. + run_seq: i32, + /// Rung by the canceller, awaited by whoever is running the child. + wake: Arc, +} + +/// Marker file (in the worktree's PRIVATE git dir, so it can never show up in +/// `git status` or be committed) recording that the folder's init command +/// completed successfully in this worktree. +const SETUP_MARKER: &str = "codeg-task-init-ok"; + +async fn setup_marker_present(wt_path: &str) -> bool { + match task_git::git_dir(wt_path).await { + Ok(dir) => dir.join(SETUP_MARKER).exists(), + // Can't resolve the git dir → assume not initialized. Re-running an + // init command is wasteful at worst; skipping one is a broken tree. + Err(_) => false, + } +} + +async fn write_setup_marker(wt_path: &str) { + match task_git::git_dir(wt_path).await { + Ok(dir) => { + if let Err(e) = tokio::fs::write(dir.join(SETUP_MARKER), b"").await { + tracing::warn!("[work_task] could not write the setup marker: {e}"); + } + } + Err(e) => tracing::warn!("[work_task] could not resolve the worktree git dir: {e}"), + } +} + +/// Best-effort kill of a child's whole process tree. Killing only the direct +/// child is not enough: the init command runs as `sh -c ""`, and the real +/// work (`pnpm install` and its downloads) happens in descendants that would +/// otherwise keep running. Deliberately not `kill_on_drop`, which SIGKILLs the +/// shell first and reparents the descendants out of reach — same rationale as +/// `commands::office_tools::stream_install_or_kill_tree`. +async fn kill_process_tree(pid: Option) { + let Some(pid) = pid else { return }; + if let Err(e) = kill_tree::tokio::kill_tree(pid).await { + tracing::warn!("[work_task] kill_tree failed for setup pid {pid}: {e}"); + } +} + async fn still_expected( conn: &sea_orm::DatabaseConnection, task_id: i32, @@ -2167,6 +2542,16 @@ fn prompt_head(blocks: &[PromptInputBlock]) -> String { /// own process. Returns (exit code, trailing output capped to /// `PREFLIGHT_TAIL_CHARS`). async fn run_shell_capture(line: &str, cwd: &str) -> Result<(Option, String), String> { + let out = shell_command(line, cwd) + .output() + .await + .map_err(|e| e.to_string())?; + Ok(combine_capture(&out)) +} + +/// The platform's shell invocation for one command line, ready to `output()` or +/// `spawn()`. stdin is null so a command waiting on input sees EOF. +fn shell_command(line: &str, cwd: &str) -> tokio::process::Command { #[cfg(not(windows))] let mut command = { let mut c = crate::process::tokio_command("/bin/sh"); @@ -2181,10 +2566,18 @@ async fn run_shell_capture(line: &str, cwd: &str) -> Result<(Option, String c }; command.current_dir(cwd); - let out = command.output().await.map_err(|e| e.to_string())?; + command.stdin(std::process::Stdio::null()); + command +} + +/// (exit code, trailing combined output) of a finished shell capture. +fn combine_capture(out: &std::process::Output) -> (Option, String) { let mut combined = String::from_utf8_lossy(&out.stdout).into_owned(); combined.push_str(&String::from_utf8_lossy(&out.stderr)); - Ok((out.status.code(), tail_chars(&combined, PREFLIGHT_TAIL_CHARS))) + ( + out.status.code(), + tail_chars(&combined, PREFLIGHT_TAIL_CHARS), + ) } fn tail_chars(s: &str, n: usize) -> String { @@ -2272,6 +2665,204 @@ mod tests { assert_eq!(prompt_head(&[]), ""); } + /// Minimal task row for prompt composition (nothing here touches the DB). + fn task_row() -> crate::db::entities::work_task::Model { + let now = chrono::Utc::now(); + crate::db::entities::work_task::Model { + id: 7, + folder_id: 1, + title: "Fix the login flow".to_string(), + config: "{}".to_string(), + status: WorkTaskStatus::Queued, + failure_reason: None, + last_error: None, + run_seq: 1, + sort_order: 0, + worktree_folder_id: None, + conversation_id: None, + connection_id: None, + base_branch: Some("main".to_string()), + base_sha: None, + work_branch: Some("task/7".to_string()), + merge_state: None, + pending_merge: None, + cleanup_state: None, + verdict: None, + result_summary: None, + files_changed: None, + additions: None, + deletions: None, + merge_commit: None, + preflight: None, + archived_at: None, + created_at: now, + updated_at: now, + started_at: None, + settled_at: None, + finished_at: None, + deleted_at: None, + } + } + + fn task_config() -> WorkTaskConfig { + WorkTaskConfig { + prompt_blocks: vec![serde_json::json!({ + "type": "text", + "text": "Fix the login flow and add tests." + })], + ..Default::default() + } + } + + fn settings_with(pairs: &[(&str, &str)]) -> WorkTaskFolderSettings { + WorkTaskFolderSettings { + stage_prompts: pairs + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect(), + ..Default::default() + } + } + + fn texts(blocks: &[PromptInputBlock]) -> Vec { + blocks + .iter() + .filter_map(|b| match b { + PromptInputBlock::Text { text } => Some(text.clone()), + _ => None, + }) + .collect() + } + + fn merge_mode() -> LaunchMode { + LaunchMode::Merge { + root_path: "/repo".to_string(), + base_branch: "main".to_string(), + work_branch: "task/7".to_string(), + strategy: "squash".to_string(), + message: None, + } + } + + #[tokio::test] + async fn stage_prompts_land_after_the_built_in_guard() { + let db = crate::db::test_helpers::fresh_in_memory_db().await; + let settings = settings_with(&[("all", "Reply in Chinese."), ("work", "Run pnpm test.")]); + let blocks = compose_prompt( + &task_config(), + &task_row(), + &LaunchMode::Fresh, + &settings, + false, + &db.conn, + ) + .await + .expect("compose"); + + let texts = texts(&blocks); + // …task blocks…, worktree guard, then the user's own instructions. + assert!(texts[texts.len() - 2].starts_with("—— Work task context ——")); + let last = texts.last().expect("trailing block"); + assert!(last.starts_with("—— Additional instructions ——")); + // "all" first, then the stage's own text. + let all_at = last.find("Reply in Chinese.").expect("all text"); + let work_at = last.find("Run pnpm test.").expect("stage text"); + assert!(all_at < work_at); + // The round marker still keys off the task's own opening line, so the + // transcript's phase dividers are unaffected. + assert_eq!(prompt_head(&blocks), "Fix the login flow and add tests."); + } + + #[tokio::test] + async fn stage_prompts_select_only_their_own_stage() { + let db = crate::db::test_helpers::fresh_in_memory_db().await; + let settings = settings_with(&[ + ("all", "EVERY-STAGE"), + ("work", "WORK-ONLY"), + ("retry", "RETRY-ONLY"), + ("return", "RETURN-ONLY"), + ("merge", "MERGE-ONLY"), + ]); + let modes = [ + (LaunchMode::Fresh, "WORK-ONLY"), + (LaunchMode::Retry, "RETRY-ONLY"), + (LaunchMode::Return("please fix the copy".to_string()), "RETURN-ONLY"), + (merge_mode(), "MERGE-ONLY"), + ]; + for (mode, expected) in modes { + let blocks = compose_prompt( + &task_config(), + &task_row(), + &mode, + &settings, + false, + &db.conn, + ) + .await + .expect("compose"); + let joined = texts(&blocks).join("\n"); + assert!(joined.contains("EVERY-STAGE"), "{expected}: missing all-stage text"); + assert!(joined.contains(expected), "{expected}: missing own text"); + for other in ["WORK-ONLY", "RETRY-ONLY", "RETURN-ONLY", "MERGE-ONLY"] { + if other != expected { + assert!(!joined.contains(other), "{expected}: leaked {other}"); + } + } + } + } + + #[tokio::test] + async fn merge_stage_keeps_its_extra_without_the_guard() { + let db = crate::db::test_helpers::fresh_in_memory_db().await; + let settings = settings_with(&[("merge", "Write the commit message in Chinese.")]); + let blocks = compose_prompt( + &task_config(), + &task_row(), + &merge_mode(), + &settings, + true, + &db.conn, + ) + .await + .expect("compose"); + + let texts = texts(&blocks); + // The merge generation replaces the guard (it forbids exactly what a + // merge must do) — the user's extra still trails it. + assert!(texts.iter().all(|t| !t.starts_with("—— Work task context ——"))); + assert!(texts[0].contains("land it onto the base branch")); + assert!(texts + .last() + .expect("trailing block") + .contains("Write the commit message in Chinese.")); + } + + #[tokio::test] + async fn blank_stage_prompts_add_nothing() { + let db = crate::db::test_helpers::fresh_in_memory_db().await; + let bare = compose_prompt( + &task_config(), + &task_row(), + &LaunchMode::Fresh, + &WorkTaskFolderSettings::default(), + false, + &db.conn, + ) + .await + .expect("compose"); + let blank = compose_prompt( + &task_config(), + &task_row(), + &LaunchMode::Fresh, + &settings_with(&[("all", " \n "), ("work", "")]), + false, + &db.conn, + ) + .await + .expect("compose"); + assert_eq!(texts(&bare), texts(&blank)); + } + #[test] fn engine_lock_is_per_data_dir() { let dir = tempfile::tempdir().expect("temp dir"); diff --git a/src-tauri/src/work_task/git.rs b/src-tauri/src/work_task/git.rs index e12393c06..3ede2cc7c 100644 --- a/src-tauri/src/work_task/git.rs +++ b/src-tauri/src/work_task/git.rs @@ -16,6 +16,24 @@ async fn run_git(path: &str, args: &[&str]) -> Result/.git/worktrees/` for a linked +/// worktree, `/.git` for the main one). Engine-private markers live here: +/// nothing under it can show up in `git status` or be committed by the agent. +pub async fn git_dir(path: &str) -> Result { + let out = run_git(path, &["rev-parse", "--absolute-git-dir"]).await?; + if !out.status.success() { + return Err(git_command_error("rev-parse --absolute-git-dir", &out.stderr)); + } + let dir = String::from_utf8_lossy(&out.stdout).trim().to_string(); + if dir.is_empty() { + return Err(AppCommandError::external_command( + "git rev-parse --absolute-git-dir returned nothing", + path.to_string(), + )); + } + Ok(std::path::PathBuf::from(dir)) +} + /// Resolve a revision to a full sha. pub async fn rev_parse(path: &str, rev: &str) -> Result { let out = run_git(path, &["rev-parse", "--verify", &format!("{rev}^{{commit}}")]).await?; diff --git a/src-tauri/tauri.conf.json b/src-tauri/tauri.conf.json index 50b25a25c..d72fe4a00 100644 --- a/src-tauri/tauri.conf.json +++ b/src-tauri/tauri.conf.json @@ -1,7 +1,7 @@ { "$schema": "https://schema.tauri.app/config/2", "productName": "codeg", - "version": "0.23.0", + "version": "0.23.1", "identifier": "app.codeg", "build": { "beforeDevCommand": "pnpm tauri:before-dev", diff --git a/src/components/conversations/conversation-detail-panel.tsx b/src/components/conversations/conversation-detail-panel.tsx index 9968f4741..d4de5ea2f 100644 --- a/src/components/conversations/conversation-detail-panel.tsx +++ b/src/components/conversations/conversation-detail-panel.tsx @@ -1938,19 +1938,25 @@ export function ConversationDetailPanel() { if (!workingDir) return null return { workingDir, folderId: activeTab.folderId } }, [tabs, activeTabId, folder?.path]) - const { disconnect: disconnectByKey } = useAcpActions() + const { disconnectIfIdle } = useAcpActions() const { addTask, updateTask } = useTaskContext() const [reloadByTabId, setReloadByTabId] = useState>({}) const [detailsOpen, setDetailsOpen] = useState(false) const exportLabels = useExportLabels() - // Disconnect the old connection immediately when a preview tab is replaced + // Release the old connection as soon as a preview tab is replaced (the next + // single-click in the sidebar takes its slot) instead of waiting for a sweep. + // Idle-gated on purpose: the replaced tab may hold a session that is still + // working — often one the user only clicked in to watch — and disconnecting + // an owner mid-turn kills the agent CLI, which lands in the transcript as an + // interrupted request. Busy owners keep running; the idle sweep reclaims them + // once they settle. useEffect(() => { return onPreviewTabReplaced((replacedTabId) => { - disconnectByKey(replacedTabId).catch(() => {}) + disconnectIfIdle(replacedTabId).catch(() => {}) }) - }, [onPreviewTabReplaced, disconnectByKey]) + }, [onPreviewTabReplaced, disconnectIfIdle]) // Background turn_complete handler: for conversations not open in tabs. // Subscribes via the context's primary `acp://event` listener (single diff --git a/src/components/conversations/sidebar-conversation-list.tsx b/src/components/conversations/sidebar-conversation-list.tsx index 45129856d..f9c168a1d 100644 --- a/src/components/conversations/sidebar-conversation-list.tsx +++ b/src/components/conversations/sidebar-conversation-list.tsx @@ -16,16 +16,17 @@ import { useTheme } from "next-themes" import { toast } from "sonner" import { Virtualizer, type VirtualizerHandle } from "virtua" import { - FolderClosed, Bot, Check, ChevronRight, Download, ExternalLink, + FolderClosed, FolderGit2, FolderOpen, FolderOpenDot, FolderRoot, + Link2, ListChecks, Loader2, MoreHorizontal, @@ -54,12 +55,12 @@ import { deleteConversation, listChildConversations, } from "@/lib/api" -import { isDesktop, openFileDialog, revealItemInDir } from "@/lib/platform" -import { getActiveRemoteConnectionId } from "@/lib/transport" +import { isDesktop, revealItemInDir } from "@/lib/platform" import type { AgentType, ConversationStatus, DbConversationSummary, + FolderDetail, } from "@/lib/types" import { getAgentLabel } from "@/lib/custom-agents" import { @@ -110,7 +111,7 @@ import { useSubsessionSync } from "@/hooks/use-subsession-sync" import { SidebarSectionHeader } from "./sidebar-section-header" import { ConversationManageDialog } from "./conversation-manage-dialog" import { CloneDialog } from "@/components/layout/clone-dialog" -import { DirectoryBrowserDialog } from "@/components/shared/directory-browser-dialog" +import { WorkspaceFolderDialog } from "@/components/layout/workspace-folder-dialog" import { Button } from "@/components/ui/button" import { Input } from "@/components/ui/input" import { @@ -180,6 +181,7 @@ const FolderHeader = memo(function FolderHeader({ onNewConversation, onImport, onManageConversations, + onManageLinks, onChangeColor, onSetAlias, onSetDefaultAgent, @@ -217,6 +219,7 @@ const FolderHeader = memo(function FolderHeader({ onNewConversation: (folderId: number) => void onImport: (folderId: number) => void onManageConversations: (folderId: number) => void + onManageLinks: (folderId: number) => void onChangeColor: (folderId: number, color: FolderThemeColor) => void onSetAlias: (folderId: number, alias: string | null) => void onSetDefaultAgent: (folderId: number, agentType: AgentType | null) => void @@ -516,6 +519,10 @@ const FolderHeader = memo(function FolderHeader({ {t("folderHeaderMenu.manageConversations")} + onManageLinks(folderId)}> + + {t("folderHeaderMenu.manageLinks")} + @@ -716,7 +723,6 @@ export function SidebarConversationList({ (s) => s.removeFolderFromWorkspace ) const reorderFolders = useAppWorkspaceStore((s) => s.reorderFolders) - const openFolder = useAppWorkspaceStore((s) => s.openFolder) const refreshFolder = useAppWorkspaceStore((s) => s.refreshFolder) const refreshing = loading const { activeFolder } = useActiveFolder() @@ -848,6 +854,8 @@ export function SidebarConversationList({ } | null>(null) const [cloneOpen, setCloneOpen] = useState(false) const [browserOpen, setBrowserOpen] = useState(false) + // Folder whose links are being managed (context menu -> Linked folders). + const [linksFolder, setLinksFolder] = useState(null) const [dragging, setDragging] = useState(null) const [reordering, setReordering] = useState(false) const [dragOrder, setDragOrder] = useState(null) @@ -1623,6 +1631,14 @@ export function SidebarConversationList({ [folderIndex] ) + const handleManageFolderLinks = useCallback( + (folderId: number) => { + const folder = allFolders.find((f) => f.id === folderId) + if (folder) setLinksFolder(folder) + }, + [allFolders] + ) + const handleRemoveFolderConfirm = useCallback(async () => { if (!removeConfirm) return const { folderId, folderName } = removeConfirm @@ -1978,43 +1994,18 @@ export function SidebarConversationList({ // Safety net: drop listeners / stop autoscroll if the list unmounts mid-drag. useEffect(() => () => teardownDragListeners(), [teardownDragListeners]) - const handleOpenFolderAction = useCallback(async () => { - // Native Tauri dialog only when running on local desktop (no active - // remote workspace). Inside a remote workspace window the path lives - // on the remote host, so we route to the in-app server-side browser - // instead — the native dialog would pick a local path the remote - // server can't open. - if (isDesktop() && getActiveRemoteConnectionId() === null) { - try { - const result = await openFileDialog({ - directory: true, - multiple: false, - }) - if (!result) return - const selected = Array.isArray(result) ? result[0] : result - await openFolder(selected) - } catch (err) { - console.error("[SidebarConversationList] failed to open folder:", err) - } - } else { - setBrowserOpen(true) - } - }, [openFolder]) + // One dialog everywhere: it owns directory selection *and* the follow-up + // step that links other folders in, so the native picker can't be a separate + // path that skips half the flow (it is still offered inside the dialog on + // local desktop). Empty deps — `setBrowserOpen` is a stable setter — so the + // memoized section header doesn't re-render on every parent render. + const handleOpenFolderAction = useCallback(() => setBrowserOpen(true), []) // Stable trigger for the Clone Repository dialog, passed to the memoized // Folders section header. Empty deps (setCloneOpen is a stable setter) so the // header doesn't re-render on every parent render. const handleOpenCloneDialog = useCallback(() => setCloneOpen(true), []) - const handleBrowserSelect = useCallback( - (path: string) => { - openFolder(path).catch((err) => { - console.error("[SidebarConversationList] failed to open folder:", err) - }) - }, - [openFolder] - ) - const handleProjectBoot = useCallback(() => { openProjectBootWindow().catch((err) => { console.error( @@ -2114,6 +2105,7 @@ export function SidebarConversationList({ onNewConversation={handleNewConversationForFolder} onImport={handleImportForFolder} onManageConversations={handleManageConversations} + onManageLinks={handleManageFolderLinks} onChangeColor={handleChangeFolderColor} onSetAlias={handleSetFolderAlias} onSetDefaultAgent={handleChangeFolderDefaultAgent} @@ -2523,11 +2515,14 @@ export function SidebarConversationList({ )} - + + {linksFolder && ( + !o && setLinksFolder(null)} + folder={linksFolder} + /> + )} ) } diff --git a/src/components/layout/aux-panel-file-tree-tab.tsx b/src/components/layout/aux-panel-file-tree-tab.tsx index 6e7b50653..34a94b036 100644 --- a/src/components/layout/aux-panel-file-tree-tab.tsx +++ b/src/components/layout/aux-panel-file-tree-tab.tsx @@ -12,9 +12,9 @@ import { type KeyboardEvent as ReactKeyboardEvent, type ReactNode, } from "react" -import { revealItemInDir } from "@/lib/platform" +import { revealItemInDir, subscribe } from "@/lib/platform" import ignore from "ignore" -import { Check, ChevronRight } from "lucide-react" +import { Check, ChevronRight, Link2 } from "lucide-react" import { useTranslations } from "next-intl" import { toast } from "sonner" import { useActiveFolder } from "@/contexts/active-folder-context" @@ -43,6 +43,7 @@ import { gitListAllBranches, gitRollbackFile, gitStatus, + listFolderLinks, moveFileTreeEntry, readFilePreview, openCommitWindow, @@ -57,7 +58,13 @@ import { type FileTreeDragPayload, } from "@/lib/file-tree-dnd" import { ScrollArea } from "@/components/ui/scroll-area" -import type { FileTreeNode, GitBranchList, GitStatusEntry } from "@/lib/types" +import { + FOLDER_LINKS_CHANGED_EVENT, + type FileTreeNode, + type FolderLinksChanged, + type GitBranchList, + type GitStatusEntry, +} from "@/lib/types" import { FileTree, FileTreeFolder, @@ -153,6 +160,13 @@ const GITIGNORE_MUTED_CLASS = "text-muted-foreground/55" */ const DesktopDropDirContext = createContext(null) +/** + * Top-level directory names that are links into other projects. Supplied via + * context rather than threaded through `RenderNode`'s prop list, which is + * recursive and already long. + */ +const LinkedDirNamesContext = createContext>(new Set()) + interface FileActionTarget { kind: "file" | "dir" path: string @@ -674,6 +688,10 @@ function RenderNode({ // Desktop native drags don't emit DOM dragover, so a directory also lights up // when it's the drop zone broadcast from the Tauri DRAG_OVER hit-test. const desktopDropDir = useContext(DesktopDropDirContext) + const linkedDirNames = useContext(LinkedDirNamesContext) + // Links are always direct children of the root, so a top-level row's rel path + // *is* the link name. + const isLinkedDir = depth === 1 && linkedDirNames.has(node.path) const isGitignoreIgnored = ancestorGitignoreIgnored || gitignoreIgnoredPaths.has(node.path) @@ -889,6 +907,14 @@ function RenderNode({ + ) : undefined + } nameClassName={ isGitignoreIgnored ? GITIGNORE_MUTED_CLASS @@ -1221,6 +1247,11 @@ export function FileTreeTab() { const loadDirectoryChildrenRef = useRef< ((dirPath: string) => Promise) | null >(null) + // `fetchTree` is defined below; effects declared above it reach it through + // this ref (naming it directly in their deps would hit the TDZ). + const fetchTreeRef = useRef< + ((options?: { silent?: boolean }) => Promise) | null + >(null) const expandedPathsRef = useRef>(new Set([FILE_TREE_ROOT_PATH])) const workspaceTreeRef = useRef([]) // The node currently being dragged (set on dragstart, cleared on drop/cancel). @@ -1251,6 +1282,51 @@ export function FileTreeTab() { lazyLoadingDirPathsRef.current.clear() }, [folder?.path]) + // Top-level directories that are symlinks the user linked in. Only used to + // badge those rows — the backend already renders them as ordinary + // directories, so a failed fetch just loses the badge. + const [linkedNames, setLinkedNames] = useState>(new Set()) + const folderId = folder?.id ?? null + const refreshLinkedNames = useCallback(async () => { + if (folderId === null) { + setLinkedNames(new Set()) + return + } + try { + const links = await listFolderLinks(folderId) + setLinkedNames(new Set(links.map((link) => link.name))) + } catch { + setLinkedNames(new Set()) + } + }, [folderId]) + + useEffect(() => { + void refreshLinkedNames() + }, [refreshLinkedNames]) + + // Links can be added from the sidebar menu or another window, so converge on + // the backend broadcast rather than only on local mutations. + useEffect(() => { + let disposed = false + let unlisten: (() => void) | undefined + void (async () => { + const dispose = await subscribe( + FOLDER_LINKS_CHANGED_EVENT, + (payload) => { + if (payload.folder_id !== folderId) return + void refreshLinkedNames() + void fetchTreeRef.current?.({ silent: true }) + } + ) + if (disposed) dispose() + else unlisten = dispose + })() + return () => { + disposed = true + unlisten?.() + } + }, [folderId, refreshLinkedNames]) + // Derive the tree's focus from the externally-active file: opening a file from // another surface (search, a file tab) — or clicking one here, which opens it — // highlights that row, and clearing the active file (closing it, or switching @@ -1412,6 +1488,10 @@ export function FileTreeTab() { loadDirectoryChildrenRef.current = loadDirectoryChildren }, [loadDirectoryChildren]) + useEffect(() => { + fetchTreeRef.current = fetchTree + }, [fetchTree]) + useEffect(() => { expandedPathsRef.current = expandedPaths }, [expandedPaths]) @@ -2802,59 +2882,61 @@ export function FileTreeTab() { {folder?.path && ( - - - {nodes.map((node) => ( - { - void openFilePreview(path) - }} - onOpenFileDiff={(path) => { - void openWorkingTreeDiff(path) - }} - onOpenDirDiff={(path) => { - void openWorkingTreeDiff(path, { - mode: "overview", - }) - }} - onOpenCommitWindow={handleOpenCommitWindow} - onRequestCompareWithBranch={ - handleRequestCompareWithBranch - } - onRequestRollback={handleRequestRollback} - onOpenDirInTerminal={handleOpenDirInTerminal} - onRequestCreate={handleRequestCreate} - onRequestAddToVcs={handleAddToVcs} - onRequestRename={handleRequestRename} - onRequestDelete={handleRequestDelete} - onRequestUpload={handleRequestUpload} - onRequestDownloadFile={(target) => - void handleRequestDownloadFile(target) - } - onRequestDownloadDir={(target) => - void handleRequestDownloadDir(target) - } - onRefresh={fetchTree} - /> - ))} - - + + + + {nodes.map((node) => ( + { + void openFilePreview(path) + }} + onOpenFileDiff={(path) => { + void openWorkingTreeDiff(path) + }} + onOpenDirDiff={(path) => { + void openWorkingTreeDiff(path, { + mode: "overview", + }) + }} + onOpenCommitWindow={handleOpenCommitWindow} + onRequestCompareWithBranch={ + handleRequestCompareWithBranch + } + onRequestRollback={handleRequestRollback} + onOpenDirInTerminal={handleOpenDirInTerminal} + onRequestCreate={handleRequestCreate} + onRequestAddToVcs={handleAddToVcs} + onRequestRename={handleRequestRename} + onRequestDelete={handleRequestDelete} + onRequestUpload={handleRequestUpload} + onRequestDownloadFile={(target) => + void handleRequestDownloadFile(target) + } + onRequestDownloadDir={(target) => + void handleRequestDownloadDir(target) + } + onRefresh={fetchTree} + /> + ))} + + + diff --git a/src/components/layout/branch-dropdown.tsx b/src/components/layout/branch-dropdown.tsx index 9add7d52c..bc292bc68 100644 --- a/src/components/layout/branch-dropdown.tsx +++ b/src/components/layout/branch-dropdown.tsx @@ -3,7 +3,6 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react" import { ChevronDown, - FolderOpen, GitBranch, GitCommitHorizontal, GitFork, @@ -51,12 +50,11 @@ import { openPushWindow, openStashWindow, } from "@/lib/api" -import { isDesktop, openFileDialog, subscribe } from "@/lib/platform" -import { getActiveRemoteConnectionId } from "@/lib/transport" +import { subscribe } from "@/lib/platform" import { RemoteManageDialog } from "@/components/layout/remote-manage-dialog" import { ConflictDialog } from "@/components/layout/conflict-dialog" import { StashDialog } from "@/components/layout/stash-dialog" -import { DirectoryBrowserDialog } from "@/components/shared/directory-browser-dialog" +import { DirectoryPathInput } from "@/components/shared/directory-path-input" import { toErrorMessage } from "@/lib/app-error" import { useSwitchToBranch } from "@/hooks/use-switch-to-branch" import { @@ -162,7 +160,6 @@ export function BranchDropdown({ folder, isChatMode }: BranchDropdownProps) { const [branchLoading, setBranchLoading] = useState(false) const [confirmAction, setConfirmAction] = useState(null) const [worktreeOpen, setWorktreeOpen] = useState(false) - const [worktreeBrowserOpen, setWorktreeBrowserOpen] = useState(false) const [worktreeBranchName, setWorktreeBranchName] = useState("") const [worktreePath, setWorktreePath] = useState("") const [manageRemotesOpen, setManageRemotesOpen] = useState(false) @@ -386,24 +383,6 @@ export function BranchDropdown({ folder, isChatMode }: BranchDropdownProps) { setWorktreeOpen(true) } - async function handleBrowseWorktreePath() { - // The worktree is created on whatever host runs the git binary — local - // for the desktop, remote for a remote workspace. The picker must - // therefore browse the matching filesystem, otherwise the user - // ends up with a path the wrong side can't resolve. - if (isDesktop() && getActiveRemoteConnectionId() === null) { - const selected = await openFileDialog({ - directory: true, - multiple: false, - }) - if (selected) { - setWorktreePath(Array.isArray(selected) ? selected[0] : selected) - } - } else { - setWorktreeBrowserOpen(true) - } - } - async function handleNewWorktree() { const name = worktreeBranchName.trim() const wtPath = worktreePath.trim() @@ -787,22 +766,12 @@ export function BranchDropdown({ folder, isChatMode }: BranchDropdownProps) {
-
- setWorktreePath(e.target.value)} - className="flex-1" - /> - -
+
@@ -821,12 +790,6 @@ export function BranchDropdown({ folder, isChatMode }: BranchDropdownProps) { - setWorktreePath(path)} - /> - (null) const repoName = useMemo( @@ -47,23 +44,6 @@ export function CloneDialog({ open, onOpenChange }: CloneDialogProps) { [url] ) - const handleBrowse = async () => { - // Clone happens on the remote host when bound to a remote workspace — - // the target dir must therefore live on that host, not on the local - // desktop. Route to the in-app browser unless we're truly local. - if (isDesktop() && getActiveRemoteConnectionId() === null) { - const selected = await openFileDialog({ - directory: true, - multiple: false, - }) - if (selected) { - setTargetDir(Array.isArray(selected) ? selected[0] : selected) - } - } else { - setBrowserOpen(true) - } - } - const resetForm = () => { setUrl("") setTargetDir("") @@ -93,86 +73,66 @@ export function CloneDialog({ open, onOpenChange }: CloneDialogProps) { } return ( - <> - { - onOpenChange(v) - if (!v) resetForm() - }} - > - - - {t("title")} - -
-
- - setUrl(e.target.value)} - disabled={cloning} - autoFocus - /> -
-
- -
- setTargetDir(e.target.value)} - disabled={cloning} - className="flex-1" - /> - -
- {targetDir && url && ( -

- {t("clonePath", { path: `${targetDir}/${repoName}` })} -

- )} -
- {error &&

{error}

} + { + onOpenChange(v) + if (!v) resetForm() + }} + > + + + {t("title")} + +
+
+ + setUrl(e.target.value)} + disabled={cloning} + autoFocus + />
- - - - - -
- setTargetDir(path)} - /> - + browseLabel={t("browseDirectory")} + /> + {targetDir && url && ( +

+ {t("clonePath", { path: `${targetDir}/${repoName}` })} +

+ )} +
+ {error &&

{error}

} + + + + + + + ) } diff --git a/src/components/layout/folder-title-bar.tsx b/src/components/layout/folder-title-bar.tsx index 3d87ce500..3744ae4ce 100644 --- a/src/components/layout/folder-title-bar.tsx +++ b/src/components/layout/folder-title-bar.tsx @@ -2,6 +2,7 @@ import { useCallback } from "react" import { + ArrowLeft, Menu, PanelRight, Settings, @@ -50,7 +51,7 @@ export function FolderTitleBar() { const { activeFolder } = useActiveFolder() const isChatMode = useIsActiveChatMode() const { openNewConversationTab, openChatModeTab } = useTabActions() - const { openConversations } = useWorkbenchRoute() + const { isConversations, openConversations } = useWorkbenchRoute() const { isMac } = usePlatform() const showMacInset = isMac && isDesktop() @@ -109,7 +110,9 @@ export function FolderTitleBar() { {/* Empty middle is a full-height window-drag region. */}
{/* Right cluster: terminal + aux + settings — the same controls the - desktop RightEdgeChrome shows, now as direct buttons (no ⋯ menu). */} + desktop RightEdgeChrome shows, now as direct buttons (no ⋯ menu), + plus that overlay's back-to-conversations exit while a full-page route + (tasks / automations) covers the workspace. */}
+ {!isConversations && ( + + )} - + setWorkspaceDialogOpen(true)}> {t("openFolder")} @@ -71,14 +48,9 @@ export function NewFolderDropdown() { - { - openFolder(path).catch((err) => { - console.error("[NewFolderDropdown] failed to open folder:", err) - }) - }} + ) diff --git a/src/components/layout/right-edge-chrome.tsx b/src/components/layout/right-edge-chrome.tsx index 31a31aae4..222896cd1 100644 --- a/src/components/layout/right-edge-chrome.tsx +++ b/src/components/layout/right-edge-chrome.tsx @@ -1,7 +1,7 @@ "use client" import { useCallback } from "react" -import { PanelRight, Settings, SquareTerminal } from "lucide-react" +import { ArrowLeft, PanelRight, Settings, SquareTerminal } from "lucide-react" import { useTranslations } from "next-intl" import { openSettingsWindow } from "@/lib/api" import { Button } from "@/components/ui/button" @@ -18,7 +18,8 @@ import { rightChromeClusterWidth } from "@/lib/window-chrome" /** * Contents of the window's fixed top-RIGHT chrome overlay: terminal + aux-panel - * toggles + settings. `FolderLayoutShell` pins this at the window's top-right + * toggles (conversations) or a back-to-conversations exit (full-page routes), + * then settings. `FolderLayoutShell` pins this at the window's top-right * corner (to the LEFT of the Windows/Linux caption buttons) so it never moves — * or re-mounts — when the aux panel opens or closes. Preserves the old title * bar's disabled predicates and active styling. A leading drag filler right- @@ -31,8 +32,8 @@ export function RightEdgeChrome() { const isChatMode = useIsActiveChatMode() // Full-page workbench routes (tasks / automations) overlay the workspace the // terminal and aux panel live in — toggling them there is invisible, so the - // two buttons hide and only the settings gear stays. - const { isConversations } = useWorkbenchRoute() + // two buttons hide and a back-to-conversations exit takes their slot instead. + const { isConversations, openConversations } = useWorkbenchRoute() const { isOpen: auxPanelOpen, toggle: toggleAuxPanel } = useAuxPanelContext() const { isOpen: terminalOpen, toggle: toggleTerminal } = useTerminalContext() const isMac = useIsMac() @@ -86,6 +87,21 @@ export function RightEdgeChrome() { )} + {/* Full-page routes (tasks / automations) cover the workspace with no + other way out of the chrome strip — this is their exit back to the + conversation workspace, sitting just left of the settings gear. */} + {!isConversations && ( + + )} + + + + ) : null} + + {view === "add-targets" ? ( + <> + + ) : null + } + /> + + + + + + ) : null} + + {view === "links" ? ( + <> +
+
+ + + {rootFolder?.path ?? ""} + + {!manageMode ? ( + + ) : null} +
+ + +
+ {loadingLinks && + links.length === 0 && + pending.length === 0 ? ( +
+ + {tBrowser("loading")} +
+ ) : null} + + {links.map((link) => ( + { + setRenamingId(link.id) + setRenameValue(link.name) + }} + onCancelRename={() => setRenamingId(null)} + onCommitRename={commitRename} + onRemove={() => handleRemove(link)} + onRepair={() => handleRepair(link)} + /> + ))} + + {pending.map((item, index) => ( + + setPending((prev) => + prev.map((p, i) => (i === index ? { ...p, name } : p)) + ) + } + onRemove={() => + setPending((prev) => prev.filter((_, i) => i !== index)) + } + /> + ))} + + {skipped.map((plan) => ( +
+ +
+
+ {plan.targetPath} +
+
+ {plan.rejection === "already_linked" && + plan.existingLinkName + ? t("rejection.alreadyLinkedAs", { + name: plan.existingLinkName, + }) + : t(`rejection.${plan.rejection ?? "not_found"}`)} +
+
+
+ ))} + + {!loadingLinks && + links.length === 0 && + pending.length === 0 && + skipped.length === 0 ? ( +
+ {t("noLinks")} +
+ ) : null} +
+ + +
+ + {pending.length > 0 ? ( + + ) : null} +
+
+ + + {pending.length > 0 ? ( + <> + + + + ) : ( + + )} + + + ) : null} + + + ) +} + +/** + * Native-picker shortcut that lives at the trailing edge of the path box. + * Icon-only — the label moves into a tooltip, so the affordance sits next to + * the thing it fills in instead of competing with the footer's real actions. + */ +function NativePickerButton({ + onPick, + disabled, +}: { + onPick: () => void + disabled: boolean +}) { + const t = useTranslations("Folder.workspaceDialog") + + return ( + // The app mounts no global tooltip provider — each surface brings its own, + // and Radix throws without one. + + + + + + + + {t("useSystemPicker")} + + + ) +} + +interface PendingLink { + targetPath: string + name: string + baseName: string + renamed: boolean + collidesWithExistingEntry: boolean +} + +function statusTone(status: FolderLinkStatus) { + switch (status) { + case "ok": + return "text-muted-foreground" + case "missing": + case "broken": + return "text-amber-500" + case "conflicted": + return "text-destructive" + } +} + +function LinkRow({ + link, + busy, + editing, + renameValue, + renameIssue, + onRenameChange, + onStartRename, + onCancelRename, + onCommitRename, + onRemove, + onRepair, +}: { + link: FolderLinkDetail + busy: boolean + editing: boolean + renameValue: string + renameIssue: LinkNameIssue | null + onRenameChange: (value: string) => void + onStartRename: () => void + onCancelRename: () => void + onCommitRename: () => void + onRemove: () => void + onRepair: () => void +}) { + const t = useTranslations("Folder.workspaceDialog") + + return ( +
+ +
+ {editing ? ( +
+ onRenameChange(e.target.value)} + onKeyDown={(e) => { + if (e.nativeEvent.isComposing || e.key === "Process") return + if (e.key === "Enter") onCommitRename() + if (e.key === "Escape") onCancelRename() + }} + className={cn( + "h-7 text-sm", + renameIssue && "border-destructive focus-visible:ring-0" + )} + autoFocus + /> + + +
+ ) : ( +
{link.name}
+ )} + {editing && renameIssue ? ( +
+ {t(`nameIssue.${renameIssue}`)} +
+ ) : ( +
+ {link.targetPath} +
+ )} + {link.status !== "ok" ? ( +
+ {t(`status.${link.status}`)} +
+ ) : null} +
+ + {!editing ? ( + // The app mounts no global tooltip provider — each surface brings its + // own, and Radix throws without one. + +
+ {link.status === "missing" || link.status === "broken" ? ( + + + + + {t("repair")} + + ) : null} + + + + + {t("rename")} + + + + + + {t("unlink")} + +
+ + ) : null} +
+ ) +} + +function PendingRow({ + item, + issue, + onNameChange, + onRemove, +}: { + item: PendingLink + issue: LinkNameIssue | null + onNameChange: (name: string) => void + onRemove: () => void +}) { + const t = useTranslations("Folder.workspaceDialog") + + return ( +
+ +
+ onNameChange(e.target.value)} + className={cn( + "h-7 text-sm", + issue && "border-destructive focus-visible:ring-0" + )} + /> +
+ {item.targetPath} +
+ {issue ? ( +
+ {t(`nameIssue.${issue}`)} +
+ ) : item.renamed ? ( +
+ {item.collidesWithExistingEntry + ? t("renamedForExistingEntry", { base: item.baseName }) + : t("renamedForDuplicate", { base: item.baseName })} +
+ ) : ( +
+ {t("willAppearAs", { name: basenameOf(item.targetPath) })} +
+ )} +
+ +
+ ) +} diff --git a/src/components/message/plan-mode-card.test.tsx b/src/components/message/plan-mode-card.test.tsx index 8b5a9c4f3..1bdd4c17f 100644 --- a/src/components/message/plan-mode-card.test.tsx +++ b/src/components/message/plan-mode-card.test.tsx @@ -1,6 +1,6 @@ -import { render, screen, waitFor } from "@testing-library/react" +import { fireEvent, render, screen, waitFor } from "@testing-library/react" import { NextIntlClientProvider } from "next-intl" -import { describe, expect, it, vi } from "vitest" +import { afterEach, describe, expect, it, vi } from "vitest" // Exercise the real Streamdown pipeline for the plan-markdown branch; only the // link-safety hook is stubbed (no bearing on plan rendering), mirroring @@ -33,7 +33,38 @@ function renderCard(props: { ) } +// jsdom does no layout, so scrollHeight/clientHeight both read 0 — already +// "not overflowing" for the default case. Patch both onto Element.prototype +// *before* rendering so the clamp's synchronous mount-time measurement (it +// never waits on the inert ResizeObserver stub in test-setup.ts) sees them. +function mockScrollMetrics(scrollHeight: number, clientHeight: number) { + const descriptors = (["scrollHeight", "clientHeight"] as const).map( + (prop) => + [prop, Object.getOwnPropertyDescriptor(Element.prototype, prop)] as const + ) + Object.defineProperty(Element.prototype, "scrollHeight", { + configurable: true, + get: () => scrollHeight, + }) + Object.defineProperty(Element.prototype, "clientHeight", { + configurable: true, + get: () => clientHeight, + }) + return () => { + for (const [prop, descriptor] of descriptors) { + if (descriptor) Object.defineProperty(Element.prototype, prop, descriptor) + } + } +} + describe("PlanModeCard", () => { + let restoreMetrics: (() => void) | null = null + + afterEach(() => { + restoreMetrics?.() + restoreMetrics = null + }) + it("renders a compact marker for EnterPlanMode (no plan body)", () => { const { container } = renderCard({ toolName: "enterplanmode", input: "{}" }) expect(screen.getByText("Entered plan mode")).toBeInTheDocument() @@ -114,6 +145,48 @@ describe("PlanModeCard", () => { expect(screen.getByText("Kept in plan mode")).toBeInTheDocument() }) + it("leaves a short plan unclamped, with no toggle", () => { + renderCard({ + toolName: "exitplanmode", + input: JSON.stringify({ plan: "# Heading" }), + }) + + expect( + screen.queryByTestId("plan-mode-card-toggle") + ).not.toBeInTheDocument() + const content = screen.getByTestId("plan-mode-card-content") + expect(content).not.toHaveClass("collapsed-content-fade") + }) + + it("clamps a long plan and toggles it open and closed", () => { + restoreMetrics = mockScrollMetrics(900, 288) + + renderCard({ + toolName: "exitplanmode", + input: JSON.stringify({ plan: "# Heading\n- a\n- b\n- c" }), + }) + + const content = screen.getByTestId("plan-mode-card-content") + const toggle = screen.getByTestId("plan-mode-card-toggle") + expect(toggle).toHaveAttribute("aria-expanded", "false") + expect(toggle).toHaveTextContent("Show more") + expect(toggle).toHaveAttribute("aria-controls", content.id) + expect(content).toHaveClass("max-h-72", "collapsed-content-fade") + + fireEvent.click(toggle) + expect(toggle).toHaveAttribute("aria-expanded", "true") + expect(toggle).toHaveTextContent("Show less") + expect(screen.getByTestId("plan-mode-card-content")).not.toHaveClass( + "max-h-72", + "collapsed-content-fade" + ) + + // Clicking again re-collapses. + fireEvent.click(toggle) + expect(toggle).toHaveAttribute("aria-expanded", "false") + expect(screen.getByTestId("plan-mode-card-content")).toHaveClass("max-h-72") + }) + it("says the decision is pending while the plan-review call is unsettled", () => { // The seeded call is `input-available` for as long as the permission card // is open. Claiming either outcome there would state a decision the user diff --git a/src/components/message/plan-mode-card.tsx b/src/components/message/plan-mode-card.tsx index 8be7485ff..32343d204 100644 --- a/src/components/message/plan-mode-card.tsx +++ b/src/components/message/plan-mode-card.tsx @@ -2,11 +2,12 @@ import { memo } from "react" import { useTranslations } from "next-intl" -import { ListTodoIcon } from "lucide-react" +import { ChevronDownIcon, ChevronUpIcon, ListTodoIcon } from "lucide-react" import type { ToolCallState } from "@/lib/adapters/ai-elements-adapter" import { asRecord, extractPlanMarkdown } from "@/lib/plan-parse" import { MessageResponse } from "@/components/ai-elements/message" +import { useCollapsibleOverflow } from "@/hooks/use-collapsible-overflow" import { cn } from "@/lib/utils" /** @@ -35,6 +36,12 @@ function parseInput(input: string | null): Record | null { } } +/** + * Plan documents are frequently long enough to bury the rest of the turn, so + * the body is clamped to the same height the sibling checklist `` + * scrolls at and gets a "Show more"/"Show less" footer once it's actually + * clipped — the same `useCollapsibleOverflow` affordance user messages use. + */ function PlanMarkdownCard({ markdown, label, @@ -42,15 +49,47 @@ function PlanMarkdownCard({ markdown: string label: string }) { + const t = useTranslations("Folder.chat.messageList") + const { contentRef, contentId, isOverflowing, expanded, toggle } = + useCollapsibleOverflow(markdown) + + const clipped = !expanded + return (
{label}
-
+
{markdown}
+ {isOverflowing && ( + + )}
) } diff --git a/src/components/project-boot/hyperframes/hyperframes-launcher.tsx b/src/components/project-boot/hyperframes/hyperframes-launcher.tsx index 11073164c..076d9d4eb 100644 --- a/src/components/project-boot/hyperframes/hyperframes-launcher.tsx +++ b/src/components/project-boot/hyperframes/hyperframes-launcher.tsx @@ -4,7 +4,6 @@ import { useState, useEffect, useCallback } from "react" import { useTranslations } from "next-intl" import { Loader2, - FolderOpen, CircleCheck, CircleX, Circle, @@ -35,8 +34,7 @@ import { FieldTitle, FieldDescription, } from "@/components/ui/field" -import { isDesktop, openFileDialog, closeCurrentWindow } from "@/lib/platform" -import { getActiveRemoteConnectionId } from "@/lib/transport" +import { closeCurrentWindow } from "@/lib/platform" import { createHyperframesProject, openFolderInWorkspace, @@ -46,7 +44,7 @@ import { } from "@/lib/api" import type { HyperframesSkillAgent } from "@/lib/types" import { extractAppCommandError, toErrorMessage } from "@/lib/app-error" -import { DirectoryBrowserDialog } from "@/components/shared/directory-browser-dialog" +import { DirectoryPathInput } from "@/components/shared/directory-path-input" import { PACKAGE_MANAGER_OPTIONS } from "../shadcn/constants" import { HYPERFRAMES_RESOLUTION_OPTIONS, @@ -69,7 +67,6 @@ export function HyperframesLauncher() { const [packageManager, setPackageManager] = useState("pnpm") const [resolution, setResolution] = useState("default") const [creating, setCreating] = useState(false) - const [browserOpen, setBrowserOpen] = useState(false) const [error, setError] = useState(null) const [pmVersion, setPmVersion] = useState(null) @@ -161,20 +158,6 @@ export function HyperframesLauncher() { } } - const handleBrowse = async () => { - // Mirror the shadcn dialog: only use the native Tauri picker when truly on - // a local desktop workspace; otherwise the scaffold host is remote and we - // must browse its filesystem instead. - if (isDesktop() && getActiveRemoteConnectionId() === null) { - const result = await openFileDialog({ directory: true, multiple: false }) - if (!result) return - const selected = Array.isArray(result) ? result[0] : result - setSaveDirectory(selected) - } else { - setBrowserOpen(true) - } - } - const handleCreate = async () => { setError(null) setCreating(true) @@ -262,24 +245,13 @@ export function HyperframesLauncher() {
-
- setSaveDirectory(e.target.value)} - placeholder={t("createDialog.saveDirectoryPlaceholder")} - disabled={creating} - className="flex-1" - /> - -
+ {saveDirectory && projectName.trim() && (

{t("createDialog.projectPath", { @@ -469,12 +441,6 @@ export function HyperframesLauncher() {

- - setSaveDirectory(path)} - />
) } diff --git a/src/components/project-boot/shadcn/create-project-dialog.tsx b/src/components/project-boot/shadcn/create-project-dialog.tsx index 7f7ae1a9f..e89c2c8ad 100644 --- a/src/components/project-boot/shadcn/create-project-dialog.tsx +++ b/src/components/project-boot/shadcn/create-project-dialog.tsx @@ -2,13 +2,7 @@ import { useState, useEffect, useCallback } from "react" import { useTranslations } from "next-intl" -import { - Loader2, - FolderOpen, - ChevronsUpDown, - CircleCheck, - CircleX, -} from "lucide-react" +import { Loader2, ChevronsUpDown, CircleCheck, CircleX } from "lucide-react" import { toast } from "sonner" import { Button } from "@/components/ui/button" import { Input } from "@/components/ui/input" @@ -34,15 +28,14 @@ import { DialogHeader, DialogTitle, } from "@/components/ui/dialog" -import { isDesktop, openFileDialog, closeCurrentWindow } from "@/lib/platform" -import { getActiveRemoteConnectionId } from "@/lib/transport" +import { closeCurrentWindow } from "@/lib/platform" import { createShadcnProject, openFolderInWorkspace, detectPackageManager, } from "@/lib/api" import { extractAppCommandError, toErrorMessage } from "@/lib/app-error" -import { DirectoryBrowserDialog } from "@/components/shared/directory-browser-dialog" +import { DirectoryPathInput } from "@/components/shared/directory-path-input" import { BASE_OPTIONS, FRAMEWORK_OPTIONS, @@ -69,7 +62,6 @@ export function CreateProjectDialog({ const [rtl, setRtl] = useState(false) const [advancedOpen, setAdvancedOpen] = useState(false) const [creating, setCreating] = useState(false) - const [browserOpen, setBrowserOpen] = useState(false) const [error, setError] = useState(null) const [pmVersion, setPmVersion] = useState(null) @@ -98,21 +90,6 @@ export function CreateProjectDialog({ } }, [open, packageManager, checkPackageManager]) - const handleBrowse = async () => { - // Project scaffolding runs on whichever host hosts the workspace — - // remote when bound to a remote workspace, local otherwise. The - // picker must match that host, so we only use the native Tauri - // dialog when we are truly on a local desktop workspace. - if (isDesktop() && getActiveRemoteConnectionId() === null) { - const result = await openFileDialog({ directory: true, multiple: false }) - if (!result) return - const selected = Array.isArray(result) ? result[0] : result - setSaveDirectory(selected) - } else { - setBrowserOpen(true) - } - } - const handleCreate = async () => { setError(null) setCreating(true) @@ -181,216 +158,197 @@ export function CreateProjectDialog({ pmInstalled === true return ( - <> - { - onOpenChange(v) - if (!v) resetForm() - }} - > - - - {t("createDialog.title")} - + { + onOpenChange(v) + if (!v) resetForm() + }} + > + + + {t("createDialog.title")} + -
-
- - setProjectName(e.target.value)} - placeholder={t("createDialog.projectNamePlaceholder")} - disabled={creating} - /> -
+
+
+ + setProjectName(e.target.value)} + placeholder={t("createDialog.projectNamePlaceholder")} + disabled={creating} + /> +
-
- -
- setSaveDirectory(e.target.value)} - placeholder={t("createDialog.saveDirectoryPlaceholder")} - disabled={creating} - className="flex-1" - /> - -
- {saveDirectory && projectName.trim() && ( -

- {t("createDialog.projectPath", { - path: `${saveDirectory}/${projectName.trim()}`, - })} -

- )} -
+
+ + + {saveDirectory && projectName.trim() && ( +

+ {t("createDialog.projectPath", { + path: `${saveDirectory}/${projectName.trim()}`, + })} +

+ )} +
-
- - - - {PACKAGE_MANAGER_OPTIONS.map((opt) => ( - - {opt.label} - - ))} - +
+ + + {PACKAGE_MANAGER_OPTIONS.map((opt) => ( - -
- {pmChecking ? ( - <> - - - {t("createDialog.pmChecking")} - - - ) : pmInstalled ? ( - <> - - - {opt.label} v{pmVersion} - - - ) : ( - <> - - - {t("createDialog.pmNotInstalled")} - - - )} -
- + + {opt.label} + ))} - -
+ + {PACKAGE_MANAGER_OPTIONS.map((opt) => ( + +
+ {pmChecking ? ( + <> + + + {t("createDialog.pmChecking")} + + + ) : pmInstalled ? ( + <> + + + {opt.label} v{pmVersion} + + + ) : ( + <> + + + {t("createDialog.pmNotInstalled")} + + + )} +
+ + ))} + +
- - - + + +
+ + - - {t("createDialog.advancedOptions")} - - - -
- - - {FRAMEWORK_OPTIONS.map((opt) => ( - - - - {opt.label} - - - - - ))} - -
+ {FRAMEWORK_OPTIONS.map((opt) => ( + + + + {opt.label} + + + + + ))} + +
-
- - - {BASE_OPTIONS.map((opt) => ( - - - - {opt.label} - - - - - ))} - -
+
+ + + {BASE_OPTIONS.map((opt) => ( + + + + {opt.label} + + + + + ))} + +
- + + - - - - - -
+ {error && ( +
+ {error} +
+ )} +
- setSaveDirectory(path)} - /> - + + + + + + ) } diff --git a/src/components/shared/directory-browser-dialog.test.tsx b/src/components/shared/directory-browser-dialog.test.tsx index c2b105546..508c8b55c 100644 --- a/src/components/shared/directory-browser-dialog.test.tsx +++ b/src/components/shared/directory-browser-dialog.test.tsx @@ -137,7 +137,9 @@ describe("DirectoryBrowserDialog", () => { fireEvent.click(screen.getByText("work")) await screen.findByDisplayValue("/home/me/work") - fireEvent.click(screen.getByTitle("Go to parent directory")) + fireEvent.click( + screen.getByRole("button", { name: "Go to parent directory" }) + ) // Parent of the INPUT (/home/me/work) is /home/me. The old rootPath-based // logic would instead have jumped to /home (parent of the tree root). diff --git a/src/components/shared/directory-browser-dialog.tsx b/src/components/shared/directory-browser-dialog.tsx index e1612f422..d9fc9fc3b 100644 --- a/src/components/shared/directory-browser-dialog.tsx +++ b/src/components/shared/directory-browser-dialog.tsx @@ -1,21 +1,7 @@ "use client" -import { - useState, - useEffect, - useLayoutEffect, - useCallback, - useRef, -} from "react" +import { useCallback, useRef, useState } from "react" import { useTranslations } from "next-intl" -import { - ChevronRight, - ChevronUp, - FolderIcon, - FolderOpenIcon, - Home, - Loader2, -} from "lucide-react" import { Dialog, DialogContent, @@ -23,13 +9,11 @@ import { DialogHeader, DialogTitle, } from "@/components/ui/dialog" -import { Input } from "@/components/ui/input" import { Button } from "@/components/ui/button" -import { ScrollArea } from "@/components/ui/scroll-area" -import { cn } from "@/lib/utils" -import { getHomeDirectory, listDirectoryEntries } from "@/lib/api" -import { parentFsPath } from "@/lib/path-utils" -import type { DirectoryEntry } from "@/lib/types" +import { + DirectoryBrowser, + type DirectoryBrowserHandle, +} from "@/components/shared/directory-browser" interface DirectoryBrowserDialogProps { open: boolean @@ -40,18 +24,10 @@ interface DirectoryBrowserDialogProps { } /** - * Strip trailing separators (POSIX `/` or Windows `\`) so otherwise-equivalent - * paths compare equal for the row highlight; an all-separator root is left - * intact rather than collapsed away. + * Single-directory picker. A thin dialog shell around [`DirectoryBrowser`], + * which owns the navigation and validation; multi-step flows embed that panel + * directly instead of nesting dialogs. */ -const normalizePath = (path: string) => path.replace(/[/\\]+$/, "") || path - -// Synchronous layout effect on the client (so the session/selection guards see -// the latest committed values before any pending async work resolves), but a -// passive effect during the static-export prerender to avoid the SSR warning. -const useIsomorphicLayoutEffect = - typeof window !== "undefined" ? useLayoutEffect : useEffect - export function DirectoryBrowserDialog({ open, onOpenChange, @@ -60,304 +36,26 @@ export function DirectoryBrowserDialog({ initialPath, }: DirectoryBrowserDialogProps) { const t = useTranslations("DirectoryBrowser") - - const [rootPath, setRootPath] = useState("") - const [pathInput, setPathInput] = useState("") - const [entries, setEntries] = useState>( - new Map() - ) - const [expandedPaths, setExpandedPaths] = useState>(new Set()) - const [loading, setLoading] = useState>(new Set()) - const [error, setError] = useState(null) + const [path, setPath] = useState(initialPath ?? "") const [confirming, setConfirming] = useState(false) + const browserRef = useRef(null) - const initialized = useRef(false) - // Monotonic session id, bumped synchronously on every real open/close - // transition. Async flows capture it before awaiting and discard their writes - // when it changes, so a slow request from a previous open can't clobber — or - // expose — state in a newer one. Guarding on the previous `open` keeps the - // bump idempotent under StrictMode's mount-time effect replay (which would - // otherwise bump again and strand the first init's in-flight writes). - const sessionGen = useRef(0) - const prevOpen = useRef(open) - useIsomorphicLayoutEffect(() => { - if (prevOpen.current !== open) { - prevOpen.current = open - sessionGen.current += 1 - } - }, [open]) - // Monotonic navigation id. Each navigateTo() bumps it and the open-time init() - // captures it, so within a single session a slower earlier navigation (or a - // late init) can't overwrite the destination of a newer one — the latest user - // intent always wins. - const navSeq = useRef(0) - // Latest committed pathInput, mirrored synchronously so a confirm validation - // can tell the selection moved (within the session) before its check resolved. - const pathInputRef = useRef(pathInput) - useIsomorphicLayoutEffect(() => { - pathInputRef.current = pathInput - }, [pathInput]) - - const loadEntries = useCallback( - async (path: string): Promise => { - // Already cached - if (entries.has(path)) return entries.get(path)! - - const gen = sessionGen.current - setLoading((prev) => new Set(prev).add(path)) - setError(null) - try { - const result = await listDirectoryEntries(path) - // Skip writes if a close/reopen happened mid-flight — they belong to a - // session that no longer exists and would surface stale data. - if (gen === sessionGen.current) { - setEntries((prev) => new Map(prev).set(path, result)) - } - return result - } catch { - if (gen === sessionGen.current) setError(t("errorLoadingDir")) - return null - } finally { - if (gen === sessionGen.current) { - setLoading((prev) => { - const next = new Set(prev) - next.delete(path) - return next - }) - } - } - }, - [entries, t] - ) - - const navigateTo = useCallback( - async (path: string) => { - const gen = sessionGen.current - const seq = (navSeq.current += 1) - const result = await loadEntries(path) - // Discard if a newer navigation started, or the load outlived its session - // (close/reopen), so the most recent navigation wins. - if (gen !== sessionGen.current || seq !== navSeq.current) return - if (result !== null) { - setRootPath(path) - setPathInput(path) - setExpandedPaths(new Set()) - } - }, - [loadEntries] - ) - - // Initialize on open - useEffect(() => { - if (!open) { - initialized.current = false - return - } - if (initialized.current) return - initialized.current = true - - const gen = sessionGen.current - const seq = navSeq.current - - // Reset synchronously so a reopened dialog never shows — or lets the user - // confirm — the previous session's path while the start dir is loading. - setRootPath("") - setPathInput(initialPath ?? "") - setExpandedPaths(new Set()) - setEntries(new Map()) - setError(null) - setLoading(new Set()) - setConfirming(false) - - const init = async () => { - try { - const startPath = initialPath || (await getHomeDirectory()) - // Drop these writes if a close/reopen superseded this init, or the user - // already navigated somewhere else while the start dir was loading. - if (gen !== sessionGen.current || seq !== navSeq.current) return - setRootPath(startPath) - setPathInput(startPath) - setLoading(new Set([startPath])) - - const result = await listDirectoryEntries(startPath) - if (gen !== sessionGen.current || seq !== navSeq.current) return - setEntries(new Map([[startPath, result]])) - setLoading(new Set()) - } catch { - if (gen !== sessionGen.current || seq !== navSeq.current) return - setError(t("errorLoadingDir")) - setLoading(new Set()) - } - } - init() - }, [open, initialPath, t]) - - const handleToggleExpand = useCallback( - async (path: string) => { - if (expandedPaths.has(path)) { - setExpandedPaths((prev) => { - const next = new Set(prev) - next.delete(path) - return next - }) - return - } - const gen = sessionGen.current - await loadEntries(path) - if (gen !== sessionGen.current) return - // Functional update so two folders expanded concurrently compose instead - // of overwriting each other with a stale snapshot. - setExpandedPaths((prev) => new Set(prev).add(path)) - }, - [expandedPaths, loadEntries] - ) - - const handleSelect = useCallback((path: string) => { - setPathInput(path) - }, []) - - const handleConfirm = useCallback(async () => { - const path = pathInput.trim() - if (!path || confirming) return - // Validate the path is a real, readable directory before committing. - // Visited dirs (clicked rows / navigated roots) are served from the - // entries cache, so this is instant in the common case; a typed path is - // verified here and keeps the dialog open with an error on failure. - const gen = sessionGen.current - setConfirming(true) - const result = await loadEntries(path) - // A stale confirm from a previous open must not touch the new session's - // state — not even its spinner — so bail before clearing `confirming`. - if (gen !== sessionGen.current) return - setConfirming(false) - // Within the session, still bail if the selection moved while validating - // (e.g. the user picked another directory after pressing Select). - if (pathInputRef.current.trim() !== path) return - if (result !== null) { - onSelect(path) - onOpenChange(false) - } - }, [pathInput, confirming, loadEntries, onSelect, onOpenChange]) - - const handleNavigateUp = useCallback(() => { - const parent = parentFsPath(pathInput.trim() || rootPath) - if (!parent) return - navigateTo(parent) - }, [pathInput, rootPath, navigateTo]) - - const handleGoHome = useCallback(async () => { - const gen = sessionGen.current - try { - const home = await getHomeDirectory() - if (gen !== sessionGen.current) return - navigateTo(home) - } catch { - if (gen !== sessionGen.current) return - setError(t("errorLoadingDir")) - } - }, [navigateTo, t]) - - const handlePathInputKeyDown = useCallback( - (e: React.KeyboardEvent) => { - if (e.key === "Enter" && pathInput.trim()) { - navigateTo(pathInput.trim()) - } - }, - [pathInput, navigateTo] - ) - - const handleDoubleClick = useCallback( - (path: string) => { - onSelect(path) + const commit = useCallback( + (selected: string) => { + onSelect(selected) onOpenChange(false) }, [onSelect, onOpenChange] ) - const renderEntries = (parentPath: string, depth: number) => { - const children = entries.get(parentPath) - const isLoading = loading.has(parentPath) - - if (isLoading) { - return ( -
- - {t("loading")} -
- ) - } - - if (!children) return null - - if (children.length === 0) { - return ( -
- {t("emptyDirectory")} -
- ) - } - - return children.map((entry) => { - const isExpanded = expandedPaths.has(entry.path) - const isSelected = normalizePath(entry.path) === normalizePath(pathInput) - - return ( -
- - {isExpanded && renderEntries(entry.path, depth + 1)} -
- ) - }) - } + // `confirming` is mirrored from the panel rather than tracked here: a confirm + // that outlives its browsing session must leave the flag set, so a stale + // round trip can never re-enable a newer session's Select button. + const handleConfirm = useCallback(async () => { + if (confirming) return + const selected = await browserRef.current?.confirm() + if (selected) commit(selected) + }, [confirming, commit]) return ( @@ -366,48 +64,15 @@ export function DirectoryBrowserDialog({ {title ?? t("title")} -
-
- - - setPathInput(e.target.value)} - onKeyDown={handlePathInputKeyDown} - placeholder={t("pathPlaceholder")} - className="flex-1 h-8 text-sm font-mono" - /> -
- - -
- {renderEntries(rootPath, 0)} - {error && !loading.size && ( -
- {error} -
- )} -
- -
+ + {isExpanded && renderEntries(entry.path, depth + 1)} + + ) + }) + } + + return ( +
+ {/* The app mounts no global tooltip provider — each surface brings its + own, and Radix throws without one. */} + + + + + + + onValueChange(e.target.value)} + onKeyDown={handlePathInputKeyDown} + placeholder={t("pathPlaceholder")} + className="h-8 text-sm font-mono" + /> + {pathInputAction ? ( + + {pathInputAction} + + ) : null} + + + + +
+ {renderEntries(rootPath, 0)} + {error && !loading.size && ( +
+ {error} +
+ )} +
+ +
+ ) +}) + +/** + * Icon-only navigation shortcut for the leading edge of the path box. The label + * lives in a tooltip so the whole row reads as one control, and is mirrored onto + * `aria-label` so the button still has an accessible name. + */ +function NavButton({ + icon: Icon, + label, + onClick, +}: { + icon: LucideIcon + label: string + onClick: () => void +}) { + return ( + + + + + + + {label} + + ) +} diff --git a/src/components/shared/directory-path-input.test.tsx b/src/components/shared/directory-path-input.test.tsx new file mode 100644 index 000000000..d7153ccd1 --- /dev/null +++ b/src/components/shared/directory-path-input.test.tsx @@ -0,0 +1,125 @@ +import { useState } from "react" +import { act, fireEvent, render, screen } from "@testing-library/react" +import { NextIntlClientProvider } from "next-intl" +import { beforeEach, describe, expect, it, vi } from "vitest" + +import enMessages from "@/i18n/messages/en.json" +import { Dialog, DialogContent, DialogTitle } from "@/components/ui/dialog" +import { DirectoryPathInput } from "./directory-path-input" + +const api = vi.hoisted(() => ({ + getHomeDirectory: vi.fn(), + listDirectoryEntries: vi.fn(), +})) +vi.mock("@/lib/api", () => api) + +const platform = vi.hoisted(() => ({ + desktop: false, + openFileDialog: vi.fn(), +})) +vi.mock("@/lib/platform", () => ({ + isDesktop: () => platform.desktop, + openFileDialog: platform.openFileDialog, +})) +vi.mock("@/lib/transport", () => ({ + getActiveRemoteConnectionId: () => null, +})) + +function Harness({ nested = false }: { nested?: boolean }) { + const [value, setValue] = useState("") + const field = ( + + ) + return ( + + {nested ? ( + // The in-app browser is a Dialog rendered from inside another Dialog's + // content — the arrangement every host now uses. + + + Host + {field} + + + ) : ( + field + )} + + ) +} + +beforeEach(() => { + vi.clearAllMocks() + platform.desktop = false + api.getHomeDirectory.mockResolvedValue("/home/me") + api.listDirectoryEntries.mockResolvedValue([]) +}) + +describe("DirectoryPathInput", () => { + it("edits the path directly", () => { + render() + fireEvent.change(screen.getByPlaceholderText("Where to?"), { + target: { value: "/typed/dir" }, + }) + expect(screen.getByDisplayValue("/typed/dir")).toBeInTheDocument() + }) + + it("fills the box from the system picker without opening a browser", async () => { + platform.desktop = true + platform.openFileDialog.mockResolvedValue("/home/me/picked") + render() + + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: "Browse directory" })) + }) + + expect(platform.openFileDialog).toHaveBeenCalledWith({ + directory: true, + multiple: false, + }) + expect(screen.getByDisplayValue("/home/me/picked")).toBeInTheDocument() + expect(api.getHomeDirectory).not.toHaveBeenCalled() + }) + + it("falls back to the in-app browser when there is no local desktop", async () => { + api.listDirectoryEntries.mockResolvedValue([ + { name: "work", path: "/home/me/work", hasChildren: false }, + ]) + render() + + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: "Browse directory" })) + }) + expect(platform.openFileDialog).not.toHaveBeenCalled() + + fireEvent.click(await screen.findByRole("button", { name: /work/ })) + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: "Select" })) + }) + + expect(screen.getByDisplayValue("/home/me/work")).toBeInTheDocument() + }) + + it("keeps the host dialog open while the nested browser runs", async () => { + api.listDirectoryEntries.mockResolvedValue([ + { name: "work", path: "/home/me/work", hasChildren: false }, + ]) + render() + + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: "Browse directory" })) + }) + fireEvent.click(await screen.findByRole("button", { name: /work/ })) + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: "Select" })) + }) + + expect(screen.getByText("Host")).toBeInTheDocument() + expect(screen.getByDisplayValue("/home/me/work")).toBeInTheDocument() + }) +}) diff --git a/src/components/shared/directory-path-input.tsx b/src/components/shared/directory-path-input.tsx new file mode 100644 index 000000000..c3fb20561 --- /dev/null +++ b/src/components/shared/directory-path-input.tsx @@ -0,0 +1,121 @@ +"use client" + +import { useCallback, useState } from "react" +import { useTranslations } from "next-intl" +import { FolderSearch, MonitorDot } from "lucide-react" + +import { + InputGroup, + InputGroupAddon, + InputGroupButton, + InputGroupInput, +} from "@/components/ui/input-group" +import { + Tooltip, + TooltipContent, + TooltipProvider, + TooltipTrigger, +} from "@/components/ui/tooltip" +import { DirectoryBrowserDialog } from "@/components/shared/directory-browser-dialog" +import { isDesktop, openFileDialog } from "@/lib/platform" +import { getActiveRemoteConnectionId } from "@/lib/transport" + +interface DirectoryPathInputProps { + value: string + onValueChange: (path: string) => void + id?: string + placeholder?: string + disabled?: boolean + className?: string + /** Tooltip and accessible name for the picker button. */ + browseLabel?: string + /** Title of the in-app browser; defaults to its own generic one. */ + browserTitle?: string + /** Directory the in-app browser opens at; defaults to the user's home. */ + initialPath?: string +} + +/** + * Directory field with the picker folded into its trailing edge. + * + * Which picker opens depends on where the path has to resolve: the OS dialog + * only when the workspace is truly local, otherwise the in-app browser, which + * walks the host that actually owns the filesystem. The icon says which one is + * coming. Either way a pick only fills the box in — committing it stays with + * the surrounding form, so the picker can never act on the user's behalf. + */ +export function DirectoryPathInput({ + value, + onValueChange, + id, + placeholder, + disabled, + className, + browseLabel, + browserTitle, + initialPath, +}: DirectoryPathInputProps) { + const t = useTranslations("DirectoryBrowser") + const [browserOpen, setBrowserOpen] = useState(false) + + const nativePicker = isDesktop() && getActiveRemoteConnectionId() === null + const label = browseLabel ?? t("title") + + const handleBrowse = useCallback(async () => { + if (!nativePicker) { + setBrowserOpen(true) + return + } + const selected = await openFileDialog({ directory: true, multiple: false }) + if (!selected) return + const path = Array.isArray(selected) ? selected[0] : selected + if (path) onValueChange(path) + }, [nativePicker, onValueChange]) + + return ( + <> + {/* The app mounts no global tooltip provider — each surface brings its + own, and Radix throws without one. */} + + + onValueChange(e.target.value)} + placeholder={placeholder} + disabled={disabled} + /> + + + + + {nativePicker ? ( + + ) : ( + + )} + + + {label} + + + + + + + + ) +} diff --git a/src/components/tasks/board-columns.test.ts b/src/components/tasks/board-columns.test.ts index d355940f1..e44475ec7 100644 --- a/src/components/tasks/board-columns.test.ts +++ b/src/components/tasks/board-columns.test.ts @@ -43,10 +43,12 @@ function task( describe("columnForStatus", () => { it("maps every DB status to its board column per the spec", () => { - // 待办 = todo + queued + // 待办 = todo + queued (queued is still waiting for a slot) expect(columnForStatus("todo")).toBe("todo") expect(columnForStatus("queued")).toBe("todo") - // 进行中 = running + // 进行中 = preparing + running — a preparing task already left the queue + // and is doing setup work (worktree, init command, agent spawn). + expect(columnForStatus("preparing")).toBe("inProgress") expect(columnForStatus("running")).toBe("inProgress") // 等你处理 = awaiting_input + review + merging + failed — a merge is an // agent turn but the card stays in the review column until it lands. diff --git a/src/components/tasks/board-columns.ts b/src/components/tasks/board-columns.ts index 3de221a97..b7c73b447 100644 --- a/src/components/tasks/board-columns.ts +++ b/src/components/tasks/board-columns.ts @@ -17,8 +17,12 @@ export const BOARD_COLUMN_IDS: BoardColumnId[] = [ export function columnForStatus(status: WorkTaskStatus): BoardColumnId { switch (status) { case "todo": + // Still waiting for a concurrency slot — nothing is happening yet. case "queued": return "todo" + // Already out of the queue and working (worktree, init command, agent + // spawn), just without a session to show yet. + case "preparing": case "running": return "inProgress" case "awaiting_input": diff --git a/src/components/tasks/setting-card.tsx b/src/components/tasks/setting-card.tsx new file mode 100644 index 000000000..8dae8487c --- /dev/null +++ b/src/components/tasks/setting-card.tsx @@ -0,0 +1,130 @@ +"use client" + +import type { LucideIcon } from "lucide-react" + +import { Label } from "@/components/ui/label" +import { cn } from "@/lib/utils" + +/** + * Bordered surface that groups related settings rows. Rows stacked inside one + * card get a hairline between them, which is what makes options that belong + * together (merge strategy + what happens to the worktree afterwards) read as + * one decision instead of two unrelated lines. + * + * `bg-muted/40` rather than `bg-card`: in the light theme `--card` is the same + * white as `--background`, so a card would only be an outline; muted gives a + * faint fill in light mode and a slight lift over the dialog in dark mode. + */ +export function SettingCard({ + className, + ...props +}: React.ComponentProps<"div">) { + return ( +
+ ) +} + +/** + * An explanatory block on the same surface as the cards around it — for the + * context a row's own description can't carry (what a whole tab is for, how + * the values there are used). Kept card-shaped so a tab never drops back to + * bare paragraphs floating between cards. + */ +export function SettingNote({ + icon: Icon, + children, + className, +}: { + icon?: LucideIcon + children: React.ReactNode + className?: string +}) { + return ( +
+ {/* The wrapper is one text line tall (`leading-5`), so the glyph centers + on the first line whatever its own size — a margin nudge would drift + the moment either the icon or the type scale changes. */} + {Icon ? ( + + + ) : null} +

{children}

+
+ ) +} + +interface SettingRowProps { + /** Small glyph in front of the title; purely decorative. */ + icon?: LucideIcon + title: React.ReactNode + /** One line explaining the setting, under the title. */ + description?: React.ReactNode + /** Ties the title to the control it labels (`Switch`, `Input`, …). */ + htmlFor?: string + /** Compact control pinned to the right of the title — switch, select, stepper. */ + control?: React.ReactNode + /** Full-width control (input, textarea, agent picker) placed under the text. */ + children?: React.ReactNode + className?: string +} + +/** + * One setting inside a {@link SettingCard}: label and explanation on the left, + * a compact control on the right, and optionally a full-width control below. + * Every option in the task settings dialog is rendered through this so the + * rhythm (type sizes, paddings, where the control sits) can't drift row to row. + */ +export function SettingRow({ + icon: Icon, + title, + description, + htmlFor, + control, + children, + className, +}: SettingRowProps) { + return ( +
+ {/* With an explanation under the title the control aligns to the title + line; without one there is only that line, so the two center on each + other instead of hanging off a taller control's top edge. */} +
+
+ + {description ? ( + + {description} + + ) : null} +
+ {control ?
{control}
: null} +
+ {children} +
+ ) +} diff --git a/src/components/tasks/task-card.tsx b/src/components/tasks/task-card.tsx index 5b9ed2d88..8afa18099 100644 --- a/src/components/tasks/task-card.tsx +++ b/src/components/tasks/task-card.tsx @@ -24,6 +24,7 @@ import type { WorkTask } from "@/lib/types" type StatusLabelKey = | "statusTodo" | "statusQueued" + | "statusPreparing" | "statusRunning" | "statusAwaitingInput" | "statusReview" @@ -38,6 +39,8 @@ export function statusLabelKey(status: WorkTask["status"]): StatusLabelKey { return "statusTodo" case "queued": return "statusQueued" + case "preparing": + return "statusPreparing" case "running": return "statusRunning" case "awaiting_input": @@ -70,6 +73,7 @@ export function StatusChip({ task }: { task: WorkTask }) { : t(statusLabelKey(task.status)) switch (task.status) { case "queued": + case "preparing": case "running": case "merging": return ( @@ -232,6 +236,7 @@ export function TaskCard({ more.push({ icon: Pencil, label: t("actionEdit"), onClick: onEdit }) break case "queued": + case "preparing": case "running": case "awaiting_input": primary = { icon: Ban, label: t("actionCancel"), onClick: onCancel } diff --git a/src/components/tasks/task-detail-sheet.tsx b/src/components/tasks/task-detail-sheet.tsx index cc23c3057..28cda10e6 100644 --- a/src/components/tasks/task-detail-sheet.tsx +++ b/src/components/tasks/task-detail-sheet.tsx @@ -52,6 +52,7 @@ import { toErrorMessage } from "@/lib/app-error" import { formatTokenCount } from "@/lib/token-format" import { onTransportReconnect, subscribe } from "@/lib/platform" import { UnifiedDiffPreview } from "@/components/diff/unified-diff-preview" +import { MessageResponse } from "@/components/ai-elements/message" import { AgentIcon } from "@/components/agent-icon" import { getAgentLabel } from "@/lib/custom-agents" import { StatusChip, statusLabelKey } from "./task-card" @@ -93,6 +94,24 @@ import type { const WORK_TASK_CHANGED_EVENT = "task://changed" +/** + * Typography for the agent's Markdown result inside the drawer's compact + * panel. Streamdown sizes its own elements for the full-width chat column + * (h1 at `text-3xl`, 24px above every heading), which is far too loud at the + * panel's 12px scale — and a descendant selector outranks the class Streamdown + * puts on the element itself, so these win without `!important`. Lists and the + * first/last block's collapsed margin already come from `MessageResponse`. + * `prose` is deliberately absent: the repo has no typography plugin, so those + * classes generate nothing. + */ +const RESULT_MARKDOWN = + "[&_h1]:text-[0.8125rem] [&_h2]:text-[0.8125rem] [&_h3]:text-xs [&_h4]:text-xs " + + "[&_h1]:font-semibold [&_h2]:font-semibold [&_h3]:font-semibold [&_h4]:font-semibold " + + "[&_h1]:mt-3 [&_h2]:mt-3 [&_h3]:mt-2 [&_h4]:mt-2 " + + "[&_h1]:mb-1 [&_h2]:mb-1 [&_h3]:mb-1 [&_h4]:mb-1 " + + "[&_p]:mt-0 [&_p]:mb-2 [&_ul]:my-2 [&_ol]:my-2 [&_li]:my-0.5 " + + "[&_blockquote]:my-2 [&_hr]:my-3" + interface TaskDetailSheetProps { open: boolean onOpenChange: (open: boolean) => void @@ -310,6 +329,7 @@ export function TaskDetailSheet({ }) break case "queued": + case "preparing": case "running": case "awaiting_input": zoneActions.push({ @@ -461,7 +481,7 @@ export function TaskDetailSheet({ on a white drawer. The brief and the result share it — they are the same kind of thing (quoted prose), and the section headings already say whose words they are. */} - +
{promptText}
@@ -474,9 +494,21 @@ export function TaskDetailSheet({

{t("detailSummary")}

-

- {task.result_summary} -

+ {/* The agent writes its verdict the way it writes anything + else — bullets, `code`, bold — so it goes through the + same renderer as the chat instead of showing its own + source. Capped taller than the brief above: this is the + thing the drawer was opened to read. */} + +
+ {task.result_summary} +
+ ) : null} @@ -926,9 +958,13 @@ function TaskDiffBody({ */ function CollapsibleBlock({ maxPx, + fadeClass = "from-background", children, }: { maxPx: number + /** Gradient source for the clip fade — it has to match the surface being + * clipped, or the fade reads as a stripe rather than a soft edge. */ + fadeClass?: string children: ReactNode }) { const t = useTranslations("Tasks") @@ -962,7 +998,14 @@ function CollapsibleBlock({ {clipped ? ( @@ -1072,6 +1115,7 @@ const EVENT_KIND_KEYS = { const STATUS_KEYS = new Set([ "todo", "queued", + "preparing", "running", "awaiting_input", "review", @@ -1086,6 +1130,7 @@ function statusDotClass(status: string): string { switch (status) { case "running": case "queued": + case "preparing": return "bg-primary" case "awaiting_input": case "review": diff --git a/src/components/tasks/task-settings-dialog.test.tsx b/src/components/tasks/task-settings-dialog.test.tsx new file mode 100644 index 000000000..c064d1b26 --- /dev/null +++ b/src/components/tasks/task-settings-dialog.test.tsx @@ -0,0 +1,235 @@ +import { render, screen, waitFor } from "@testing-library/react" +import userEvent from "@testing-library/user-event" +import { NextIntlClientProvider } from "next-intl" +import { beforeEach, describe, expect, it, vi } from "vitest" +import enMessages from "@/i18n/messages/en.json" +import type { WorkTaskFolderSettings } from "@/lib/types" + +const getMock = vi.fn() +const getOwnMock = vi.fn() +const setMock = vi.fn().mockResolvedValue(undefined) +const deleteMock = vi.fn().mockResolvedValue(undefined) + +vi.mock("@/lib/api", () => ({ + workTaskSettingsGet: (...args: unknown[]) => getMock(...args), + workTaskSettingsGetOwn: (...args: unknown[]) => getOwnMock(...args), + workTaskSettingsSet: (...args: unknown[]) => setMock(...args), + workTaskSettingsDelete: (...args: unknown[]) => deleteMock(...args), + listFolderCommands: () => Promise.resolve([]), +})) + +// The agent surface probes a live CLI; none of it is under test here. +vi.mock("@/components/chat/agent-selector", () => ({ + AgentSelector: () =>
, +})) +vi.mock("@/components/automations/agent-config-section", () => ({ + AgentConfigSection: () =>
, + effectiveSelections: ( + _snapshot: unknown, + modeId: string | null, + configValues: Record + ) => ({ mode_id: modeId, config_values: configValues }), + snapshotLabels: () => ({}), +})) +vi.mock("@/components/automations/use-agent-options", () => ({ + useAgentOptions: () => ({ + snapshot: null, + loading: false, + error: null, + reload: vi.fn(), + ensure: () => Promise.resolve(null), + }), +})) +vi.mock("@/lib/custom-agents", () => ({ + getAgentLabel: (agent: string) => agent, +})) +vi.mock("@/stores/app-workspace-store", () => { + const state = { + folders: [ + { + id: 1, + name: "proj", + alias: null, + parent_id: null, + kind: "regular", + path: "/tmp/proj", + default_agent_type: "claude_code", + }, + ], + } + const useStore = (selector: (s: typeof state) => unknown) => selector(state) + useStore.getState = () => state + return { useAppWorkspaceStore: useStore } +}) + +import { TaskSettingsDialog } from "./task-settings-dialog" + +function settings( + overrides?: Partial +): WorkTaskFolderSettings { + return { + default_agent_type: null, + mode_id: null, + config_values: {}, + auto_process: false, + max_concurrent: 2, + merge_strategy: "squash", + delete_worktree_default: true, + ...overrides, + } +} + +function renderDialog(folderId: number | null) { + return render( + + {}} folderId={folderId} /> + + ) +} + +/** Wait for the initial load — Save is disabled until the settings land. */ +async function saveButton() { + const save = await screen.findByRole("button", { name: "Save" }) + await waitFor(() => expect(save).toBeEnabled()) + return save +} + +async function openPromptsTab(user: ReturnType) { + await user.click(screen.getByRole("tab", { name: "Prompts" })) +} + +beforeEach(() => { + getMock.mockReset().mockResolvedValue(settings()) + getOwnMock.mockReset().mockResolvedValue(settings()) + setMock.mockClear() + deleteMock.mockClear() +}) + +describe("TaskSettingsDialog stage prompts", () => { + it("saves the text typed for one stage under that stage's key", async () => { + const user = userEvent.setup() + renderDialog(1) + const save = await saveButton() + + await openPromptsTab(user) + await user.click(screen.getByRole("tab", { name: "Merge" })) + await user.type( + screen.getByRole("textbox", { name: "Merge" }), + "Write the commit message in Chinese." + ) + await user.click(save) + + await waitFor(() => expect(setMock).toHaveBeenCalled()) + const [, saved] = setMock.mock.calls[0] as [number, WorkTaskFolderSettings] + expect(saved.stage_prompts).toEqual({ + merge: "Write the commit message in Chinese.", + }) + }) + + it("shows saved stage text and marks the stages that carry it", async () => { + getOwnMock.mockResolvedValue( + settings({ + stage_prompts: { all: "Reply in Chinese.", work: "Be brief." }, + }) + ) + const user = userEvent.setup() + renderDialog(1) + await saveButton() + await openPromptsTab(user) + + // "All stages" is selected first, so its text is the one on screen. + expect(screen.getByRole("textbox", { name: "All stages" })).toHaveValue( + "Reply in Chinese." + ) + // Configured stages carry a dot; untouched ones do not. + expect( + screen + .getByRole("tab", { name: "Retry run" }) + .querySelector(".bg-primary") + ).toBeNull() + expect( + screen.getByRole("tab", { name: "Task run" }).querySelector(".bg-primary") + ).not.toBeNull() + + await user.click(screen.getByRole("tab", { name: "Task run" })) + expect(screen.getByRole("textbox", { name: "Task run" })).toHaveValue( + "Be brief." + ) + }) + + it("gives each stage its own example placeholder", async () => { + const user = userEvent.setup() + renderDialog(1) + await saveButton() + await openPromptsTab(user) + + const placeholders: string[] = [] + for (const stage of [ + "All stages", + "Task run", + "Retry run", + "Rework", + "Merge", + ]) { + await user.click(screen.getByRole("tab", { name: stage })) + const box = screen.getByRole("textbox", { name: stage }) + placeholders.push(box.getAttribute("placeholder") ?? "") + } + + expect(placeholders.every((p) => p.length > 0)).toBe(true) + expect(new Set(placeholders).size).toBe(placeholders.length) + }) + + it("drops stages whose text is only whitespace", async () => { + const user = userEvent.setup() + renderDialog(1) + const save = await saveButton() + + await openPromptsTab(user) + await user.type(screen.getByRole("textbox", { name: "All stages" }), " ") + await user.click(save) + + await waitFor(() => expect(setMock).toHaveBeenCalled()) + const [, saved] = setMock.mock.calls[0] as [number, WorkTaskFolderSettings] + expect(saved.stage_prompts).toEqual({}) + }) +}) + +describe("TaskSettingsDialog scope", () => { + it("seeds a folder with no row of its own from the global defaults", async () => { + getOwnMock.mockResolvedValue(null) + getMock.mockResolvedValue( + settings({ max_concurrent: 5, stage_prompts: { all: "From global." } }) + ) + const user = userEvent.setup() + renderDialog(1) + const save = await saveButton() + + // Following the global row: the form stays hidden until it is detached. + expect(screen.queryByRole("tab", { name: "Prompts" })).toBeNull() + await user.click(screen.getByRole("tab", { name: "Custom" })) + await openPromptsTab(user) + expect(screen.getByRole("textbox", { name: "All stages" })).toHaveValue( + "From global." + ) + + await user.click(save) + await waitFor(() => expect(setMock).toHaveBeenCalled()) + const [, saved] = setMock.mock.calls[0] as [number, WorkTaskFolderSettings] + expect(saved.max_concurrent).toBe(5) + expect(saved.stage_prompts).toEqual({ all: "From global." }) + expect(deleteMock).not.toHaveBeenCalled() + }) + + it("drops the folder's own row when it is set back to the global defaults", async () => { + const user = userEvent.setup() + renderDialog(1) + const save = await saveButton() + + await user.click(screen.getByRole("tab", { name: "Global defaults" })) + await user.click(save) + + await waitFor(() => expect(deleteMock).toHaveBeenCalledWith(1)) + expect(setMock).not.toHaveBeenCalled() + }) +}) diff --git a/src/components/tasks/task-settings-dialog.tsx b/src/components/tasks/task-settings-dialog.tsx index 6be4c3969..5d09a8880 100644 --- a/src/components/tasks/task-settings-dialog.tsx +++ b/src/components/tasks/task-settings-dialog.tsx @@ -3,6 +3,17 @@ import { useEffect, useMemo, useState } from "react" import { useTranslations } from "next-intl" import { toast } from "sonner" +import { + Bot, + Gauge, + GitMerge, + Info, + MessageSquarePlus, + PackagePlus, + ShieldCheck, + Trash2, + Zap, +} from "lucide-react" import { useAppWorkspaceStore } from "@/stores/app-workspace-store" import { AgentSelector } from "@/components/chat/agent-selector" import { @@ -20,8 +31,13 @@ import { workTaskSettingsSet, } from "@/lib/api" import { toErrorMessage } from "@/lib/app-error" +import { cn } from "@/lib/utils" +import { + SettingCard, + SettingNote, + SettingRow, +} from "@/components/tasks/setting-card" import { Button } from "@/components/ui/button" -import { Checkbox } from "@/components/ui/checkbox" import { Dialog, DialogContent, @@ -31,7 +47,6 @@ import { DialogTitle, } from "@/components/ui/dialog" import { Input } from "@/components/ui/input" -import { Label } from "@/components/ui/label" import { Switch } from "@/components/ui/switch" import { Select, @@ -40,12 +55,74 @@ import { SelectTrigger, SelectValue, } from "@/components/ui/select" -import { Tabs, TabsList, TabsTrigger } from "@/components/ui/tabs" +import { Tabs, TabsContent, TabsList, TabsTrigger } from "@/components/ui/tabs" +import { Textarea } from "@/components/ui/textarea" import type { AgentType, WorkTaskFolderSettings } from "@/lib/types" /** Sentinel folder id of the global-defaults settings row (backend contract). */ const GLOBAL_SCOPE = 0 +/** + * The launch stages a task flows through, in the order they can happen. The + * keys are the engine's own stage ids (the `round` event's `kind`), so the + * labels here are literally the ones the transcript shows above each round — + * `all` is the extra bucket appended to every stage. + */ +const PROMPT_STAGES = [ + { + key: "all", + labelKey: "settingsPromptStageAll", + hintKey: "settingsPromptHintAll", + placeholderKey: "settingsPromptPlaceholderAll", + }, + { + key: "work", + labelKey: "phaseWork", + hintKey: "settingsPromptHintWork", + placeholderKey: "settingsPromptPlaceholderWork", + }, + { + key: "retry", + labelKey: "phaseRetry", + hintKey: "settingsPromptHintRetry", + placeholderKey: "settingsPromptPlaceholderRetry", + }, + { + key: "return", + labelKey: "phaseReturn", + hintKey: "settingsPromptHintReturn", + placeholderKey: "settingsPromptPlaceholderReturn", + }, + { + key: "merge", + labelKey: "phaseMerge", + hintKey: "settingsPromptHintMerge", + placeholderKey: "settingsPromptPlaceholderMerge", + }, +] as const + +/** + * The two ways a finished task can land. Rendered as side-by-side option cards + * rather than a dropdown so both trade-offs are readable at once — the choice + * is made rarely and the wording ("tidier history" vs "every step traceable") + * is the whole decision. + */ +const MERGE_STRATEGIES = [ + { + value: "squash", + labelKey: "strategySquash", + hintKey: "strategySquashHint", + }, + { value: "merge", labelKey: "strategyMerge", hintKey: "strategyMergeHint" }, +] as const + +/** + * Shared tab body. The floor is the natural height of the Prompts tab, so it + * and General settle at exactly the same height with no dead space under their + * last card; only Workflow, which is genuinely taller, grows the dialog. + */ +const TAB_BODY_CLASS = "mt-1 flex min-h-[19.5rem] flex-col gap-3" + interface TaskSettingsDialogProps { open: boolean onOpenChange: (open: boolean) => void @@ -54,11 +131,18 @@ interface TaskSettingsDialogProps { } /** - * Task defaults, per folder or global: the default processing agent + its - * ACP-probed mode/model options, auto-process, max concurrency, merge/cleanup - * defaults, worktree init command and the preflight command. The scope - * selector at the top switches which settings row is being edited; the global - * row (folder id 0) applies wholesale to folders that never saved their own. + * Task defaults, per folder or global, grouped into three tabs: General (the + * processing agent + its ACP-probed mode/model options, auto-process, max + * concurrency), Workflow (how a task lands, plus the worktree init and + * preflight commands) and Prompts (per-stage instructions appended to what the + * engine sends the agent). The scope selector above the tabs switches which + * settings row is being edited; the global row (folder id 0) applies wholesale + * to folders that never saved their own. + * + * Every option is rendered as a `SettingRow` inside a `SettingCard`, and the + * cards are the grouping: options that are one decision (merge strategy and + * what happens to the worktree afterwards; auto-process and its concurrency + * limit) share a card so they read together instead of as a flat list. */ export function TaskSettingsDialog({ open, @@ -67,7 +151,7 @@ export function TaskSettingsDialog({ }: TaskSettingsDialogProps) { return ( - + {open ? ( >({}) + const [stage, setStage] = useState(PROMPT_STAGES[0].key) + const [tab, setTab] = useState("general") const [saving, setSaving] = useState(false) + const activeStage = + PROMPT_STAGES.find((s) => s.key === stage) ?? PROMPT_STAGES[0] useEffect(() => { let cancelled = false @@ -170,6 +259,7 @@ function TaskSettingsBody({ setMergeStrategy(s.merge_strategy === "merge" ? "merge" : "squash") setDeleteWorktreeDefault(s.delete_worktree_default) setInitCommand(s.init_command ?? "") + setStagePrompts(s.stage_prompts ?? {}) const legacy = s.preflight_command_id != null ? (commands.find((c) => c.id === s.preflight_command_id)?.command ?? @@ -229,6 +319,13 @@ function TaskSettingsBody({ preflight_command_id: null, preflight_command: preflightCommand.trim() || null, init_command: initCommand.trim() || null, + // Blank stages are dropped rather than stored as "" — the engine + // trims anyway, and an empty entry would only add noise to the blob. + stage_prompts: Object.fromEntries( + Object.entries(stagePrompts) + .map(([key, text]) => [key, text.trim()] as const) + .filter(([, text]) => text.length > 0) + ), } await workTaskSettingsSet(folderId, settings) onClose() @@ -250,200 +347,322 @@ function TaskSettingsBody({ -
-
- - -
- - {/* Folder scope: which source is in effect — following the global - defaults, or this folder's own row. Seeded from the DB truth. */} - {!isGlobal ? ( -
-
- - - setSource(v === "global" ? "global" : "custom") - } - > - - - {t("settingsSourceGlobal")} - - - {t("settingsSourceCustom")} - - - -
- - {source === "global" - ? t("settingsSourceGlobalFollow") - : t("settingsSourceCustomHint")} +
+ {/* Which settings row is on screen — chrome, not a setting. Left + deliberately without a card so it reads as part of the dialog + header: muted mini-labels against the `text-sm` titles the cards + below use, so the eye separates "what am I editing" from "what am + I changing" without a box around either. */} +
+
+ + {t("settingsScope")} +
- ) : null} - - {editing ? ( - <> -
- -
- { - setAgentType(a) - setModeId(null) - setConfigValues({}) - }} - onFallback={setAgentType} - /> -
- - setConfigValues((prev) => { - const next = { ...prev } - if (valueId === null) delete next[optionId] - else next[optionId] = valueId - return next - }) - } - /> -
- -
-
- - - {t("settingsAutoProcessHint")} - -
- -
- -
-
- - - {t("settingsMaxConcurrentHint")} - -
- - setMaxConcurrent(e.target.value.replace(/[^0-9]/g, "")) - } - className="w-20 text-right" - /> -
- {/* Plain-language options (no git jargon); the hint under the - row explains whichever one is selected — same pattern as the - settings-source switcher above. */} -
+ {/* Folder scope: which source is in effect — following the global + defaults, or this folder's own row. Seeded from the DB truth. */} + {!isGlobal ? ( + <>
- - + + + {t("settingsSourceGlobal")} + + + {t("settingsSourceCustom")} + + +
- - {mergeStrategy === "squash" - ? t("strategySquashHint") - : t("strategyMergeHint")} + + {source === "global" + ? t("settingsSourceGlobalFollow") + : t("settingsSourceCustomHint")} -
+ + ) : null} +
-
- - - {t("settingsInitCommandHint")} - - setInitCommand(e.target.value)} - placeholder={t("settingsInitCommandPlaceholder")} - className="font-mono text-xs" - /> -
+ {editing ? ( + + + + {t("settingsTabGeneral")} + + + {t("settingsTabWorkflow")} + + + {t("settingsTabPrompts")} + + -
- - - {t("settingsPreflightHint")} - - setPreflightCommand(e.target.value)} - placeholder={t("settingsPreflightCustomPlaceholder")} - className="font-mono text-xs" - /> -
+ + + +
+
+ { + setAgentType(a) + setModeId(null) + setConfigValues({}) + }} + onFallback={setAgentType} + /> +
+ + setConfigValues((prev) => { + const next = { ...prev } + if (valueId === null) delete next[optionId] + else next[optionId] = valueId + return next + }) + } + /> +
+ + + + {/* The switch that starts work and the valve that limits it — + one card, because reading either alone tells you half. */} + + + } + /> + + setMaxConcurrent(e.target.value.replace(/[^0-9]/g, "")) + } + className="h-8 w-16 bg-background text-center" + /> + } + /> + + + + + {/* Everything about landing a task lives in one card: how the + commits are recorded, and what happens to the worktree right + after. Plain-language options, no git jargon. */} + + +
+ {MERGE_STRATEGIES.map((opt) => { + const active = mergeStrategy === opt.value + return ( + + ) + })} +
+ + + } + /> + - - + {/* The two shell hooks around a task's life, in run order. */} + + + setInitCommand(e.target.value)} + placeholder={t("settingsInitCommandPlaceholder")} + className="h-8 bg-background font-mono text-xs" + /> + + + setPreflightCommand(e.target.value)} + placeholder={t("settingsPreflightCustomPlaceholder")} + className="h-8 bg-background font-mono text-xs" + /> + + + + + {/* Free-form text appended after the built-in instructions of one + launch stage. The stage strip doubles as the field's label — + a dot marks the stages that already carry text, so nothing + stays hidden behind an unselected segment. */} + + {/* What this tab is: the engine composes a fixed prompt per stage + and appends this text under "Additional instructions" — see + compose_prompt in work_task/engine.rs. Saying so up front is + what stops people from re-stating the built-ins here. */} + {t("settingsPromptsIntro")} + + + {PROMPT_STAGES.map((s) => ( + + {t(s.labelKey)} + {stagePrompts[s.key]?.trim() ? ( + + ) : null} + + ))} + + + {/* The card restates the selected stage, so the text you type is + always framed by when it will be sent — and each stage carries + its own example, since what belongs in "merge" is nothing like + what belongs in "rework". */} + + +