feat: add ChatUI workspace file browser - #9432
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
astrbot-docs | fdb7098 | Commit Preview URL Branch Preview URL |
Jul 28 2026, 03:51 PM |
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The workspace root resolution logic (including
fallback_umoandos.path.realpath/normcasehandling) is repeated acrosslist_workspace_files,get_workspace_file, andget_workspace_file_location; consider extracting this into a single private helper to avoid divergence in future changes. - In
downloadSelectedFileon the frontend, you always derive the download filename from the last path segment; consider preferring a server-provided filename from theContent-Dispositionheader when available so that downloads respect backend naming.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The workspace root resolution logic (including `fallback_umo` and `os.path.realpath`/`normcase` handling) is repeated across `list_workspace_files`, `get_workspace_file`, and `get_workspace_file_location`; consider extracting this into a single private helper to avoid divergence in future changes.
- In `downloadSelectedFile` on the frontend, you always derive the download filename from the last path segment; consider preferring a server-provided filename from the `Content-Disposition` header when available so that downloads respect backend naming.
## Individual Comments
### Comment 1
<location path="dashboard/src/components/chat/WorkspaceFilesPanel.vue" line_range="234" />
<code_context>
+const props = defineProps<[{
</code_context>
<issue_to_address>
**nitpick:** `projectTitle` prop is unused in the panel component
`WorkspaceFilesPanel` declares `projectTitle` but never uses it. Either remove this prop to simplify the API, or wire it into the UI (e.g., header or dialog title) if it’s intended to be displayed.
</issue_to_address>
### Comment 2
<location path="astrbot/dashboard/services/chatui_project_service.py" line_range="147" />
<code_context>
) -> list[dict]:
return await self.get_project_sessions(username, project_id)
+ async def list_workspace_files(
+ self,
+ username: str,
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting shared workspace-root resolution, path normalization, and strict file traversal into reusable helpers and standardizing on Path to simplify these workspace methods.
You can reduce complexity and duplication without changing behavior by extracting the common pieces into small helpers and standardizing on `Path` as much as possible.
### 1. Centralize workspace root resolution
Both `list_workspace_files` and `get_workspace_file_location` repeat the same logic. Move it into a private helper that returns a `Path`:
```python
from pathlib import Path
class ChatUIProjectService:
...
async def _get_workspace_root(self, username: str, project_id: str) -> Path:
project = await self._get_owned_project(username, project_id)
fallback_umo = f"webchat:FriendMessage:webchat!{project.creator}!default"
root = resolve_project_workspace_root(project, fallback_umo=fallback_umo)
# Preserve normcase/realpath semantics while returning a Path
return Path(os.path.normcase(os.path.realpath(root)))
```
Then reuse it:
```python
async def list_workspace_files(...):
workspace_root = await self._get_workspace_root(username, project_id)
...
# instead of workspace_root_path / os.path.join
target_dir = (workspace_root / normalized_path).resolve()
...
async def get_workspace_file_location(...):
workspace_root = await self._get_workspace_root(username, project_id)
...
target_file = workspace_root
...
```
This removes the repeated `project` / `fallback_umo` / `resolve_project_workspace_root` / `normcase` / `realpath` chain and makes any future change to those invariants localized.
### 2. Normalize path validation into a single helper
You currently validate relative paths slightly differently in `list_workspace_files` and `get_workspace_file_location`. A shared helper clarifies the invariant and reduces drift:
```python
def _normalize_workspace_relative_path(
self,
raw_path: str | None,
*,
allow_empty: bool,
) -> Path:
raw = (raw_path or "").strip()
normalized = Path(raw.replace("\\", "/") or ".")
if not raw and not allow_empty:
raise ChatUIProjectServiceError("Invalid workspace path")
if normalized.is_absolute() or ".." in normalized.parts:
raise ChatUIProjectServiceError("Invalid workspace path")
return normalized
```
Then:
```python
async def list_workspace_files(...):
workspace_root = await self._get_workspace_root(username, project_id)
normalized_path = self._normalize_workspace_relative_path(
relative_path,
allow_empty=True,
)
...
async def get_workspace_file_location(...):
workspace_root = await self._get_workspace_root(username, project_id)
normalized_path = self._normalize_workspace_relative_path(
relative_path,
allow_empty=False,
)
...
```
This keeps the same constraints but removes duplicated checks and string handling.
### 3. Encapsulate the manual traversal in a helper
Even if you keep the conservative segment-by-segment traversal and symlink policy, you can move it into a narrowly scoped helper used by `get_workspace_file_location`. That makes the public method short and keeps all the tricky logic in one place:
```python
def _resolve_workspace_file_strict(self, root: Path, normalized_path: Path) -> Path:
# Match server-enumerated entries so request values never form a file path.
target = root
parts = normalized_path.parts
for index, part in enumerate(parts):
try:
children = {entry.name: entry for entry in target.iterdir()}
except OSError as exc:
raise ChatUIProjectServiceError("Workspace file cannot be read") from exc
child = children.get(part)
if child is None:
raise ChatUIProjectServiceError("Workspace file not found")
if child.is_symlink():
# Preserve current behavior: reject any symlink, even if inside root.
if not child.resolve(strict=False).is_relative_to(root):
raise ChatUIProjectServiceError(
"Workspace path escapes project directory"
)
raise ChatUIProjectServiceError("Workspace file not found")
if index < len(parts) - 1 and not child.is_dir():
raise ChatUIProjectServiceError("Workspace file not found")
target = child
if not parts or not target.is_file():
raise ChatUIProjectServiceError("Workspace file not found")
return target
```
Usage in `get_workspace_file_location`:
```python
async def get_workspace_file_location(...):
workspace_root = await self._get_workspace_root(username, project_id)
normalized_path = self._normalize_workspace_relative_path(
relative_path,
allow_empty=False,
)
target_file = self._resolve_workspace_file_strict(workspace_root, normalized_path)
return workspace_root, target_file
```
This preserves the exact traversal semantics and error messages, but the public method becomes easy to read and reason about.
### 4. Reduce mixed `os.path` / `Path` usage in listing
You can simplify `list_workspace_files` by leaning on `Path` instead of mixing in `os.path`:
```python
async def list_workspace_files(...):
workspace_root = await self._get_workspace_root(username, project_id)
normalized_path = self._normalize_workspace_relative_path(
relative_path,
allow_empty=True,
)
target_dir = (workspace_root / normalized_path).resolve()
# Equivalent of your normcase/startswith check:
if target_dir != workspace_root and not target_dir.is_relative_to(workspace_root):
raise ChatUIProjectServiceError("Workspace path escapes project directory")
if not workspace_root.exists() and normalized_path == Path("."):
return {"path": "", "entries": []}
if not target_dir.is_dir():
raise ChatUIProjectServiceError("Workspace directory not found")
...
# existing iterdir/stat loop unchanged
```
If you need `normcase` specifically for case-insensitive roots, you can still apply it when creating `workspace_root` in `_get_workspace_root` and keep the rest of the code in terms of `Path`/`is_relative_to`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| const props = defineProps<{ | ||
| modelValue: boolean; | ||
| projectId: string; | ||
| projectTitle?: string; |
There was a problem hiding this comment.
nitpick: projectTitle prop is unused in the panel component
WorkspaceFilesPanel declares projectTitle but never uses it. Either remove this prop to simplify the API, or wire it into the UI (e.g., header or dialog title) if it’s intended to be displayed.
| ) -> list[dict]: | ||
| return await self.get_project_sessions(username, project_id) | ||
|
|
||
| async def list_workspace_files( |
There was a problem hiding this comment.
issue (complexity): Consider extracting shared workspace-root resolution, path normalization, and strict file traversal into reusable helpers and standardizing on Path to simplify these workspace methods.
You can reduce complexity and duplication without changing behavior by extracting the common pieces into small helpers and standardizing on Path as much as possible.
1. Centralize workspace root resolution
Both list_workspace_files and get_workspace_file_location repeat the same logic. Move it into a private helper that returns a Path:
from pathlib import Path
class ChatUIProjectService:
...
async def _get_workspace_root(self, username: str, project_id: str) -> Path:
project = await self._get_owned_project(username, project_id)
fallback_umo = f"webchat:FriendMessage:webchat!{project.creator}!default"
root = resolve_project_workspace_root(project, fallback_umo=fallback_umo)
# Preserve normcase/realpath semantics while returning a Path
return Path(os.path.normcase(os.path.realpath(root)))Then reuse it:
async def list_workspace_files(...):
workspace_root = await self._get_workspace_root(username, project_id)
...
# instead of workspace_root_path / os.path.join
target_dir = (workspace_root / normalized_path).resolve()
...
async def get_workspace_file_location(...):
workspace_root = await self._get_workspace_root(username, project_id)
...
target_file = workspace_root
...This removes the repeated project / fallback_umo / resolve_project_workspace_root / normcase / realpath chain and makes any future change to those invariants localized.
2. Normalize path validation into a single helper
You currently validate relative paths slightly differently in list_workspace_files and get_workspace_file_location. A shared helper clarifies the invariant and reduces drift:
def _normalize_workspace_relative_path(
self,
raw_path: str | None,
*,
allow_empty: bool,
) -> Path:
raw = (raw_path or "").strip()
normalized = Path(raw.replace("\\", "/") or ".")
if not raw and not allow_empty:
raise ChatUIProjectServiceError("Invalid workspace path")
if normalized.is_absolute() or ".." in normalized.parts:
raise ChatUIProjectServiceError("Invalid workspace path")
return normalizedThen:
async def list_workspace_files(...):
workspace_root = await self._get_workspace_root(username, project_id)
normalized_path = self._normalize_workspace_relative_path(
relative_path,
allow_empty=True,
)
...
async def get_workspace_file_location(...):
workspace_root = await self._get_workspace_root(username, project_id)
normalized_path = self._normalize_workspace_relative_path(
relative_path,
allow_empty=False,
)
...This keeps the same constraints but removes duplicated checks and string handling.
3. Encapsulate the manual traversal in a helper
Even if you keep the conservative segment-by-segment traversal and symlink policy, you can move it into a narrowly scoped helper used by get_workspace_file_location. That makes the public method short and keeps all the tricky logic in one place:
def _resolve_workspace_file_strict(self, root: Path, normalized_path: Path) -> Path:
# Match server-enumerated entries so request values never form a file path.
target = root
parts = normalized_path.parts
for index, part in enumerate(parts):
try:
children = {entry.name: entry for entry in target.iterdir()}
except OSError as exc:
raise ChatUIProjectServiceError("Workspace file cannot be read") from exc
child = children.get(part)
if child is None:
raise ChatUIProjectServiceError("Workspace file not found")
if child.is_symlink():
# Preserve current behavior: reject any symlink, even if inside root.
if not child.resolve(strict=False).is_relative_to(root):
raise ChatUIProjectServiceError(
"Workspace path escapes project directory"
)
raise ChatUIProjectServiceError("Workspace file not found")
if index < len(parts) - 1 and not child.is_dir():
raise ChatUIProjectServiceError("Workspace file not found")
target = child
if not parts or not target.is_file():
raise ChatUIProjectServiceError("Workspace file not found")
return targetUsage in get_workspace_file_location:
async def get_workspace_file_location(...):
workspace_root = await self._get_workspace_root(username, project_id)
normalized_path = self._normalize_workspace_relative_path(
relative_path,
allow_empty=False,
)
target_file = self._resolve_workspace_file_strict(workspace_root, normalized_path)
return workspace_root, target_fileThis preserves the exact traversal semantics and error messages, but the public method becomes easy to read and reason about.
4. Reduce mixed os.path / Path usage in listing
You can simplify list_workspace_files by leaning on Path instead of mixing in os.path:
async def list_workspace_files(...):
workspace_root = await self._get_workspace_root(username, project_id)
normalized_path = self._normalize_workspace_relative_path(
relative_path,
allow_empty=True,
)
target_dir = (workspace_root / normalized_path).resolve()
# Equivalent of your normcase/startswith check:
if target_dir != workspace_root and not target_dir.is_relative_to(workspace_root):
raise ChatUIProjectServiceError("Workspace path escapes project directory")
if not workspace_root.exists() and normalized_path == Path("."):
return {"path": "", "entries": []}
if not target_dir.is_dir():
raise ChatUIProjectServiceError("Workspace directory not found")
...
# existing iterdir/stat loop unchangedIf you need normcase specifically for case-insensitive roots, you can still apply it when creating workspace_root in _get_workspace_root and keep the rest of the code in terms of Path/is_relative_to.
GitDiffSidebar (spcode) supersedes WorkspaceFilesPanel for workspace inspection, so the upstream entry point (AstrBotDevs#9432) is unused on this branch. Toggle v-if bound to false; component still mounted in Chat.vue and import kept, so future upstream rebases can revert in one line.
Summary
Why
ChatUI project users need a lightweight way to inspect files in the active project workspace without leaving the conversation view.
Impact
The project header now exposes a workspace-files toggle when a project is open. Repeated list, preview, and download requests are idempotent and do not modify workspace contents.
Validation
pnpm generate:apipnpm typecheckruff check astrbot/dashboard/api/chat_projects.py astrbot/dashboard/services/chatui_project_service.py tests/unit/test_chatui_project_service.pypytest tests/unit/test_chatui_project_service.py -q(13 passed)Summary by Sourcery
Add a workspace file browser to ChatUI projects, including backend APIs, frontend panel integration, and secure file access and download support.
New Features:
Enhancements:
Tests: