feat(workbench): ✨ add integrated file management with editor, preview and CRUD#183
feat(workbench): ✨ add integrated file management with editor, preview and CRUD#183HayWolf wants to merge 11 commits into
Conversation
… CRUD Implement a unified file panel in the right-side drawer that combines browsing, editing, previewing, creating, deleting and renaming files, inspired by OpenChamber's FilesView but adapted for TiyCode's AI-first sidebar layout. Key changes: Rust backend (5 new Tauri commands): - file_read/file_write/file_create/file_delete/file_rename with workspace-scoped path safety (canonicalize + starts_with guard) - Binary detection, 5 MB size limit, image base64 encoding - Path traversal rejection for ".." and path separators in names Frontend file editor (CodeMirror 6): - Tab management store with preview/pinned tab semantics (VS Code style) - Auto-save with 1.5s debounce, dirty state detection, Cmd+S - Language detection by extension (TS/JS/JSON/CSS/HTML/MD/Rust/Python) - Preview modes for Markdown, HTML and images - External content sync when Agent modifies open files Workbench integration: - Resizable drawer width (drag handle, min 320px, max 50vw, persisted) - Split pane layout: file tree above, editor below (draggable divider) - Dropdown menu (⋮ icon) replacing copy button for file CRUD actions - New File/Folder dialogs with type toggle, Rename and Delete confirm Agent file-changed event pipeline: - ToolCompleted event now carries tool_name field - AgentRunManager emits "agent-file-changed" for write/edit/patch tools - Editor auto-reloads non-dirty tabs on receiving the event
… path Add buttons in the project panel toolbar menu to create new files and folders at the workspace root. Also add a button to copy the root project path to clipboard. Refactor tree file opening logic to support directories, allowing them to be opened in the configured external application. The `NewFileDialog` component is now exported for reuse at the root level.
Replace the inline tree/editor split-pane layout with a modal WorkbenchPreviewOverlay for file previews. This frees up panel space and provides a focused editing view. - Remove treeSplitRatio state, local storage persistence, and drag resize handling from file-editor-store and project-panel - Open files via overlay instead of inline split pane; add temporary click-through during the open transition - Refactor file type icons to CSS variables for adaptive light/dark theming and add folder-specific opacity modifier - Add overlayClassName prop to WorkbenchPreviewOverlay for custom pointer-events control - Fix flex overflow behavior with min-h-0 on editor and tree containers
…evel Move the file editor preview overlay from ProjectPanel up to DashboardOverlays to centralize overlay management at the dashboard level. Changes include: - Lift isFileEditorOverlayOpen and isFileEditorOverlayClickThrough state from local React state into UILayoutStore - Render WorkbenchPreviewOverlay and FileEditorView inside DashboardOverlays instead of ProjectPanel - Pass resolvedWorkspaceId as a prop from DashboardWorkbench rather than deriving it inside DashboardOverlays - Handle Escape key dismissal of the file editor overlay alongside other dashboard overlays This improves separation of concerns and keeps overlay orchestration in a single location.
AI Code Review SummaryPR: #183 (feat(workbench): ✨ add integrated file management with editor, preview and CRUD) Overall AssessmentDetected 15 actionable findings, prioritize CRITICAL/HIGH before merge. Major Findings by Severity
Actionable Suggestions
Potential Risks
Test Suggestions
File-Level Coverage Notes
Inline Downgraded Items (processed but not inline)
Coverage Status
Uncovered list:
No-patch covered list:
Runtime/Budget
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| AppError::internal(ErrorSource::System, format!("Failed to read file: {e}")) | ||
| })?; | ||
|
|
||
| // Image files → base64 data URI |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| _ => {} | ||
| } | ||
|
|
||
| // Broadcast file-changed event when a file-mutating tool completes, |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| // Path safety tests (unit-level, no DB needed) | ||
| // --------------------------------------------------------------------------- | ||
|
|
||
| #[test] |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| // Path safety tests (unit-level, no DB needed) | ||
| // --------------------------------------------------------------------------- | ||
|
|
||
| #[test] |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Add comprehensive unit tests for internal file command helpers, including path resolution, image detection, binary content identification, and MIME type mapping. Extract the inline MIME matching logic from `file_read` into a dedicated `mime_for_extension` function to improve code clarity and enable direct testing of the mapping behavior. Test coverage includes: - safe_resolve path traversal protection and valid path handling - is_image_extension case-insensitive recognition - is_binary_content null-byte detection - End-to-end PNG data URI generation with base64 encoding - Exhaustive MIME mapping for all supported image extensions
| path: String, | ||
| ) -> Result<(), AppError> { | ||
| let root = resolve_workspace_root(&state, &workspace_id).await?; | ||
| let resolved = safe_resolve(&root, &path)?; |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| // --------------------------------------------------------------------------- | ||
|
|
||
| #[tauri::command] | ||
| pub async fn file_read( |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| // --------------------------------------------------------------------------- | ||
|
|
||
| #[test] | ||
| fn file_write_read_roundtrip() { |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| } | ||
|
|
||
| // Enforce MAX_TABS — evict oldest unpinned, non-dirty tab | ||
| while (nextTabs.length > MAX_TABS) { |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| // Auto-save debounce management | ||
| // --------------------------------------------------------------------------- | ||
|
|
||
| const autoSaveTimers = new Map<string, ReturnType<typeof setTimeout>>(); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| // Initial state | ||
| // --------------------------------------------------------------------------- | ||
|
|
||
| function readSavedDrawerWidth(): number { |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| const filterResults = filterState.data?.results ?? []; | ||
| const isFiltering = normalizedFilter.length > 0; | ||
|
|
||
| const handleOpenFileInEditor = useCallback((path: string) => { |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| const saved = window.localStorage.getItem(DRAWER_WIDTH_STORAGE_KEY); | ||
| if (saved) { | ||
| const n = parseInt(saved, 10); | ||
| if (!isNaN(n) && n >= 320) return n; |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| <div> | ||
| <AlertCircle className="mx-auto mb-2 size-5 text-destructive" /> | ||
| <p className="text-xs text-muted-foreground"> | ||
| {activeTab.error === "Binary file — cannot edit" |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| onClick={() => handleCreateRootEntry(false)} | ||
| > | ||
| <FilePlus className="size-4 shrink-0 text-app-subtle" /> | ||
| <span className="min-w-0 flex-1 truncate text-[12px] font-medium">New File</span> |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…firmation dialog Add context menu on Tracked and Untracked change groups in the Git panel with a destructive 'Discard Changes' / 'Delete File' action. Executing the action shows a confirmation dialog before proceeding. - Tracked files: runs 'git restore -- <paths>' to revert to HEAD - Untracked files: runs 'git clean -f -- <paths>' to delete from disk Backend: add GitMutationAction::Restore/Clean variants, CLI executor functions, git_manager methods, authorize_and_run_git_mutation commands, policy engine registration, and lib.rs invoke_handler. Frontend: add gitRestore/gitClean bridge (returns GitMutationResponseDto), ContextMenu on ChangeGroup rows, discard confirmation dialog, i18n keys for en/zh-CN, and mock mode support.
| } | ||
|
|
||
| if resolved.is_dir() { | ||
| tokio::fs::remove_dir_all(&resolved).await.map_err(|e| { |
There was a problem hiding this comment.
[CRITICAL] file_delete can recursively delete the workspace root
When path is '.' or an empty string, safe_resolve returns the workspace root directory, and file_delete proceeds to remove_dir_all the entire workspace.
Suggestion: Reject deletion when the resolved path equals the workspace root (e.g., explicitly block '.' and '' in safe_resolve or add a root-equality guard in file_delete).
Risk: Complete loss of the workspace directory and all its contents.
Confidence: 0.95
| let parent = resolved_old.parent().ok_or_else(|| { | ||
| AppError::validation(ErrorSource::System, "Cannot determine parent directory") | ||
| })?; | ||
| let new_path = parent.join(&new_name); |
There was a problem hiding this comment.
[HIGH] file_rename allows workspace escape via new_name == '..'
An attacker or buggy caller can pass new_name='..' to move a file from the workspace root to the parent directory, escaping the intended boundary.
Suggestion: Reject new_name values that are '.', '..', or empty, and assert new_path.starts_with(root) before renaming.
Risk: Files can be moved outside the workspace, leading to information disclosure or damage to external files.
Confidence: 0.92
| const onMove = (ev: MouseEvent) => { | ||
| const maxW = Math.floor(window.innerWidth * MAX_DRAWER_WIDTH_RATIO); | ||
| // Dragging left = increasing width (drawer is on right side) | ||
| const newWidth = Math.max(MIN_DRAWER_WIDTH, Math.min(maxW, startWidth + (startX - ev.clientX))); |
There was a problem hiding this comment.
[HIGH] Drawer resize commits store update on every mousemove
The drawer resize handler writes width changes to the global uiLayoutStore on every mousemove pixel, forcing DashboardWorkbench and any other drawerWidth subscribers to re-render continuously during drag.
Suggestion: Mutate the DOM element width directly via ref during the drag and only commit the final width to the store on mouseup, or throttle/RAF the store updates.
Risk: UI jank and dropped frames during drawer resizing, especially on lower-end hardware.
Confidence: 0.88
| // If the joined path were canonicalized, it would NOT start_with(root) | ||
| // (unless by coincidence). The command code rejects ".." early. | ||
| assert!( | ||
| !joined.starts_with(root) || bad.contains(".."), |
There was a problem hiding this comment.
[MEDIUM] Tautological assertion in path traversal test
The test claiming to reject '..' traversal uses an assertion that can never fail, providing no confidence in the actual path-safety logic.
Suggestion: Replace the assertion with a real call to the safe_resolve helper (or the command under test) and assert that the returned result is an error for out-of-bound paths.
Risk: False sense of security around path traversal protection; regressions in safe_resolve would not be caught.
Confidence: 0.95
| <DropdownMenuItem | ||
| className="gap-2 text-xs" | ||
| onClick={() => { | ||
| const absolutePath = workspaceRoot |
There was a problem hiding this comment.
[MEDIUM] Mixed path separators on Windows when copying absolute path
The Copy Path feature concatenates workspaceRoot and nodePath with a forward slash, which on Windows produces mixed separators (e.g., C:\project\folder/file.ts).
Suggestion: Use the platform-specific separator from the backend or normalize the resulting path before copying.
Risk: Windows users will get malformed absolute paths in the clipboard.
Confidence: 0.90
| // If the joined path were canonicalized, it would NOT start_with(root) | ||
| // (unless by coincidence). The command code rejects ".." early. | ||
| assert!( | ||
| !joined.starts_with(root) || bad.contains(".."), |
There was a problem hiding this comment.
[LOW] Flawed path traversal test assertion provides false assurance
The test rejects_dotdot_traversal asserts that a path is unsafe using !joined.starts_with(root) || bad.contains(".."). Because the input ../../etc/passwd always contains "..", the right-hand side is always true, making the assertion a tautology that can never fail regardless of whether the actual path-safety logic works.
Suggestion: Rewrite the test to invoke the actual safe_resolve helper or command function and assert that it returns an error or that the resolved path remains within the workspace root.
Risk: Provides false confidence that path traversal protections are validated by tests.
Confidence: 0.85
| <div> | ||
| <AlertCircle className="mx-auto mb-2 size-5 text-destructive" /> | ||
| <p className="text-xs text-muted-foreground"> | ||
| {activeTab.error === "Binary file — cannot edit" |
There was a problem hiding this comment.
[LOW] Binary file detection relies on hardcoded backend error string
The UI switches to a friendly binary-file message only when the error exactly matches a hardcoded English string from the backend.
Suggestion: Use a typed error code (e.g., error.code === 'BINARY_FILE') instead of comparing human-readable text.
Risk: Backend wording changes or localization will break the special-case UI rendering.
Confidence: 0.85
| variant="outline" | ||
| onClick={clearDiscardTarget} | ||
| > | ||
| Cancel |
There was a problem hiding this comment.
[LOW] Discard dialog uses hardcoded Cancel label
The discard confirmation dialog button uses the literal string 'Cancel' instead of a translation key.
Suggestion: Replace 'Cancel' with t('common.cancel') or an existing equivalent translation key.
Risk: Untranslated UI in non-English locales.
Confidence: 0.95
| "webpack.config.ts": "webpack", | ||
| "esbuild.config.js": "esbuild", | ||
| "tsup.config.ts": "tsup", | ||
| "components.json": "shadcn-ui", |
There was a problem hiding this comment.
[LOW] Dead icon mappings reference missing sprite IDs
Several filename and extension mappings resolve to icon IDs that are not present in the sprite set, causing silent fallback to generic icons.
Suggestion: Remove or replace the missing IDs with valid ones from FILE_TYPE_ICON_IDS, and consider adding a static assertion or test that every map value exists in the set.
Risk: User-facing UI shows generic document icons for files that should have custom icons.
Confidence: 0.90
2 participants
Summary
Changes
file-editor-view,file-context-menu,file-editor-store,file-type-iconsmodules; updatedproject-panel,context-menu,dashboard-overlays, andui-layout-storecommands/file.rswith file CRUD IPC commands,model/file.rsmodel updates, and 183-line integration test suite infile_commands.rsfile-commands.tsservice layer connecting frontend to Tauri backendTest Plan
cargo test --locked --manifest-path src-tauri/Cargo.tomlnpm run typecheck🤖 Generated with TiyCode