Add workspace chat tabs#348
Conversation
Co-Authored-By: Claude Opus 4.7 <[email protected]>
There was a problem hiding this comment.
Review mode: initial
Findings
-
Minor Missing changeset for user-visible feature — the PR adds workspace chat tabs, plus/close buttons, and new i18n strings. Per
CLAUDE.mdandCONTRIBUTING, user-visible changes require a changeset (pnpm changeset). The diff does not include one.
Suggested fix: runpnpm changesetwith a patch bump and a short description like "feat: add workspace chat tabs with fresh conversation support". -
Minor
chatWorkspaceKeyinSidebar.tsx(apps/desktop/src/renderer/src/components/Sidebar.tsx:70) usesglobalThis.navigator?.platformto decide case-sensitivity for path comparison. This is deprecated and unreliable — in some environments (headless, CI, sandboxed) it may be empty or incorrect, leading to inconsistent workspace grouping on Windows. The importedworkspacePathComparisonKeyfrom../lib/workspace-pathalready handles the platform check.
Suggested fix: remove the localchatWorkspaceKeywrapper and callworkspacePathComparisonKey(workspacePath)directly (the function takes a single argument perworkspace-path.ts:36). If the renderer version has a different signature, align it to the main process version or pass the platform from main via preload. -
Nit
ChatSessionTabBarinSidebar.tsx(~100 lines with tabs, close button, and modal) would be cleaner as its own file, e.g.,ChatSessionTabBar.tsx. Not a blocker but helps maintainability as chat-tab features grow. -
Nit
chatWorkspaceKeyduplicates the logic ofworkspacePathComparisonKey— it manually normalizes and case-folds. Remove it and use the shared function directly (see Minor finding above).
Summary
This PR implements workspace-scoped chat tabs: multiple designs (sessions) can share the same workspace directory, a + button creates a fresh conversation without copying history/snapshots, and the hub views collapse designs sharing a workspace into a single card. The main-process IPC is extended with a workspaceReuse flag to bypass the workspace-binding conflict check. The extraction of workspacePathComparisonKey into workspace-path.ts and its use in generation-ipc.ts is a good cross-cutting improvement. UI rendering is refactored to split the file-tab bar from the files-tab bar. No blockers or major issues; the missing changeset should be addressed before merging, and the navigator.platform usage should be hardened.
Testing
- New Vitest tests cover:
bindWorkspacewithallowExistingWorkspaceBinding,acquireInFlightWorkspaceGenerationWindows case folding,registerSnapshotsIpcfresh-conversation flow and conflict rejection,collapseWorkspaceDesignsworkspace dedup, and store actioncreateNewConversationForCurrentWorkspacewith clean UI state. - Existing tests pass locally per PR metadata.
- No E2E (Playwright) tests added for the chat-tab UI interactions (click, switch, close, new conversation). Consider adding a Playwright test for the sidebar tab row if this area becomes more interactive.
Open-CoDesign Bot
Co-Authored-By: Claude Opus 4.7 <[email protected]>
There was a problem hiding this comment.
Review mode: follow-up after new commits
Findings
No new issues. All previous findings have been addressed:
- ✅ Changeset added:
.changeset/workspace-chat-tabs.mdnow present (patchbumps for desktop and i18n). - ✅ ChatSessionTabBar extracted to
ChatSessionTabBar.tsxwith its own test file, removing inline code fromSidebar.tsx. - ✅
navigator.platformusage removed:ChatSessionTabBar.tsximportsworkspacePathComparisonKeyfrom../lib/workspace-pathwithout passing a platform argument, relying on the newisWindowsPath()heuristic. The previouschatWorkspaceKeyfunction inSidebar.tsxis gone. The renderer-sideworkspace-path.tsnow falls back to path-shape detection whenplatformisundefined, making it robust in headless/CI environments.
Residual observations
- The
collapseWorkspaceDesignsfunction inRecentTab.tsxduplicates the case-folding and trailing-slash logic thatworkspacePathComparisonKeyalready handles. This is intentional — the hub view operates onDesignshape without importing the lib function — and the duplication is minimal. Not a blocker.
Summary
The PR is now ready to merge. The workspace chat-tabs feature is well-implemented: IPC supports workspaceReuse: 'fresh-conversation', the renderer state creates clean conversations, the hub views collapse workspace-shared designs, and cross-cutting path normalization is consistent across main and renderer. No blockers or major issues remain.
Testing
- All new tests pass locally per PR description.
- Existing tests still pass.
- No E2E tests for sidebar tab interactions; acceptable for this scope.
Open-CoDesign Bot
Co-Authored-By: Claude Opus 4.7 <[email protected]>
There was a problem hiding this comment.
Review mode: follow-up after new commits
Findings
No new issues. The incremental changes since the previous review are all clean improvements:
- ✅
fallbackChatAfterCloseextracted and tested — the inline fallback logic is now a separate pure function with unit coverage for the active-chat-only fallback semantics. - ✅ Duplicate normalization removed in
workspaceGroupKey— now delegates toworkspacePathComparisonKeyinstead of inline lowercasing, eliminating the residual duplication noted in the previous review. - ✅ Root-path edge case covered — the new test ensures that
C:/andc:/root workspaces (which have no meaningful basename) are not incorrectly collapsed. - ✅ Changeset description updated — reflects the hub card deduplication behavior.
Summary
The PR is ready to merge. All prior findings have been resolved, and the follow-up commits are focused, well-tested, and improve consistency.
Testing
- New unit tests for
fallbackChatAfterClose, root path handling, and updatedworkspaceGroupKeyall pass. - No E2E tests added for the tab bar interactions; acceptable for this scope and consistent with project practices.
Open-CoDesign Bot
Summary
Test plan
pnpm --filter @open-codesign/desktop build.Principles