[codex] Show CLI workspaces in sidebar by default#4897
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR refactors sidebar workspace visibility and hidden-state handling: a new ChangesSidebar Workspace Visibility and Hidden-State Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
|
Ready to review this PR? Stage has broken it down into 4 individual chapters for you: 
Chapters generated by Stage for commit 50c136c on May 24, 2026 6:28am UTC. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 12 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.ts">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.ts:267">
P2: Section workspace ordering can regress because sidebar workspaces are no longer sorted by tab order before being pushed into `section.workspaces`.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| pendingTransaction: workspaceTransactionsById[workspace.id] ?? null, | ||
| })), | ||
| [hostsByMachineId, rawSidebarWorkspaces, workspaceTransactionsById], | ||
| rawSidebarWorkspaces |
There was a problem hiding this comment.
P2: Section workspace ordering can regress because sidebar workspaces are no longer sorted by tab order before being pushed into section.workspaces.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.ts, line 267:
<comment>Section workspace ordering can regress because sidebar workspaces are no longer sorted by tab order before being pushed into `section.workspaces`.</comment>
<file context>
@@ -209,41 +208,81 @@ export function useDashboardSidebarData() {
- pendingTransaction: workspaceTransactionsById[workspace.id] ?? null,
- })),
- [hostsByMachineId, rawSidebarWorkspaces, workspaceTransactionsById],
+ rawSidebarWorkspaces
+ .filter((workspace) =>
+ shouldIncludeSidebarWorkspace(workspace, currentUserId),
</file context>
Greptile SummaryThis PR makes CLI-created worktree workspaces appear in the dashboard sidebar by default, without requiring explicit local sidebar state. It also updates all delete/remove flows to write
Confidence Score: 4/5The core two-stage filtering logic and hidden-marker approach are sound; the main risks are narrow edge cases in The new default-visibility flow correctly handles the create, show, hide, and re-hide lifecycle for CLI workspaces. The two edge-case observations do not affect normal user flows today: cross-project workspace sidebar assignment is not supported in the UI, and all workspaces should have a
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/sidebarDefaultVisibility.ts | New pure helper module with shouldIncludeSidebarWorkspace and buildSidebarProjects; logic and tests look correct |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.ts | Query rearchitected to drive visibility from v2Workspaces with left-join on local state; two-stage filtering (shouldIncludeSidebarWorkspace → getVisibleSidebarWorkspaces) is correct |
| apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts | writeHiddenWorkspaceSidebarState factored out; removeProjectFromSidebar now writes hidden markers for all workspaces of a project, including those without existing local state; see comment on cross-project edge case |
| apps/desktop/src/renderer/commandPalette/modules/workspace/commands.tsx | Delete command now guarded by projectId truthiness in addition to !isMain; silently suppresses delete when projectId is absent |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/sidebarDefaultVisibility.test.ts | Comprehensive tests for both helpers covering explicit state, default visibility, user identity, and project ordering; well-structured |
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
apps/desktop/src/renderer/commandPalette/modules/workspace/commands.tsx:71-72
**Delete command silently disappears when `projectId` is falsy**
The new guard `if (!isMain && projectId)` hides the delete command whenever `workspace.projectId` is absent. Since `DeleteWorkspaceTarget.projectId` is now a required (non-optional) `string`, a workspace that somehow lacks a `projectId` would simply have no delete entry in the command palette with no feedback. Consider falling back to `workspaceProjectId` or asserting with a log so the omission is observable.
### Issue 2 of 2
apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts:500-507
**`removeProjectFromSidebar` may clobber local state for workspaces pinned under a different project**
The new `workspaceIds` set now includes every workspace whose `workspace.projectId === projectId` (from `v2Workspaces`), not just those whose `sidebarState.projectId === projectId`. If a workspace's canonical project is `projectId` but was manually pinned to a *different* project in the sidebar (`sidebarState.projectId` ≠ `projectId`), calling `writeHiddenWorkspaceSidebarState` will overwrite its local state — changing `sidebarState.projectId` to `projectId` and setting `isHidden: true` — silently removing it from the other project's visible sidebar lane. Cross-project sidebar assignment is not currently available in the UI, so this is theoretical today, but defensive code could skip workspaces whose existing local state already references a different project.
Reviews (1): Last reviewed commit: "Show CLI workspaces in sidebar by defaul..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const projectId = workspace.projectId; | ||
| if (!isMain && projectId) { |
There was a problem hiding this comment.
projectId is falsy
The new guard if (!isMain && projectId) hides the delete command whenever workspace.projectId is absent. Since DeleteWorkspaceTarget.projectId is now a required (non-optional) string, a workspace that somehow lacks a projectId would simply have no delete entry in the command palette with no feedback. Consider falling back to workspaceProjectId or asserting with a log so the omission is observable.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/commandPalette/modules/workspace/commands.tsx
Line: 71-72
Comment:
**Delete command silently disappears when `projectId` is falsy**
The new guard `if (!isMain && projectId)` hides the delete command whenever `workspace.projectId` is absent. Since `DeleteWorkspaceTarget.projectId` is now a required (non-optional) `string`, a workspace that somehow lacks a `projectId` would simply have no delete entry in the command palette with no feedback. Consider falling back to `workspaceProjectId` or asserting with a log so the omission is observable.
How can I resolve this? If you propose a fix, please make it concise.| const workspaceIds = new Set([ | ||
| ...Array.from(collections.v2WorkspaceLocalState.state.values()) | ||
| .filter((item) => item.sidebarState.projectId === projectId) | ||
| .map((item) => item.workspaceId), | ||
| ...Array.from(collections.v2Workspaces.state.values()) | ||
| .filter((item) => item.projectId === projectId) | ||
| .map((item) => item.id), | ||
| ]); |
There was a problem hiding this comment.
removeProjectFromSidebar may clobber local state for workspaces pinned under a different project
The new workspaceIds set now includes every workspace whose workspace.projectId === projectId (from v2Workspaces), not just those whose sidebarState.projectId === projectId. If a workspace's canonical project is projectId but was manually pinned to a different project in the sidebar (sidebarState.projectId ≠ projectId), calling writeHiddenWorkspaceSidebarState will overwrite its local state — changing sidebarState.projectId to projectId and setting isHidden: true — silently removing it from the other project's visible sidebar lane. Cross-project sidebar assignment is not currently available in the UI, so this is theoretical today, but defensive code could skip workspaces whose existing local state already references a different project.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts
Line: 500-507
Comment:
**`removeProjectFromSidebar` may clobber local state for workspaces pinned under a different project**
The new `workspaceIds` set now includes every workspace whose `workspace.projectId === projectId` (from `v2Workspaces`), not just those whose `sidebarState.projectId === projectId`. If a workspace's canonical project is `projectId` but was manually pinned to a *different* project in the sidebar (`sidebarState.projectId` ≠ `projectId`), calling `writeHiddenWorkspaceSidebarState` will overwrite its local state — changing `sidebarState.projectId` to `projectId` and setting `isHidden: true` — silently removing it from the other project's visible sidebar lane. Cross-project sidebar assignment is not currently available in the UI, so this is theoretical today, but defensive code could skip workspaces whose existing local state already references a different project.
How can I resolve this? If you propose a fix, please make it concise.hideWorkspaceInSidebar now resolves projectId itself from v2WorkspaceLocalState (preserving explicit reparenting) or v2Workspaces. Removes the projectId arg from DeleteWorkspaceTarget, DeleteTarget, WorkspaceMissingWorktreeStateProps, and the command palette intent payload. Also deletes the now-unused removeWorkspaceFromSidebar — every caller switched to hideWorkspaceInSidebar in the previous commit.
TanStack DB joins only accept a single eq() — the (org, host, user) and() in the leftJoin against v2UsersHosts threw "Join condition must be an equality expression" at runtime, taking down the sidebar. Replace with a separate v2UsersHosts live query filtered by userId, then check (orgId, hostId) membership via Set in the visibility filter. Same pattern useAccessibleV2Workspaces already uses.
1 participant
Summary
Why
The sidebar was previously driven by local sidebar state rows. Workspaces created through the CLI or automations can exist in synced workspace data without a local sidebar row, so they stayed out of the sidebar until something explicitly pinned them. Making missing local state default to visible fixes that, but delete and remove flows also needed hidden markers so intentional removal still wins over the default.
Validation
bun test apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/sidebarDefaultVisibility.test.tsbun run lintbun --filter @superset/desktop typecheckgit diff --checkSummary by cubic
Show CLI- and automation-created workspaces in the sidebar by default, and auto-add their projects when needed. Deletions/removals now write hidden markers so items stay gone after sync.
New Features
Bug Fixes
v2UsersHostsjoin with a user-scoped query to prevent sidebar crashes; filter access by(orgId, hostId).hideWorkspaceInSidebareverywhere and dropremoveWorkspaceFromSidebar; default visibility and project derivation are factored into a tested helper.Written for commit 50c136c. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Refactor
Tests