-
Notifications
You must be signed in to change notification settings - Fork 125
feat: support todo workspace #1989
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -15,7 +15,7 @@ DEFAULT_VERSION="1.0.0" | |||||
|
|
||||||
| # Function to show help | ||||||
| show_help() { | ||||||
| echo "Build docker images for Wegent components" | ||||||
| echo "Build docker images for Wegent components"aass | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Remove the unintended The current command prints Proposed fix- echo "Build docker images for Wegent components"aass
+ echo "Build docker images for Wegent components"📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| echo "" | ||||||
| echo "Usage: $0 [OPTIONS]" | ||||||
| echo "" | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,312 @@ | ||
| use std::path::{Component, Path, PathBuf}; | ||
|
|
||
| use serde::Serialize; | ||
| use tauri::Manager; | ||
|
|
||
| #[derive(Serialize)] | ||
| #[serde(rename_all = "camelCase")] | ||
| pub struct TodoWorkspaceEntry { | ||
| path: String, | ||
| name: String, | ||
| node_type: &'static str, | ||
| size: u64, | ||
| modified_at_ms: u128, | ||
| absolute_path: String, | ||
| } | ||
|
|
||
| fn store_root(app: &tauri::AppHandle) -> Result<PathBuf, String> { | ||
| app.path() | ||
| .app_data_dir() | ||
| .map(|path| path.join("todo")) | ||
| .map_err(|error| format!("Failed to resolve app data directory: {error}")) | ||
| } | ||
|
|
||
| fn safe_key(value: &str) -> Result<&str, String> { | ||
| if value.is_empty() | ||
| || !value | ||
| .chars() | ||
| .all(|character| character.is_ascii_alphanumeric() || matches!(character, '-' | '_')) | ||
| { | ||
| return Err("Invalid TODO storage key".to_string()); | ||
| } | ||
| Ok(value) | ||
| } | ||
|
|
||
| fn safe_relative_path(value: &str) -> Result<PathBuf, String> { | ||
| let path = Path::new(value); | ||
| if path.as_os_str().is_empty() || path.is_absolute() { | ||
| return Err("Workspace path must be relative".to_string()); | ||
| } | ||
| if path.components().any(|component| { | ||
| matches!( | ||
| component, | ||
| Component::ParentDir | Component::RootDir | Component::Prefix(_) | ||
| ) | ||
| }) { | ||
| return Err("Workspace path escapes the TODO directory".to_string()); | ||
| } | ||
| Ok(path.to_path_buf()) | ||
|
Comment on lines
+35
to
+48
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win Normalize paths before enforcing protected entries.
Proposed fix fn safe_relative_path(value: &str) -> Result<PathBuf, String> {
let path = Path::new(value);
if path.as_os_str().is_empty() || path.is_absolute() {
return Err("Workspace path must be relative".to_string());
}
if path.components().any(|component| {
matches!(
component,
- Component::ParentDir | Component::RootDir | Component::Prefix(_)
+ Component::CurDir
+ | Component::ParentDir
+ | Component::RootDir
+ | Component::Prefix(_)
)
}) {
- return Err("Workspace path escapes the TODO directory".to_string());
+ return Err("Workspace path must be normalized".to_string());
}
Ok(path.to_path_buf())
}Also add regression cases for Also applies to: 154-159, 190-201 🤖 Prompt for AI Agents |
||
| } | ||
|
Comment on lines
+35
to
+49
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win Reject normalized aliases of protected paths.
Proposed fix if path.components().any(|component| {
matches!(
component,
- Component::ParentDir | Component::RootDir | Component::Prefix(_)
+ Component::CurDir
+ | Component::ParentDir
+ | Component::RootDir
+ | Component::Prefix(_)
)
}) { fn is_protected_workspace_path(path: &Path) -> bool {
- matches!(
- path.to_string_lossy().as_ref(),
- "README.md" | "context" | "work"
- )
+ let mut components = path.components()
+ matches!(
+ (components.next(), components.next()),
+ (Some(Component::Normal(name)), None)
+ if matches!(name.to_str(), Some("README.md" | "context" | "work"))
+ )
}Also applies to: 154-209, 282-311 🤖 Prompt for AI Agents |
||
|
|
||
| fn workspace_root(app: &tauri::AppHandle, item_id: &str) -> Result<PathBuf, String> { | ||
| Ok(store_root(app)?.join("workspaces").join(safe_key(item_id)?)) | ||
| } | ||
|
|
||
| fn reject_symlink_components(root: &Path, relative: &Path) -> Result<(), String> { | ||
| let mut current = root.to_path_buf(); | ||
| for component in relative.components() { | ||
| current.push(component.as_os_str()); | ||
| match std::fs::symlink_metadata(¤t) { | ||
| Ok(metadata) if metadata.file_type().is_symlink() => { | ||
| return Err("Workspace paths cannot traverse symbolic links".to_string()); | ||
| } | ||
| Ok(_) => {} | ||
| Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} | ||
| Err(error) => return Err(format!("Failed to inspect workspace path: {error}")), | ||
| } | ||
| } | ||
| Ok(()) | ||
| } | ||
|
|
||
| #[tauri::command] | ||
| pub fn load_todo_store(app: tauri::AppHandle, scope: String) -> Result<Option<String>, String> { | ||
| let path = store_root(&app)?.join(format!("{}.json", safe_key(&scope)?)); | ||
| if !path.exists() { | ||
| return Ok(None); | ||
| } | ||
| std::fs::read_to_string(path) | ||
| .map(Some) | ||
| .map_err(|error| format!("Failed to read TODO store: {error}")) | ||
| } | ||
|
|
||
| #[tauri::command] | ||
| pub fn save_todo_store( | ||
| app: tauri::AppHandle, | ||
| scope: String, | ||
| contents: String, | ||
| ) -> Result<(), String> { | ||
| serde_json::from_str::<serde_json::Value>(&contents) | ||
| .map_err(|error| format!("TODO store must contain valid JSON: {error}"))?; | ||
| let root = store_root(&app)?; | ||
| std::fs::create_dir_all(&root) | ||
| .map_err(|error| format!("Failed to create TODO store directory: {error}"))?; | ||
| let target = root.join(format!("{}.json", safe_key(&scope)?)); | ||
| let temporary = target.with_extension("json.tmp"); | ||
| std::fs::write(&temporary, contents) | ||
| .map_err(|error| format!("Failed to write TODO store: {error}"))?; | ||
| std::fs::rename(temporary, target) | ||
| .map_err(|error| format!("Failed to commit TODO store: {error}")) | ||
|
Comment on lines
+83
to
+98
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Inspect target-platform configuration and all persistence call sites.
fd -i 'Cargo.toml|Cargo.lock|rust-toolchain.toml|*.yml|*.yaml' \
--exec rg -n -C2 'windows|macos|linux|target_os|std::fs::rename' {}
rg -n -C3 --type=ts --type=tsx '\bsaveLocalWorkItems\s*\(' .
rg -n -C3 --type=rust '\bsave_todo_store\b|std::fs::rename\s*\(' .Repository: wecode-ai/Wegent Length of output: 2646 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- todo_store.rs (relevant section) ---'
sed -n '70,110p' wework/src-tauri/src/todo_store.rs
echo
echo '--- todoModel.ts (relevant section) ---'
sed -n '200,245p' wework/src/features/todo/todoModel.ts
echo
echo '--- search for save_todo_store call handling ---'
rg -n -C2 "invoke\\('save_todo_store'|save_todo_store\\(|saveLocalWorkItems\\(" wework/src
echo
echo '--- search for other rename uses in todo_store.rs ---'
rg -n -C2 'std::fs::rename|with_extension\("json.tmp"\)|create_dir_all' wework/src-tauri/src/todo_store.rsRepository: wecode-ai/Wegent Length of output: 6569 🌐 Web query:
💡 Result: On Windows, Rust's std::fs::rename will overwrite an existing destination file [1][2]. The implementation details of this operation have evolved to provide safer and more POSIX-like behavior [3][4]: 1. Core Semantics: By default, std::fs::rename replaces the destination file if it already exists [1][2]. If the destination is an existing directory, the operation will generally fail [1][2]. 2. Underlying APIs: Rust does not rely on a single Windows API call but instead attempts a series of strategies depending on OS and filesystem support [3][5]: - Modern POSIX Semantics: On newer Windows versions (Windows 10 1709+), Rust attempts to use SetFileInformationByHandle with the FILE_RENAME_FLAG_POSIX_SEMANTICS and FILE_RENAME_FLAG_REPLACE_IF_EXISTS flags [4][6]. This allows for atomic renames even if the destination file is currently open (provided it was opened with appropriate sharing permissions) [3][4]. - Legacy Fallback: If the advanced POSIX-style rename is not supported (e.g., due to filesystem limitations or older OS versions), Rust falls back to standard Windows APIs like MoveFileExW (using the MOVEFILE_REPLACE_EXISTING flag) [1][3][7][5]. 3. Evolution: Recent changes in the Rust standard library (post-2024) have improved reliability by attempting non-atomic renames first in certain environments (like some Windows Server configurations) and falling back to atomic ones only if necessary, ensuring better compatibility across diverse Windows storage environments [5]. In summary, you can rely on std::fs::rename to overwrite an existing destination file on Windows [1][2], with the library automatically handling the underlying complexity of choosing the best available Windows API to perform the rename safely [3][5]. Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- todoModel.ts saveLocalWorkItems and nearby callers ---'
sed -n '200,320p' wework/src/features/todo/todoModel.ts
echo
echo '--- direct references to saveLocalWorkItems within wework/src ---'
rg -n -C3 '\bsaveLocalWorkItems\s*\(' wework/src --glob '*.ts' --glob '*.tsx'
echo
echo '--- direct references to save_todo_store within repo ---'
rg -n -C3 '\bsave_todo_store\b' wework/src-tauri wework/src --glob '*.rs' --glob '*.ts' --glob '*.tsx'Repository: wecode-ai/Wegent Length of output: 6420 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- todoModel.ts around load/save flow ---'
sed -n '170,240p' wework/src/features/todo/todoModel.ts
echo
echo '--- todo_store.rs load path ---'
sed -n '1,70p' wework/src-tauri/src/todo_store.rsRepository: wecode-ai/Wegent Length of output: 4670 Serialize TODO persistence writes per scope.
Queue or await the save path so only the latest snapshot reaches disk. 📍 Affects 2 files
🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| #[tauri::command] | ||
| pub fn ensure_todo_workspace( | ||
| app: tauri::AppHandle, | ||
| item_id: String, | ||
| title: String, | ||
| objective: String, | ||
| ) -> Result<String, String> { | ||
| let root = workspace_root(&app, &item_id)?; | ||
| std::fs::create_dir_all(root.join("context")) | ||
| .and_then(|_| std::fs::create_dir_all(root.join("work"))) | ||
| .map_err(|error| format!("Failed to initialize TODO workspace: {error}"))?; | ||
| let readme = root.join("README.md"); | ||
| if !readme.exists() { | ||
| std::fs::write(&readme, format!("# {title}\n\n{objective}\n")) | ||
| .map_err(|error| format!("Failed to create TODO README: {error}"))?; | ||
| } | ||
| Ok(root.to_string_lossy().into_owned()) | ||
| } | ||
|
|
||
| #[tauri::command] | ||
| pub fn ensure_todo_work_directory( | ||
| app: tauri::AppHandle, | ||
| item_id: String, | ||
| work_type: String, | ||
| ) -> Result<String, String> { | ||
| let directory = workspace_root(&app, &item_id)? | ||
| .join("work") | ||
| .join(safe_key(&work_type)?); | ||
| std::fs::create_dir_all(&directory) | ||
| .map_err(|error| format!("Failed to create work directory: {error}"))?; | ||
| Ok(directory.to_string_lossy().into_owned()) | ||
| } | ||
|
|
||
| #[tauri::command] | ||
| pub fn write_todo_workspace_file( | ||
| app: tauri::AppHandle, | ||
| item_id: String, | ||
| relative_path: String, | ||
| bytes: Vec<u8>, | ||
| ) -> Result<String, String> { | ||
| let root = workspace_root(&app, &item_id)?; | ||
| let relative = safe_relative_path(&relative_path)?; | ||
| reject_symlink_components(&root, &relative)?; | ||
| let target = root.join(relative); | ||
| if let Some(parent) = target.parent() { | ||
| std::fs::create_dir_all(parent) | ||
| .map_err(|error| format!("Failed to create workspace directory: {error}"))?; | ||
| } | ||
| std::fs::write(&target, bytes) | ||
| .map_err(|error| format!("Failed to write workspace file: {error}"))?; | ||
| Ok(target.to_string_lossy().into_owned()) | ||
| } | ||
|
|
||
| fn is_protected_workspace_path(path: &Path) -> bool { | ||
| matches!( | ||
| path.to_string_lossy().as_ref(), | ||
| "README.md" | "context" | "work" | ||
| ) | ||
| } | ||
|
|
||
| #[tauri::command] | ||
| pub fn rename_todo_workspace_entry( | ||
| app: tauri::AppHandle, | ||
| item_id: String, | ||
| from_path: String, | ||
| to_path: String, | ||
| ) -> Result<(), String> { | ||
| let root = workspace_root(&app, &item_id)?; | ||
| let from = safe_relative_path(&from_path)?; | ||
| let to = safe_relative_path(&to_path)?; | ||
| reject_symlink_components(&root, &from)?; | ||
| reject_symlink_components(&root, &to)?; | ||
| if is_protected_workspace_path(&from) || is_protected_workspace_path(&to) { | ||
| return Err("Core TODO workspace entries cannot be renamed".to_string()); | ||
| } | ||
| let source = root.join(from); | ||
| let target = root.join(to); | ||
| if target.exists() { | ||
| return Err("Workspace destination already exists".to_string()); | ||
| } | ||
| if let Some(parent) = target.parent() { | ||
| std::fs::create_dir_all(parent) | ||
| .map_err(|error| format!("Failed to create workspace directory: {error}"))?; | ||
| } | ||
| std::fs::rename(source, target) | ||
| .map_err(|error| format!("Failed to rename workspace entry: {error}")) | ||
| } | ||
|
|
||
| #[tauri::command] | ||
| pub fn delete_todo_workspace_entry( | ||
| app: tauri::AppHandle, | ||
| item_id: String, | ||
| relative_path: String, | ||
| ) -> Result<(), String> { | ||
| let root = workspace_root(&app, &item_id)?; | ||
| let relative = safe_relative_path(&relative_path)?; | ||
| reject_symlink_components(&root, &relative)?; | ||
| if is_protected_workspace_path(&relative) { | ||
| return Err("Core TODO workspace entries cannot be deleted".to_string()); | ||
| } | ||
| let target = root.join(relative); | ||
| if target.is_dir() { | ||
| std::fs::remove_dir_all(target) | ||
| .map_err(|error| format!("Failed to delete workspace directory: {error}")) | ||
| } else { | ||
| std::fs::remove_file(target) | ||
| .map_err(|error| format!("Failed to delete workspace file: {error}")) | ||
| } | ||
| } | ||
|
|
||
| #[tauri::command] | ||
| pub fn get_todo_workspace_path(app: tauri::AppHandle, item_id: String) -> Result<String, String> { | ||
| let root = workspace_root(&app, &item_id)?; | ||
| if !root.exists() { | ||
| return Err("TODO workspace does not exist".to_string()); | ||
| } | ||
| Ok(root.to_string_lossy().into_owned()) | ||
| } | ||
|
|
||
| fn collect_entries( | ||
| root: &Path, | ||
| directory: &Path, | ||
| entries: &mut Vec<TodoWorkspaceEntry>, | ||
| ) -> Result<(), String> { | ||
| for result in std::fs::read_dir(directory) | ||
| .map_err(|error| format!("Failed to read TODO workspace: {error}"))? | ||
| { | ||
| let entry = result.map_err(|error| format!("Failed to read workspace entry: {error}"))?; | ||
| let path = entry.path(); | ||
| let metadata = entry | ||
| .metadata() | ||
| .map_err(|error| format!("Failed to inspect workspace entry: {error}"))?; | ||
| let file_type = entry | ||
| .file_type() | ||
| .map_err(|error| format!("Failed to inspect workspace entry type: {error}"))?; | ||
| let relative = path | ||
| .strip_prefix(root) | ||
| .map_err(|error| format!("Failed to resolve workspace entry: {error}"))?; | ||
| entries.push(TodoWorkspaceEntry { | ||
| path: relative.to_string_lossy().replace('\\', "/"), | ||
| name: entry.file_name().to_string_lossy().into_owned(), | ||
| node_type: if file_type.is_dir() { | ||
| "directory" | ||
| } else { | ||
| "file" | ||
| }, | ||
| size: metadata.len(), | ||
| modified_at_ms: metadata | ||
| .modified() | ||
| .ok() | ||
| .and_then(|value| value.duration_since(std::time::UNIX_EPOCH).ok()) | ||
| .map_or(0, |value| value.as_millis()), | ||
| absolute_path: path.to_string_lossy().into_owned(), | ||
| }); | ||
| if file_type.is_dir() && !file_type.is_symlink() { | ||
| collect_entries(root, &path, entries)?; | ||
| } | ||
| } | ||
| Ok(()) | ||
| } | ||
|
|
||
| #[tauri::command] | ||
| pub fn list_todo_workspace( | ||
| app: tauri::AppHandle, | ||
| item_id: String, | ||
| ) -> Result<Vec<TodoWorkspaceEntry>, String> { | ||
| let root = workspace_root(&app, &item_id)?; | ||
| if !root.exists() { | ||
| return Ok(Vec::new()); | ||
| } | ||
| let mut entries = Vec::new(); | ||
| collect_entries(&root, &root, &mut entries)?; | ||
| entries.sort_by(|left, right| left.path.cmp(&right.path)); | ||
| Ok(entries) | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::{is_protected_workspace_path, reject_symlink_components, safe_relative_path}; | ||
| use std::path::Path; | ||
|
|
||
| #[test] | ||
| fn workspace_paths_cannot_escape() { | ||
| assert!(safe_relative_path("context/brief.md").is_ok()); | ||
| assert!(safe_relative_path("../secret").is_err()); | ||
| assert!(safe_relative_path("/tmp/secret").is_err()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn core_workspace_entries_are_protected() { | ||
| assert!(is_protected_workspace_path(Path::new("README.md"))); | ||
| assert!(is_protected_workspace_path(Path::new("context"))); | ||
| assert!(!is_protected_workspace_path(Path::new("context/brief.md"))); | ||
| } | ||
|
|
||
| #[cfg(unix)] | ||
| #[test] | ||
| fn workspace_paths_cannot_traverse_symbolic_links() { | ||
| let root = std::env::temp_dir().join(format!("wework-todo-{}", std::process::id())); | ||
| let outside = root.with_extension("outside"); | ||
| let _ = std::fs::remove_dir_all(&root); | ||
| let _ = std::fs::remove_dir_all(&outside); | ||
| std::fs::create_dir_all(&root).unwrap(); | ||
| std::fs::create_dir_all(&outside).unwrap(); | ||
| std::os::unix::fs::symlink(&outside, root.join("linked")).unwrap(); | ||
|
|
||
| assert!(reject_symlink_components(&root, Path::new("linked/secret.txt")).is_err()); | ||
|
|
||
| std::fs::remove_dir_all(&root).unwrap(); | ||
| std::fs::remove_dir_all(&outside).unwrap(); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Remove the unintended
aasssuffix.The command remains syntactically valid, but
--helpprintsBuild docker images for Wegent componentsaassinstead of the expected header.Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents