feat(directory): manage environments - #1787
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 (10)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughImplements a complete environment directory lifecycle management system. A new SQLite ChangesComplete Directory Management
Sequence Diagram(s)sequenceDiagram
participant User
participant EnvironmentsSettings
participant ProjectStore
participant ProjectClient
participant RoutesDispatcher
participant ProjectPresenter
participant NewEnvironmentPreferencesTable
participant EventBus
User->>EnvironmentsSettings: drag reorder / archive / restore / remove
EnvironmentsSettings->>ProjectStore: reorderEnvironments(paths) / archiveEnvironment(path) / removeEnvironment(path)
ProjectStore->>ProjectClient: invoke route with action + path
ProjectClient->>RoutesDispatcher: IPC invoke project.*Environment route
RoutesDispatcher->>ProjectPresenter: reorderEnvironments / archiveEnvironment / restoreEnvironment / removeEnvironment
ProjectPresenter->>NewEnvironmentPreferencesTable: reorderActive / markArchived / markRemoved (+ clearProjectDir tx)
NewEnvironmentPreferencesTable-->>ProjectPresenter: done / clearedSessionIds[]
ProjectPresenter-->>RoutesDispatcher: result
RoutesDispatcher->>EventBus: publishProjectEnvironmentsChanged(action, path, version)
EventBus-->>ProjectStore: onEnvironmentsChanged → refreshProjectData
RoutesDispatcher-->>ProjectClient: result
ProjectClient-->>ProjectStore: resolve promise
ProjectStore->>ProjectStore: update local state / rollback on error
ProjectStore-->>EnvironmentsSettings: environments / archivedEnvironments / removedEnvironments updated
EnvironmentsSettings-->>User: re-render tabs with updated data
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 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)
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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/main/presenter/projectPresenter/index.ts (1)
46-50: ⚡ Quick winMake recency ordering explicit before applying the limit.
getRecentProjectscurrently slices after filtering but does not sort bylast_accessed_atin-method. This makes “recent” behavior depend onnewProjectsTable.getAll()implementation details.Proposed fix
async getRecentProjects(limit: number = 10): Promise<Project[]> { const rows = this.sqlitePresenter.newProjectsTable.getAll() return rows .filter((row) => !this.isRemovedEnvironment(row.path)) + .sort((left, right) => right.last_accessed_at - left.last_accessed_at) .slice(0, limit) .map((row) => ({ path: row.path, name: row.name, icon: row.icon, lastAccessedAt: row.last_accessed_at })) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/presenter/projectPresenter/index.ts` around lines 46 - 50, The getRecentProjects method in projectPresenter currently filters and slices the projects without explicitly sorting by recency, making the "recent" behavior dependent on implementation details of getAll(). Add an explicit sort step after filtering but before slicing that orders the rows by last_accessed_at in descending order to ensure the most recently accessed projects are returned first, making the recency ordering explicit and deterministic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/renderer/settings/components/EnvironmentsSettings.vue`:
- Around line 330-332: The `reorderActivePaths` function is async and can
reject, but callers invoke it without awaiting or handling errors
(fire-and-forget pattern), creating unhandled promise rejections and loss of
error feedback. Add a try-catch block inside the `reorderActivePaths` function
body to catch any errors thrown by `projectStore.reorderEnvironments(paths)`,
and handle the error appropriately with user-facing error feedback (such as
displaying an error notification or logging the failure).
In `@src/renderer/src/i18n/vi-VN/settings.json`:
- Line 2001: The hideTemp translation in the Vietnamese settings file is
currently set to "Ẩn nhiệt độ" which means "hide temperature", but in the
context of the settings UI this key refers to hiding the Temp folder, not
temperature. Locate the hideTemp key in the settings.json file and change its
value from "Ẩn nhiệt độ" to "Ẩn Temp" to accurately reflect that this feature
pertains to the Temp folder rather than temperature.
In `@src/renderer/src/stores/ui/project.ts`:
- Around line 214-227: The reorderEnvironments call sends the unfiltered
normalizedPaths which may contain paths that were excluded by the filter on the
previous line, causing backend failures. In the
projectClient.reorderEnvironments() call, replace the normalizedPaths argument
with orderedPaths instead, which contains only the actively persisted
environment paths that have corresponding entries in the byPath collection.
In `@src/shared/contracts/routes/project.routes.ts`:
- Around line 29-33: The route validation in the projectReorderEnvironmentsRoute
(and other similar routes at lines 39-43, 49-53, and 59-63) accepts
whitespace-only strings because z.string().min(1) only checks length, not actual
content. Update the Zod schema definitions for the paths array validation across
all mentioned route contracts to properly reject strings that are empty or
contain only whitespace by adding validation that either trims the input first
or uses a custom refinement to ensure at least one non-whitespace character
exists in each path string.
In `@test/e2e/specs/21-project-readonly-route.smoke.spec.ts`:
- Around line 23-28: The assertion for archived tab content is too broad because
the locator matching environments-archived-empty or environment-row can find
elements anywhere on the page, not just within the archived-tab content area. To
fix this, scope the locator to only search within the archived tab container by
chaining the locator selection (first locate the archived-tab element or its
content container, then find the environments-archived-empty or environment-row
descendants within it), or alternatively verify the archived tab is in an
active/selected state before checking for the empty or row visibility to ensure
the test specifically validates archived-tab rendering.
---
Nitpick comments:
In `@src/main/presenter/projectPresenter/index.ts`:
- Around line 46-50: The getRecentProjects method in projectPresenter currently
filters and slices the projects without explicitly sorting by recency, making
the "recent" behavior dependent on implementation details of getAll(). Add an
explicit sort step after filtering but before slicing that orders the rows by
last_accessed_at in descending order to ensure the most recently accessed
projects are returned first, making the recency ordering explicit and
deterministic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 276a73dd-8e1b-4857-8490-1501ae3ea0d2
📒 Files selected for processing (76)
AGENTS.mddocs/features/complete-directory-management/plan.mddocs/features/complete-directory-management/sidebar-frontend-technical-plan.mddocs/features/complete-directory-management/spec.mddocs/features/complete-directory-management/tasks.mdsrc/main/presenter/index.tssrc/main/presenter/projectPresenter/index.tssrc/main/presenter/sqlitePresenter/index.tssrc/main/presenter/sqlitePresenter/schemaCatalog.tssrc/main/presenter/sqlitePresenter/tables/newEnvironmentPreferences.tssrc/main/presenter/sqlitePresenter/tables/newSessions.tssrc/main/routes/index.tssrc/renderer/api/ProjectClient.tssrc/renderer/settings/components/EnvironmentsSettings.vuesrc/renderer/src/components/WindowSideBar.vuesrc/renderer/src/i18n/da-DK/chat.jsonsrc/renderer/src/i18n/da-DK/settings.jsonsrc/renderer/src/i18n/de-DE/chat.jsonsrc/renderer/src/i18n/de-DE/settings.jsonsrc/renderer/src/i18n/en-US/chat.jsonsrc/renderer/src/i18n/en-US/settings.jsonsrc/renderer/src/i18n/es-ES/chat.jsonsrc/renderer/src/i18n/es-ES/settings.jsonsrc/renderer/src/i18n/fa-IR/chat.jsonsrc/renderer/src/i18n/fa-IR/settings.jsonsrc/renderer/src/i18n/fr-FR/chat.jsonsrc/renderer/src/i18n/fr-FR/settings.jsonsrc/renderer/src/i18n/he-IL/chat.jsonsrc/renderer/src/i18n/he-IL/settings.jsonsrc/renderer/src/i18n/id-ID/chat.jsonsrc/renderer/src/i18n/id-ID/settings.jsonsrc/renderer/src/i18n/it-IT/chat.jsonsrc/renderer/src/i18n/it-IT/settings.jsonsrc/renderer/src/i18n/ja-JP/chat.jsonsrc/renderer/src/i18n/ja-JP/settings.jsonsrc/renderer/src/i18n/ko-KR/chat.jsonsrc/renderer/src/i18n/ko-KR/settings.jsonsrc/renderer/src/i18n/ms-MY/chat.jsonsrc/renderer/src/i18n/ms-MY/settings.jsonsrc/renderer/src/i18n/pl-PL/chat.jsonsrc/renderer/src/i18n/pl-PL/settings.jsonsrc/renderer/src/i18n/pt-BR/chat.jsonsrc/renderer/src/i18n/pt-BR/settings.jsonsrc/renderer/src/i18n/ru-RU/chat.jsonsrc/renderer/src/i18n/ru-RU/settings.jsonsrc/renderer/src/i18n/tr-TR/chat.jsonsrc/renderer/src/i18n/tr-TR/settings.jsonsrc/renderer/src/i18n/vi-VN/chat.jsonsrc/renderer/src/i18n/vi-VN/settings.jsonsrc/renderer/src/i18n/zh-CN/chat.jsonsrc/renderer/src/i18n/zh-CN/settings.jsonsrc/renderer/src/i18n/zh-HK/chat.jsonsrc/renderer/src/i18n/zh-HK/settings.jsonsrc/renderer/src/i18n/zh-TW/chat.jsonsrc/renderer/src/i18n/zh-TW/settings.jsonsrc/renderer/src/pages/NewThreadPage.vuesrc/renderer/src/stores/ui/project.tssrc/shared/contracts/domainSchemas.tssrc/shared/contracts/events.tssrc/shared/contracts/events/project.events.tssrc/shared/contracts/routes.tssrc/shared/contracts/routes/project.routes.tssrc/shared/types/agent-interface.d.tssrc/shared/types/presenters/project.presenter.d.tstest/e2e/specs/21-project-readonly-route.smoke.spec.tstest/main/presenter/projectPresenter/projectPresenter.test.tstest/main/presenter/sqlitePresenter/newEnvironmentPreferencesTable.test.tstest/main/presenter/sqlitePresenter/newSessionsTable.test.tstest/main/routes/dispatcher.test.tstest/renderer/api/clients.test.tstest/renderer/components/EnvironmentsSettings.test.tstest/renderer/components/NewThreadPage.onboarding.test.tstest/renderer/components/NewThreadPage.test.tstest/renderer/components/WindowSideBar.test.tstest/renderer/pages/NewThreadPage.test.tstest/renderer/stores/projectStore.test.ts
Summary
Validation
pnpm run formatpnpm run i18npnpm run lintpnpm run typecheck:nodepnpm run typecheck:webpnpm exec vitest run test/renderer/components/EnvironmentsSettings.test.ts test/renderer/stores/projectStore.test.ts test/main/routes/contracts.test.tspnpm run buildtest/e2e/specs/21-project-readonly-route.smoke.spec.tsCloses #1785
Summary by CodeRabbit
Release Notes
New Features
Documentation
Localization