Manage workspaces using trpc/lowdb, implement recent projects#112
Manage workspaces using trpc/lowdb, implement recent projects#112saddlepaddle merged 3 commits intomainfrom
Conversation
WalkthroughThis PR introduces a comprehensive project and workspace management system for the desktop app. It adds a local database layer using lowdb, creates TRPC routers for projects and workspaces CRUD operations, provides React Query hooks for state management and cache invalidation, and builds new UI components while migrating existing components from local stores to hook-based interactions. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI Component
participant Hook as React Query Hook
participant TRPC as TRPC Router
participant DB as Database
participant Cache as React Query Cache
Note over UI,Cache: Opening a Project
UI->>Hook: Call mutation (openProject)
activate Hook
Hook->>TRPC: mutate() → trpc.projects.openProject
activate TRPC
TRPC->>UI: Show folder picker dialog
TRPC->>DB: Add/update recent project entry
TRPC-->>Hook: Return { success, path, name }
deactivate TRPC
Hook->>Cache: Invalidate workspaces queries
Hook->>UI: onSuccess callback
deactivate Hook
UI->>UI: Re-render with updated data
Note over UI,Cache: Creating a Workspace
UI->>Hook: Call mutation (createWorkspace)
activate Hook
Hook->>TRPC: mutate() → trpc.workspaces.create
activate TRPC
TRPC->>DB: Create workspace with order & timestamps
TRPC->>DB: Update lastActiveWorkspaceId setting
TRPC-->>Hook: Return created workspace
deactivate TRPC
Hook->>Cache: Invalidate all workspace queries
Hook->>UI: onSuccess callback
deactivate Hook
UI->>UI: Re-render workspace list
Note over UI,Cache: Deleting a Workspace
UI->>Hook: Call mutation (deleteWorkspace)
activate Hook
Hook->>TRPC: mutate({ id })
activate TRPC
TRPC->>DB: Remove workspace by ID
TRPC->>DB: Clean up orphaned recent projects
TRPC->>DB: Reassign lastActiveWorkspaceId if needed
TRPC-->>Hook: Return { success }
deactivate TRPC
Hook->>Cache: Invalidate all workspace queries
Hook->>UI: onSuccess callback
deactivate Hook
UI->>UI: Re-render & update active workspace
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (14)
apps/desktop/src/renderer/react-query/workspaces/useSetActiveWorkspace.ts (1)
1-23: Good implementation; optional parallel invalidationThe hook looks correct and consistent with other workspace mutations. If you care about tiny perf/latency wins, you could run the two invalidations in parallel:
await Promise.all([ utils.workspaces.getActive.invalidate(), utils.workspaces.getAll.invalidate(), ]);Not required, just a small optional tweak.
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/AddWorkspaceButton.tsx (1)
2-11: Consider guarding against duplicate clicks while create is pendingThe mutation wiring looks correct. To avoid users spamming the button and enqueueing multiple “New Workspace” creations, consider disabling the button while the mutation is in flight, e.g.:
const createWorkspace = useCreateWorkspace(); <Button variant="ghost" size="icon" onClick={handleAddWorkspace} disabled={createWorkspace.isPending} aria-label="Add new workspace" >Nice-to-have, not blocking.
Also applies to: 17-17
apps/desktop/src/shared/types.ts (1)
149-154: ClarifylastOpenedAtsemantics for RecentProjectThe
RecentProjectshape matches how the UI uses it, butlastOpenedAt: numberis a bit opaque. Consider documenting the units (e.g., UNIX ms since epoch) via a short comment or JSDoc to avoid confusion or inconsistent usage later.apps/desktop/src/renderer/react-query/workspaces/useUpdateWorkspace.ts (1)
1-22: Solid wrapper; consider guarding useronSuccessagainst invalidate failuresThe mutation wrapper correctly:
- Uses the same options type as the underlying mutation.
- Centralizes workspace cache invalidation before calling the user-provided
onSuccess.One refinement: if
utils.workspaces.invalidate()ever rejects, the user’sonSuccesswill be skipped. A smalltry/finallykeeps the user callback reliable:onSuccess: async (...args) => { try { await utils.workspaces.invalidate(); } finally { await options?.onSuccess?.(...args); } },This preserves your behavior while avoiding accidental coupling between cache invalidation success and the caller’s side effects.
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsx (1)
12-15: Guard width calculation for empty list and confirm workspace namingTwo small points here:
- Width calc when there are no workspaces
const availableWidth = containerWidth - ADD_BUTTON_WIDTH; const calculatedWidth = Math.max( MIN_WORKSPACE_WIDTH, Math.min(MAX_WORKSPACE_WIDTH, availableWidth / workspaces.length), );When
workspaces.length === 0,availableWidth / 0yieldsInfinity, and the math happens to clamp this toMAX_WORKSPACE_WIDTH. It works, but it’s non-obvious. A simple guard is clearer:if (workspaces.length === 0) { setWorkspaceWidth(MAX_WORKSPACE_WIDTH); return; }
- Confirm the
namefield on workspace<WorkspaceItem id={workspace.id} title={workspace.name} // ... />Assuming the workspace objects from your query expose
name(and not an oldertitlefield), this is correct. If any callers or backend types still usetitle, they’ll need aligning to avoid blank labels in the tabs.Also applies to: 39-43, 92-92
apps/desktop/src/renderer/screens/main/components/WorkspaceView/NewWorkspaceView/components/RecentSection/RecentSection.tsx (1)
1-38: RecentSection logic is sound; consider making height configurableThe component correctly:
- No-ops when there are no recents.
- Delegates open/remove behavior via
onOpenRecent/onRemoveRecent.- Uses
project.pathas a stable key.One small UX refinement:
className="h-[400px]"makes the scroll area fixed-height, which may feel cramped on small windows or overly constrained on large ones. Exposing the height as a prop or tying it to viewport/container size would make this section more adaptable if the layout needs to evolve.apps/desktop/src/renderer/react-query/workspaces/useReorderWorkspaces.ts (1)
14-16: Consider using broader invalidation for consistency.This hook invalidates only
utils.workspaces.getAll, while other workspace mutation hooks (useCreateWorkspace,useSetActiveWorkspace, etc.) invalidate all workspace queries usingutils.workspaces.invalidate(). This inconsistency could lead to stale data if other workspace-related queries exist.Apply this diff for consistency:
- await utils.workspaces.getAll.invalidate(); + await utils.workspaces.invalidate();Alternatively, if the specific invalidation is intentional for performance reasons, consider documenting why this hook differs from the others.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/NewWorkspaceView.tsx (1)
9-15: Factor shared success logic and clarify behavior whenactiveWorkspaceis missingBoth
openProjectandopenRecenthave the sameonSuccessbody. You can DRY this up and make the behavior aroundactiveWorkspacemore explicit:
- Extract a small helper that takes
{ success, path, name }and, ifsuccessandactiveWorkspaceare truthy, runsupdateWorkspace.mutateAsyncandaddTab.- Optionally log or otherwise handle the case where
result.success === truebutactiveWorkspaceisundefined(e.g. query still loading, workspace deleted), instead of silently no‑op’ing.Example refactor:
- const updateWorkspace = useUpdateWorkspace(); - - const openProject = useOpenProject({ - onSuccess: async (result) => { - if (result.success && activeWorkspace) { - // Update workspace in DB with path and wait for it to complete - await updateWorkspace.mutateAsync({ - id: activeWorkspace.id, - patch: { - path: result.path, - name: result.name, - }, - }); - - // Add a tab for the project (still using Zustand for now) - addTab(activeWorkspace.id); - } - }, - }); - - const openRecent = useOpenRecent({ - onSuccess: async (result) => { - if (result.success && activeWorkspace) { - // Update workspace in DB with path and wait for it to complete - await updateWorkspace.mutateAsync({ - id: activeWorkspace.id, - patch: { - path: result.path, - name: result.name, - }, - }); - - // Add a tab for the project (still using Zustand for now) - addTab(activeWorkspace.id); - } - }, - }); + const updateWorkspace = useUpdateWorkspace(); + + const handleProjectOpenSuccess = async (result: { + success: boolean; + path: string; + name: string; + }) => { + if (!result.success || !activeWorkspace) return; + + await updateWorkspace.mutateAsync({ + id: activeWorkspace.id, + patch: { + path: result.path, + name: result.name, + }, + }); + + addTab(activeWorkspace.id); + }; + + const openProject = useOpenProject({ + onSuccess: handleProjectOpenSuccess, + }); + + const openRecent = useOpenRecent({ + onSuccess: handleProjectOpenSuccess, + });Also applies to: 16-32, 34-50
apps/desktop/src/lib/trpc/routers/projects/projects.ts (2)
19-52: Consider bounding recents list size to avoid unbounded growth
openProjectappends todata.recentProjectsandgetRecentssimply returns all entries sorted. Over time this will grow without bound, which may not be desirable for a “recent projects” list.If you intend to show only a limited number (e.g. last 10–20):
- await db.update((data) => { + await db.update((data) => { const existingIndex = data.recentProjects.findIndex( (p) => p.path === path, ); if (existingIndex !== -1) { data.recentProjects[existingIndex].lastOpenedAt = Date.now(); } else { data.recentProjects.push({ path, name, lastOpenedAt: Date.now(), }); + // Keep only the N most recent entries + data.recentProjects + .sort((a, b) => b.lastOpenedAt - a.lastOpenedAt) + .splice(20); // or whatever max length you want } });Also applies to: 72-76
53-71: Clarify “not found” behavior and keep docs in sync with implementationTwo small points here:
openRecentandremoveRecentboth return success even if the path does not exist inrecentProjects. That’s fine if callers only ever pass paths fromgetRecents, but if you ever need to distinguish “not found” from “updated/removed”, consider returningsuccess: false(or an additional flag) when no matching entry is found.- The file-level comment mentions “workspace creation”, but this router currently only handles project selection and recents. It would be good to update the comment to reflect the actual responsibilities.
Also applies to: 77-87, 9-12
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx (1)
5-9: Reordering via mutation onhovermay be heavier than neededThe TRPC wiring for
setActiveanddeleteWorkspacelooks good, but for reordering you’re now callingreorderWorkspaces.mutateinside the DnDhoverhandler. That means each boundary crossed during a drag issues a mutation and a DB write.Consider an approach like:
- Keep the existing
hoverlogic purely local (e.g. update a local/Zustand ordering to keep drag feedback snappy).- Fire a single
reorderWorkspaces.mutate({ fromIndex, toIndex })in thedrophandler once the drag ends.That preserves the UX from the old implementation while avoiding multiple persistence calls per drag gesture.
Also applies to: 32-35, 47-55, 71-72, 94-97
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (2)
15-44: Computeorderinside thedb.updatecall to avoid stale reads
maxOrderis computed fromdb.data.workspacesbefore callingawait db.update(...). Ifdb.updatedoes not synchronously mutatedb.databefore returning its Promise, two concurrentcreatecalls could both see the samemaxOrderand assign duplicateordervalues, which would make ordering ambiguous.A safer pattern is to compute
maxOrderand construct the workspace inside thedb.updatecallback, then return it:create: publicProcedure .input( z.object({ name: z.string(), path: z.string().nullable().optional(), }), ) .mutation(async ({ input }) => { let created = null as (typeof db.data.workspaces)[number] | null; await db.update((data) => { const maxOrder = data.workspaces.length > 0 ? Math.max(...data.workspaces.map((w) => w.order)) : -1; const workspace = { id: nanoid(), name: input.name, path: input.path ?? null, order: maxOrder + 1, createdAt: Date.now(), updatedAt: Date.now(), lastOpenedAt: Date.now(), }; data.workspaces.push(workspace); data.settings.lastActiveWorkspaceId = workspace.id; created = workspace; }); return created!; });This keeps the read/modify sequence atomic with respect to other updates.
82-113: Consider decouplingupdatedAtfromlastOpenedAton generic updatesIn the update mutation, every patch (including simple renames or path changes) sets both
updatedAtandlastOpenedAttoDate.now().If you intend
lastOpenedAtto reflect when the workspace was last activated/used, it may be clearer to only set:
updatedAtinupdate- both
updatedAtandlastOpenedAtinsetActive(and any “open project” mutations)This keeps “edited” and “opened” timelines distinct:
// inside update workspace.updatedAt = Date.now(); // leave lastOpenedAt unchangedUp to you whether to keep current behavior or narrow it.
apps/desktop/src/main/lib/db/schemas.ts (1)
6-29: Be mindful of duplicated and diverging type definitions vsshared/typesThere are similarly named interfaces in
apps/desktop/src/shared/types.ts:
RecentProjectwith the same fields.TabandWorkspacewith different fields and timestamp types (strings vs numbers, additional props likerepoPath,worktrees, etc.).Having two
Workspace/Tabtypes with different shapes can be confusing and can drift over time. Consider:
- Renaming these DB-layer interfaces (e.g.,
DbWorkspace,DbTab,DbRecentProject), or- Importing and reusing the shared
RecentProjectwhere feasible, and clearly documenting where the DB types intentionally differ.This will reduce accidental misuse of the wrong type in main vs renderer code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (31)
apps/desktop/package.json(2 hunks)apps/desktop/src/lib/trpc/routers/index.ts(2 hunks)apps/desktop/src/lib/trpc/routers/projects/index.ts(1 hunks)apps/desktop/src/lib/trpc/routers/projects/projects.ts(1 hunks)apps/desktop/src/lib/trpc/routers/workspaces/index.ts(1 hunks)apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts(1 hunks)apps/desktop/src/main/index.ts(2 hunks)apps/desktop/src/main/lib/db/index.ts(1 hunks)apps/desktop/src/main/lib/db/schemas.ts(1 hunks)apps/desktop/src/renderer/react-query/projects/index.ts(1 hunks)apps/desktop/src/renderer/react-query/projects/useOpenProject.ts(1 hunks)apps/desktop/src/renderer/react-query/projects/useOpenRecent.ts(1 hunks)apps/desktop/src/renderer/react-query/projects/useRemoveRecent.ts(1 hunks)apps/desktop/src/renderer/react-query/workspaces/index.ts(1 hunks)apps/desktop/src/renderer/react-query/workspaces/useCreateWorkspace.ts(1 hunks)apps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.ts(1 hunks)apps/desktop/src/renderer/react-query/workspaces/useReorderWorkspaces.ts(1 hunks)apps/desktop/src/renderer/react-query/workspaces/useSetActiveWorkspace.ts(1 hunks)apps/desktop/src/renderer/react-query/workspaces/useUpdateWorkspace.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/AddWorkspaceButton.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx(5 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsx(3 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/NewWorkspaceView.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/NewWorkspaceView/components/RecentSection/RecentProjectItem.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/NewWorkspaceView/components/RecentSection/RecentSection.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/NewWorkspaceView/components/RecentSection/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/NewWorkspaceView/components/StartSection/StartSection.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/NewWorkspaceView/components/StartSection/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/NewWorkspaceView/components/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsx(1 hunks)apps/desktop/src/shared/types.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (17)
apps/desktop/src/renderer/react-query/projects/useRemoveRecent.ts (1)
apps/desktop/src/renderer/react-query/projects/index.ts (1)
useRemoveRecent(3-3)
apps/desktop/src/renderer/react-query/projects/useOpenRecent.ts (1)
apps/desktop/src/renderer/react-query/projects/index.ts (1)
useOpenRecent(2-2)
apps/desktop/src/renderer/react-query/workspaces/useSetActiveWorkspace.ts (1)
apps/desktop/src/renderer/react-query/workspaces/index.ts (1)
useSetActiveWorkspace(4-4)
apps/desktop/src/renderer/react-query/workspaces/useReorderWorkspaces.ts (1)
apps/desktop/src/renderer/react-query/workspaces/index.ts (1)
useReorderWorkspaces(5-5)
apps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.ts (1)
apps/desktop/src/renderer/react-query/workspaces/index.ts (1)
useDeleteWorkspace(3-3)
apps/desktop/src/renderer/react-query/workspaces/useUpdateWorkspace.ts (1)
apps/desktop/src/renderer/react-query/workspaces/index.ts (1)
useUpdateWorkspace(2-2)
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (2)
apps/desktop/src/lib/trpc/routers/workspaces/index.ts (2)
createWorkspacesRouter(1-1)WorkspacesRouter(2-2)apps/desktop/src/lib/trpc/index.ts (1)
router(15-15)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/NewWorkspaceView.tsx (7)
apps/desktop/src/renderer/stores/tabs/store.ts (1)
useAddTab(624-624)apps/desktop/src/renderer/react-query/workspaces/useUpdateWorkspace.ts (1)
useUpdateWorkspace(7-22)apps/desktop/src/renderer/react-query/projects/useOpenProject.ts (1)
useOpenProject(7-22)apps/desktop/src/renderer/react-query/projects/useOpenRecent.ts (1)
useOpenRecent(7-22)apps/desktop/src/renderer/react-query/projects/useRemoveRecent.ts (1)
useRemoveRecent(7-22)apps/desktop/src/renderer/screens/main/components/WorkspaceView/NewWorkspaceView/components/StartSection/StartSection.tsx (1)
StartSection(9-26)apps/desktop/src/renderer/screens/main/components/WorkspaceView/NewWorkspaceView/components/RecentSection/RecentSection.tsx (1)
RecentSection(12-38)
apps/desktop/src/main/index.ts (1)
apps/desktop/src/main/lib/db/index.ts (1)
initDb(11-17)
apps/desktop/src/lib/trpc/routers/projects/projects.ts (2)
apps/desktop/src/lib/trpc/routers/projects/index.ts (2)
createProjectsRouter(1-1)ProjectsRouter(2-2)apps/desktop/src/lib/trpc/index.ts (1)
router(15-15)
apps/desktop/src/main/lib/db/index.ts (1)
apps/desktop/src/main/lib/db/schemas.ts (2)
Database(35-39)defaultDatabase(44-48)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/AddWorkspaceButton.tsx (1)
apps/desktop/src/renderer/react-query/workspaces/useCreateWorkspace.ts (1)
useCreateWorkspace(7-22)
apps/desktop/src/renderer/react-query/workspaces/useCreateWorkspace.ts (1)
apps/desktop/src/renderer/react-query/workspaces/index.ts (1)
useCreateWorkspace(1-1)
apps/desktop/src/main/lib/db/schemas.ts (1)
apps/desktop/src/shared/types.ts (3)
RecentProject(150-154)Tab(44-56)Workspace(70-82)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/NewWorkspaceView/components/RecentSection/RecentSection.tsx (2)
apps/desktop/src/shared/types.ts (1)
RecentProject(150-154)apps/desktop/src/renderer/screens/main/components/WorkspaceView/NewWorkspaceView/components/RecentSection/RecentProjectItem.tsx (1)
RecentProjectItem(32-70)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/NewWorkspaceView/components/RecentSection/RecentProjectItem.tsx (3)
apps/desktop/src/main/lib/db/schemas.ts (1)
RecentProject(6-10)apps/desktop/src/shared/types.ts (1)
RecentProject(150-154)apps/desktop/src/renderer/screens/main/components/WorkspaceView/NewWorkspaceView/components/RecentSection/index.ts (1)
RecentProjectItem(2-2)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx (3)
apps/desktop/src/renderer/react-query/workspaces/useSetActiveWorkspace.ts (1)
useSetActiveWorkspace(7-23)apps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.ts (1)
useDeleteWorkspace(7-22)apps/desktop/src/renderer/react-query/workspaces/useReorderWorkspaces.ts (1)
useReorderWorkspaces(7-22)
🔇 Additional comments (27)
apps/desktop/src/renderer/react-query/workspaces/useDeleteWorkspace.ts (1)
1-21: useDeleteWorkspace hook pattern looks solidHook correctly wraps the TRPC mutation, invalidates workspace queries, and then delegates to any user-provided
onSuccess; matches the pattern used inuseCreateWorkspace.apps/desktop/src/renderer/react-query/projects/index.ts (1)
1-3: Barrel exports look goodCleanly re-exports the project hooks and matches the pattern used for other React Query barrels.
apps/desktop/src/main/index.ts (1)
6-6: DB initialization placement makes sense; verify no earlydbaccessInitializing the DB right after
app.whenReady()and beforemakeAppSetupis a good place forinitDb(). Please double-check that no other main-process module accesses thedbproxy at import time (e.g., top-level code in TRPC routers), otherwise the proxy’s guard will throw beforeinitDbhas run.Also applies to: 33-38
apps/desktop/package.json (1)
52-53: New runtime dependencies look appropriate; check for version alignmentAdding
nanoidandzodhere aligns with the new DB/TRPC/validation work. Please verify there aren’t conflicting versions of these packages in other workspace packages so that tooling and bundling stay consistent.Also applies to: 67-67
apps/desktop/src/renderer/screens/main/components/WorkspaceView/NewWorkspaceView/components/StartSection/index.ts (1)
1-1: StartSection barrel export is fineStraightforward re-export; keeps imports tidy for NewWorkspaceView.
apps/desktop/src/lib/trpc/routers/projects/index.ts (1)
1-2: Barrel re-exports look consistent and correctThe router factory and type are re-exported cleanly from the projects module; this matches the intended TRPC router structure and keeps imports tidy.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/NewWorkspaceView/components/index.ts (1)
1-2: Component barrel exports are straightforwardPublicly exposing
StartSectionandRecentSectionfrom this index keeps imports ergonomic and matches the surrounding folder structure.apps/desktop/src/renderer/screens/main/components/WorkspaceView/NewWorkspaceView/components/RecentSection/index.ts (1)
1-2: Recent section barrel exports are cleanRe-exporting both
RecentSectionandRecentProjectItemfrom this index keeps the public surface simple and matches how other component folders are structured.apps/desktop/src/lib/trpc/routers/workspaces/index.ts (1)
1-2: LGTM! Clean barrel export.The re-exports follow standard module organization patterns and correctly expose the workspaces router factory and type.
apps/desktop/src/renderer/react-query/projects/useOpenProject.ts (1)
1-22: LGTM! Solid mutation hook with proper cache invalidation.The hook correctly wraps the TRPC mutation, invalidates workspace queries on success, and forwards the user's callback. The pattern is consistent with similar hooks in the codebase.
apps/desktop/src/lib/trpc/routers/index.ts (1)
4-5: LGTM! AppRouter correctly extended.The projects and workspaces routers are properly integrated into the main router composition. The type inference via
ReturnType<typeof createAppRouter>will automatically reflect these additions.Also applies to: 14-15
apps/desktop/src/renderer/react-query/projects/useOpenRecent.ts (1)
1-22: LGTM! Consistent with useOpenProject pattern.The hook follows the same pattern as
useOpenProject, properly invalidating workspace queries when a recent project is opened.apps/desktop/src/renderer/screens/main/components/WorkspaceView/NewWorkspaceView/components/RecentSection/RecentProjectItem.tsx (2)
11-30: LGTM! Clean timestamp formatting logic.The relative timestamp logic is well-structured and provides appropriate granularity. The "Yesterday" case (days === 1, representing 24-48 hours ago) is reasonable for a relative time display.
32-70: LGTM! Solid component implementation.The component properly handles user interactions with good accessibility practices (semantic buttons, proper event handling with stopPropagation). The UI layout with folder icon, project details, timestamp, and remove button is well-structured.
apps/desktop/src/renderer/react-query/workspaces/useCreateWorkspace.ts (1)
1-22: LGTM! Consistent invalidation pattern.The hook properly uses broad invalidation (
utils.workspaces.invalidate()) to ensure all workspace-related queries are refreshed after creation. This aligns with the pattern used by most other workspace mutation hooks.apps/desktop/src/renderer/react-query/projects/useRemoveRecent.ts (1)
1-22: LGTM! Appropriate cache invalidation for recents removal.The hook correctly invalidates the specific
getRecentsquery after removing a recent project. This differs fromuseOpenRecent(which invalidates workspace queries) because the operations have different cache impact scopes—removal affects the recents list, while opening affects workspace state.apps/desktop/src/renderer/screens/main/components/WorkspaceView/NewWorkspaceView.tsx (1)
52-64: New handlers + two‑column layout look clean and composableThe handler wiring (
handleOpenProject,handleOpenRecent,handleRemoveRecent) and the new two‑column layout withStartSection/RecentSectionare straightforward and keep the view nicely decoupled from TRPC details. PassingopenProject.isPendingdown to disable the button is a good touch.Also applies to: 66-101
apps/desktop/src/renderer/screens/main/components/WorkspaceView/NewWorkspaceView/components/StartSection/StartSection.tsx (1)
1-26: StartSection API and implementation are minimal and solidProps surface (
onOpenProject,isLoading) is clear, and the button wiring (including disabled state) is correct. No changes needed here.apps/desktop/src/renderer/react-query/workspaces/index.ts (1)
1-5: Barrel exports are consistent and help keep imports tidyThe barrel cleanly re-exports all workspace mutation hooks and matches the underlying filenames. This will simplify imports across the renderer.
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (6)
1-4: Router structure and db wiring look coherentOverall router composition and use of a shared
dbinstance for all procedures is consistent and keeps workspace state centralized. No structural issues here.Also applies to: 10-14
49-63: Basic read queries (get/getAll) are straightforward and correct
getandgetAllread directly fromdb.data.workspaces, returningnullwhen not found and always sorting byorder. This matches the stored schema and should behave as expected.
68-76:getActivebehavior is consistent withsettings.lastActiveWorkspaceIdLooking up the active workspace via
settings.lastActiveWorkspaceIdand returningnullwhen it is unset or when the workspace is missing is reasonable and aligns with the delete/setActive logic.
119-157: Delete flow and recents cleanup logic are soundThe delete mutation:
- Short-circuits with
{ success: false, error: "Workspace not found" }when appropriate.- Removes the workspace by
id.- Only prunes
recentProjectsforworkspace.pathwhen no other workspace still uses that path.- Recomputes
lastActiveWorkspaceIdbased onlastOpenedAtordering.This is a solid set of side effects and should keep recents and settings consistent.
162-177:setActivecorrectly updates both settings and activity timestampsFinding the workspace by
id, throwing when missing, then updating:
settings.lastActiveWorkspaceIdworkspace.lastOpenedAtworkspace.updatedAtis consistent with the rest of the router and with how
getActiveand the delete fallback work.
216-216: ExportedWorkspacesRoutertype is useful for composing the app routerExporting
WorkspacesRouterasReturnType<typeof createWorkspacesRouter>is a good pattern for keeping TRPC router types in sync with implementation.apps/desktop/src/main/lib/db/schemas.ts (2)
6-39: Schema shapes align with router usage and lowdb needsThe
RecentProject,Tab,Workspace,Settings, andDatabaseinterfaces are simple, numeric-timestamp structures well-suited for lowdb and match how the workspaces router interacts withworkspaces,recentProjects, andsettings. No shape issues here.
41-48:defaultDatabaseis minimal and consistent with the schemaInitializing
workspacesandrecentProjectsas empty arrays andsettingsas{}matches theDatabaseinterface (all fields present;lastActiveWorkspaceIdoptional). This should give a safe, predictable initial state.
| reorder: publicProcedure | ||
| .input( | ||
| z.object({ | ||
| fromIndex: z.number(), | ||
| toIndex: z.number(), | ||
| }), | ||
| ) | ||
| .mutation(async ({ input }) => { | ||
| await db.update((data) => { | ||
| const { fromIndex, toIndex } = input; | ||
|
|
||
| // Get all workspaces sorted by order | ||
| const workspaces = data.workspaces | ||
| .slice() | ||
| .sort((a, b) => a.order - b.order); | ||
|
|
||
| // Move workspace from fromIndex to toIndex | ||
| const [removed] = workspaces.splice(fromIndex, 1); | ||
| workspaces.splice(toIndex, 0, removed); | ||
|
|
||
| // Update order fields to reflect new positions | ||
| workspaces.forEach((workspace, index) => { | ||
| const ws = data.workspaces.find((w) => w.id === workspace.id); | ||
| if (ws) { | ||
| ws.order = index; | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| return { success: true }; | ||
| }), |
There was a problem hiding this comment.
Guard against out-of-range indices in reorder to avoid runtime errors
reorder assumes fromIndex and toIndex are valid in the sorted workspaces array:
const [removed] = workspaces.splice(fromIndex, 1);
workspaces.splice(toIndex, 0, removed);If fromIndex is out of bounds, removed becomes undefined, and workspace.id in the later forEach will throw a TypeError. Similarly, an invalid toIndex can insert undefined into the array.
Add explicit bounds checks before splicing:
await db.update((data) => {
const { fromIndex, toIndex } = input;
const workspaces = data.workspaces
.slice()
.sort((a, b) => a.order - b.order);
const lastIndex = workspaces.length - 1;
if (
fromIndex < 0 ||
fromIndex > lastIndex ||
toIndex < 0 ||
toIndex > lastIndex
) {
// Either no-op or throw a typed error here
return;
}
const [removed] = workspaces.splice(fromIndex, 1);
workspaces.splice(toIndex, 0, removed);
workspaces.forEach((workspace, index) => {
const ws = data.workspaces.find((w) => w.id === workspace.id);
if (ws) {
ws.order = index;
}
});
});This prevents malformed state and makes the mutation robust to bad indices.
🤖 Prompt for AI Agents
In apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts around lines
182–212, the reorder mutation assumes fromIndex/toIndex are valid and can splice
undefined into the workspaces array; add an explicit bounds check before any
splicing: compute lastIndex = workspaces.length - 1 and if fromIndex or toIndex
are outside 0..lastIndex return early (or throw a typed TRPC error), and ensure
you only proceed with splice and order reassignment when the removed item is
defined so you never access workspace.id on undefined.
| import { JSONFilePreset } from "lowdb/node"; | ||
| import { join } from "node:path"; | ||
| import { app } from "electron"; | ||
| import type { Database } from "./schemas"; | ||
| import { defaultDatabase } from "./schemas"; | ||
|
|
||
| type DB = Awaited<ReturnType<typeof JSONFilePreset<Database>>>; | ||
|
|
||
| let _db: DB | null = null; | ||
|
|
||
| export async function initDb(): Promise<void> { | ||
| if (_db) return; | ||
|
|
||
| const dbPath = join(app.getPath("userData"), "db.json"); | ||
| _db = await JSONFilePreset<Database>(dbPath, defaultDatabase); | ||
| console.log(`Database initialized at: ${dbPath}`); | ||
| } | ||
|
|
||
| export const db = new Proxy({} as DB, { | ||
| get(_target, prop) { | ||
| if (!_db) { | ||
| throw new Error("Database not initialized. Call initDb() first."); | ||
| } | ||
| return _db[prop as keyof DB]; | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
Revisit DB type alias and be mindful of import-time db access
The lazy init + proxy pattern is nice, but two things to double-check:
- The
DBtype alias
type DB = Awaited<ReturnType<typeof JSONFilePreset<Database>>>;Using typeof JSONFilePreset<Database> is likely not doing what you intend in TypeScript (you generally can’t apply type arguments on the value-side like this). Consider either:
- Importing the DB type from lowdb and parameterizing it directly (e.g.,
Low<Database>), or - Falling back to
type DB = Awaited<ReturnType<typeof JSONFilePreset>>;if you’re OK with looser typing.
Please run the typecheck to confirm this compiles as expected.
- Proxy usage
The proxy correctly throws when _db is null, but that also means any top-level access of db in other modules (outside of functions/handlers called after initDb) will hard-fail at import time. Make sure all consumers only touch db after initDb has been awaited (as wired in apps/desktop/src/main/index.ts).
I'll verify the concerns raised in the review comment by checking TypeScript compilation, inspecting how db is used throughout the codebase, and verifying the initialization sequence.
🏁 Script executed:
#!/bin/bash
# Check if TypeScript compilation succeeds
cd /repo
if [ -f "tsconfig.json" ]; then
echo "=== Running TypeScript type check ==="
npx tsc --noEmit 2>&1 | head -50
else
echo "tsconfig.json not found"
fiLength of output: 79
🏁 Script executed:
#!/bin/bash
# Find all usages of the db export from index.ts
cd /repo
echo "=== Searching for imports/usages of db from db/index.ts ==="
rg -n "from ['\"].*db/index['\"]|from ['\"].*db['\"]" --type=ts --type=tsx -A 2 -B 2Length of output: 143
🏁 Script executed:
#!/bin/bash
# Check the main entry point for initialization order
cd /repo
echo "=== Checking apps/desktop/src/main/index.ts for initDb call ==="
if [ -f "apps/desktop/src/main/index.ts" ]; then
cat "apps/desktop/src/main/index.ts"
else
echo "File not found, searching for main entry point..."
find apps/desktop/src/main -name "index.ts" -o -name "main.ts" | head -5
fiLength of output: 1268
🏁 Script executed:
#!/bin/bash
# Search for direct db access patterns outside of function scopes
cd /repo
echo "=== Searching for top-level db accesses ==="
rg -n "\bdb\." --type=ts --type=tsx | grep -v "function\|const\|let\|async\|=>|//|initDb" | head -20Length of output: 127
I see the issue with the previous scripts. Let me regenerate them to work with the correct environment and settings.
🏁 Script executed:
#!/bin/bash
# Check if TypeScript exists and run type check
echo "=== Checking TypeScript and running type check ==="
if command -v tsc &> /dev/null; then
tsc --noEmit 2>&1 | grep -i "error\|db" | head -30
else
echo "tsc not found, checking for lowdb types..."
find . -name "lowdb" -type d | head -5
find . -name "package.json" | head -1 | xargs grep -l "lowdb" 2>/dev/null
fiLength of output: 93
🏁 Script executed:
#!/bin/bash
# Search for all db imports and usages
echo "=== Searching for db imports ==="
rg -n "from.*db/index|import.*db" --type=js -A 3 -B 1 | head -80Length of output: 32
🏁 Script executed:
#!/bin/bash
# Check lowdb version and exported types in package.json
echo "=== Checking lowdb package ==="
find . -name "package.json" | xargs grep -A 2 -B 2 "lowdb" 2>/dev/null | head -20Length of output: 846
🏁 Script executed:
#!/bin/bash
# Look for top-level db property access (not inside functions)
echo "=== Checking for direct db accesses ==="
rg -n "db\.\w+" --type=js -B 5 -A 2 | grep -v "function\|async\|=>\|//" | head -50Length of output: 39
🏁 Script executed:
#!/bin/bash
# Verify the db/schemas file exists and check Database type
echo "=== Checking db/schemas file ==="
if [ -f "apps/desktop/src/main/lib/db/schemas.ts" ]; then
echo "=== Content of schemas.ts ==="
cat "apps/desktop/src/main/lib/db/schemas.ts"
else
find . -name "schemas.ts" -path "*db*" | xargs cat 2>/dev/null
fiLength of output: 1029
Let me search more thoroughly for db usages and verify the lowdb type behavior.
[/run_scripts]
[/web_search]
🏁 Script executed:
#!/bin/bash
# Search with different patterns for db usage
echo "=== Searching all .ts files for db references ==="
find . -name "*.ts" -type f | xargs grep -l "db\." 2>/dev/null | head -20Length of output: 981
🏁 Script executed:
#!/bin/bash
# Check the storage-ipcs handler mentioned in main entry
echo "=== Checking registerStorageHandlers ==="
find . -path "*storage-ipcs*" -name "*.ts" | head -5 | xargs cat 2>/dev/null | head -100Length of output: 646
🏁 Script executed:
#!/bin/bash
# Look for any imports of db from db/index
echo "=== Searching for db imports from db/index ==="
grep -r "from.*db/index\|from.*db\"" --include="*.ts" . 2>/dev/null | head -30Length of output: 324
🏁 Script executed:
#!/bin/bash
# Check if db is ever accessed at module level (not in functions)
echo "=== Checking db/index.ts for the DB type alias ==="
cat "apps/desktop/src/main/lib/db/index.ts" | head -30Length of output: 777
🌐 Web query:
lowdb v7 JSONFilePreset type signature TypeScript
💡 Result:
Signature (TypeScript):
async function JSONFilePreset(filename: fs.PathLike | string, defaultData: T): Promise<Low>
Usage example:
const db = await JSONFilePreset('db.json', defaultData);
Sources: lowdb README and v7 release notes. [1][2]
References
[1] lowdb README (usage / presets).
[2] lowdb v7 release notes (presets renamed to JSONFilePreset).
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Check how db is used in projects router
echo "=== Checking projects/projects.ts db usage ==="
cat "./apps/desktop/src/lib/trpc/routers/projects/projects.ts" | grep -A 10 -B 5 "db\."Length of output: 1307
🏁 Script executed:
#!/bin/bash
# Check how db is used in workspaces router
echo "=== Checking workspaces/workspaces.ts db usage ==="
cat "./apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts" | grep -A 10 -B 5 "db\."Length of output: 3760
🏁 Script executed:
#!/bin/bash
# Verify the actual type returned by JSONFilePreset
echo "=== Checking lowdb Low type structure ==="
find . -path "*node_modules/lowdb*" -name "*.d.ts" | head -5 | xargs grep -A 3 "class Low\|interface Low\|export.*Low" 2>/dev/null | head -30Length of output: 41
Fix the DB type alias to preserve TypeScript type safety
The lazy init + proxy pattern is sound, and all db consumers correctly access it only within handler callbacks (never at module level). However, the type alias does have an issue:
type DB = Awaited<ReturnType<typeof JSONFilePreset<Database>>>;typeof JSONFilePreset<Database> is invalid in TypeScript—type parameters cannot be applied on the value side. This causes the alias to resolve to Low<unknown>, losing the Database type information. TypeScript will not validate that consumers are accessing the correct properties.
To fix, import Low from lowdb and parameterize it directly:
import { Low } from "lowdb";
type DB = Low<Database>;Alternatively, if you prefer avoiding the extra import, use typeof JSONFilePreset without the type parameter (though this will still result in Low<unknown>).
The import-time access concern is addressed by proper initialization sequencing in apps/desktop/src/main/index.ts—all db accesses occur only within handler callbacks after initDb() is awaited.
🤖 Prompt for AI Agents
In apps/desktop/src/main/lib/db/index.ts lines 1-26, the DB type alias uses
Awaited<ReturnType<typeof JSONFilePreset<Database>>> which misapplies a type
parameter to a value and collapses to Low<unknown>; replace that with an
explicit Low<Database> type by importing Low from "lowdb" and change the DB
alias to Low<Database>, and update imports accordingly so TypeScript preserves
the Database shape.
| const { data: activeWorkspace } = trpc.workspaces.getActive.useQuery(); | ||
|
|
||
| // If no workspace or workspace has no path, show new workspace view | ||
| const isNew = !activeWorkspace || activeWorkspace.path === null; | ||
|
|
||
| if (activeWorkspace?.isNew) { | ||
| if (isNew) { | ||
| return ( |
There was a problem hiding this comment.
🧩 Analysis chain
Loading/error state currently masquerades as “new workspace”
Right now isNew is computed from data only:
const { data: activeWorkspace } = trpc.workspaces.getActive.useQuery();
// ...
const isNew = !activeWorkspace || activeWorkspace.path === null;This means:
- While the query is still loading, or if it errors and
datastaysundefined, the view will renderNewWorkspaceView, potentially prompting users to create a new workspace even though one exists or an error occurred. - The logic also assumes
activeWorkspace.pathexists and becomes exactlynullfor “new” workspaces; if the backend ever usesundefined/empty string instead, this check will misclassify the state.
Consider:
- Explicitly handling
isLoading/error(or using Suspense) so those states don’t fall into the “new workspace” path. - Treating the path as “unset” via a null-safe check like
activeWorkspace?.path == nullor, if available, delegating to a dedicated flag from the backend (e.g.,activeWorkspace.isNew).
This will avoid incorrect rendering and UX flicker around startup and error cases.
I'll verify the concerns in this review comment by examining the codebase. Let me start by gathering the necessary information.
---
🏁 Script executed:
```shell
#!/bin/bash
# Read the WorkspaceView file to see current implementation
cat apps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsx
Length of output: 708
🏁 Script executed:
#!/bin/bash
# Find workspace type definition
find . -type f \( -name "*.ts" -o -name "*.tsx" \) -path "*/workspace*" | head -20Length of output: 1227
🏁 Script executed:
#!/bin/bash
# Search for trpc workspaces getActive query definition
rg -n "workspaces\.getActive" --type ts --type tsx -A 5Length of output: 27
Let me continue verifying with corrected commands:
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for workspaces getActive query definition (fix tsx issue)
rg -n "getActive" --type ts -A 8 apps/desktop/src/lib/trpc/routers/workspaces/
Length of output: 800
🏁 Script executed:
#!/bin/bash
# Check workspace type definition
cat apps/desktop/src/types/workspace.tsLength of output: 67
🏁 Script executed:
#!/bin/bash
# Look at workspaces router implementation
cat apps/desktop/src/lib/trpc/routers/workspaces/index.tsLength of output: 108
🏁 Script executed:
#!/bin/bash
# Check if there's a Suspense boundary or layout wrapper handling loading
rg -n "useQuery\|isLoading\|Suspense" apps/desktop/src/renderer/screens/main/ -B 2 -A 2Length of output: 0
Let me search more broadly for the workspace type and check the parent component:
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for workspace interface/type definition
rg -n "interface.*Workspace\|type.*Workspace\|export.*Workspace" --type ts -B 1 -A 5 | head -50
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check parent components/layouts that might wrap WorkspaceView
find apps/desktop/src/renderer/screens/main -name "*.tsx" -type f | xargs cat | head -100Length of output: 2386
🏁 Script executed:
#!/bin/bash
# Check the main app entry or layout
cat apps/desktop/src/renderer/screens/main/index.tsx 2>/dev/null || cat apps/desktop/src/renderer/App.tsx 2>/dev/null || find apps/desktop/src/renderer -name "App.tsx" -o -name "index.tsx" | head -1Length of output: 617
🏁 Script executed:
#!/bin/bash
# Look for useQuery destructuring patterns to see if isLoading is typically extracted elsewhere
rg -n "useQuery.*isLoading\|useQuery.*error" apps/desktop/src/renderer --type ts -B 1 -A 1 | head -30Length of output: 0
Handle loading and error states explicitly in the isNew logic
The issue is confirmed. During the query's loading phase, activeWorkspace is undefined, causing isNew to evaluate to true and render NewWorkspaceView—even though a workspace may exist and is just being fetched. Similarly, if the query errors, the same fallback occurs.
The fix is to extract isLoading from the useQuery hook:
const { data: activeWorkspace, isLoading } = trpc.workspaces.getActive.useQuery();
const isNew = !isLoading && (!activeWorkspace || activeWorkspace.path === null);This ensures the view doesn't render NewWorkspaceView while the query is in flight or errored, preventing UX flicker and confusion at startup. Alternatively, return a loading/error state from this component instead of falling through to the new workspace view.
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsx
around lines 7–13, the component treats undefined query data as a "new"
workspace during the query's loading or error states; extract isLoading (and
optionally isError) from trpc.workspaces.getActive.useQuery and change the isNew
calculation to only consider absence of a workspace after the query has finished
(e.g., isNew = !isLoading && !isError && (no workspace or workspace.path ===
null)), or alternatively render an explicit loading/error UI while
isLoading/isError are true so NewWorkspaceView is not shown during fetch or
error conditions.
1 participant
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.