fix: add project path validation across all project switching methods #341
Conversation
- Introduced ProjectPathValidationDialog component to handle scenarios where project paths are invalid or inaccessible. - Implemented path validation logic in project selection and trash operations, ensuring users are notified of invalid paths and can update or remove projects accordingly. - Enhanced project cycling functionality to skip invalid paths automatically, improving user experience when navigating through project history. - Added validateProjectPath utility to check the existence and accessibility of project directories.
📝 WalkthroughWalkthroughAdds end-to-end project-path validation: new validator, dialog component, and hooks that detect invalid project folders, prompt the user (refresh/remove/dismiss), and ensure project switching, cycling, restoration, and trash-restore flows validate paths before navigation. (≈29 words) Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Selector / Shortcut / Root
participant Validator as validateProjectPath()
participant Dialog as ProjectPathValidationDialog
participant Store as AppStore
participant FS as File Browser / Electron API
participant Router as Router
User->>UI: Request switch or app init
UI->>Validator: validate target project path
alt Path valid
Validator-->>UI: true
UI->>Store: set currentProject / update history
UI->>Router: navigate /board
else Path invalid
Validator-->>UI: false
UI->>Dialog: open(invalidProject)
Dialog-->>User: show options (Locate / Remove / Dismiss)
alt User chooses Locate
User->>Dialog: open file browser
Dialog->>FS: pick new path
FS-->>Dialog: newPath
Dialog->>Validator: validate newPath
alt newPath valid
Validator-->>Dialog: true
Dialog->>Store: update project.path, set currentProject
Dialog->>Router: navigate /board
else newPath invalid
Validator-->>Dialog: false
Dialog-->>User: show error toast
end
else User chooses Remove
User->>Dialog: remove project
Dialog->>Store: remove project, clear currentProject
Dialog->>Router: navigate /home
else User chooses Dismiss
User->>Dialog: dismiss
Dialog-->>UI: close dialog
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (2)📚 Learning: 2025-12-30T01:02:07.114ZApplied to files:
📚 Learning: 2025-12-28T05:07:48.147ZApplied to files:
🧬 Code graph analysis (3)apps/ui/src/components/layout/sidebar/hooks/use-trash-operations.ts (3)
apps/ui/src/components/layout/sidebar.tsx (2)
apps/ui/src/routes/__root.tsx (5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (11)
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 |
Summary of ChangesHello @illia1f, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the application's robustness by implementing a comprehensive project path validation system. It addresses issues where projects could become inaccessible due to filesystem changes, leading to silent failures. By integrating validation at every project access point and providing clear user feedback through a dedicated dialog, the PR ensures a more stable and user-friendly experience, preventing data loss and improving overall application reliability. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a robust path validation system for projects, which is a great improvement to prevent silent failures. The changes are well-structured, with new hooks and components for validation logic and UI. The validation is consistently applied across different ways of switching projects.
My review focuses on improving maintainability by reducing code duplication and enhancing type safety. I've identified several areas where similar logic is repeated across components (__root.tsx and project-selector-with-options.tsx) and within hooks (use-validated-project-cycling.ts). Extracting this duplicated logic into shared hooks or helper functions would make the code cleaner and easier to maintain. I also found a minor bug where a dialog doesn't close as expected. Additionally, I've suggested some minor improvements to logging and typing for better developer experience and robustness.
Overall, this is a solid contribution that addresses an important issue. The suggested refactorings will help to improve the quality of the new code even further.
apps/ui/src/components/layout/sidebar/components/project-selector-with-options.tsx
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/ui/src/hooks/use-store-hydration.ts (1)
12-12: Improve type safety for the store parameter.The
anytype bypasses TypeScript's type checking. Define an interface or use a constrained generic type to ensure the store has the expectedpersistAPI.🔎 Proposed fix with a constrained interface
+interface StoreWithPersist { + persist?: { + hasHydrated?: () => boolean; + onFinishHydration?: (callback: () => void) => (() => void) | undefined; + }; +} + -export function useStoreHydration(store: any) { +export function useStoreHydration(store: StoreWithPersist) { const [hydrated, setHydrated] = useState(() => store.persist?.hasHydrated?.() ?? false);apps/ui/src/lib/validate-project-path.ts (1)
4-39: Consider optimizing the validation flow.The function makes two separate API calls:
exists()followed bystat(). Sincestat()will fail if the path doesn't exist, you could simplify by using onlystat()and checking bothsuccessandisDirectoryin one operation. This would reduce API calls and improve performance.🔎 Proposed optimization
export const validateProjectPath = async (project: Project): Promise<boolean> => { try { if (!project?.path) { console.error('[Validation] No project path provided'); return false; } console.log('[Validation] Checking path:', project.path); const api = getElectronAPI(); - // Check if path exists - const exists = await api.exists(project.path); - console.log('[Validation] Exists result:', exists); - - if (exists !== true) { - console.error('[Validation] Path does not exist'); - return false; - } - - // Verify it's a directory const statResult = await api.stat(project.path); console.log('[Validation] Stat result:', statResult); if (!statResult.success || !statResult.stats?.isDirectory) { - console.error('[Validation] Path is not a directory or stat failed'); + console.error('[Validation] Path does not exist, is not a directory, or stat failed'); return false; } console.log('[Validation] Path is valid!'); return true; } catch (error) { // Treat errors as invalid (permissions, network issues, etc.) console.error('[Validation] Exception during validation:', error); return false; } };apps/ui/src/components/layout/sidebar/components/project-selector-with-options.tsx (1)
101-132: Consider extracting shared validation handlers to reduce duplication.The
handleRefreshPathandhandleRemoveProjectimplementations here are nearly identical to those inapps/ui/src/routes/__root.tsx(lines 365-404). Both:
- Validate the new path with
validateProjectPath- Update projects via
setProjectswith the same mapping logic- Update
currentProjectand navigate- Show the same toast messages
Consider extracting these handlers into a shared hook (e.g.,
useProjectPathValidation) to centralize the logic and reduce maintenance burden.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/ui/src/components/dialogs/project-path-validation-dialog.tsxapps/ui/src/components/layout/sidebar.tsxapps/ui/src/components/layout/sidebar/components/project-selector-with-options.tsxapps/ui/src/components/layout/sidebar/hooks/use-trash-operations.tsapps/ui/src/hooks/index.tsapps/ui/src/hooks/use-store-hydration.tsapps/ui/src/hooks/use-validated-project-cycling.tsapps/ui/src/lib/validate-project-path.tsapps/ui/src/routes/__root.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always import from shared packages (@automaker/*), never from old relative paths
Files:
apps/ui/src/hooks/index.tsapps/ui/src/hooks/use-store-hydration.tsapps/ui/src/lib/validate-project-path.tsapps/ui/src/hooks/use-validated-project-cycling.tsapps/ui/src/routes/__root.tsxapps/ui/src/components/dialogs/project-path-validation-dialog.tsxapps/ui/src/components/layout/sidebar/hooks/use-trash-operations.tsapps/ui/src/components/layout/sidebar/components/project-selector-with-options.tsxapps/ui/src/components/layout/sidebar.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
resolveModelString()from @automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names
Files:
apps/ui/src/hooks/index.tsapps/ui/src/hooks/use-store-hydration.tsapps/ui/src/lib/validate-project-path.tsapps/ui/src/hooks/use-validated-project-cycling.tsapps/ui/src/routes/__root.tsxapps/ui/src/components/dialogs/project-path-validation-dialog.tsxapps/ui/src/components/layout/sidebar/hooks/use-trash-operations.tsapps/ui/src/components/layout/sidebar/components/project-selector-with-options.tsxapps/ui/src/components/layout/sidebar.tsx
🧠 Learnings (2)
📚 Learning: 2025-12-30T01:02:07.114Z
Learnt from: illia1f
Repo: AutoMaker-Org/automaker PR: 324
File: apps/ui/src/components/views/board-view/components/kanban-card/kanban-card.tsx:122-131
Timestamp: 2025-12-30T01:02:07.114Z
Learning: Tailwind CSS v4 uses postfix syntax for the important modifier: append ! at the end of the utility class (e.g., backdrop-blur-[0px]! or hover:bg-red-500!). The older v3 style used a prefix (!) at the start (e.g., !backdrop-blur-[0px]); prefer the postfix form for consistency across TSX files.
Applied to files:
apps/ui/src/routes/__root.tsxapps/ui/src/components/dialogs/project-path-validation-dialog.tsxapps/ui/src/components/layout/sidebar/components/project-selector-with-options.tsxapps/ui/src/components/layout/sidebar.tsx
📚 Learning: 2025-12-28T05:07:48.147Z
Learnt from: CR
Repo: AutoMaker-Org/automaker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T05:07:48.147Z
Learning: Frontend UI must use TanStack Router for file-based routing, organize components in components/views/, implement stores with Zustand, and use custom hooks in hooks/ directory
Applied to files:
apps/ui/src/components/layout/sidebar/components/project-selector-with-options.tsx
🧬 Code graph analysis (7)
apps/ui/src/hooks/use-store-hydration.ts (1)
apps/ui/src/hooks/index.ts (1)
useStoreHydration(11-11)
apps/ui/src/lib/validate-project-path.ts (2)
apps/ui/src/lib/electron.ts (1)
getElectronAPI(728-737)apps/ui/src/lib/http-api-client.ts (1)
exists(820-825)
apps/ui/src/hooks/use-validated-project-cycling.ts (1)
apps/ui/src/lib/validate-project-path.ts (1)
validateProjectPath(4-39)
apps/ui/src/routes/__root.tsx (4)
apps/ui/src/hooks/use-store-hydration.ts (1)
useStoreHydration(12-29)apps/ui/src/lib/electron.ts (2)
Project(2826-2832)getElectronAPI(728-737)apps/ui/src/lib/validate-project-path.ts (1)
validateProjectPath(4-39)apps/ui/src/components/dialogs/project-path-validation-dialog.tsx (1)
ProjectPathValidationDialog(24-129)
apps/ui/src/components/dialogs/project-path-validation-dialog.tsx (1)
apps/ui/src/components/ui/hotkey-button.tsx (1)
HotkeyButton(169-288)
apps/ui/src/components/layout/sidebar/components/project-selector-with-options.tsx (3)
apps/ui/src/hooks/use-validated-project-cycling.ts (1)
useValidatedProjectCycling(20-144)apps/ui/src/lib/validate-project-path.ts (1)
validateProjectPath(4-39)apps/ui/src/components/dialogs/project-path-validation-dialog.tsx (1)
ProjectPathValidationDialog(24-129)
apps/ui/src/components/layout/sidebar.tsx (2)
apps/ui/src/hooks/index.ts (1)
useValidatedProjectCycling(12-12)apps/ui/src/hooks/use-validated-project-cycling.ts (1)
useValidatedProjectCycling(20-144)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (16)
apps/ui/src/hooks/index.ts (1)
11-12: LGTM!The new exports follow the existing pattern and correctly expose the new hooks.
apps/ui/src/components/dialogs/project-path-validation-dialog.tsx (3)
62-68: LGTM: Dialog interaction correctly prevents dismissal.Preventing
onInteractOutsideand hiding the close button appropriately forces users to make an explicit choice (Locate/Remove/Dismiss) when facing an invalid project path, which aligns well with the PR objectives to prevent silent failures.
114-124: LGTM: Keyboard shortcut implementation is correct.The HotkeyButton configuration with
Cmd/Ctrl+Enterfollows the expected API and provides good keyboard accessibility for the primary action.
34-49: Review comment is not applicable — dialog closes after successful path refresh.Both parent component implementations (
__root.tsxandproject-selector-with-options.tsx) properly close the dialog by callingsetValidationDialogOpen(false)afteronRefreshPathcompletes successfully. The dialog's state management is delegated to the parent component, which is an appropriate architectural pattern.apps/ui/src/hooks/use-validated-project-cycling.ts (3)
26-36: LGTM: State update helper is correctly implemented.The
switchProjectForCyclinghelper appropriately updates all related state fields without reordering history, matching the cycling semantics described in the comments.
38-85: Excellent validation and error handling.Both cycling functions properly:
- Guard against concurrent operations with
isValidating- Filter to valid history before cycling
- Validate each candidate path before switching
- Handle the case where all projects are invalid gracefully
- Use correct wraparound arithmetic for array cycling
The implementation aligns well with the PR objectives to prevent silent failures.
Also applies to: 87-137
38-85: Cycling direction semantics are correct—no changes needed.The projectHistory array in app-store.ts is explicitly documented as "MRU order (most recent first)" (line 403), with new projects added to the front via
[project.id, ...filteredHistory](line 1129). The cycling logic aligns correctly:cyclePrevProjectmoves to higher indices (older projects), andcycleNextProjectmoves to lower indices (newer/recent projects). The naming and implementation match typical user expectations for MRU history navigation.apps/ui/src/components/layout/sidebar/hooks/use-trash-operations.ts (1)
22-53: LGTM! Well-structured async validation before restore.The validation flow correctly:
- Looks up the trashed project first
- Validates path existence before calling
restoreTrashedProject- Provides clear error messaging with actionable guidance
- Updates the dependency array appropriately
apps/ui/src/components/layout/sidebar.tsx (3)
6-6: LGTM! Clean integration of validated project cycling.The import correctly uses the
@/hookspath alias, and the hook cleanly encapsulates the validation logic that was previously missing from keyboard shortcuts.
73-74: Good refactor: Validation now integrated into cycling.Sourcing
cyclePrevProjectandcycleNextProjectfrom the new hook ensures keyboard shortcuts (Q/E) validate paths before switching, addressing the silent failure issue described in the PR objectives.
146-151: Correct:trashedProjectspassed to support path validation in restore flow.This aligns with the updated
useTrashOperationsinterface that now requirestrashedProjectsfor path validation during project restoration.apps/ui/src/components/layout/sidebar/components/project-selector-with-options.tsx (2)
231-243: LGTM! Validation before project switch is correct.The async
onSelecthandler properly:
- Closes the dropdown immediately (good UX)
- Validates the path before switching
- Opens the validation dialog if invalid, otherwise sets the current project
One consideration: if validation fails, the dropdown is already closed but no project switch occurs. This is acceptable UX since the dialog provides next steps.
426-432: Dialog integration looks correct.The
ProjectPathValidationDialogis properly wired with all required props. The dialog will display when a user selects a project with an invalid path.apps/ui/src/routes/__root.tsx (3)
365-410: Dialog handlers work correctly but duplicate logic from project-selector.As noted in the project-selector review, these handlers (
handleRefreshPath,handleRemoveProject,handleDismiss) share significant logic with the same-named handlers inproject-selector-with-options.tsx. ThehandleDismisshere additionally clearscurrentProject, which is appropriate for the startup context where the user dismisses an invalid project.
46-47: Good: Hydration hooks ensure validation runs after persisted state loads.Using
useStoreHydrationfor bothappHydratedandsetupHydratedensures the restoration logic waits for Zustand persistence to complete before attempting to validate thecurrentProjectpath. This prevents race conditions where validation runs against stale or uninitialized state.
507-514: LGTM! Dialog correctly placed and wired.The
ProjectPathValidationDialogis rendered with all required props including theonDismisshandler, which is specific to the root context (clearscurrentProjectwhen user dismisses).
- Introduced useProjectPathValidation hook to centralize project path validation logic, enhancing code reusability and maintainability. - Updated ProjectSelectorWithOptions component to utilize the new hook, streamlining project path handling and validation. - Refactored RootLayoutContent to integrate project restoration logic with path validation, improving user experience during project navigation. - Added useProjectRestoration hook for managing project restoration on app initialization, ensuring valid project paths before navigation.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/ui/src/components/layout/sidebar/components/project-selector-with-options.tsx (1)
199-210: Add error handling for the async validation inonSelect.The
validateProjectPathcall could throw an exception, but there's no try-catch wrapper. While the validation function has internal error handling that returnsfalse, wrapping in try-catch would provide defense-in-depth.🔎 Suggested fix
onSelect={async (p) => { setIsProjectPickerOpen(false); - // Validate path before switching - const isValid = await validateProjectPath(p); - - if (!isValid) { - showValidationDialog(p); - } else { - setCurrentProject(p); + try { + // Validate path before switching + const isValid = await validateProjectPath(p); + + if (!isValid) { + showValidationDialog(p); + } else { + setCurrentProject(p); + } + } catch (error) { + console.error('Failed to validate project path:', error); + showValidationDialog(p); } }}
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/ui/src/components/layout/sidebar/components/project-selector-with-options.tsxapps/ui/src/hooks/index.tsapps/ui/src/hooks/use-project-path-validation.tsapps/ui/src/hooks/use-project-restoration.tsapps/ui/src/hooks/use-store-hydration.tsapps/ui/src/lib/validate-project-path.tsapps/ui/src/routes/__root.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/ui/src/lib/validate-project-path.ts
- apps/ui/src/hooks/use-store-hydration.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always import from shared packages (@automaker/*), never from old relative paths
Files:
apps/ui/src/hooks/index.tsapps/ui/src/routes/__root.tsxapps/ui/src/hooks/use-project-restoration.tsapps/ui/src/components/layout/sidebar/components/project-selector-with-options.tsxapps/ui/src/hooks/use-project-path-validation.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
resolveModelString()from @automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names
Files:
apps/ui/src/hooks/index.tsapps/ui/src/routes/__root.tsxapps/ui/src/hooks/use-project-restoration.tsapps/ui/src/components/layout/sidebar/components/project-selector-with-options.tsxapps/ui/src/hooks/use-project-path-validation.ts
🧠 Learnings (2)
📚 Learning: 2025-12-28T05:07:48.147Z
Learnt from: CR
Repo: AutoMaker-Org/automaker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T05:07:48.147Z
Learning: Frontend UI must use TanStack Router for file-based routing, organize components in components/views/, implement stores with Zustand, and use custom hooks in hooks/ directory
Applied to files:
apps/ui/src/routes/__root.tsx
📚 Learning: 2025-12-30T01:02:07.114Z
Learnt from: illia1f
Repo: AutoMaker-Org/automaker PR: 324
File: apps/ui/src/components/views/board-view/components/kanban-card/kanban-card.tsx:122-131
Timestamp: 2025-12-30T01:02:07.114Z
Learning: Tailwind CSS v4 uses postfix syntax for the important modifier: append ! at the end of the utility class (e.g., backdrop-blur-[0px]! or hover:bg-red-500!). The older v3 style used a prefix (!) at the start (e.g., !backdrop-blur-[0px]); prefer the postfix form for consistency across TSX files.
Applied to files:
apps/ui/src/routes/__root.tsxapps/ui/src/components/layout/sidebar/components/project-selector-with-options.tsx
🧬 Code graph analysis (2)
apps/ui/src/routes/__root.tsx (4)
apps/ui/src/hooks/use-store-hydration.ts (1)
useStoreHydration(19-36)apps/ui/src/hooks/use-project-path-validation.ts (1)
useProjectPathValidation(16-95)apps/ui/src/hooks/use-project-restoration.ts (1)
useProjectRestoration(22-86)apps/ui/src/components/dialogs/project-path-validation-dialog.tsx (1)
ProjectPathValidationDialog(24-129)
apps/ui/src/hooks/use-project-restoration.ts (4)
apps/ui/src/hooks/index.ts (1)
useProjectRestoration(13-13)apps/ui/src/lib/electron.ts (1)
getElectronAPI(728-737)apps/ui/src/lib/validate-project-path.ts (1)
validateProjectPath(4-34)apps/ui/src/lib/http-api-client.ts (1)
isElectronMode(114-122)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (15)
apps/ui/src/hooks/index.ts (1)
11-14: LGTM!The new hook exports follow the established pattern and are correctly re-exported from the barrel file.
apps/ui/src/components/layout/sidebar/components/project-selector-with-options.tsx (1)
36-39: LGTM!The new imports for validation hooks and dialog component are correctly structured.
apps/ui/src/routes/__root.tsx (5)
25-26: LGTM!Clean import of the new hooks and dialog component from the centralized locations.
41-42: LGTM!The
useStoreHydrationhook is correctly applied to both stores, replacing the previous manual hydration tracking logic.
47-55: LGTM!The
useProjectPathValidationhook is correctly initialized with default options, and all necessary values are destructured for use in the component.
286-295: LGTM!The
useProjectRestorationhook is correctly wired with all necessary state and the validation dialog callback, fulfilling the PR objective of validating project paths on app startup.
413-420: LGTM!The
ProjectPathValidationDialogis correctly integrated with all necessary props, includingonDismissfor proper cleanup when the user dismisses the dialog.apps/ui/src/hooks/use-project-restoration.ts (4)
1-5: LGTM!Imports are correctly structured, using local lib paths as appropriate for this UI app.
7-15: LGTM!The options interface is well-structured with clear typing for all restoration prerequisites.
36-66: LGTM!The
validateAndRestorecallback correctly implements the IPC check, path validation, and navigation flow. TheisValidatingRefpattern properly prevents concurrent validation attempts, addressing previous review feedback.
68-85: LGTM!The effect has comprehensive guards and the dependency array correctly includes all reactive values. The
isValidatingRefcheck prevents duplicate validation runs even whenvalidateAndRestoreis recreated due to dependency changes.apps/ui/src/hooks/use-project-path-validation.ts (4)
1-6: LGTM!Imports are correctly structured and include all necessary dependencies.
70-79: LGTM!The
handleRemoveProjectcallback correctly removes the project, clears state, closes the dialog, and navigates to the home page.
81-84: LGTM!The
handleDismisscallback correctly clears the current project to prevent the user from being left in a broken state with an invalid project selected.
86-94: LGTM!The hook returns a complete API with all necessary state and handlers for managing the validation dialog.
apps/ui/src/components/layout/sidebar/components/project-selector-with-options.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/ui/src/hooks/use-validated-project-cycling.ts (1)
26-36: Consider tightening the project parameter type.The parameter
project: typeof currentProjectinfersProject | null, but this helper is always called with a non-nullProject(line 77, inside theif (targetProject && ...)guard). Changing the type toproject: Projectwould more accurately reflect the call sites.🔎 Suggested type refinement
const switchProjectForCycling = useCallback( - (project: typeof currentProject, validHistory: string[], newIndex: number) => { + (project: Project, validHistory: string[], newIndex: number) => { useAppStore.setState({
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/ui/src/hooks/use-project-path-validation.tsapps/ui/src/hooks/use-validated-project-cycling.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/ui/src/hooks/use-project-path-validation.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always import from shared packages (@automaker/*), never from old relative paths
Files:
apps/ui/src/hooks/use-validated-project-cycling.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
resolveModelString()from @automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names
Files:
apps/ui/src/hooks/use-validated-project-cycling.ts
🧬 Code graph analysis (1)
apps/ui/src/hooks/use-validated-project-cycling.ts (1)
apps/ui/src/lib/validate-project-path.ts (1)
validateProjectPath(4-34)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (2)
apps/ui/src/hooks/use-validated-project-cycling.ts (2)
39-94: Excellent consolidation of cycling logic!The shared
cycleProjecthelper with a direction parameter eliminates duplication and addresses the previous review's suggestion. The index calculation correctly cycles through all projects (with wraparound) while skipping the current project, and the sequential validation approach is appropriate for keyboard-driven cycling.
96-102: LGTM!The wrapper functions provide a clean, direction-specific interface while reusing the shared logic.
|
e2e tests |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/ui/tests/utils/project/setup.ts (1)
790-790: Consider implications of the fallback path.The fallback to
/mock/test-projectmaintains backward compatibility, but tests that don't provideprojectPathwill fail validation if they trigger path validation flows. While the JSDoc makes the requirement clear, existing tests using this function may need updates.Consider whether tests that require path validation should explicitly fail without a valid
projectPathrather than silently using a non-existent fallback path. However, the current approach with clear documentation is acceptable.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/ui/tests/profiles/profiles-crud.spec.tsapps/ui/tests/utils/project/setup.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always import from shared packages (@automaker/*), never from old relative paths
Files:
apps/ui/tests/utils/project/setup.tsapps/ui/tests/profiles/profiles-crud.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
resolveModelString()from @automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names
Files:
apps/ui/tests/utils/project/setup.tsapps/ui/tests/profiles/profiles-crud.spec.ts
🧬 Code graph analysis (1)
apps/ui/tests/profiles/profiles-crud.spec.ts (2)
apps/ui/tests/utils/git/worktree.ts (2)
createTempDirPath(52-55)cleanupTempDir(157-161)apps/ui/tests/utils/project/setup.ts (1)
setupMockProjectWithProfiles(777-884)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (4)
apps/ui/tests/utils/project/setup.ts (1)
782-783: Good documentation of the new parameter.The JSDoc comment clearly indicates that
projectPathis required for path validation, helping test authors understand when to provide a real filesystem path.apps/ui/tests/profiles/profiles-crud.spec.ts (3)
8-8: Appropriate imports for test infrastructure.The
fsimport and test utility imports (createTempDirPath,cleanupTempDir) are necessary for managing the temporary directory required for path validation testing.Also applies to: 20-21
24-38: Well-structured temporary directory lifecycle.The implementation properly manages the test directory lifecycle:
- Unique path per test run prevents conflicts
- Directory created in
beforeAllensures it exists for path validation- Cleanup in
afterAllprevents test pollution
41-44: Test properly adapted for path validation.Passing
projectPath: TEST_TEMP_DIRensures the test uses a real filesystem path, allowing the new project path validation flows to succeed. This change aligns with the PR's objective of implementing centralized path validation.
Tweaked and work :) |
|
closing because of conflicts, reopen if this is still an issue |
2 participants
Summary
Fixes #283 - Resolves silent failures when project folders are moved, renamed, or deleted from the filesystem.
Changes
Path Validation System
validate-project-path.ts): Centralized async path validation with existence and directory checksproject-path-validation-dialog.tsx): User-friendly dialog when invalid paths are detected, offering:Consistent Validation Across All Entry Points
use-validated-project-cycling.ts: New hook validates paths before cycling, automatically skips invalid projectsproject-selector-with-options.tsx: Validates on manual selection__root.tsx: ValidatescurrentProjectbefore auto-restoring to board viewuse-trash-operations.ts: Prevents restoring projects with missing directoriesSupporting Infrastructure
use-store-hydration.ts: Ensures validation only runs after persisted state loadssidebar.tsx): Uses validated cycling instead of direct store methodsBehavior
Before: Keyboard shortcuts bypassed validation, app would silently break on invalid paths, data loss on restart
After:
Screenshots
Testing
Verified:
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.