feat(workspace): support workspace tab (#656)#657
feat(workspace): support workspace tab (#656)#657Tabll wants to merge 1 commit intoValueCell-ai:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new “Workspace” feature that lets users browse per-agent workspace directories and preview common file types from within the app, wired into the sidebar navigation and Electron host API.
Changes:
- Add a new Workspace page with file tree browsing and file preview (image/code/markdown/html).
- Add Electron host API routes to list agents, return workspace file trees, and return file contents.
- Add i18n strings + hook up the new route in the app router and sidebar.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pages/Workspace/index.tsx | New UI for browsing workspace tree and previewing files. |
| electron/api/routes/workspace.ts | New host API endpoints for agents/tree/file preview data. |
| electron/api/server.ts | Registers the new workspace route handler. |
| src/App.tsx | Adds /workspace route. |
| src/components/layout/Sidebar.tsx | Adds Workspace nav item. |
| src/i18n/index.ts | Registers workspace namespace in i18n resources. |
| src/i18n/locales/en/workspace.json | English strings for Workspace page. |
| src/i18n/locales/zh/workspace.json | Chinese strings for Workspace page. |
| src/i18n/locales/ja/workspace.json | Japanese strings for Workspace page. |
| src/i18n/locales/en/common.json | Adds sidebar label for Workspace. |
| src/i18n/locales/zh/common.json | Adds sidebar label for Workspace. |
| src/i18n/locales/ja/common.json | Adds sidebar label for Workspace. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ); | ||
| } | ||
|
|
||
| const fileName = filePath.split('/').pop() || filePath; |
There was a problem hiding this comment.
fileName is derived via filePath.split('/'), which won’t work correctly if the backend returns Windows-style \ separators (or if paths ever include backslashes). Use a separator-agnostic split (e.g. /[\\/]/) or a shared basename helper.
| const fileName = filePath.split('/').pop() || filePath; | |
| const fileName = filePath.split(/[\\/]/).pop() || filePath; |
| const fetchTree = useCallback(async (agentId: string) => { | ||
| setTreeLoading(true); | ||
| setError(null); | ||
| try { | ||
| const result = await hostApiFetch<{ | ||
| success: boolean; | ||
| tree: FileTreeNode[]; | ||
| workspace: string; | ||
| error?: string; | ||
| }>(`/api/workspace/tree?agent=${encodeURIComponent(agentId)}`); | ||
| if (result.success) { | ||
| setFileTree(result.tree); | ||
| setWorkspacePath(result.workspace); | ||
| } else { | ||
| setError(result.error || 'Failed to load workspace'); | ||
| setFileTree([]); | ||
| } | ||
| } catch (err) { | ||
| setError(String(err)); | ||
| setFileTree([]); | ||
| } finally { | ||
| setTreeLoading(false); | ||
| } |
There was a problem hiding this comment.
Potential race condition: fetchTree(agentId) updates fileTree/workspacePath/error when it resolves, even if the user has already switched to a different agent. This can show the wrong tree for the selected agent. Consider tracking a request id / abort signal and only applying results if it matches the latest selected agent.
| const fetchFileContent = useCallback(async (agentId: string, filePath: string) => { | ||
| setFileLoading(true); | ||
| try { | ||
| const result = await hostApiFetch<FileContent>( | ||
| `/api/workspace/file?agent=${encodeURIComponent(agentId)}&path=${encodeURIComponent(filePath)}` | ||
| ); | ||
| setFileContent(result); | ||
| } catch (err) { | ||
| setFileContent({ success: false, fileType: 'binary', error: String(err) }); | ||
| } finally { | ||
| setFileLoading(false); | ||
| } | ||
| }, []); |
There was a problem hiding this comment.
Potential race condition: fetchFileContent sets fileContent when it resolves without checking that selectedAgentId + selectedFile are still the same. If the user changes selection quickly, an older request can overwrite the preview. Consider aborting in-flight requests or guarding state updates with a request token.
| function isPathWithinRoot(root: string, requestedPath: string): boolean { | ||
| const resolvedRoot = resolve(root); | ||
| const resolvedPath = resolve(root, requestedPath); | ||
| return resolvedPath.startsWith(resolvedRoot); | ||
| } |
There was a problem hiding this comment.
isPathWithinRoot uses resolvedPath.startsWith(resolvedRoot), which is vulnerable to path traversal via prefix matches (e.g. root /a/b will allow /a/b2, and ../b2 can resolve outside). Use a path-separator aware check (e.g. resolvedPath === resolvedRoot or resolvedPath.startsWith(resolvedRoot + sep)) or compare relative(resolvedRoot, resolvedPath) to ensure it doesn’t start with .. and isn’t absolute.
|
|
||
| if (entry.isDirectory()) { | ||
| const children = await buildFileTree(fullPath, rootPath, depth + 1, maxDepth); | ||
| nodes.push({ | ||
| name: entry.name, | ||
| path: relativePath, | ||
| type: 'directory', | ||
| children, | ||
| }); | ||
| } else { | ||
| nodes.push({ | ||
| name: entry.name, | ||
| path: relativePath, |
There was a problem hiding this comment.
relative(rootPath, fullPath) will return platform-specific separators (e.g. \ on Windows). The renderer treats paths as /-separated (e.g. split('/')), which can break filename extraction and display. Consider normalizing API paths to POSIX separators (e.g. replace \\ with /) before returning them, and/or standardize on / across both tree and file endpoints.
| if (entry.isDirectory()) { | |
| const children = await buildFileTree(fullPath, rootPath, depth + 1, maxDepth); | |
| nodes.push({ | |
| name: entry.name, | |
| path: relativePath, | |
| type: 'directory', | |
| children, | |
| }); | |
| } else { | |
| nodes.push({ | |
| name: entry.name, | |
| path: relativePath, | |
| const apiPath = relativePath.replace(/\\/g, '/'); | |
| if (entry.isDirectory()) { | |
| const children = await buildFileTree(fullPath, rootPath, depth + 1, maxDepth); | |
| nodes.push({ | |
| name: entry.name, | |
| path: apiPath, | |
| type: 'directory', | |
| children, | |
| }); | |
| } else { | |
| nodes.push({ | |
| name: entry.name, | |
| path: apiPath, |
| for (const rule of rules) { | ||
| for (const keyword of rule.keywords) { | ||
| const regex = new RegExp(`\\b(${keyword})\\b`, 'g'); | ||
| result = result.replace(regex, (_, kw) => hold(`<span class="${rule.className}">${kw as string}</span>`)); | ||
| } | ||
| } |
There was a problem hiding this comment.
highlightCode does per-line, per-keyword regex replacement (creating a new RegExp for every keyword on every line). Combined with the backend allowing up to 5MB text previews, this can easily freeze the UI on large source files. Consider limiting syntax highlighting to smaller files / first N KB, and/or precompiling keyword regexes (single alternation) instead of nested loops.
| <button | ||
| className={cn( | ||
| 'flex w-full items-center gap-1.5 rounded-lg px-2 py-1.5 text-[13px] hover:bg-black/5 dark:hover:bg-white/5 transition-colors', | ||
| 'text-foreground/80' | ||
| )} | ||
| style={{ paddingLeft: `${depth * 16 + 8}px` }} | ||
| onClick={() => setExpanded(!expanded)} |
There was a problem hiding this comment.
The directory toggle button doesn’t expose expanded/collapsed state to assistive tech. Consider adding aria-expanded (and optionally aria-controls) and type="button" to prevent unintended form submits if this component is ever used inside a <form>.
| <button | |
| className={cn( | |
| 'flex w-full items-center gap-1.5 rounded-lg px-2 py-1.5 text-[13px] hover:bg-black/5 dark:hover:bg-white/5 transition-colors', | |
| 'text-foreground/80' | |
| )} | |
| style={{ paddingLeft: `${depth * 16 + 8}px` }} | |
| onClick={() => setExpanded(!expanded)} | |
| <button | |
| type="button" | |
| className={cn( | |
| 'flex w-full items-center gap-1.5 rounded-lg px-2 py-1.5 text-[13px] hover:bg-black/5 dark:hover:bg-white/5 transition-colors', | |
| 'text-foreground/80' | |
| )} | |
| style={{ paddingLeft: `${depth * 16 + 8}px` }} | |
| onClick={() => setExpanded(!expanded)} | |
| aria-expanded={expanded} |
| const fullPath = join(workspacePath, normalizedRelPath); | ||
| const fileStat = await stat(fullPath); | ||
|
|
||
| if (!fileStat.isFile()) { | ||
| sendJson(res, 400, { success: false, error: 'Not a file' }); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
The file read path check uses stat(fullPath), which follows symlinks. A symlink inside the workspace that points outside the workspace root would bypass the traversal protection and allow reading arbitrary files. Consider rejecting symlinks via lstat (and/or resolving realpath for both root and file and re-validating containment).
| // Links | ||
| result = result.replace(/\[([^\]]+)\]\(([^)]+)\)/g, '<a href="$2" class="text-blue-500 underline" target="_blank" rel="noopener noreferrer">$1</a>'); |
There was a problem hiding this comment.
renderInlineMarkdown turns markdown links into <a href="..."> without validating/sanitizing the URL scheme. A workspace markdown file containing [x](javascript:...) would generate an executable link in the renderer. Restrict to safe schemes (e.g. http/https/mailto) or sanitize URLs before embedding them.
| // Links | |
| result = result.replace(/\[([^\]]+)\]\(([^)]+)\)/g, '<a href="$2" class="text-blue-500 underline" target="_blank" rel="noopener noreferrer">$1</a>'); | |
| // Links (only allow safe URL schemes) | |
| result = result.replace(/\[([^\]]+)\]\(([^)]+)\)/g, (match, linkText, rawUrl) => { | |
| const url = String(rawUrl).trim(); | |
| const lowerUrl = url.toLowerCase(); | |
| // Allow http, https, mailto, and relative/anchor URLs without a scheme. | |
| const isHttp = lowerUrl.startsWith('http:') || lowerUrl.startsWith('https:'); | |
| const isMailto = lowerUrl.startsWith('mailto:'); | |
| // Heuristic for "no explicit scheme": no ":" before "/", "?", or "#" | |
| const firstColon = lowerUrl.indexOf(':'); | |
| const firstSlash = lowerUrl.indexOf('/'); | |
| const firstQuery = lowerUrl.indexOf('?'); | |
| const firstHash = lowerUrl.indexOf('#'); | |
| const firstTerminator = [firstSlash, firstQuery, firstHash] | |
| .filter((idx) => idx !== -1) | |
| .reduce((min, idx) => (min === -1 || idx < min ? idx : min), -1); | |
| const hasScheme = | |
| firstColon !== -1 && (firstTerminator === -1 || firstColon < firstTerminator); | |
| const isRelative = !hasScheme; | |
| if (isHttp || isMailto || isRelative) { | |
| return `<a href="${url}" class="text-blue-500 underline" target="_blank" rel="noopener noreferrer">${linkText}</a>`; | |
| } | |
| // Unsafe URL scheme: render only the link text without a clickable link. | |
| return linkText; | |
| }); |
| // Markdown preview | ||
| if (fileContent.fileType === 'text' && fileContent.language === 'markdown' && fileContent.content) { | ||
| const htmlContent = renderMarkdownToHtml(fileContent.content); | ||
| return ( | ||
| <div className="flex flex-1 flex-col overflow-hidden"> | ||
| <div className="border-b border-black/10 dark:border-white/10 px-4 py-2.5 text-[13px] text-foreground/60 flex items-center gap-2 shrink-0 bg-black/[0.02] dark:bg-white/[0.02]"> | ||
| <FileText className="h-4 w-4" /> | ||
| <span className="truncate font-medium">{filePath}</span> | ||
| {fileContent.size && ( | ||
| <span className="ml-auto text-[12px] shrink-0 text-foreground/40">{formatFileSize(fileContent.size)}</span> | ||
| )} | ||
| </div> | ||
| <div | ||
| className="flex-1 overflow-auto px-8 py-6 prose prose-sm dark:prose-invert max-w-none" | ||
| dangerouslySetInnerHTML={{ __html: htmlContent }} | ||
| /> |
There was a problem hiding this comment.
Markdown rendering uses dangerouslySetInnerHTML with a custom parser (renderMarkdownToHtml). The repo already uses react-markdown + remark-gfm elsewhere (e.g. chat) which avoids raw HTML injection risks and edge cases. Consider reusing that approach here to reduce XSS surface area and maintenance burden.
Summary
增加工作空间显示的功能
支持切换多agent的工作空间目录
支持预览图片、代码、Markdown、html网页等文件
Related Issue(s)
#656
Type of Change
Validation
基本功能:

文件预览:



图片:
代码:
网页:
Checklist