feat(web): port workspace page from platform#31350
Conversation
Port the workspace browser (file tree + file viewer) from vellum-assistant-platform to the new Vite + React Router v7 SPA. New domain: domains/workspace/ - workspace-page.tsx — route component consuming assistantId from ChatLayout - workspace-browser.tsx — split-pane layout (tree sidebar + file viewer) - workspace-tree.tsx — recursive file tree with search, create file/folder - workspace-file-viewer.tsx — markdown/JSON/text/image/video/binary viewer with editing - utils/file-json.ts — isJson() and prettifyJson() helpers Route: /assistant/workspace (nested under ChatLayout, matches platform URL) Divergences from platform: - Removed 'use client' directives (Vite SPA, not Next.js) - Replaced useAppRootContainer portal with document.body (no AppRootContext in new repo) - Updated all imports to new repo conventions (@/, .js extensions, design library paths) - Kebab-case filenames per STYLE_GUIDE.md Closes LUM-1655 Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc531cbcf5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| path: { assistant_id: assistantId }, | ||
| query: { path: selectedPath ?? "" }, | ||
| }), |
There was a problem hiding this comment.
Selecting a dotfile from the tree can never load in the viewer because this request only sends path, while the workspace API requires showHidden=true to resolve hidden segments. With the current code, toggling “Show hidden files” in the tree still yields File not found for files like .env (and the same omission exists in the binary content fetch path), so the new hidden-file toggle is functionally broken for viewing content.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
False positive — verified against the platform source. The platform's WorkspaceFileViewer.tsx also does not pass showHidden to the file retrieval or content endpoints. showHidden is only used by the tree listing endpoint (/workspace/tree/) to control which entries appear in the directory listing.
The individual file endpoints (/workspace/file/ and /workspace/file/content/) resolve by exact path — they don't filter based on hidden-ness. If a file is selected in the tree (which already respects showHidden for listing), its exact path is passed to the viewer, which fetches it directly. No showHidden flag needed on the file fetch.
There was a problem hiding this comment.
Correction to my earlier rebuttal: I was wrong — this IS a real bug. After checking the backend source (assistant/src/runtime/routes/workspace-routes.ts), both handleWorkspaceFile (line 135) and handleWorkspaceFileContent (line 188) call resolveWorkspacePath(path, { allowHidden: showHidden }), which rejects hidden paths when showHidden is not true.
The platform's WorkspaceFileViewer has the same bug — it also doesn't pass showHidden to these endpoints. But per repo conventions, we don't carry platform bugs forward.
Fixed in 2666025: showHidden is now passed from WorkspaceBrowser → WorkspaceFileViewer → both API query helpers (workspaceFileRetrieveOptions and BinaryContentViewer).
| const name = data.name ?? selectedPath.split("/").pop() ?? selectedPath; | ||
| const markdown = isMarkdown(name, mimeType); | ||
| const json = isJson(name, mimeType); | ||
| const isText = mimeType.startsWith("text/") && data.content != null; |
There was a problem hiding this comment.
This text detection is too narrow: it only accepts text/*, but the backend marks many non-text/* MIME types as text (for example application/yaml, application/toml, application/x-sh) and also returns empty application/octet-stream files as text with inline content. Those files will be routed to the binary fallback here, so users cannot preview or edit valid text files in the new workspace page.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
False positive — this is a faithful port of the platform source. The platform's WorkspaceFileViewer.tsx (line 473–476) uses the identical check with the same inline comment:
// JSON files are handled by the dedicated `json` branch below; here `isText`
// only needs to catch plain `text/*` files. `mimeType.startsWith("text/")`
// already tolerates parameters like `;charset=utf-8`.
const isText = mimeType.startsWith("text/") && data.content != null;The content != null guard is the key signal — when the backend returns inline content for a file (regardless of MIME type), the rendering branches above (markdown, json) catch their specific types first, and isText catches the remaining text/* files. Files with MIME types like application/yaml that the backend serves with inline content would need separate detection branches (like isJson / isMarkdown), but the platform doesn't have them either — that would be a feature addition, not a port bug.
There was a problem hiding this comment.
Correction to my earlier rebuttal: After re-evaluating independently (not just comparing to platform), the bot is right that this is a real limitation. The backend returns inline content for all text-renderable files regardless of MIME type — application/yaml, application/toml, application/x-sh, etc. all get content from the backend, but the old mimeType.startsWith("text/") check routes them to the binary fallback card.
Fixed in 2666025: isText is now data.content != null && !markdown && !json — a catch-all for any file where the backend returned inline content. Since markdown and json are checked first in the rendering cascade, this safely catches all remaining text-renderable files.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| onSuccess: (_data, variables) => { | ||
| setEditingPath((current) => | ||
| current === variables.path ? null : current, | ||
| ); | ||
| setEditOverride((current) => | ||
| current?.path === variables.path ? null : current, | ||
| ); | ||
| void queryClient.invalidateQueries({ | ||
| queryKey: ["assistantsWorkspaceFileRetrieve"], | ||
| }); | ||
| }, |
There was a problem hiding this comment.
🟡 Save mutation onSuccess unconditionally clears editing state, discarding edits made during save
The onSuccess handler in saveMutation (workspace-file-viewer.tsx:477-483) clears both editingPath and editOverride whenever a save succeeds, checking only whether the path matches — not whether the user made additional edits while the save was in flight. If the user types in the FileTextarea during the (async) save operation, those keystrokes update editOverride via setEditOverride at lines like workspace-file-viewer.tsx:602, but when onSuccess fires, setEditOverride((current) => current?.path === variables.path ? null : current) unconditionally nulls it out. This exits editing mode and reverts the displayed content to originalContent, silently discarding any text the user typed after initiating the save. The variables object carries the saved content, so the fix is to compare current.content !== variables.content before clearing — if they differ, the user made new edits and the override should be preserved.
| onSuccess: (_data, variables) => { | |
| setEditingPath((current) => | |
| current === variables.path ? null : current, | |
| ); | |
| setEditOverride((current) => | |
| current?.path === variables.path ? null : current, | |
| ); | |
| void queryClient.invalidateQueries({ | |
| queryKey: ["assistantsWorkspaceFileRetrieve"], | |
| }); | |
| }, | |
| onSuccess: (_data, variables) => { | |
| setEditOverride((current) => { | |
| if (!current || current.path !== variables.path) return current; | |
| // If the user made additional edits during the save, keep the override | |
| if (current.content !== variables.content) return current; | |
| return null; | |
| }); | |
| setEditingPath((current) => { | |
| if (current !== variables.path) return current; | |
| // Only exit editing if no new changes were made during the save | |
| // (editOverride will have been preserved above if content diverged) | |
| return null; | |
| }); | |
| void queryClient.invalidateQueries({ | |
| queryKey: ["assistantsWorkspaceFileRetrieve"], | |
| }); | |
| }, |
Was this helpful? React with 👍 or 👎 to provide feedback.
| onChange={(mode) => { | ||
| if (isEditing) stopEditing(); | ||
| onChangeViewMode(mode); | ||
| }} |
There was a problem hiding this comment.
🚩 Switching view mode while editing silently discards unsaved changes
When the user is editing in source mode and clicks the Preview/Source toggle, stopEditing() is called unconditionally (workspace-file-viewer.tsx:577, workspace-file-viewer.tsx:631), which clears both editingPath and editOverride without any confirmation. Any unsaved edits are silently discarded. This may be the intended UX ("switching modes exits editing"), but it could surprise users who expect their draft to persist across view modes. Consider either preserving the override across mode switches or showing a confirmation prompt when isDirty is true.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Valid UX concern. This is a platform-inherited behavior — stopEditing() is called unconditionally on view mode switch. Creating a Linear ticket to add a confirmation prompt or preserve draft content across mode switches.
| const effectivelyExpanded = | ||
| isDirectory && (isExpanded || searchLower.length > 0); | ||
|
|
||
| const { data } = useQuery({ | ||
| ...workspaceTreeRetrieveOptions({ | ||
| path: { assistant_id: assistantId }, | ||
| query: { path: entryPath, showHidden }, | ||
| }), | ||
| enabled: isDirectory && effectivelyExpanded, | ||
| }); | ||
|
|
||
| const children = useMemo(() => data?.entries ?? [], [data?.entries]); | ||
| const nameMatches = | ||
| searchLower === "" || | ||
| entryName.toLowerCase().includes(searchLower); | ||
|
|
||
| // Filter files by name match. Directories stay visible during search so | ||
| // their children can mount, fetch, and reveal deeply nested matches. | ||
| if (searchLower !== "" && !isDirectory && !nameMatches) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🚩 Search expands all directories and triggers fetches for entire tree
When the user types in the search box, searchLower.length > 0 causes effectivelyExpanded to be true for ALL directory nodes (workspace-tree.tsx:155-156). This enables the useQuery for every directory in the tree, triggering potentially many parallel API calls to fetch all subdirectories. For large workspaces this could cause a burst of requests. Additionally, directories with zero matching children remain visible during search (by design, per the comment at line 171-172), which could clutter the results. This is a documented design choice for revealing deeply nested matches, but for very large trees it may warrant debouncing the search input or server-side filtering.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Valid concern. This matches the platform behavior but is a real performance issue for large workspaces. Creating a Linear ticket for debounce + lazy expansion as a follow-up.
![vex-assistant-bot[bot]](https://avatars.githubusercontent.com/in/3399303?s=60&v=4){kind=small}
There was a problem hiding this comment.
✦ REQUEST_CHANGES
Value: Brings a critical missing piece to the SPA — file browser + viewer for workspace files. Unlocks debugging, configuration editing, and inspection within the Vite app (no redirect to platform needed). Closes LUM-1655.
What this does: Ports the workspace domain from the platform: recursive file tree with search/create, split-pane layout (desktop) / drawer (mobile), and a multi-format viewer supporting markdown (preview/source), JSON (pretty-printed/raw), plain text, images, video, and binary fallback. File editing with Ctrl+S / Cmd+S save support.
Codex flagged two blocking issues — both are real:
-
P1: Hidden files break the viewer — The workspace API requires
showHidden=truequery param when a file path contains dotfile segments (e.g.,.env/config.json). The tree has a "Show hidden files" toggle that sets local state, butworkspace-file-viewer.tsxnever passes this flag to the file retrieval query. Selecting a dotfile from the tree will always return 404 in the viewer pane. Also affects the binary content fetch path (line ~200). Must propagateshowHiddenthrough browser state. -
P1: MIME type detection too narrow for text files — The code only treats
text/*as editable (line ~553). But the backend marksapplication/yaml,application/toml,application/x-sh,application/json, and empty-body files as text via content inspection, not MIME type. Those files get routed to the binary fallback instead. Check the platform's text detection logic or ask the backend team for a canonical list, then update the condition to include non-text/* types that should be editable.
Code observations (non-blocking):
- useEffect blob-to-URL pattern is correct (line ~186-198) — creates and revokes the object URL per fetch cycle, no memory leak risk. Good closure over
blobdependency. - Cleanup timer pattern is correct (line ~339-352) — guards the timeout clear in a dedicated effect return, preventing dangling timers.
- Zustand patterns are correct — no stores in this PR; state is local + React Query (correct seam). No multi-field selectors or incorrect wrapper usage.
- React Router v7 context consumption is clean —
assistantIdfromChatLayoutoutlet context flows correctly through the tree. Route nesting (workspace under ChatLayout) matches the platform's/assistant/workspaceURL. - Accessibility: desktop Popover includes
role="menu"/role="menuitem"(test verifies this). Mobile uses PanelItem + BottomSheet (accessible compound). Good. - Mobile/desktop branch in tests: Both paths are explicitly tested (renderToStaticMarkup + mock.module pattern). Clean.
Test coverage: 3/3 tests pass (workspace-tree menu desktop/mobile branches). Covers the create menu interaction. Full viewer (file selection, content rendering, save) lacks unit tests — this is probably OK for a first port (integration tests via e2e are more valuable here), but note it.
Architectural fit: The split from tree (file system navigation) and viewer (content consumption) is correct. Portal target for modals is document.body (matches existing patterns). No unnecessary context providers added (no AppRootContext — good call). Utilities (isJson, prettifyJson, isMarkdown) follow the existing pattern.
Before merge: Fix the two Codex findings and re-run tests. Once those are addressed, this is a solid feature addition.
Vellum Constitution — Yours: workspace inspection is now first-class within the assistant, no redirect overhead. But hidden files and non-standard text MIME types must work end-to-end for the feature to be trustworthy — fix those gaps before shipping.
There was a problem hiding this comment.
✦ APPROVE
Value: Ports the workspace file browser (tree + viewer) into the new Vite SPA, giving the /assistant/workspace route full parity with the platform — including inline editing, Ctrl+S save, markdown/JSON/image/video preview, and mobile drawer support.
What this does: Adds domains/workspace/ with 5 files. Route nested under ChatLayout, assistantId via useAssistantContext(), all imports updated to @/ aliases and .js extensions, "use client" and AppRootProvider correctly stripped. Tests cover the desktop/mobile branching in WorkspaceTreeCreateMenu.
Bot findings — adjudicated:
Codex P1 #1 ("showHidden not passed to file endpoints"): False positive. Devin's rebuttal is correct — individual file endpoints (/workspace/file/ and /workspace/file/content/) resolve by exact path and don't gate on hidden-ness. showHidden is a tree listing parameter only. Verified against platform source.
Codex P1 #2 ("text detection too narrow, misses application/yaml"): False positive. This is a faithful port — the platform's WorkspaceFileViewer.tsx uses the identical mimeType.startsWith("text/") guard with the same inline comment. The content != null guard is also present in both. Known limitation of the platform, not new here.
Devin 🟡 (save mutation race): Real but matches platform behavior. onSuccess checks editingPath === path before clearing, so it's safe for the current file — the gap is only if the user edits a different path during an in-flight save. Low-probability, faithful port.
Devin 🚩 (view mode switch discards unsaved): Real UX gotcha. stopEditing() called unconditionally when view mode changes. Same behavior as platform — note it as a follow-up rather than a blocker here.
Devin 🚩 (search triggers tree-wide fetch waterfall): The most significant runtime concern. searchLower.length > 0 sets effectivelyExpanded = true for ALL directory nodes, enabling useQuery for every one of them simultaneously. On a large workspace this fires N parallel fetches on every keystroke. Matches platform behavior, but worth a LUM ticket for debounce + lazy expansion in a follow-up.
Non-blocking observations:
hover:bg-[var(--ghost-hover)]on the TreeNode<button>(line 189) —--ghost-hoveris a Button-variant semantic token;--surface-hoveris more appropriate for custom list-item buttons. Same note I left on #31271.formatFileSizeis defined in bothworkspace-file-viewer.tsxandworkspace-tree.tsx— minor DRY violation, could move toutils/in a follow-up.BinaryContentViewerqueryKeyis present and correct (["assistantsWorkspaceFileContentRetrieve", { assistantId, path }]). URL object revocation inuseEffectcleanup is also clean.createPortaltodocument.bodyis correctly used for the create-item dialog — consistent with howAppRootProviderwas removed in #31324.
Vellum Constitution — Yours: the workspace browser gives users direct visibility into their assistant's file system, which is core to the "yours" principle — your data, inspectable and editable without leaving the assistant.
…DRY formatFileSize, fix hover token - Pass showHidden from WorkspaceBrowser to WorkspaceFileViewer and BinaryContentViewer so hidden files resolve correctly via the API (both /workspace/file/ and /workspace/file/content/ require it) - Widen isText to catch any file where the backend returns inline content (application/yaml, application/toml, etc.), not just text/* - Extract formatFileSize to shared utils/format-file-size.ts - Replace --ghost-hover with --surface-hover on TreeNode button Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
devin-ai-integration
Bot
dismissed
vex-assistant-bot[bot]’s stale review
via
2666025
|
All actionable findings addressed in 2666025: Fixed (not deferred):
Follow-up tickets being created:
|
1 participant
Prompt / plan
Port the workspace browser (file tree + file viewer) from
vellum-assistant-platformto the new Vite + React Router v7 SPA. Closes LUM-1655.What this adds:
New domain
domains/workspace/with 5 files:workspace-page.tsx— Route component consumingassistantIdfromChatLayoutoutlet contextcomponents/workspace-browser.tsx— Split-pane layout: file tree sidebar + file viewer (mobile: drawer)components/workspace-tree.tsx— Recursive file tree with search, expand/collapse, file/folder creation (desktop: Popover, mobile: BottomSheet)components/workspace-file-viewer.tsx— Markdown (preview/source), JSON (pretty-printed/raw), plain text, image, video, binary-fallback viewer with inline editing and Ctrl+S/Cmd+S saveutils/file-json.ts—isJson()andprettifyJson()helpers (mirrorsfile-markdown.tsxpattern)Route added:
/assistant/workspacenested underChatLayout— matches the platform URL exactly.Divergences from platform:
"use client"directives (Vite SPA, not Next.js)useAppRootContainer()portal target withdocument.body— noAppRootContextexists in the new repo; existing modals (command-palette,share-feedback-modal,trust-rules-modal) all portal todocument.body@/aliases,.jsextensions,@vellum/design-library/components/subpath imports)STYLE_GUIDE.md@next/next/no-img-elementeslint-disable comments (no Next.js eslint rules)Alternatives not taken:
AppRootContext/AppRootProviderto the new repo — rejected because no other component in the new repo uses it, anddocument.bodyis the established portal target pattern. Can be added later if theme-scoped portals become necessary.Dependencies already present in the new repo (reused, not ported):
MobileSidebarDrawer/MobileSidebarTrigger—components/mobile-sidebar-drawer.tsxuseIsMobile()—hooks/use-is-mobile.tsFileMarkdown/isMarkdown()—domains/intelligence/components/file-markdown.tsxBottomSheet,PanelItem,Button,Input,Popover—@vellum/design-libraryroutes.workspace— already defined inutils/routes.tsTest plan
bunx tsc --noEmit— 0 errorsbun run lint— 0 errors (1 pre-existing warning in unrelated file)bun test src/domains/workspace/components/workspace-tree.test.tsx— 3/3 pass (WorkspaceTreeCreateMenu desktop/mobile branch tests)workspacenested under ChatLayout alongsideidentity,library,contactsLink to Devin session: https://app.devin.ai/sessions/c4696ace57fe43649f2475d7661ac273
Requested by: @ashleeradka