feat: support todo workspace - #1989
Conversation
📝 WalkthroughWalkthroughThis PR introduces a unified TODO work-item and workflow model with local and Tauri-backed persistence, adds a Rust-based todo workspace filesystem store with security guards, reworks the TODO UI (board, detail panel, my-work, workflow dialog), and wires a ChangesTodo Workflow and Workspace
Estimated code review effort: 4 (Complex) | ~75 minutes Build Help-Text Correction
Sequence Diagram(s)sequenceDiagram
participant TodoWorkspace
participant todoModel
participant localStorage
participant TauriTodoStore
TodoWorkspace->>todoModel: hydrateLocalWorkItems(userId)
todoModel->>TauriTodoStore: load_todo_store(scope)
TauriTodoStore-->>todoModel: serialized work items
todoModel->>localStorage: cache normalized work items
TodoWorkspace->>todoModel: saveLocalWorkItems(userId, items)
todoModel->>localStorage: persist work items
todoModel->>TauriTodoStore: save_todo_store(scope, contents)
sequenceDiagram
participant TodoWorkspace
participant TodoDetailPanel
participant createProjectRuntimeTask
participant sendPreparedRuntimeMessage
TodoWorkspace->>TodoDetailPanel: run local work item
TodoDetailPanel->>TodoWorkspace: onRunTodo(request)
TodoWorkspace->>createProjectRuntimeTask: create task with collaborationMode
createProjectRuntimeTask->>sendPreparedRuntimeMessage: forward collaborationMode
sendPreparedRuntimeMessage-->>TodoWorkspace: runtime task creation result
TodoWorkspace->>TodoDetailPanel: update state and events
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wework/src/features/todo/TodoWorkItems.tsx (1)
86-103: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHandle global creation independently of the visible board columns.
createRequestis ignored in list layout, and the board can omitinboxwhen filters orshowEmptyGroupshide it. In either case, the header/sidebar Create action sets a request but no input opens. Forward the request toTodoListand ensure the requested board state is rendered.Also applies to: 399-411
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/features/todo/TodoWorkItems.tsx` around lines 86 - 103, The global create request is only handled by the board when its target column is visible. Forward createRequest to TodoList, and update TodoBoard’s column-selection/rendering logic to ensure the requested state column is included even when filters or showEmptyGroups would otherwise hide it, so header/sidebar Create opens the appropriate input in both layouts.
🧹 Nitpick comments (1)
wework/src/features/todo/TodoWorkspace.tsx (1)
234-242: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSplit this orchestration component before it grows further.
TodoWorkspacenow runs from Line 234 through Line 1027 and owns persistence, workflow generation, execution, filtering, and the full view. Extract focused hooks/components for work-item storage, workflow actions, and header/content rendering.As per coding guidelines, keep functions focused and split source files over 1000 lines.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/features/todo/TodoWorkspace.tsx` around lines 234 - 242, The TodoWorkspace component spans over 1000 lines and combines persistence logic, workflow generation, execution, filtering, and view rendering. Extract custom hooks to isolate work-item storage state and persistence, workflow action handlers (generate, execute, filter), and separate the header and content rendering into focused sub-components. Pass these extracted concerns to TodoWorkspace as props or through composition to keep the main component under the 1000-line guideline while maintaining focused responsibilities for each extracted piece.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@build_image.sh`:
- Line 18: Remove the unintended “aass” suffix from the echo command in
build_image.sh so the output header is exactly “Build docker images for Wegent
components”.
In `@wework/src-tauri/src/todo_store.rs`:
- Around line 35-48: Normalize the path in safe_relative_path before
protected-entry checks so equivalent forms such as context/., work/., and
./context resolve to their canonical relative paths. Update
is_protected_workspace_path and the rename/delete flows to use this normalized
value, preserving rejection of parent, root, and prefix components. Add
regression coverage for all three bypass forms.
- Around line 83-98: Serialize TODO persistence writes per scope: update
saveLocalWorkItems in wework/src/features/todo/todoModel.ts:221-228 to await or
queue save_todo_store calls so rapid edits commit in order and only the latest
snapshot reaches disk; update save_todo_store in
wework/src-tauri/src/todo_store.rs:83-98 to prevent overlapping saves for the
same scope from sharing and stomping the temporary file, using per-scope
serialization while preserving atomic rename behavior.
In `@wework/src/features/todo/TodoDetailPanel.tsx`:
- Around line 214-218: Update the workspace open/reveal handlers, including
revealWorkspace and the corresponding flow around lines 632-641, to catch
rejected openLocalFile and revealLocalFile promises and assign the caught error
to workspaceError. Preserve the existing path and workspace-item checks while
ensuring both failure paths provide user feedback through that error state.
- Around line 427-444: The TodoDetailPanel controls use undersized desktop
dimensions. Update the standard inputs around the blocker and next-action
fields, plus the referenced control ranges, from h-7 to h-8; update icon-only
controls from h-6 w-6 to h-8 w-8 while preserving their existing behavior and
styling.
- Around line 421-745: The TodoDetailPanel component has grown too large and
contains several unrelated UI sections. Extract the workflow section around
item.kind === 'draft' into a focused component, the shared workspace section
around workspaceEntries into another, and the Properties section into a third
component, passing only the required item data, callbacks, state, and
translation helpers; update TodoDetailPanel to render these components while
preserving current behavior.
- Around line 160-165: Update the workspace-loading useEffect around
listTodoWorkspace so previous requests cannot overwrite results for a newer item
or update. Clear workspace entries when item.workspaceItemId changes, and cancel
or sequence each request so only the latest request may call
setWorkspaceEntries; preserve the empty-list fallback for the active request’s
failure.
- Around line 427-442: Update the TodoDetailPanel inputs identified by
todo-detail-blocker-input and todo-detail-next-action-input so their DOM state
resets when item.id changes, either by keying the detail panel or these inputs
with item.id. Preserve the existing trimmed onBlur updates while ensuring
switching items cannot retain or submit the previous item’s values.
In `@wework/src/features/todo/todoModel.ts`:
- Around line 208-218: Update hydrateLocalWorkItems to fall back to
loadLocalWorkItems only when load_todo_store returns null; remove the
catch-based fallback so rejected reads and malformed JSON propagate. Also update
the listing logic at wework/src/features/todo/todoModel.ts lines 256-263 to
propagate listing errors rather than treating them as an empty workspace.
- Around line 73-129: Replace the hardcoded Chinese status and workflow template
names in DEFAULT_TODO_WORKFLOW and TODO_WORKFLOW_TEMPLATES with stable
translation keys, then resolve them through useTranslation wherever these
configs are displayed. Add matching entries for every status and template key
under the appropriate Wework namespaces in both en and zh-CN locale trees,
preserving the current Chinese text in zh-CN and providing English text in en.
- Around line 265-279: The workspace upload flow must prevent unbounded memory
use. In wework/src/features/todo/todoModel.ts lines 265-279, update
writeTodoWorkspaceFile to enforce the documented size limit before calling
file.arrayBuffer(), or pass a file path to a streaming backend command. In
wework/src-tauri/src/todo_store.rs lines 135-151, enforce the same limit
server-side and replace whole-file collection and synchronous writing with
buffered or streamed I/O for supported large files.
In `@wework/src/features/todo/TodoMyWork.tsx`:
- Around line 5-7: Update TodoMyWork and the TodoDetailItem model to carry
stable signed-in user, assignee, and confirmer IDs, then filter “My work” groups
by matching those identities instead of only assigneeType === 'human'. Ensure
review items are included only when the signed-in user matches the confirmer.
In `@wework/src/features/todo/TodoWorkflowDialog.tsx`:
- Around line 108-116: Apply the responsive control-size contract to the close
button in TodoWorkflowDialog.tsx (108-116) and the quick-create and related
column/list actions in TodoWorkItems.tsx (586-596): use at least 44px touch
targets on mobile while preserving h-8 w-8 for desktop icon-only controls.
- Around line 50-63: The TodoWorkflowDialog dependency graph must reject cycles
before accepting changes or saving. Update toggleDependency and the
save/runDraft validation path to detect whether the proposed workTypes
dependencies contain any cycle, prevent the toggle or save when cyclic, and
preserve acyclic configurations.
- Around line 263-273: Add descriptive data-testid attributes to the
clear-workflow and cancel Buttons in TodoWorkflowDialog, using stable,
action-specific selector values. Preserve their existing onClick handlers,
labels, and styling.
In `@wework/src/features/todo/TodoWorkspace.test.tsx`:
- Around line 559-564: Update the localStorage assertion in the TodoWorkspace
test to parse the stored JSON and locate the specific implementation child by
its ID or workTypeKey, rather than searching the entire serialized store. Assert
that this child has the expected title “等待 API 定稿”, state “backlog”, and type
“ai”, preserving the existing waitFor behavior.
- Around line 303-322: Extend coverage beyond the component tests in
TodoWorkspace.test.tsx by adding E2E tests for work-item creation, AI launch,
workflow application, completion, persistence, IPC, and recovery using the real
backend and Tauri runtime. Preserve the existing tests, and add isolated
ai:verify evidence that exercises these flows with real backend requests and
validates runtime behavior rather than injected callbacks or localStorage alone.
In `@wework/src/features/todo/TodoWorkspace.tsx`:
- Around line 452-453: Replace the hardcoded Chinese summaries in the persisted
event entries within TodoWorkspace with stable event types or translation keys,
ensuring stored history remains locale-independent. Add the corresponding keys
to both English and Chinese locale trees, and use the translation wrapper when
rendering these events so summaries are localized at display time.
- Around line 379-388: Update the projectItemCounts useMemo calculation to
exclude runtime workItems that are linked by local items, matching the displayed
list’s filtering behavior. Ensure each linked runtime task is omitted before
counting, while retaining all local root items and unlinked runtime tasks
without double-counting.
- Around line 415-421: Update the TodoWorkspace hydration and persistence
effects around hydrateLocalWorkItems and saveLocalWorkItems to track hydration
completion for the current user scope. Apply the hydrated result
unconditionally, including an empty array, and prevent saves until that scope’s
asynchronous hydration finishes; reset or invalidate the hydration state when
user?.id changes so prior-user items cannot be persisted under the new scope.
---
Outside diff comments:
In `@wework/src/features/todo/TodoWorkItems.tsx`:
- Around line 86-103: The global create request is only handled by the board
when its target column is visible. Forward createRequest to TodoList, and update
TodoBoard’s column-selection/rendering logic to ensure the requested state
column is included even when filters or showEmptyGroups would otherwise hide it,
so header/sidebar Create opens the appropriate input in both layouts.
---
Nitpick comments:
In `@wework/src/features/todo/TodoWorkspace.tsx`:
- Around line 234-242: The TodoWorkspace component spans over 1000 lines and
combines persistence logic, workflow generation, execution, filtering, and view
rendering. Extract custom hooks to isolate work-item storage state and
persistence, workflow action handlers (generate, execute, filter), and separate
the header and content rendering into focused sub-components. Pass these
extracted concerns to TodoWorkspace as props or through composition to keep the
main component under the 1000-line guideline while maintaining focused
responsibilities for each extracted piece.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c6748923-0276-495e-992d-1042daace5e1
📒 Files selected for processing (17)
build_image.shwework/src-tauri/src/lib.rswework/src-tauri/src/todo_store.rswework/src/components/layout/DesktopWorkbenchLayout.tsxwework/src/features/todo/TodoDetailPanel.tsxwework/src/features/todo/TodoMyWork.tsxwework/src/features/todo/TodoNavigation.tsxwework/src/features/todo/TodoWorkItems.tsxwework/src/features/todo/TodoWorkflowDialog.tsxwework/src/features/todo/TodoWorkspace.test.tsxwework/src/features/todo/TodoWorkspace.tsxwework/src/features/todo/todoModel.test.tswework/src/features/todo/todoModel.tswework/src/features/workbench/useWorkbenchRuntimeMessaging.tswework/src/features/workbench/workbenchContextTypes.tswework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.json
| # Function to show help | ||
| show_help() { | ||
| echo "Build docker images for Wegent components" | ||
| echo "Build docker images for Wegent components"aass |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Remove the unintended aass suffix.
The command remains syntactically valid, but --help prints Build docker images for Wegent componentsaass instead of the expected header.
Proposed fix
- echo "Build docker images for Wegent components"aass
+ echo "Build docker images for Wegent components"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| echo "Build docker images for Wegent components"aass | |
| echo "Build docker images for Wegent components" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@build_image.sh` at line 18, Remove the unintended “aass” suffix from the echo
command in build_image.sh so the output header is exactly “Build docker images
for Wegent components”.
| 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()) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Normalize paths before enforcing protected entries.
safe_relative_path accepts current-directory components, while protection uses the original string. Inputs such as context/., work/., or ./context therefore resolve to protected directories but bypass is_protected_workspace_path, allowing them to be renamed or recursively deleted.
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 context/., work/., and ./context.
Also applies to: 154-159, 190-201
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src-tauri/src/todo_store.rs` around lines 35 - 48, Normalize the path
in safe_relative_path before protected-entry checks so equivalent forms such as
context/., work/., and ./context resolve to their canonical relative paths.
Update is_protected_workspace_path and the rename/delete flows to use this
normalized value, preserving rejection of parent, root, and prefix components.
Add regression coverage for all three bypass forms.
| 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}")) |
There was a problem hiding this comment.
🗄️ 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:
Rust std::fs::rename Windows overwrite existing destination semantics MoveFileExW REPLACE_EXISTING
💡 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:
- 1: https://doc.rust-lang.org/std/fs/fn.rename.html
- 2: https://doc.rust-lang.org/stable/std/fs/fn.rename.html
- 3: Win: Use POSIX rename semantics for
std::fs::renameif available rust-lang/rust#131072 - 4:
std::fs::renamesometimes fails on Windows due to missingFILE_RENAME_POSIX_SEMANTICSrust-lang/rust#123985 - 5: Win: Fix std::fs::rename failing on Windows Server by attempting the non-atomic rename first rust-lang/rust#138133
- 6: https://github.com/rust-lang/rust/blob/master/library/std/src/sys/fs/windows.rs
- 7: https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-movefileexw
🏁 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.
wework/src/features/todo/todoModel.ts#L221-L228:saveLocalWorkItems()firessave_todo_storefire-and-forget, so rapid edits can persist out of order.wework/src-tauri/src/todo_store.rs#L83-L98: every save for a scope uses the same temp file path, so overlapping writes can stomp each other’s in-progress contents.
Queue or await the save path so only the latest snapshot reaches disk.
📍 Affects 2 files
wework/src-tauri/src/todo_store.rs#L83-L98(this comment)wework/src/features/todo/todoModel.ts#L221-L228
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src-tauri/src/todo_store.rs` around lines 83 - 98, Serialize TODO
persistence writes per scope: update saveLocalWorkItems in
wework/src/features/todo/todoModel.ts:221-228 to await or queue save_todo_store
calls so rapid edits commit in order and only the latest snapshot reaches disk;
update save_todo_store in wework/src-tauri/src/todo_store.rs:83-98 to prevent
overlapping saves for the same scope from sharing and stomping the temporary
file, using per-scope serialization while preserving atomic rename behavior.
| useEffect(() => { | ||
| if (!item.workspaceItemId) return | ||
| void listTodoWorkspace(item.workspaceItemId) | ||
| .then(setWorkspaceEntries) | ||
| .catch(() => setWorkspaceEntries([])) | ||
| }, [item.workspaceItemId, item.updatedAt]) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Prevent stale workspace requests from overwriting newer results.
A slower request for the previous item or update can replace the current workspace list, exposing incorrect file actions. Cancel or sequence requests and clear entries when the workspace ID changes.
Proposed fix
useEffect(() => {
- if (!item.workspaceItemId) return
- void listTodoWorkspace(item.workspaceItemId)
- .then(setWorkspaceEntries)
- .catch(() => setWorkspaceEntries([]))
+ const itemId = item.workspaceItemId
+ if (!itemId) {
+ setWorkspaceEntries([])
+ return
+ }
+ let cancelled = false
+ void listTodoWorkspace(itemId).then(entries => {
+ if (!cancelled) setWorkspaceEntries(entries)
+ })
+ return () => {
+ cancelled = true
+ }
}, [item.workspaceItemId, item.updatedAt])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/features/todo/TodoDetailPanel.tsx` around lines 160 - 165, Update
the workspace-loading useEffect around listTodoWorkspace so previous requests
cannot overwrite results for a newer item or update. Clear workspace entries
when item.workspaceItemId changes, and cancel or sequence each request so only
the latest request may call setWorkspaceEntries; preserve the empty-list
fallback for the active request’s failure.
| const revealWorkspace = async () => { | ||
| if (!item.workspaceItemId) return | ||
| const path = await getTodoWorkspacePath(item.workspaceItemId) | ||
| if (path) await revealLocalFile(path) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Surface workspace open and reveal failures.
openLocalFile and revealLocalFile may reject, but these paths leave an unhandled promise and no user feedback. Catch failures and assign them to workspaceError.
Also applies to: 632-641
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/features/todo/TodoDetailPanel.tsx` around lines 214 - 218, Update
the workspace open/reveal handlers, including revealWorkspace and the
corresponding flow around lines 632-641, to catch rejected openLocalFile and
revealLocalFile promises and assign the caught error to workspaceError. Preserve
the existing path and workspace-item checks while ensuring both failure paths
provide user feedback through that error state.
| await quickCreate('Investigate and fix the bug') | ||
| await userEvent.click(screen.getByText('Investigate and fix the bug')) | ||
| await userEvent.selectOptions(screen.getByTestId('todo-detail-assignee-select'), 'ai') | ||
| await userEvent.click(screen.getByTestId('todo-detail-run')) | ||
|
|
||
| await waitFor(() => expect(onRunTodo).toHaveBeenCalledTimes(1)) | ||
| expect(onRunTodo).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| project: expect.objectContaining({ id: 7 }), | ||
| message: 'Investigate and fix the bug', | ||
| goal: 'Ship a verified fix', | ||
| goal: undefined, | ||
| attachments: [], | ||
| collaborationMode: 'plan', | ||
| }) | ||
| ) | ||
| await waitFor(() => expect(screen.queryByTestId('todo-create-dialog')).not.toBeInTheDocument()) | ||
| await waitFor(() => { | ||
| const stored = window.localStorage.getItem('wework:todo:work-items:1') | ||
| expect(stored).toContain('created-task') | ||
| expect(stored).toContain('已关联 AI 执行会话') | ||
| }) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Add real-backend and real-Tauri coverage for these core flows.
These are component tests using localStorage and injected callbacks; they do not validate actual runtime creation, workspace persistence, IPC, or recovery. Keep them, but add E2E coverage and isolated ai:verify evidence for creation, launch, workflow application, and completion.
As per coding guidelines, core flows require automated E2E regression coverage, and Wework runtime/Tauri changes require isolated real-Tauri verification using real backend requests.
Also applies to: 455-592
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/features/todo/TodoWorkspace.test.tsx` around lines 303 - 322,
Extend coverage beyond the component tests in TodoWorkspace.test.tsx by adding
E2E tests for work-item creation, AI launch, workflow application, completion,
persistence, IPC, and recovery using the real backend and Tauri runtime.
Preserve the existing tests, and add isolated ai:verify evidence that exercises
these flows with real backend requests and validates runtime behavior rather
than injected callbacks or localStorage alone.
Source: Coding guidelines
| await waitFor(() => { | ||
| const stored = window.localStorage.getItem('wework:todo:work-items:1') | ||
| expect(stored).toContain('等待 API 定稿') | ||
| expect(stored).toContain('"state":"backlog"') | ||
| expect(stored).toContain('"type":"ai"') | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the updated child instead of searching the whole store.
Other workflow items already contain "state":"backlog" and an AI assignee, so these substring checks can pass even if the selected implementation child was updated incorrectly. Parse the JSON, locate that child by ID or workTypeKey, and assert its exact fields.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/features/todo/TodoWorkspace.test.tsx` around lines 559 - 564,
Update the localStorage assertion in the TodoWorkspace test to parse the stored
JSON and locate the specific implementation child by its ID or workTypeKey,
rather than searching the entire serialized store. Assert that this child has
the expected title “等待 API 定稿”, state “backlog”, and type “ai”, preserving the
existing waitFor behavior.
| const projectItemCounts = useMemo( | ||
| () => | ||
| Object.fromEntries( | ||
| projectEntries.map(entry => [ | ||
| entry.project.id, | ||
| entry.workspaces.reduce((total, workspace) => total + workspace.tasks.length, 0) + | ||
| drafts.filter(draft => draft.projectId === entry.project.id).length, | ||
| workItems.filter(item => item.projectId === entry.project.id && !item.parentId).length, | ||
| ]) | ||
| ), | ||
| [drafts, projectEntries] | ||
| [workItems, projectEntries] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Exclude linked runtime tasks from sidebar counts.
The displayed list removes runtime tasks referenced by local items, but this count still adds every runtime task plus every local root. After running a TODO, the sidebar counts that work item twice.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/features/todo/TodoWorkspace.tsx` around lines 379 - 388, Update
the projectItemCounts useMemo calculation to exclude runtime workItems that are
linked by local items, matching the displayed list’s filtering behavior. Ensure
each linked runtime task is omitted before counting, while retaining all local
root items and unlinked runtime tasks without double-counting.
| useEffect(() => { | ||
| try { | ||
| window.localStorage.setItem( | ||
| `${TODO_DRAFTS_STORAGE_KEY}:${user?.id ?? 'local'}`, | ||
| JSON.stringify(drafts) | ||
| ) | ||
| } catch { | ||
| // Drafts remain available for the current session. | ||
| } | ||
| }, [drafts, user?.id]) | ||
| void hydrateLocalWorkItems(user?.id).then(items => { | ||
| if (items.length > 0) setWorkItems(items) | ||
| }) | ||
| }, [user?.id]) | ||
|
|
||
| useEffect(() => saveLocalWorkItems(user?.id, workItems), [user?.id, workItems]) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift
Gate persistence until the current user scope is hydrated.
Line 416 starts an asynchronous load, while Line 421 immediately saves the initial local state. In Tauri this can overwrite the authoritative store before loading finishes; on user changes it can also save the previous user's items into the new scope. Always apply the hydrated result—including []—and suppress saves until hydration for the current scope completes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/features/todo/TodoWorkspace.tsx` around lines 415 - 421, Update
the TodoWorkspace hydration and persistence effects around hydrateLocalWorkItems
and saveLocalWorkItems to track hydration completion for the current user scope.
Apply the hydrated result unconditionally, including an empty array, and prevent
saves until that scope’s asynchronous hydration finishes; reset or invalidate
the hydration state when user?.id changes so prior-user items cannot be
persisted under the new scope.
| events: [ | ||
| { id: createLocalWorkItemId(), type: 'created', summary: '事项已创建', createdAt: now }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Localize persisted event summaries.
These Chinese literals appear in persisted history for every locale and cannot be retranslated later. Store stable event types/translation keys and translate when rendering, with entries in both locale trees.
As per coding guidelines, new Wework copy must use the translation wrapper and be added to both English and Chinese locales.
Also applies to: 476-481, 569-574, 629-630, 969-975, 994-1000
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/features/todo/TodoWorkspace.tsx` around lines 452 - 453, Replace
the hardcoded Chinese summaries in the persisted event entries within
TodoWorkspace with stable event types or translation keys, ensuring stored
history remains locale-independent. Add the corresponding keys to both English
and Chinese locale trees, and use the translation wrapper when rendering these
events so summaries are localized at display time.
Source: Coding guidelines
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
wework/src/features/todo/TodoWorkItems.tsx (2)
86-103: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winForward global create requests to list layout.
createRequestonly reachesTodoBoard, so the global create action does nothing whenlayout === 'list'. Pass it toTodoListand open/reset the requested state when its token changes.Also applies to: 622-637
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/features/todo/TodoWorkItems.tsx` around lines 86 - 103, Update the TodoList branch in TodoWorkItems to pass createRequest, then handle request-token changes inside TodoList by opening the create UI and resetting the requested state. Preserve the existing TodoBoard behavior and ensure repeated requests are processed only when the token changes.
409-427: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAlways render the requested quick-create state.
The requested state can be removed by
stateFilterorshowEmptyGroups === false, so noTodoColumnreceivesforceCreateToken. IncludecreateRequest.stateinstateswhile the request is active.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/features/todo/TodoWorkItems.tsx` around lines 409 - 427, Update the state list used by the TodoColumn map in TodoWorkItems so an active createRequest.state is included even when stateFilter removes it or showEmptyGroups hides empty groups. Preserve the existing state filtering behavior when no quick-create request is active, ensuring the matching TodoColumn receives forceCreateToken.
🧹 Nitpick comments (2)
wework/src/features/todo/TodoWorkspace.tsx (1)
234-242: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftExtract the Todo persistence and workflow orchestration.
TodoWorkspacenow contains hundreds of lines of persistence, workflow, runtime, and UI logic. Move these concerns into focused hooks such asuseTodoPersistenceanduseTodoWorkflowExecution.As per coding guidelines, keep functions focused, preferably under 50 lines, and split source files over 1000 lines.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/features/todo/TodoWorkspace.tsx` around lines 234 - 242, Refactor TodoWorkspace into focused hooks, extracting persistence responsibilities into useTodoPersistence and workflow/runtime orchestration into useTodoWorkflowExecution. Keep TodoWorkspace focused on composing hooks and rendering UI, preserve existing behavior and dependencies, and split the implementation into appropriately sized files with functions preferably under 50 lines.Source: Coding guidelines
wework/src/features/todo/TodoDetailPanel.tsx (1)
965-1032: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSplit the detail panel into cohesive modules.
The file now exceeds 1000 lines. Extract the workflow, shared-workspace, properties, and activity sections into focused components.
As per coding guidelines, source files over 1000 lines must be split.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/features/todo/TodoDetailPanel.tsx` around lines 965 - 1032, Split TodoDetailPanel into focused components/modules, extracting the workflow, shared-workspace, properties (including EditableProperty, Property, formatDetailDate, and formatBytes), and activity sections while preserving existing behavior and interfaces. Keep TodoDetailPanel responsible only for composition and coordination, and move each cohesive section into appropriately scoped components.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@build_image.sh`:
- Line 18: Remove the unintended “aass” suffix from the help banner echo command
so it prints only “Build docker images for Wegent components”.
In `@wework/src-tauri/src/todo_store.rs`:
- Around line 35-49: Update safe_relative_path to reject Component::CurDir and
validate protected paths using normalized components rather than the raw input
string, covering aliases such as ".", "./README.md", and "context/". Apply the
same validation to the affected deletion flows and add regression tests for
these path forms.
In `@wework/src/features/todo/TodoDetailPanel.tsx`:
- Around line 676-679: Update the Properties heading in TodoDetailPanel to use
the existing t(...) localization function, then add the corresponding key under
the appropriate Wework namespace in both English and Chinese locale files, with
the correct translated values.
- Around line 421-446: Update the editable blocker and next-action inputs in
TodoDetailPanel so their displayed values reset when item.id changes. Either
control both inputs with state synchronized to the selected item, or key each
input by item.id while preserving the existing trimmed onBlur updates.
- Around line 160-165: Update the workspace-loading useEffect around
listTodoWorkspace so responses are applied only if they belong to the latest
item request. Track cancellation or request identity within the effect, ignore
stale successes and failures after cleanup, and preserve clearing entries when
the active request fails.
In `@wework/src/features/todo/TodoMyWork.tsx`:
- Around line 15-40: Update TodoMyWork to receive a stable current principal ID
and restrict the “action” group to human-assigned items whose explicit assignee
ID matches that principal. Replace the assigneeType-only filter in the action
group while preserving the existing state exclusions and leave the AI/review
grouping unchanged.
In `@wework/src/features/todo/TodoWorkflowDialog.tsx`:
- Around line 263-273: Add descriptive, stable data-testid attributes to the
clear-workflow and cancel Buttons in TodoWorkflowDialog’s footer. Use distinct
selectors that identify each action, while preserving their existing onClick
behavior and labels.
- Around line 90-96: Update the dialog section’s width classes in the
TodoWorkflowDialog markup to use full available width while retaining a 560px
maximum, replacing the fixed w-[560px] sizing with w-full max-w-[560px].
Preserve the existing layout and styling classes.
- Around line 50-63: Prevent cyclic dependencies in the workflow state before
saving: update toggleDependency or the save handler to detect whether
dependencyKey can already reach workTypeKey through the existing dependsOn
links, and reject the change when it would create a cycle. Preserve valid
dependency updates and ensure the save path cannot persist cyclic graphs.
- Around line 108-116: Update the mobile TODO controls to provide at least 44px
touch targets: adjust the close and other h-7/h-8 controls in
wework/src/features/todo/TodoWorkflowDialog.tsx:108-116, enlarge the
quick-create, submit, and list-creation actions in
wework/src/features/todo/TodoWorkItems.tsx:513-521, and enlarge the workspace,
workflow, property, and confirmation actions in
wework/src/features/todo/TodoDetailPanel.tsx:588-605, using responsive sizing
while preserving desktop dimensions.
In `@wework/src/features/todo/TodoWorkspace.tsx`:
- Around line 592-597: Update deleteDraft to invoke and await a Tauri command
that removes the filesystem workspace for draftId before calling setWorkItems or
clearing the selection. Surface command failures and preserve the existing state
only after deletion succeeds; add the command implementation using the workspace
root path and ensure it removes the entire workspace directory.
- Around line 415-421: Update the hydration and persistence effects in
TodoWorkspace to track the current user’s hydration generation, always apply
hydrated results including empty arrays, and gate saveLocalWorkItems until
hydration for that same user scope completes. Ensure stale hydration completions
cannot update or enable persistence for a newer user, and add a regression
covering switching from user A with items to empty user B. Verify the Tauri
behavior change with isolated real-Tauri execution via scripts/ai-verify.mjs.
- Around line 552-558: Update the onRunTodo call in TodoWorkspace so its message
is always non-empty: use draft.description when it contains meaningful text,
otherwise fall back to draft.objective or the todo title. Preserve the existing
goal, attachments, and collaborationMode behavior.
---
Outside diff comments:
In `@wework/src/features/todo/TodoWorkItems.tsx`:
- Around line 86-103: Update the TodoList branch in TodoWorkItems to pass
createRequest, then handle request-token changes inside TodoList by opening the
create UI and resetting the requested state. Preserve the existing TodoBoard
behavior and ensure repeated requests are processed only when the token changes.
- Around line 409-427: Update the state list used by the TodoColumn map in
TodoWorkItems so an active createRequest.state is included even when stateFilter
removes it or showEmptyGroups hides empty groups. Preserve the existing state
filtering behavior when no quick-create request is active, ensuring the matching
TodoColumn receives forceCreateToken.
---
Nitpick comments:
In `@wework/src/features/todo/TodoDetailPanel.tsx`:
- Around line 965-1032: Split TodoDetailPanel into focused components/modules,
extracting the workflow, shared-workspace, properties (including
EditableProperty, Property, formatDetailDate, and formatBytes), and activity
sections while preserving existing behavior and interfaces. Keep TodoDetailPanel
responsible only for composition and coordination, and move each cohesive
section into appropriately scoped components.
In `@wework/src/features/todo/TodoWorkspace.tsx`:
- Around line 234-242: Refactor TodoWorkspace into focused hooks, extracting
persistence responsibilities into useTodoPersistence and workflow/runtime
orchestration into useTodoWorkflowExecution. Keep TodoWorkspace focused on
composing hooks and rendering UI, preserve existing behavior and dependencies,
and split the implementation into appropriately sized files with functions
preferably under 50 lines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c6748923-0276-495e-992d-1042daace5e1
📒 Files selected for processing (17)
build_image.shwework/src-tauri/src/lib.rswework/src-tauri/src/todo_store.rswework/src/components/layout/DesktopWorkbenchLayout.tsxwework/src/features/todo/TodoDetailPanel.tsxwework/src/features/todo/TodoMyWork.tsxwework/src/features/todo/TodoNavigation.tsxwework/src/features/todo/TodoWorkItems.tsxwework/src/features/todo/TodoWorkflowDialog.tsxwework/src/features/todo/TodoWorkspace.test.tsxwework/src/features/todo/TodoWorkspace.tsxwework/src/features/todo/todoModel.test.tswework/src/features/todo/todoModel.tswework/src/features/workbench/useWorkbenchRuntimeMessaging.tswework/src/features/workbench/workbenchContextTypes.tswework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.json
| # Function to show help | ||
| show_help() { | ||
| echo "Build docker images for Wegent components" | ||
| echo "Build docker images for Wegent components"aass |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Remove the unintended aass suffix from the help banner.
The current command prints Build docker images for Wegent componentsaass, degrading the user-facing help output.
Proposed fix
- echo "Build docker images for Wegent components"aass
+ echo "Build docker images for Wegent components"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| echo "Build docker images for Wegent components"aass | |
| echo "Build docker images for Wegent components" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@build_image.sh` at line 18, Remove the unintended “aass” suffix from the help
banner echo command so it prints only “Build docker images for Wegent
components”.
| 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()) | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Reject normalized aliases of protected paths.
relativePath='.' passes validation and makes deletion target the entire workspace. Aliases such as ./README.md or context/ can also bypass the raw-string protection check. Reject Component::CurDir and detect protected entries from normalized components; add regression tests for these forms.
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
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src-tauri/src/todo_store.rs` around lines 35 - 49, Update
safe_relative_path to reject Component::CurDir and validate protected paths
using normalized components rather than the raw input string, covering aliases
such as ".", "./README.md", and "context/". Apply the same validation to the
affected deletion flows and add regression tests for these path forms.
| useEffect(() => { | ||
| if (!item.workspaceItemId) return | ||
| void listTodoWorkspace(item.workspaceItemId) | ||
| .then(setWorkspaceEntries) | ||
| .catch(() => setWorkspaceEntries([])) | ||
| }, [item.workspaceItemId, item.updatedAt]) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Discard stale workspace-loading responses.
Switching items before an earlier request resolves can overwrite the new item’s entries with the old workspace. Subsequent rename/delete actions then use the current item ID with stale paths.
Proposed fix
useEffect(() => {
- if (!item.workspaceItemId) return
+ let cancelled = false
+ setWorkspaceEntries([])
+ if (!item.workspaceItemId) return
void listTodoWorkspace(item.workspaceItemId)
- .then(setWorkspaceEntries)
- .catch(() => setWorkspaceEntries([]))
+ .then(entries => {
+ if (!cancelled) setWorkspaceEntries(entries)
+ })
+ return () => {
+ cancelled = true
+ }
}, [item.workspaceItemId, item.updatedAt])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| if (!item.workspaceItemId) return | |
| void listTodoWorkspace(item.workspaceItemId) | |
| .then(setWorkspaceEntries) | |
| .catch(() => setWorkspaceEntries([])) | |
| }, [item.workspaceItemId, item.updatedAt]) | |
| useEffect(() => { | |
| let cancelled = false | |
| setWorkspaceEntries([]) | |
| if (!item.workspaceItemId) return | |
| void listTodoWorkspace(item.workspaceItemId) | |
| .then(entries => { | |
| if (!cancelled) setWorkspaceEntries(entries) | |
| }) | |
| return () => { | |
| cancelled = true | |
| } | |
| }, [item.workspaceItemId, item.updatedAt]) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/features/todo/TodoDetailPanel.tsx` around lines 160 - 165, Update
the workspace-loading useEffect around listTodoWorkspace so responses are
applied only if they belong to the latest item request. Track cancellation or
request identity within the effect, ignore stale successes and failures after
cleanup, and preserve clearing entries when the active request fails.
| {onUpdateItem && ( | ||
| <section className="mt-[18px] grid gap-2 sm:grid-cols-2"> | ||
| <label className="rounded-lg border border-border bg-muted/30 p-3"> | ||
| <span className="text-[10px] font-semibold text-text-muted"> | ||
| {t('todo.blocked', '阻塞')} | ||
| </span> | ||
| <input | ||
| data-testid="todo-detail-blocker-input" | ||
| defaultValue={item.blocker} | ||
| onBlur={event => onUpdateItem({ blocker: event.target.value.trim() })} | ||
| placeholder={t('todo.blocker_placeholder', '没有阻塞')} | ||
| className="mt-1 h-7 w-full bg-transparent text-[11px] text-text-primary outline-none placeholder:text-text-muted" | ||
| /> | ||
| </label> | ||
| <label className="rounded-lg border border-border bg-muted/30 p-3"> | ||
| <span className="text-[10px] font-semibold text-text-muted"> | ||
| {t('todo.next_action', '下一步')} | ||
| </span> | ||
| <input | ||
| data-testid="todo-detail-next-action-input" | ||
| defaultValue={item.nextAction} | ||
| onBlur={event => onUpdateItem({ nextAction: event.target.value.trim() })} | ||
| placeholder={t('todo.next_action_placeholder', '写下明确的下一步')} | ||
| className="mt-1 h-7 w-full bg-transparent text-[11px] text-text-primary outline-none placeholder:text-text-muted" | ||
| /> | ||
| </label> |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reset editable fields when the selected item changes.
Because these inputs use defaultValue, selecting a child or another item leaves the previous item’s text in the DOM; blurring can save that stale value onto the new item. Use controlled state or key each input by item.id.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/features/todo/TodoDetailPanel.tsx` around lines 421 - 446, Update
the editable blocker and next-action inputs in TodoDetailPanel so their
displayed values reset when item.id changes. Either control both inputs with
state synchronized to the selected item, or key each input by item.id while
preserving the existing trimmed onBlur updates.
| <section className="mt-[18px]"> | ||
| <h3 className="mb-2 text-[13px] font-semibold text-[#343A40] dark:text-text-primary"> | ||
| Properties | ||
| </h3> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Localize the new Properties heading.
Properties remains English in the Chinese UI. Replace it with t(...) and add the key to both locale files.
As per coding guidelines, new copy must be added to the appropriate Wework namespace in both English and Chinese locales.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/features/todo/TodoDetailPanel.tsx` around lines 676 - 679, Update
the Properties heading in TodoDetailPanel to use the existing t(...)
localization function, then add the corresponding key under the appropriate
Wework namespace in both English and Chinese locale files, with the correct
translated values.
Source: Coding guidelines
| <button | ||
| type="button" | ||
| data-testid="todo-workflow-close" | ||
| onClick={onClose} | ||
| className="flex h-8 w-8 items-center justify-center rounded-md text-text-muted hover:bg-muted" | ||
| aria-label={t('workbench.close', '关闭')} | ||
| > | ||
| <X className="h-4 w-4" /> | ||
| </button> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Make all new mobile actions meet the 44px touch-target requirement.
The new workflow and TODO controls consistently retain desktop dimensions on mobile.
wework/src/features/todo/TodoWorkflowDialog.tsx#L108-L116: use responsive 44px sizing for the close action and audit the dialog’s h-7/h-8 controls.wework/src/features/todo/TodoWorkItems.tsx#L513-L521: enlarge quick-create, submit, and list creation actions on mobile.wework/src/features/todo/TodoDetailPanel.tsx#L588-L605: enlarge workspace, workflow, property, and confirmation actions on mobile.
As per coding guidelines, mobile controls must be at least 44px × 44px.
📍 Affects 3 files
wework/src/features/todo/TodoWorkflowDialog.tsx#L108-L116(this comment)wework/src/features/todo/TodoWorkItems.tsx#L513-L521wework/src/features/todo/TodoDetailPanel.tsx#L588-L605
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/features/todo/TodoWorkflowDialog.tsx` around lines 108 - 116,
Update the mobile TODO controls to provide at least 44px touch targets: adjust
the close and other h-7/h-8 controls in
wework/src/features/todo/TodoWorkflowDialog.tsx:108-116, enlarge the
quick-create, submit, and list-creation actions in
wework/src/features/todo/TodoWorkItems.tsx:513-521, and enlarge the workspace,
workflow, property, and confirmation actions in
wework/src/features/todo/TodoDetailPanel.tsx:588-605, using responsive sizing
while preserving desktop dimensions.
Source: Coding guidelines
| <footer className="flex h-14 items-center justify-end gap-2 border-t border-border px-5"> | ||
| <Button | ||
| type="button" | ||
| variant="ghost" | ||
| onClick={() => setWorkTypes(DEFAULT_TODO_WORKFLOW.workTypes)} | ||
| > | ||
| {t('todo.clear_workflow', '清空流程')} | ||
| </Button> | ||
| <Button type="button" variant="secondary" onClick={onClose}> | ||
| {t('common.cancel', '取消')} | ||
| </Button> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add stable selectors to the new footer actions.
Both clear and cancel are new interactive elements without descriptive data-testid values.
As per coding guidelines, all new interactive elements must have descriptive data-testid values.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/features/todo/TodoWorkflowDialog.tsx` around lines 263 - 273, Add
descriptive, stable data-testid attributes to the clear-workflow and cancel
Buttons in TodoWorkflowDialog’s footer. Use distinct selectors that identify
each action, while preserving their existing onClick behavior and labels.
Source: Coding guidelines
| useEffect(() => { | ||
| try { | ||
| window.localStorage.setItem( | ||
| `${TODO_DRAFTS_STORAGE_KEY}:${user?.id ?? 'local'}`, | ||
| JSON.stringify(drafts) | ||
| ) | ||
| } catch { | ||
| // Drafts remain available for the current session. | ||
| } | ||
| }, [drafts, user?.id]) | ||
| void hydrateLocalWorkItems(user?.id).then(items => { | ||
| if (items.length > 0) setWorkItems(items) | ||
| }) | ||
| }, [user?.id]) | ||
|
|
||
| useEffect(() => saveLocalWorkItems(user?.id, workItems), [user?.id, workItems]) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Gate persistence until the matching user scope is hydrated.
On a user change, Line 421 immediately saves the previous workItems under the new user before hydration resolves. Empty hydration results are then ignored by Line 417, leaking or restoring stale items across user scopes. Track the hydration generation, always apply empty results, and enable saves only after that same scope finishes hydrating.
Add a regression covering user A with items switching to an empty user B.
As per coding guidelines, Tauri behavior changes require isolated real-Tauri verification using scripts/ai-verify.mjs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/features/todo/TodoWorkspace.tsx` around lines 415 - 421, Update
the hydration and persistence effects in TodoWorkspace to track the current
user’s hydration generation, always apply hydrated results including empty
arrays, and gate saveLocalWorkItems until hydration for that same user scope
completes. Ensure stale hydration completions cannot update or enable
persistence for a newer user, and add a regression covering switching from user
A with items to empty user B. Verify the Tauri behavior change with isolated
real-Tauri execution via scripts/ai-verify.mjs.
Source: Coding guidelines
| const address = await onRunTodo({ | ||
| project, | ||
| message: draft.markdown, | ||
| goal: draft.goal || undefined, | ||
| message: draft.description, | ||
| goal: draft.objective || undefined, | ||
| attachments: draft.attachments, | ||
| collaborationMode: draft.runtimeRefs.length === 0 ? 'plan' : 'default', | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Provide non-empty input when launching workflow stages.
Auto-created and manually added children have description: '', but createProjectRuntimeTask rejects an empty trimmed message. Every such AI stage therefore fails to launch. Fall back to its objective or title.
Proposed fix
- message: draft.description,
+ message: draft.description.trim() || draft.objective.trim() || draft.title,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const address = await onRunTodo({ | |
| project, | |
| message: draft.markdown, | |
| goal: draft.goal || undefined, | |
| message: draft.description, | |
| goal: draft.objective || undefined, | |
| attachments: draft.attachments, | |
| collaborationMode: draft.runtimeRefs.length === 0 ? 'plan' : 'default', | |
| }) | |
| const address = await onRunTodo({ | |
| project, | |
| message: draft.description.trim() || draft.objective.trim() || draft.title, | |
| goal: draft.objective || undefined, | |
| attachments: draft.attachments, | |
| collaborationMode: draft.runtimeRefs.length === 0 ? 'plan' : 'default', | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/features/todo/TodoWorkspace.tsx` around lines 552 - 558, Update
the onRunTodo call in TodoWorkspace so its message is always non-empty: use
draft.description when it contains meaningful text, otherwise fall back to
draft.objective or the todo title. Preserve the existing goal, attachments, and
collaborationMode behavior.
| const deleteDraft = (draftId: string) => { | ||
| setDrafts(current => current.filter(draft => draft.id !== draftId)) | ||
| setWorkItems(current => | ||
| current.filter(item => item.id !== draftId && item.parentId !== draftId) | ||
| ) | ||
| setSelectedItemId(null) | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Delete the filesystem workspace with the work item.
This removes the persisted model but leaves todo/workspaces/<id> and all user files behind indefinitely. Add a Tauri command that removes the root workspace, await it, surface failures, and only then remove the item from state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/features/todo/TodoWorkspace.tsx` around lines 592 - 597, Update
deleteDraft to invoke and await a Tauri command that removes the filesystem
workspace for draftId before calling setWorkItems or clearing the selection.
Surface command failures and preserve the existing state only after deletion
succeeds; add the command implementation using the workspace root path and
ensure it removes the entire workspace directory.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes