fix: New Project Initialize does not work - throws error but project created #708
Conversation
📝 WalkthroughWalkthroughIntroduce a per-project Git preference ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AddModal as AddProjectModal
participant IPC as IPC Handler
participant Init as initializeProject
participant Store as ProjectStore
participant Sidebar
User->>AddModal: create project (initGit = false)
AddModal->>IPC: createProject request (project data)
IPC->>Init: initializeProject(path, { requireGit: false })
Init-->>IPC: success (git checks skipped)
IPC->>Store: addProject(...)
IPC->>Store: updateProjectSettings(id, { useGit: false })
Store-->>AddModal: persisted result
AddModal-->>User: project created / show status
rect rgba(200,200,255,0.12)
Note right of Sidebar: Later, user selects project
User->>Sidebar: select project
Sidebar->>Sidebar: sees settings.useGit === false
Sidebar-->>User: skip git status, hide GitSetupModal
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 (2 passed)
✨ Finishing touches
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 @StillKnotKnown, 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 implements a crucial fix for project initialization, specifically targeting scenarios where users choose to skip Git setup. By introducing a "useGit" preference within project settings, the system can now correctly identify and handle projects that do not require Git, preventing previous initialization failures. This change ensures that the application's behavior aligns with user choices, providing a more robust and user-friendly experience when creating or managing projects without Git integration. Highlights
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 effectively addresses the issue of project initialization failing when git is not used. The introduction of a useGit project setting allows for conditionally skipping git-related checks, which is a clean solution. The changes are well-structured and include new unit tests for both the backend logic and the frontend component behavior, which is great to see. I have a couple of suggestions to improve error handling and test code maintainability.
| } catch { | ||
| // Non-fatal - continue without updating git preference |
There was a problem hiding this comment.
Silently swallowing errors can hide underlying problems and make debugging harder. While this operation is marked as non-fatal, logging the error provides valuable information if the setting fails to persist, without interrupting the user flow.
| } catch { | |
| // Non-fatal - continue without updating git preference | |
| } catch (e) { | |
| console.error('Failed to persist \"useGit: false\" setting for new project.', e); |
| function createTestProject(overrides: Partial<Project> = {}): Project { | ||
| return { | ||
| id: 'project-1', | ||
| name: 'Test Project', | ||
| path: '/path/to/project', | ||
| autoBuildPath: '', | ||
| settings: { | ||
| model: 'opus', | ||
| memoryBackend: 'file', | ||
| linearSync: false, | ||
| notifications: { | ||
| onTaskComplete: true, | ||
| onTaskFailed: true, | ||
| onReviewNeeded: true, | ||
| sound: false | ||
| }, | ||
| graphitiMcpEnabled: true, | ||
| useGit: true | ||
| }, | ||
| createdAt: new Date(), | ||
| updatedAt: new Date(), | ||
| ...overrides | ||
| }; | ||
| } |
There was a problem hiding this comment.
The current implementation of createTestProject uses a shallow merge for overrides. When a test provides a settings object, it completely replaces the default settings. This makes tests brittle, as they have to redefine all default settings just to change one value. This can lead to maintenance issues when new settings are added to the default project.
To make this helper more robust, I'd recommend modifying it to perform a deeper merge, especially for the settings object. This way, tests only need to specify the settings they want to change.
Here's a suggested implementation:
function createTestProject(overrides: Partial<Project> = {}): Project {
const baseProject = {
id: 'project-1',
name: 'Test Project',
path: '/path/to/project',
autoBuildPath: '',
settings: {
model: 'opus',
memoryBackend: 'file',
linearSync: false,
notifications: {
onTaskComplete: true,
onTaskFailed: true,
onReviewNeeded: true,
sound: false
},
graphitiMcpEnabled: true,
useGit: true
},
createdAt: new Date(),
updatedAt: new Date(),
};
return {
...baseProject,
...overrides,
settings: {
...baseProject.settings,
...(overrides.settings || {}),
},
};
}With this change, the test case can be simplified to:
const project = createTestProject({
settings: { useGit: false },
});![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @apps/frontend/src/main/__tests__/project-initializer.test.ts:
- Around line 19-29: Add a test that asserts Git commands are not invoked when
initializeProject is called with { requireGit: false }: spy or mock the Node
child_process methods your code uses for Git (e.g., child_process.execSync /
exec / spawnSync) before calling initializeProject(projectPath, { requireGit:
false }), call the function, then assert the mocked Git method was not called;
also add a complementary test that when requireGit: true the Git method is
invoked. Reference initializeProject and the requireGit option and the
child_process method names so the test hooks the exact call sites.
In @apps/frontend/src/renderer/components/AddProjectModal.tsx:
- Around line 128-134: The updateProjectSettings call inside the initGit branch
currently swallows all errors; change this to surface failures by catching the
error from updateProjectSettings(project.id, { useGit: false }) and either log
it (via the app logger or console with contextual info including project.id and
useGit:false) and/or surface a user-facing notification before continuing, or
make it a fatal error by showing an error dialog and aborting the add flow;
locate the block around initGit, updateProjectSettings and the subsequent
onProjectAdded call and ensure the catch handler records the error details and
informs the user (or returns/throws) so Git initialization won’t proceed
silently with the wrong preference.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/frontend/src/main/project-initializer.ts (1)
268-306: Update the docstring to reflect conditional Git requirement.The function docstring still states "Requires: Project must be a git repository with at least one commit" (lines 274-276), but the implementation now makes this requirement conditional based on
options.requireGit. The documentation should reflect that Git is required by default but can be skipped whenrequireGit: falseis passed.🔎 Suggested documentation update
/** * Initialize auto-claude data directory in a project. * * Creates .auto-claude/ with data directories (specs, ideation, insights, roadmap). * The framework code runs from the source repo - only data is stored here. * * Requires: * - Project directory must exist - * - Project must be a git repository with at least one commit + * - Project must be a git repository with at least one commit (unless requireGit is false) + * + * @param projectPath - Path to the project directory + * @param options - Initialization options + * @param options.requireGit - Whether to require Git (defaults to true) */
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
apps/frontend/src/main/__tests__/project-initializer.test.tsapps/frontend/src/main/ipc-handlers/project-handlers.tsapps/frontend/src/main/project-initializer.tsapps/frontend/src/renderer/components/AddProjectModal.tsxapps/frontend/src/renderer/components/Sidebar.tsxapps/frontend/src/renderer/components/__tests__/Sidebar.test.tsxapps/frontend/src/shared/constants/config.tsapps/frontend/src/shared/types/project.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/shared/types/project.tsapps/frontend/src/main/__tests__/project-initializer.test.tsapps/frontend/src/main/ipc-handlers/project-handlers.tsapps/frontend/src/renderer/components/__tests__/Sidebar.test.tsxapps/frontend/src/renderer/components/AddProjectModal.tsxapps/frontend/src/renderer/components/Sidebar.tsxapps/frontend/src/main/project-initializer.tsapps/frontend/src/shared/constants/config.ts
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/shared/types/project.tsapps/frontend/src/main/__tests__/project-initializer.test.tsapps/frontend/src/main/ipc-handlers/project-handlers.tsapps/frontend/src/renderer/components/__tests__/Sidebar.test.tsxapps/frontend/src/renderer/components/AddProjectModal.tsxapps/frontend/src/renderer/components/Sidebar.tsxapps/frontend/src/main/project-initializer.tsapps/frontend/src/shared/constants/config.ts
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/shared/types/project.tsapps/frontend/src/main/__tests__/project-initializer.test.tsapps/frontend/src/main/ipc-handlers/project-handlers.tsapps/frontend/src/renderer/components/__tests__/Sidebar.test.tsxapps/frontend/src/renderer/components/AddProjectModal.tsxapps/frontend/src/renderer/components/Sidebar.tsxapps/frontend/src/main/project-initializer.tsapps/frontend/src/shared/constants/config.ts
🧠 Learnings (2)
📚 Learning: 2026-01-04T23:59:45.209Z
Learnt from: MikeeBuilds
Repo: AndyMik90/Auto-Claude PR: 661
File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189
Timestamp: 2026-01-04T23:59:45.209Z
Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings) can be deferred for future i18n cleanup passes. Do not fix such issues in PRs that do not introduce new i18n violations, especially in frontend TSX components (e.g., apps/frontend/**/*.tsx). If a PR adds new i18n violations, address them in that PR.
Applied to files:
apps/frontend/src/renderer/components/__tests__/Sidebar.test.tsxapps/frontend/src/renderer/components/AddProjectModal.tsxapps/frontend/src/renderer/components/Sidebar.tsx
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Use git worktrees for isolated agent builds with branches following the naming pattern `auto-claude/{spec-name}`
Applied to files:
apps/frontend/src/shared/constants/config.ts
🧬 Code graph analysis (3)
apps/frontend/src/renderer/components/__tests__/Sidebar.test.tsx (2)
apps/frontend/src/shared/types/project.ts (1)
Project(5-13)apps/frontend/src/renderer/components/Sidebar.tsx (1)
Sidebar(97-478)
apps/frontend/src/renderer/components/Sidebar.tsx (2)
apps/frontend/src/main/project-store.ts (1)
updateProjectSettings(224-235)apps/frontend/src/renderer/stores/project-store.ts (1)
updateProjectSettings(380-404)
apps/frontend/src/main/project-initializer.ts (1)
apps/frontend/src/shared/types/project.ts (1)
InitializationResult(450-453)
⏰ 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). (2)
- GitHub Check: CodeQL (python)
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (8)
apps/frontend/src/shared/types/project.ts (1)
29-30: LGTM! Clean type addition.The new
useGitfield is properly typed, well-documented, and follows the same pattern as other optional settings in the interface.apps/frontend/src/renderer/components/__tests__/Sidebar.test.tsx (1)
96-136: LGTM! Test effectively validates git opt-out behavior.The test correctly verifies that when
useGitis false, the Sidebar component skips Git status checks and keeps the Git setup modal closed. The mock setup is comprehensive and the assertions are appropriate.apps/frontend/src/shared/constants/config.ts (1)
74-76: LGTM! Default setting established correctly.The addition of
useGit: trueto the default project settings is correct and properly documented. This ensures projects use Git by default unless explicitly opted out.apps/frontend/src/main/ipc-handlers/project-handlers.ts (1)
347-348: LGTM! Git requirement correctly derived from project settings.The logic correctly defaults to requiring Git unless explicitly disabled via
useGit: false. The optional chaining handles undefined settings gracefully, maintaining backward compatibility for existing projects without the setting.apps/frontend/src/main/project-initializer.ts (2)
234-236: LGTM! Clean interface definition.The
InitializationOptionsinterface is well-defined with an optionalrequireGitfield, enabling flexible Git requirement configuration per initialization call.
278-281: LGTM! Backward-compatible signature update.The updated function signature maintains backward compatibility by defaulting
optionsto an empty object, ensuring existing callers without the parameter continue to work with default behavior.apps/frontend/src/renderer/components/Sidebar.tsx (2)
193-197: LGTM! Git checks correctly skipped when opt-out is set.The early return when
useGitis false properly prevents Git status checks and suppresses the Git setup modal for projects that have explicitly opted out of Git integration.
228-232: LGTM! Git setup skip handler correctly persists user preference.The
handleGitSetupSkipfunction properly updates project settings to persist the Git opt-out choice (useGit: false) and closes the modal. The null check on line 229 prevents errors if no project is selected. The wiring toGitSetupModalviaonSkipprop (line 474) enables users to skip Git setup from the UI.Also applies to: 474-474
StillKnotKnown
left a comment
StillKnotKnown
left a comment
There was a problem hiding this comment.
🤖 Auto Claude PR Review
Merge Verdict: 🟠 NEEDS REVISION
6 issue(s) must be addressed (3 required, 3 recommended), 4 suggestions
Risk Assessment
| Factor | Level | Notes |
|---|---|---|
| Complexity | Medium | Based on lines changed |
| Security Impact | None | Based on security findings |
| Scope Coherence | Good | Based on structural review |
Findings Summary
- High: 3 issue(s)
- Medium: 3 issue(s)
- Low: 4 issue(s)
Generated by Auto Claude PR Review
Findings (10 selected of 10 total)
🟠 [HIGH] Git check bypasses project's useGit setting during task start
📁 apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts:109
When a user sets useGit: false on a project, tasks still fail due to git checks. The code calls checkGitStatus() without checking project.settings?.useGit first. This creates an inconsistent UX where users can opt out of git during initialization but cannot run tasks afterward. This issue exists in 3 locations in execution-handlers.ts (lines 109, 530, 856).
Suggested fix:
Add a check for project.settings?.useGit before calling checkGitStatus(). Pattern: if (project.settings?.useGit !== false) { const gitStatus = checkGitStatus(project.path); if (!gitStatus.isGitRepo || !gitStatus.hasCommits) { /* error */ } } Apply this pattern to all three locations (lines 109, 530, 856).
🟠 [HIGH] Git check bypasses project's useGit setting during task auto-start
📁 apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts:530
When task status changes to 'in_progress' and auto-start triggers, the git check doesn't respect project.settings?.useGit. This is the second of three locations in execution-handlers.ts with the same issue.
Suggested fix:
Wrap git check in conditional: if (project.settings?.useGit !== false) { const gitStatusCheck = checkGitStatus(project.path); ... }
🟠 [HIGH] Git check bypasses project's useGit setting during task auto-restart
📁 apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts:856
During task recovery with autoRestart=true, the git check doesn't check project.settings?.useGit. This is the third location in execution-handlers.ts with the same issue.
Suggested fix:
Wrap git check in conditional: if (project.settings?.useGit !== false) { const gitStatusForRestart = checkGitStatus(project.path); ... }
🟡 [MEDIUM] Silent error swallowing in settings update
📁 apps/frontend/src/renderer/components/AddProjectModal.tsx:128
The empty catch block silently swallows any error when updating project settings to useGit: false. If the update fails, the project is created with useGit: true (default) despite the user unchecking the box. This creates a silent data inconsistency between UI state and persisted state without any logging for debugging.
Suggested fix:
Add console.error logging: } catch (error) { console.error('[AddProjectModal] Failed to update useGit setting:', error); } This maintains non-fatal behavior while providing visibility for troubleshooting. Consider also showing a non-blocking toast notification.
🟡 [MEDIUM] Missing test coverage for default requireGit behavior
📁 apps/frontend/src/main/__tests__/project-initializer.test.ts:19
The test only covers requireGit: false explicitly, but doesn't test the default behavior (undefined or true). The test should verify that requireGit defaults to true when options are omitted or undefined, ensuring git checks are required by default.
Suggested fix:
Add test case: 'it('requires git when options are undefined', () => { const result = initializeProject(projectPath, {}); expect(result.success).toBe(false); expect(result.error).toContain('Git'); });'
🟡 [MEDIUM] Missing test coverage for project with undefined settings.useGit
📁 apps/frontend/src/renderer/components/__tests__/Sidebar.test.tsx:96
The test only covers useGit: false explicitly. For existing projects created before this feature, settings.useGit would be undefined. The test should verify this edge case properly defaults to checking git (undefined !== false should be true).
Suggested fix:
Add test case with createTestProject({ settings: { ...baseSettings, useGit: undefined } }) and verify checkGitStatus IS called, ensuring default behavior works for legacy projects.
🔵 [LOW] Inconsistent error handling pattern lacks logging
📁 apps/frontend/src/renderer/components/AddProjectModal.tsx:76
There are three empty catch blocks in this file (lines 76-78, 129-133, 145-147) with 'Non-fatal' comments. While this follows existing codebase patterns, the lack of logging makes debugging difficult when these operations fail.
Suggested fix:
Standardize on adding debug logging: } catch (error) { console.debug('[AddProjectModal] Non-fatal error during operation:', error); }
🔵 [LOW] Double-negative pattern less readable than nullish coalescing
📁 apps/frontend/src/main/project-initializer.ts:293
The code uses options.requireGit !== false which is functionally correct but less readable than options.requireGit ?? true. The double-negative is harder to reason about and could confuse future maintainers.
Suggested fix:
Use nullish coalescing for clarity: const requireGit = options.requireGit ?? true;
🔵 [LOW] Test helper does shallow merge for nested settings object
📁 apps/frontend/src/renderer/components/__tests__/Sidebar.test.tsx:56
The createTestProject helper uses spread operator which performs shallow merge. When tests provide a settings object override, they must manually specify ALL required settings properties. This is verbose and error-prone if DEFAULT_PROJECT_SETTINGS adds new properties.
Suggested fix:
Use deep merge for settings: const settings = { ...DEFAULT_PROJECT_SETTINGS, ...overrides.settings }; Then return { ..., settings, ...overrides, settings };
🔵 [LOW] Inconsistent boolean check pattern across codebase
📁 apps/frontend/src/renderer/components/Sidebar.tsx:193
The code uses selectedProject.settings?.useGit === false while other parts use !== false (project-handlers.ts:347). Both are functionally correct but stylistically inconsistent with the existing useClaudeMd checks that use !== false pattern.
Suggested fix:
Consider standardizing on !== false pattern throughout for consistency. Change Sidebar.tsx:193 to useGit !== false and invert the conditional logic.
This review was generated by Auto Claude.
There was a problem hiding this comment.
Actionable comments posted: 3
Fix all issues with AI Agents 🤖
In @apps/frontend/src/renderer/components/__tests__/Sidebar.test.tsx:
- Around line 57-69: The createTestProject helper currently does a shallow merge
for settings (DEFAULT_PROJECT_SETTINGS + overrides.settings) which breaks nested
defaults; change it to perform a deep merge so tests can override only the
nested keys they care about. Update createTestProject to deep-merge
DEFAULT_PROJECT_SETTINGS with overrides.settings (e.g., use lodash.merge or a
small deepMerge utility) and then spread the rest of overrides into the returned
Project so the resulting settings retains nested defaults while applying
override values; keep the function name createTestProject and the
DEFAULT_PROJECT_SETTINGS reference intact.
In @apps/frontend/src/renderer/components/AddProjectModal.tsx:
- Around line 128-140: The error message is misleading when persisting the Git
preference fails: locate the block using initGit and updateProjectSettings
(inside AddProjectModal) where setError(t('addProject.failedToCreate')) is
called on catch, and change the translation key to a specific one like
'addProject.failedToSaveGitPreference' (and ensure setError uses
t('addProject.failedToSaveGitPreference')); also update i18n resources to add
that new key so users are informed the project was created but saving the Git
preference failed.
In @apps/frontend/src/renderer/components/Sidebar.tsx:
- Around line 228-232: handleGitSetupSkip currently closes the modal without
verifying updateProjectSettings succeeded; change it to await
updateProjectSettings(selectedProject.id, { useGit: false }) and check its
result or catch errors, only calling setShowGitSetupModal(false) when the call
returns success, and on failure keep the modal open and surface an error (e.g.
via existing toast/error handler) so the preference isn't lost; reference the
handleGitSetupSkip function, updateProjectSettings call, selectedProject.id, and
setShowGitSetupModal when making the change.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
apps/frontend/src/main/__tests__/project-initializer.test.tsapps/frontend/src/main/ipc-handlers/task/execution-handlers.tsapps/frontend/src/main/project-initializer.tsapps/frontend/src/renderer/components/AddProjectModal.tsxapps/frontend/src/renderer/components/Sidebar.tsxapps/frontend/src/renderer/components/__tests__/Sidebar.test.tsx
🧰 Additional context used
📓 Path-based instructions (3)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/renderer/components/__tests__/Sidebar.test.tsxapps/frontend/src/main/__tests__/project-initializer.test.tsapps/frontend/src/renderer/components/AddProjectModal.tsxapps/frontend/src/renderer/components/Sidebar.tsxapps/frontend/src/main/project-initializer.tsapps/frontend/src/main/ipc-handlers/task/execution-handlers.ts
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/renderer/components/__tests__/Sidebar.test.tsxapps/frontend/src/main/__tests__/project-initializer.test.tsapps/frontend/src/renderer/components/AddProjectModal.tsxapps/frontend/src/renderer/components/Sidebar.tsxapps/frontend/src/main/project-initializer.tsapps/frontend/src/main/ipc-handlers/task/execution-handlers.ts
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/renderer/components/__tests__/Sidebar.test.tsxapps/frontend/src/main/__tests__/project-initializer.test.tsapps/frontend/src/renderer/components/AddProjectModal.tsxapps/frontend/src/renderer/components/Sidebar.tsxapps/frontend/src/main/project-initializer.tsapps/frontend/src/main/ipc-handlers/task/execution-handlers.ts
🧠 Learnings (1)
📚 Learning: 2026-01-04T23:59:45.209Z
Learnt from: MikeeBuilds
Repo: AndyMik90/Auto-Claude PR: 661
File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189
Timestamp: 2026-01-04T23:59:45.209Z
Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings) can be deferred for future i18n cleanup passes. Do not fix such issues in PRs that do not introduce new i18n violations, especially in frontend TSX components (e.g., apps/frontend/**/*.tsx). If a PR adds new i18n violations, address them in that PR.
Applied to files:
apps/frontend/src/renderer/components/__tests__/Sidebar.test.tsxapps/frontend/src/renderer/components/AddProjectModal.tsxapps/frontend/src/renderer/components/Sidebar.tsx
🧬 Code graph analysis (5)
apps/frontend/src/renderer/components/__tests__/Sidebar.test.tsx (3)
apps/frontend/src/shared/types/project.ts (1)
Project(5-13)apps/frontend/src/shared/constants/config.ts (1)
DEFAULT_PROJECT_SETTINGS(60-77)apps/frontend/src/renderer/components/Sidebar.tsx (1)
Sidebar(97-478)
apps/frontend/src/main/__tests__/project-initializer.test.ts (1)
apps/frontend/src/main/project-initializer.ts (1)
initializeProject(278-346)
apps/frontend/src/renderer/components/Sidebar.tsx (1)
apps/frontend/src/renderer/stores/project-store.ts (1)
updateProjectSettings(380-404)
apps/frontend/src/main/project-initializer.ts (1)
apps/frontend/src/shared/types/project.ts (1)
InitializationResult(450-453)
apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts (2)
apps/frontend/src/main/project-initializer.ts (1)
checkGitStatus(34-93)apps/frontend/src/shared/constants/ipc.ts (1)
IPC_CHANNELS(6-511)
⏰ 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). (2)
- GitHub Check: CodeQL (python)
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (11)
apps/frontend/src/main/project-initializer.ts (2)
234-236: LGTM! Clean interface definition.The
InitializationOptionsinterface is well-structured and properly exported. The optionalrequireGitfield enables the new Git-skipping behavior while maintaining backward compatibility.
278-306: LGTM! Solid implementation of conditional Git validation.The changes correctly implement the Git-skipping behavior:
- Backward compatibility maintained via default
requireGit ?? true- Git validation properly guarded and skipped when
requireGitis false- Debug logging aids troubleshooting
- Error messages remain clear and actionable
apps/frontend/src/main/__tests__/project-initializer.test.ts (1)
1-72: LGTM! Comprehensive test coverage.The test suite effectively validates the
requireGitbehavior:
- ✅ Verifies initialization succeeds without Git when
requireGit: false- ✅ Confirms Git commands are not invoked when Git is disabled
- ✅ Tests default behavior (Git required when options omitted)
- ✅ Validates Git commands are invoked when
requireGit: trueThe mock verification approach addresses the previous review comment about definitively proving Git operations are skipped.
apps/frontend/src/renderer/components/AddProjectModal.tsx (1)
56-56: LGTM! Appropriate logging for non-fatal errors.The console warnings provide helpful debugging context for non-fatal failures (loading default location, detecting main branch, selecting directory) without disrupting the user experience.
Also applies to: 77-77, 95-95, 152-152
apps/frontend/src/renderer/components/__tests__/Sidebar.test.tsx (1)
86-166: LGTM! Tests effectively validate Git check skipping behavior.The test cases provide good coverage of the Sidebar's Git-related behavior:
- ✅ Verifies Git status checks are skipped when
useGitis false- ✅ Confirms GitSetupModal remains closed when Git is disabled
- ✅ Validates Git status checks run when
useGitis undefined (default behavior)The mock setup and assertions are appropriate for verifying the intended behavior.
apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts (3)
108-129: LGTM! Git validation correctly guarded in TASK_START.The implementation properly:
- Checks
useGit !== falseto determine whether Git validation is needed- Skips Git checks entirely when disabled
- Provides clear error messages via IPC when Git requirements aren't met
- Maintains early return pattern to prevent task execution without valid Git setup
531-545: LGTM! Consistent Git validation in TASK_UPDATE_STATUS.The auto-start logic correctly guards Git validation:
- Same conditional pattern as TASK_START (
useGit !== false)- Appropriate error propagation via IPC
- Early return prevents auto-start when Git is required but unavailable
859-876: LGTM! Appropriate Git validation in TASK_RECOVER_STUCK.The recovery handler correctly handles Git validation for auto-restart:
- Git checks only performed when attempting to restart
- Recovery succeeds even if restart is blocked by Git requirements
- Clear messaging about why restart couldn't proceed
- Maintains consistency with other handlers
The nuanced approach (recovery succeeds, restart optional) is appropriate for this use case.
apps/frontend/src/renderer/components/Sidebar.tsx (3)
46-47: LGTM!The new imports are necessary for persisting the Git preference when users skip Git setup.
193-209: LGTM! Good addition of error handling.The git status check logic correctly respects the
useGitpreference:
- Skips git checks when
useGit === false- Treats
undefinedastrue(sensible default: check git unless explicitly disabled)- Clears git status and hides the modal when git is disabled
- The added try-catch with
console.errorimproves error handling
474-474: Sidebar.tsx integration is correct; address i18n violations in GitSetupModal.The
onSkipprop integration in Sidebar.tsx line 474 is working correctly—GitSetupModal properly accepts and handles the prop. However, GitSetupModal contains hardcoded user-facing strings that violate the i18n guideline. Update the following to use translation keys:
- "Skip for now" (line 147) → use
t('gitSetup.skipButton')or similar- "Initialize Git" (line 151) → use
t('gitSetup.initButton')or similar- "Open a terminal in your project folder and run:" (line 131) → use
t('gitSetup.manualInstructions')or similarThese strings are already referenced in the dialogs translation namespace that the component imports.
⛔ Skipped due to learnings
Learnt from: CR Repo: AndyMik90/Auto-Claude PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-30T16:38:36.314Z Learning: Applies to apps/frontend/src/**/*.{ts,tsx,jsx} : Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded stringsLearnt from: MikeeBuilds Repo: AndyMik90/Auto-Claude PR: 661 File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189 Timestamp: 2026-01-04T23:59:45.209Z Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings that should be localized) can be deferred to future i18n cleanup passes rather than requiring immediate fixes in PRs that don't introduce new i18n violations.Learnt from: CR Repo: AndyMik90/Auto-Claude PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-30T16:38:36.314Z Learning: Applies to apps/frontend/src/shared/i18n/locales/**/*.json : When implementing new frontend features, add translation keys to all language files (minimum: en/*.json and fr/*.json)Learnt from: CR Repo: AndyMik90/Auto-Claude PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-30T16:38:36.314Z Learning: Applies to apps/frontend/src/**/*.{ts,tsx} : Use `useTranslation()` hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
apps/frontend/src/renderer/components/__tests__/Sidebar.test.tsx
Outdated
Show resolved
Hide resolved
StillKnotKnown
changed the title
There was a problem hiding this comment.
Actionable comments posted: 4
Fix all issues with AI Agents 🤖
In @apps/frontend/src/renderer/components/__tests__/Sidebar.test.tsx:
- Around line 111-151: The test is supplying a full settings object instead of
using the deep merge provided by createTestProject; update the test to only
override the minimal setting by calling createTestProject({ settings: { useGit:
false } }) (or otherwise merge with DEFAULT_PROJECT_SETTINGS) so the test
remains resilient to added defaults; locate and change the project creation in
the test (the createTestProject invocation) and remove the large explicit
settings block so the rest of the test still asserts that
window.electronAPI.checkGitStatus is not called and 'git-setup-modal' is closed.
- Around line 153-191: The test should only override the specific setting it
cares about instead of supplying the whole settings object; update the test that
uses createTestProject and mockUseProjectStore for Sidebar to build the project
via the existing deep merge helper (used in the other test) so you pass only {
settings: { useGit: undefined } } and let the helper merge defaults, then keep
the same render and assertion that window.electronAPI.checkGitStatus is called
with project.path.
In @apps/frontend/src/renderer/components/AddProjectModal.tsx:
- Around line 128-140: The catch block for updateProjectSettings currently calls
setError(...) and returns, blocking the rest of the flow; instead let the flow
continue so the project creation completes and onProjectAdded(...) runs: remove
the early return in the catch, keep or replace setError with a non-blocking
notification (e.g., toast) if desired, and ensure onProjectAdded(project,
initGit) is still invoked whether or not updateProjectSettings fails; touch the
catch around updateProjectSettings (function/updateProjectSettings usage) and
the subsequent call to onProjectAdded to guarantee continuation.
In @apps/frontend/src/renderer/components/Sidebar.tsx:
- Around line 230-255: handleGitSetupSkip duplicates the same toast in both the
false-return branch and the catch block; consolidate by extracting a single
helper that shows the error toast (e.g., showSaveGitPreferenceError) and call it
from both places. Update handleGitSetupSkip to call that helper when
updateProjectSettings returns false or when an exception is caught, and keep the
existing console.warn only in the catch branch (or include project context in
the helper if desired) so the UI toast logic is centralized and de-duplicated.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
apps/frontend/src/renderer/components/AddProjectModal.tsxapps/frontend/src/renderer/components/Sidebar.tsxapps/frontend/src/renderer/components/__tests__/Sidebar.test.tsxapps/frontend/src/shared/i18n/locales/en/dialogs.jsonapps/frontend/src/shared/i18n/locales/fr/dialogs.json
🧰 Additional context used
📓 Path-based instructions (4)
apps/frontend/src/shared/i18n/locales/**/*.json
📄 CodeRabbit inference engine (CLAUDE.md)
apps/frontend/src/shared/i18n/locales/**/*.json: Store translation strings in namespace-organized JSON files atapps/frontend/src/shared/i18n/locales/{lang}/*.jsonfor each supported language
When implementing new frontend features, add translation keys to all language files (minimum: en/.json and fr/.json)
Files:
apps/frontend/src/shared/i18n/locales/en/dialogs.jsonapps/frontend/src/shared/i18n/locales/fr/dialogs.json
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/renderer/components/Sidebar.tsxapps/frontend/src/renderer/components/__tests__/Sidebar.test.tsxapps/frontend/src/renderer/components/AddProjectModal.tsx
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/renderer/components/Sidebar.tsxapps/frontend/src/renderer/components/__tests__/Sidebar.test.tsxapps/frontend/src/renderer/components/AddProjectModal.tsx
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/renderer/components/Sidebar.tsxapps/frontend/src/renderer/components/__tests__/Sidebar.test.tsxapps/frontend/src/renderer/components/AddProjectModal.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: MikeeBuilds
Repo: AndyMik90/Auto-Claude PR: 661
File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189
Timestamp: 2026-01-04T23:59:45.209Z
Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings that should be localized) can be deferred to future i18n cleanup passes rather than requiring immediate fixes in PRs that don't introduce new i18n violations.
📚 Learning: 2026-01-04T23:59:45.209Z
Learnt from: MikeeBuilds
Repo: AndyMik90/Auto-Claude PR: 661
File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189
Timestamp: 2026-01-04T23:59:45.209Z
Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings) can be deferred for future i18n cleanup passes. Do not fix such issues in PRs that do not introduce new i18n violations, especially in frontend TSX components (e.g., apps/frontend/**/*.tsx). If a PR adds new i18n violations, address them in that PR.
Applied to files:
apps/frontend/src/renderer/components/Sidebar.tsxapps/frontend/src/renderer/components/__tests__/Sidebar.test.tsxapps/frontend/src/renderer/components/AddProjectModal.tsx
🧬 Code graph analysis (3)
apps/frontend/src/renderer/components/Sidebar.tsx (3)
apps/frontend/src/renderer/hooks/use-toast.ts (2)
useToast(192-192)toast(192-192)apps/frontend/src/main/project-store.ts (1)
updateProjectSettings(224-235)apps/frontend/src/renderer/stores/project-store.ts (1)
updateProjectSettings(380-404)
apps/frontend/src/renderer/components/__tests__/Sidebar.test.tsx (2)
apps/frontend/src/shared/types/project.ts (2)
Project(5-13)ProjectSettings(15-31)apps/frontend/src/shared/constants/config.ts (1)
DEFAULT_PROJECT_SETTINGS(60-77)
apps/frontend/src/renderer/components/AddProjectModal.tsx (1)
apps/frontend/src/renderer/components/settings/utils/hookProxyFactory.ts (1)
setError(19-19)
⏰ 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). (2)
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: CodeQL (python)
🔇 Additional comments (9)
apps/frontend/src/shared/i18n/locales/fr/dialogs.json (1)
119-119: LGTM! French localization added correctly.The French translation for the Git preference save failure is accurate and consistent with the English version.
apps/frontend/src/shared/i18n/locales/en/dialogs.json (1)
119-119: LGTM! English localization added correctly.The error message clearly communicates that the project was created successfully but saving the Git preference failed, which accurately reflects the non-fatal error scenario.
apps/frontend/src/renderer/components/__tests__/Sidebar.test.tsx (2)
57-77: LGTM! Deep merge utility correctly handles nested objects.The
deepMergeimplementation properly merges nested objects recursively, ensuring that partial overrides of nested properties (likenotifications) are handled correctly without replacing the entire object.
79-94: LGTM! Test helper now uses deep merge as recommended.The
createTestProjecthelper correctly usesdeepMergeto merge settings, allowing tests to override only the specific settings they care about while retaining all defaults. This addresses the previous review feedback.apps/frontend/src/renderer/components/Sidebar.tsx (3)
46-47: LGTM! Imports added for Git preference handling.The imports of
updateProjectSettingsanduseToastare correctly added to support persisting the user's choice to skip Git setup and provide error feedback.Also applies to: 54-54
195-211: LGTM! Git checks properly gated by useGit setting.The logic correctly skips Git status checks when
useGitis explicitly false, avoiding initialization errors for projects where the user opted out of Git. The fallback behavior (whenuseGitis undefined or true) retains the original Git validation flow.
497-497: LGTM! GitSetupModal wired to persist skip choice.The
onSkipprop correctly connects the modal's skip action to thehandleGitSetupSkiphandler, enabling persistence of the user's decision to opt out of Git.apps/frontend/src/renderer/components/AddProjectModal.tsx (2)
16-16: LGTM! Import added for Git preference persistence.The import of
updateProjectSettingsenables saving the user's Git preference for newly created projects.
56-56: LGTM! Non-fatal errors degraded to warnings.Changing from errors to
console.warnfor these non-critical operations (loading default location, detecting main branch) is appropriate, as these failures don't prevent core functionality.Also applies to: 77-77, 95-95, 152-152
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/frontend/src/renderer/components/AddProjectModal.tsx (1)
128-156: Inconsistent error handling disrupts user experience.When
updateProjectSettingsfails (line 130), the code callssetError(line 137) to display an error message in the modal. However, the flow continues to execute lines 154-155, which close the modal and trigger the callback. This creates a race condition where:
- The error message is set but the modal closes immediately
- The user never sees the error message
- The project was successfully created and the initialization dialog should appear, but the user is left confused
Since the project creation succeeded (line 126) and only the preference persistence failed, this is a non-fatal issue that shouldn't block the user flow. The past review comments suggested allowing the flow to continue, which the current code does, but using
setErroris incorrect because it's intended for blocking errors that keep the modal open.🔎 Use non-blocking notification instead of setError
Replace
setErrorwith a non-blocking toast notification that allows the flow to continue without disrupting the modal close:if (!initGit) { try { await updateProjectSettings(project.id, { useGit: false }); } catch (err) { console.warn('[AddProjectModal] Failed to persist git preference', { projectId: project.id, useGit: false, error: err }); - setError(t('addProject.failedToSaveGitPreference')); + // Non-blocking warning - project was created successfully + // Note: You'll need to import useToast at the top of the file + // import { useToast } from '../hooks/use-toast'; + // And add const { toast } = useToast(); in the component + toast?.({ + title: t('common:labels.warning'), + description: t('addProject.failedToSaveGitPreference'), + variant: 'default' + }); } }Alternatively, if you don't want to show any user-facing message for this non-fatal failure, simply remove the
setErrorline and rely on the console warning.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
apps/frontend/src/renderer/components/AddProjectModal.tsxapps/frontend/src/renderer/components/Sidebar.tsxapps/frontend/src/renderer/components/__tests__/Sidebar.test.tsx
🧰 Additional context used
📓 Path-based instructions (3)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/renderer/components/Sidebar.tsxapps/frontend/src/renderer/components/AddProjectModal.tsxapps/frontend/src/renderer/components/__tests__/Sidebar.test.tsx
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/renderer/components/Sidebar.tsxapps/frontend/src/renderer/components/AddProjectModal.tsxapps/frontend/src/renderer/components/__tests__/Sidebar.test.tsx
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/renderer/components/Sidebar.tsxapps/frontend/src/renderer/components/AddProjectModal.tsxapps/frontend/src/renderer/components/__tests__/Sidebar.test.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: MikeeBuilds
Repo: AndyMik90/Auto-Claude PR: 661
File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189
Timestamp: 2026-01-04T23:59:45.209Z
Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings that should be localized) can be deferred to future i18n cleanup passes rather than requiring immediate fixes in PRs that don't introduce new i18n violations.
📚 Learning: 2026-01-04T23:59:45.209Z
Learnt from: MikeeBuilds
Repo: AndyMik90/Auto-Claude PR: 661
File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189
Timestamp: 2026-01-04T23:59:45.209Z
Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings) can be deferred for future i18n cleanup passes. Do not fix such issues in PRs that do not introduce new i18n violations, especially in frontend TSX components (e.g., apps/frontend/**/*.tsx). If a PR adds new i18n violations, address them in that PR.
Applied to files:
apps/frontend/src/renderer/components/Sidebar.tsxapps/frontend/src/renderer/components/AddProjectModal.tsxapps/frontend/src/renderer/components/__tests__/Sidebar.test.tsx
🧬 Code graph analysis (1)
apps/frontend/src/renderer/components/Sidebar.tsx (2)
apps/frontend/src/renderer/hooks/use-toast.ts (2)
useToast(192-192)toast(192-192)apps/frontend/src/renderer/stores/project-store.ts (1)
updateProjectSettings(380-404)
⏰ 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). (2)
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: CodeQL (python)
🔇 Additional comments (11)
apps/frontend/src/renderer/components/__tests__/Sidebar.test.tsx (5)
1-113: LGTM! Comprehensive test setup.The test setup is well-structured with appropriate mocks for all dependencies. The
gitSetupModalSpycorrectly provides testable attributes, and theelectronAPImock covers all required methods.
57-77: LGTM! Well-implemented deep merge utility.The
deepMergehelper correctly handles nested object merging with proper type guards. TheisPlainObjectcheck appropriately excludes arrays and null values, ensuring safe recursive merging.
79-98: LGTM! Test helper correctly implements deep merge.The
createTestProjecthelper properly usesdeepMergefor settings, allowing tests to specify only the properties they care about while preserving defaults. TheProjectOverridestype definition correctly separatessettingsfor partial typing.
115-143: LGTM! Test correctly verifies Git check skipping.The test properly verifies that when
useGitisfalse, the Sidebar component skips Git status checks and keeps the GitSetupModal closed. The simplified test data usingcreateTestProject({ settings: { useGit: false } })makes the test maintainable and focused.
145-171: LGTM! Test correctly verifies Git check invocation.The test properly verifies that when
useGitisundefined, the Sidebar component performs Git status checks. The assertion confirms the correct project path is passed tocheckGitStatus.apps/frontend/src/renderer/components/Sidebar.tsx (4)
46-47: LGTM! Dependencies correctly imported.The new imports for
updateProjectSettingsanduseToastare properly added to support Git preference persistence and error notification functionality.Also applies to: 54-54, 117-117
192-217: LGTM! Git status check correctly guarded by useGit preference.The logic properly skips Git checks when
useGitisfalseand clears the Git status state. The optional chaining safely handles projects without settings, and the try-catch block appropriately logs errors without disrupting the UI flow.
230-255: LGTM! Error handling properly consolidated.The
showSaveGitPreferenceErrorhelper eliminates duplication as suggested in past reviews. ThehandleGitSetupSkipfunction correctly:
- Persists the
useGit: falsepreference viaupdateProjectSettings- Closes the modal only on success
- Shows error toast for both failure and exception cases
- Includes diagnostic context in console warnings
- Uses proper i18n keys per coding guidelines
491-498: LGTM! GitSetupModal correctly wired with skip handler.The
onSkipprop is properly connected tohandleGitSetupSkip, enabling users to persist their preference when skipping Git setup.apps/frontend/src/renderer/components/AddProjectModal.tsx (2)
16-16: LGTM! Required dependency imported.The
updateProjectSettingsimport is correctly added to support Git preference persistence.
48-60: LGTM! Non-fatal errors properly logged.The console warnings for non-fatal failures (default location, directory selection, main branch detection) improve observability without disrupting the user flow. The consistent use of
[AddProjectModal]prefix aids debugging.Also applies to: 62-86, 88-97, 142-153
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @apps/frontend/src/renderer/components/AddProjectModal.tsx:
- Around line 130-145: In AddProjectModal, update the toast description
translation key to include the dialogs namespace: replace the current
'addProject.failedToSaveGitPreference' with
'dialogs:addProject.failedToSaveGitPreference' where the toast is invoked
(inside the catch block that calls updateProjectSettings and toast); ensure the
change is made in the toast({ description: t(...) }) call so it matches the
namespace pattern used elsewhere like in Sidebar.tsx.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/frontend/src/renderer/components/AddProjectModal.tsx (1)
148-159: Consider consistent error surfacing for settings persistence failures.Main branch detection failure (line 157) only logs a warning, while Git preference save failure (lines 139-143) shows both a warning and a toast notification. Both are non-fatal settings persistence failures after successful project creation.
For consistent user experience, consider either:
- Adding a toast notification for main branch detection failure (to match Git preference handling), or
- Documenting why Git preference failures warrant user notification while main branch failures don't
💡 Option 1: Add toast for main branch detection failure
} catch { console.warn('[AddProjectModal] Failed to detect main branch for new project'); + toast({ + title: t('common:labels.warning'), + description: t('addProject.failedToDetectMainBranch'), + variant: 'default' + }); }Note: This would require adding the
addProject.failedToDetectMainBranchtranslation key.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
apps/frontend/src/renderer/components/AddProjectModal.tsx
🧰 Additional context used
📓 Path-based instructions (3)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/renderer/components/AddProjectModal.tsx
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/renderer/components/AddProjectModal.tsx
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/renderer/components/AddProjectModal.tsx
🧠 Learnings (3)
📓 Common learnings
Learnt from: MikeeBuilds
Repo: AndyMik90/Auto-Claude PR: 661
File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189
Timestamp: 2026-01-04T23:59:45.209Z
Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings that should be localized) can be deferred to future i18n cleanup passes rather than requiring immediate fixes in PRs that don't introduce new i18n violations.
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/**/*.{ts,tsx,jsx} : Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Applied to files:
apps/frontend/src/renderer/components/AddProjectModal.tsx
📚 Learning: 2026-01-04T23:59:45.209Z
Learnt from: MikeeBuilds
Repo: AndyMik90/Auto-Claude PR: 661
File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189
Timestamp: 2026-01-04T23:59:45.209Z
Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings) can be deferred for future i18n cleanup passes. Do not fix such issues in PRs that do not introduce new i18n violations, especially in frontend TSX components (e.g., apps/frontend/**/*.tsx). If a PR adds new i18n violations, address them in that PR.
Applied to files:
apps/frontend/src/renderer/components/AddProjectModal.tsx
🧬 Code graph analysis (1)
apps/frontend/src/renderer/components/AddProjectModal.tsx (1)
apps/frontend/src/renderer/hooks/use-toast.ts (2)
useToast(192-192)toast(192-192)
⏰ 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). (2)
- GitHub Check: CodeQL (python)
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (3)
apps/frontend/src/renderer/components/AddProjectModal.tsx (3)
16-17: LGTM - Imports support new Git preference persistence.The added imports (
updateProjectSettings,useToast, andcommontranslation namespace) are necessary for persisting the Git preference and providing user feedback on failures.Also applies to: 29-30
58-58: LGTM - Improved error visibility for debugging.Replacing silent failures with
console.warnprovides valuable debugging information without disrupting the user flow for non-fatal operations.Also applies to: 79-79, 97-97, 157-157
160-161: The initialization logic correctly handles the default Git setting. WhenupdateProjectSettingsfails to persistuseGit: false, the project retains the defaultuseGit: truevalue, and the initialization handler properly respects this with theproject.settings?.useGit !== falsecheck inproject-handlers.ts:347, ensuring thatrequireGitis determined accurately based on the persisted setting.
StillKnotKnown
left a comment
StillKnotKnown
left a comment
There was a problem hiding this comment.
🤖 Auto Claude PR Review
⚠️ Follow-up Review: Merge With Changes
Resolution Status
- ✅ Resolved: 10 previous findings addressed
- ❌ Unresolved: 0 previous findings remain
- 🆕 New Issues: 7 new findings in recent changes
Verdict
All CI checks passing. All 10 previous findings (3 high, 3 medium, 4 low) have been verified as resolved. The new-code-reviewer identified 7 additional findings, all of which are low-severity quality and test suggestions. No critical, high, or medium-severity logic/security issues remain. The implementation properly adds useGit opt-out functionality with type safety, backward compatibility, consistent checks, error handling, and test coverage. The low-severity findings can be addressed post-merge as polish items.
Review Process
Agents invoked: resolution-verifier, new-code-reviewer
This is an AI-generated follow-up review using parallel specialist analysis with finding validation.
Findings (7 selected of 7 total)
🟡 [MEDIUM] Inconsistent error handling for git preference save failure
📁 apps/frontend/src/renderer/components/AddProjectModal.tsx:130
When git init is disabled, the code attempts to persist useGit:false but catches errors and shows a toast. The project creation still completes even though the setting failed to save, creating an inconsistent state.
Suggested fix:
Current implementation correctly notifies user via toast. This is acceptable as the user is informed.
🔵 [LOW] Git check error message is generic
📁 apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts:531
The error message returned when git check fails during auto-start is generic and doesn't mention the useGit setting context.
Suggested fix:
The error message is appropriate for the context. No change needed.
🔵 [LOW] Duplicate error handling code for git preference save
📁 apps/frontend/src/renderer/components/Sidebar.tsx:238
The showSaveGitPreferenceError function is duplicated between Sidebar.tsx and AddProjectModal.tsx.
Suggested fix:
Consider extracting to a shared utility function. However, this is minor and acceptable.
🔵 [LOW] Default useGit setting verification
📁 apps/frontend/src/shared/constants/config.ts:76
Verification that the default useGit: true setting works correctly with the !== false pattern.
Suggested fix:
This is correct. The !== false pattern allows existing projects to default to git-enabled behavior.
🔵 [LOW] InitializationOptions type is properly exported
📁 apps/frontend/src/main/project-initializer.ts:234
Verification that the InitializationOptions interface is correctly exported and used.
Suggested fix:
The interface is correctly exported and used. No action needed.
🔵 [LOW] Test coverage could explicitly test undefined requireGit
📁 apps/frontend/src/main/__tests__/project-initializer.test.ts:50
While the test covers the default behavior when options are omitted, it could explicitly test requireGit: undefined.
Suggested fix:
Current test coverage is sufficient as it tests the behavior when options parameter is omitted.
🔵 [LOW] Test coverage for useGit false case is comprehensive
📁 apps/frontend/src/renderer/components/__tests__/Sidebar.test.tsx:115
The test properly validates that when useGit is false, git checks are skipped and the git setup modal is not shown.
Suggested fix:
Test coverage is good. No action needed.
This review was generated by Auto Claude.
…lize-does-not-work-throws-error-but
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI Agents
In @apps/frontend/src/main/__tests__/project-initializer.test.ts:
- Around line 50-74: The two tests are duplicates verifying default/undefined
requireGit behavior; remove the second test block and keep a single test that
calls initializeProject(projectPath) (or initializeProject(projectPath, {
requireGit: undefined }) if you prefer) to assert execFileSyncMock was called
and result.success is false; locate the tests by the test descriptions and the
initializeProject, requireGit and execFileSyncMock symbols and delete the
redundant "requires git when requireGit is undefined" test, or consolidate both
expectations into one test if you want to explicitly show both call forms.
In @apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts:
- Around line 108-129: Add unit/integration tests for the three handlers that
include the new Git preference conditional: exercise TASK_START,
TASK_UPDATE_STATUS, and TASK_RECOVER_STUCK flows by mocking
project.settings.useGit and the checkGitStatus function; assert that when useGit
is false the handler proceeds without needing a git repo, and when useGit is
true or undefined the handler calls checkGitStatus and that
mainWindow.webContents.send is invoked with IPC_CHANNELS.TASK_ERROR and the
exact error messages used in the handlers when isGitRepo is false or hasCommits
is false; ensure you stub checkGitStatus to return both failure modes and a
success mode and verify behavior for each handler (use mocks/spies on
mainWindow.webContents.send and taskId to validate correct error routing).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
apps/frontend/src/main/__tests__/project-initializer.test.tsapps/frontend/src/main/ipc-handlers/task/execution-handlers.tsapps/frontend/src/renderer/components/AddProjectModal.tsxapps/frontend/src/renderer/components/Sidebar.tsxapps/frontend/src/renderer/lib/toast-utils.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/renderer/components/Sidebar.tsxapps/frontend/src/main/ipc-handlers/task/execution-handlers.tsapps/frontend/src/main/__tests__/project-initializer.test.tsapps/frontend/src/renderer/components/AddProjectModal.tsxapps/frontend/src/renderer/lib/toast-utils.ts
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/renderer/components/Sidebar.tsxapps/frontend/src/main/ipc-handlers/task/execution-handlers.tsapps/frontend/src/main/__tests__/project-initializer.test.tsapps/frontend/src/renderer/components/AddProjectModal.tsxapps/frontend/src/renderer/lib/toast-utils.ts
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/renderer/components/Sidebar.tsxapps/frontend/src/main/ipc-handlers/task/execution-handlers.tsapps/frontend/src/main/__tests__/project-initializer.test.tsapps/frontend/src/renderer/components/AddProjectModal.tsxapps/frontend/src/renderer/lib/toast-utils.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: MikeeBuilds
Repo: AndyMik90/Auto-Claude PR: 661
File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189
Timestamp: 2026-01-04T23:59:45.209Z
Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings that should be localized) can be deferred to future i18n cleanup passes rather than requiring immediate fixes in PRs that don't introduce new i18n violations.
📚 Learning: 2026-01-04T23:59:45.209Z
Learnt from: MikeeBuilds
Repo: AndyMik90/Auto-Claude PR: 661
File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189
Timestamp: 2026-01-04T23:59:45.209Z
Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings) can be deferred for future i18n cleanup passes. Do not fix such issues in PRs that do not introduce new i18n violations, especially in frontend TSX components (e.g., apps/frontend/**/*.tsx). If a PR adds new i18n violations, address them in that PR.
Applied to files:
apps/frontend/src/renderer/components/Sidebar.tsxapps/frontend/src/renderer/components/AddProjectModal.tsx
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/**/*.{ts,tsx,jsx} : Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Applied to files:
apps/frontend/src/renderer/components/AddProjectModal.tsx
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/**/*.{ts,tsx} : Use `useTranslation()` hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Applied to files:
apps/frontend/src/renderer/components/AddProjectModal.tsx
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/shared/i18n/locales/**/*.json : When implementing new frontend features, add translation keys to all language files (minimum: en/*.json and fr/*.json)
Applied to files:
apps/frontend/src/renderer/components/AddProjectModal.tsx
🧬 Code graph analysis (4)
apps/frontend/src/renderer/components/Sidebar.tsx (4)
apps/frontend/src/renderer/hooks/use-toast.ts (2)
useToast(192-192)toast(192-192)apps/frontend/src/main/project-store.ts (1)
updateProjectSettings(224-235)apps/frontend/src/renderer/stores/project-store.ts (1)
updateProjectSettings(380-404)apps/frontend/src/renderer/lib/toast-utils.ts (1)
showGitPreferenceSaveError(6-12)
apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts (1)
apps/frontend/src/main/project-initializer.ts (1)
checkGitStatus(34-93)
apps/frontend/src/main/__tests__/project-initializer.test.ts (1)
apps/frontend/src/main/project-initializer.ts (1)
initializeProject(278-346)
apps/frontend/src/renderer/lib/toast-utils.ts (1)
apps/frontend/src/renderer/hooks/use-toast.ts (2)
useToast(192-192)toast(192-192)
⏰ 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). (2)
- GitHub Check: CodeQL (python)
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (13)
apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts (1)
108-129: Well-implemented Git preference guards across all execution paths.The conditional Git checks are correctly implemented:
- The guard condition
project.settings?.useGit !== falseproperly defaults to requiring Git (when undefined/null/true) while allowing explicit opt-out withfalse- Consistent application across TASK_START, TASK_UPDATE_STATUS auto-start, and TASK_RECOVER_STUCK auto-restart paths
- Proper early returns prevent execution when Git requirements aren't met
- Authentication checks remain mandatory regardless of Git preference (correct)
- Error messages provide actionable guidance including the new "disable Git in project settings" option
The differing return patterns are appropriate for each context:
- TASK_START and TASK_UPDATE_STATUS return errors when Git checks fail
- TASK_RECOVER_STUCK returns success with
autoRestarted: falsebecause the recovery itself succeeded even though the optional restart couldn't proceedAlso applies to: 531-547, 861-878
apps/frontend/src/main/__tests__/project-initializer.test.ts (2)
1-10: LGTM!The import structure and mock setup for
child_processare correct. The mock will properly interceptexecFileSynccalls during tests.
12-27: LGTM!The test setup and teardown logic is well-structured. Resetting mocks in
beforeEachensures test isolation, and the cleanup inafterEachproperly removes all temporary directories.apps/frontend/src/renderer/lib/toast-utils.ts (2)
1-4: LGTM!The type definition for
ToastFncorrectly extracts the toast function signature from theuseToasthook return type.
6-12: LGTM!The function properly uses namespace-prefixed translation keys (
common:labels.erroranddialogs:addProject.failedToSaveGitPreference) as required by the coding guidelines. This abstraction promotes code reuse across components.apps/frontend/src/renderer/components/Sidebar.tsx (4)
46-47: LGTM!The new imports for
updateProjectSettings,useToast, andshowGitPreferenceSaveErrorare correctly integrated, and the toast hook is properly initialized.Also applies to: 54-55, 118-118
196-212: LGTM!The Git status check correctly respects the
useGitpreference with proper default behavior (Git checks enabled unless explicitly disabled). The try/catch wrapper prevents crashes and provides appropriate error logging.
231-248: LGTM!The
handleGitSetupSkipfunction properly persists the Git preference and handles both failure modes (false return and exception) with appropriate user feedback via the extractedshowGitPreferenceSaveErrorhelper, which addresses the duplication concern from previous reviews.
490-490: LGTM!The
onSkipprop correctly wires the persistence logic to the Git setup modal, enabling the user to opt out of Git setup with their preference saved.apps/frontend/src/renderer/components/AddProjectModal.tsx (4)
16-18: LGTM!The new imports and hooks are correctly integrated. The
useTranslationhook properly includes both'dialogs'and'common'namespaces as required by the translation keys used in this component.Also applies to: 30-31
59-59: LGTM!Converting silent failures to
console.warncalls improves debuggability by surfacing non-fatal issues. The log messages are properly prefixed with[AddProjectModal]for easy filtering.Also applies to: 80-80, 98-98, 157-157
146-168: LGTM!The main branch detection for Git-initialized projects is properly wrapped in try/catch, with failures logged as non-fatal warnings. The callback and modal closure complete the project creation flow appropriately.
131-145: Inconsistent error handling for Git preference persistence.When
updateProjectSettingsreturnsfalse(line 134), the function shows an error toast and returns early (line 136), preventing the modal from closing. However, whenupdateProjectSettingsthrows an exception (caught at line 137), the function shows an error toast but does not return, allowing the flow to continue to lines 160-161.This inconsistency means:
- Save returning false: project added to store, modal stays open, user notified
- Save throwing exception: project added to store, modal closes, user notified but flow completes
Both error paths should behave identically—either both should return early or both should continue.
🔎 Add return statement for consistency
} catch (err) { console.warn('[AddProjectModal] Failed to persist git preference', { projectId: project.id, useGit: false, error: err }); showGitPreferenceSaveError(toast, t); + return; }Likely an incorrect or invalid review comment.
| it('invokes git commands when requireGit is true', () => { | ||
| const projectPath = mkdtempSync(path.join(tmpdir(), 'auto-claude-init-')); | ||
| tempDirs.push(projectPath); | ||
| execFileSyncMock.mockImplementation(() => ''); | ||
|
|
||
| initializeProject(projectPath, { requireGit: true }); | ||
|
|
||
| expect(execFileSyncMock).toHaveBeenCalled(); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Verify initialization success in this test.
The test mocks Git commands to succeed (returning empty string) but doesn't assert that initialization completes successfully. This leaves a gap in test coverage.
🔎 Add success assertions
it('invokes git commands when requireGit is true', () => {
const projectPath = mkdtempSync(path.join(tmpdir(), 'auto-claude-init-'));
tempDirs.push(projectPath);
execFileSyncMock.mockImplementation(() => '');
- initializeProject(projectPath, { requireGit: true });
+ const result = initializeProject(projectPath, { requireGit: true });
expect(execFileSyncMock).toHaveBeenCalled();
+ expect(result.success).toBe(true);
+ expect(result.error).toBeUndefined();
+ expect(existsSync(path.join(projectPath, '.auto-claude'))).toBe(true);
});| const gitRequirementMessage = gitStatusCheck.error | ||
| || 'Git repository with commits required to run tasks. Initialize git or disable Git in project settings.'; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider i18n for new user-facing error messages.
The new error messages at lines 535-536 and 873 (mentioning "disable Git in project settings") are hardcoded English strings. Per coding guidelines, user-facing text should use i18n translation keys. While this file follows the existing pattern of the main process sending English strings via IPC, consider implementing i18n support for main process error messages in a future cleanup pass.
Based on learnings, this can be deferred to future i18n work.
Also applies to: 873-873
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @apps/frontend/src/main/__tests__/ipc-handlers.test.ts:
- Line 158: The mock property isRunning on mockAgentManager is unused; either
remove it from the mock and its type (delete the isRunning: ReturnType<typeof
vi.fn> declaration and any assignment in mockAgentManager) or add a test that
asserts its behavior (e.g., set isRunning.mockReturnValueOnce/true and assert
code paths that call mockAgentManager.isRunning are exercised), or if it’s
intentionally reserved for future tests, add an inline comment next to isRunning
in the mock explaining why it is present and when it will be used; update any
related imports or type annotations to keep the test file consistent.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
apps/frontend/src/main/__tests__/ipc-handlers.test.tsapps/frontend/src/main/__tests__/project-initializer.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/main/__tests__/project-initializer.test.tsapps/frontend/src/main/__tests__/ipc-handlers.test.ts
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/main/__tests__/project-initializer.test.tsapps/frontend/src/main/__tests__/ipc-handlers.test.ts
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/main/__tests__/project-initializer.test.tsapps/frontend/src/main/__tests__/ipc-handlers.test.ts
🧬 Code graph analysis (1)
apps/frontend/src/main/__tests__/project-initializer.test.ts (1)
apps/frontend/src/main/project-initializer.ts (1)
initializeProject(278-346)
🔇 Additional comments (9)
apps/frontend/src/main/__tests__/project-initializer.test.ts (5)
1-10: LGTM! Clean test setup.The imports and mock configuration are appropriate for testing the project initialization flow. Mocking
execFileSyncallows isolated testing of Git-related behavior.
12-27: LGTM! Proper test lifecycle management.The setup and teardown hooks ensure test isolation by resetting mocks and cleaning up temporary directories.
29-39: LGTM! Comprehensive success case.The test properly verifies both the successful return value and the expected directory structure when Git checks are bypassed.
41-48: LGTM! Focused verification of Git bypass.This test definitively proves that Git operations are skipped when
requireGitis false, addressing the concern from earlier reviews.
50-61: LGTM! Default behavior correctly tested.The test verifies that Git checks are performed by default and that initialization fails appropriately when Git is unavailable.
apps/frontend/src/main/__tests__/ipc-handlers.test.ts (4)
10-11: LGTM! Import additions support the new test suite.The new imports for
IPC_CHANNELS,AUTO_BUILD_PATHS,DEFAULT_PROJECT_SETTINGS, and type definitions are properly utilized throughout the new Git checks test suite.
182-183: Good addition for test isolation.Clearing ipcMain listeners and handlers in
beforeEachensures clean test state and prevents test interference.
519-612: Well-structured test setup with good isolation.The helper functions and
setupTaskExecutionTestfixture factory provide excellent test reusability and isolation:
- Clean separation of concerns with
ensureSpecDirandwritePlanFilehelpers- Comprehensive mocking of dependencies (findTaskAndProject, checkGitStatus, fileWatcher)
- Proper cleanup function that restores all spies
- Flexible configuration via parameters for different test scenarios
614-882: Use i18n translation keys for user-facing error messages instead of hardcoded strings.The test error message assertions are correctly synchronized with the implementation. However, these error messages are sent to the renderer via IPC (
mainWindow.webContents.send) making them user-facing, yet they use hardcoded strings instead of i18n translation keys. Per coding guidelines, all user-facing text in the frontend should use i18n translation keys with namespace prefixes (e.g.,'errors:git.repositoryRequired'). This applies to:
- TASK_START error messages (missing repo, no commits)
- TASK_UPDATE_STATUS error messages (git/auth checks during auto-start)
- TASK_RECOVER_STUCK error messages (git checks before auto-restart)
Extract these strings to an i18n translation file and reference them via
useTranslation()or a similar i18n resolution mechanism.⛔ Skipped due to learnings
Learnt from: CR Repo: AndyMik90/Auto-Claude PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-30T16:38:36.314Z Learning: Applies to apps/frontend/src/**/*.{ts,tsx,jsx} : Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded stringsLearnt from: MikeeBuilds Repo: AndyMik90/Auto-Claude PR: 661 File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189 Timestamp: 2026-01-04T23:59:45.209Z Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings that should be localized) can be deferred to future i18n cleanup passes rather than requiring immediate fixes in PRs that don't introduce new i18n violations.Learnt from: CR Repo: AndyMik90/Auto-Claude PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-30T16:38:36.314Z Learning: Applies to apps/frontend/src/shared/i18n/locales/**/*.json : When implementing new frontend features, add translation keys to all language files (minimum: en/*.json and fr/*.json)
| startQAProcess: ReturnType<typeof vi.fn>; | ||
| killTask: ReturnType<typeof vi.fn>; | ||
| configure: ReturnType<typeof vi.fn>; | ||
| isRunning: ReturnType<typeof vi.fn>; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Get the full test file to understand context
wc -l apps/frontend/src/main/__tests__/ipc-handlers.test.tsRepository: AndyMik90/Auto-Claude
Length of output: 122
🏁 Script executed:
#!/bin/bash
# Search for isRunning usage in the test file
rg -n "isRunning" apps/frontend/src/main/__tests__/ipc-handlers.test.tsRepository: AndyMik90/Auto-Claude
Length of output: 149
🏁 Script executed:
#!/bin/bash
# Get context around line 158 and 196-197
sed -n '150,170p' apps/frontend/src/main/__tests__/ipc-handlers.test.tsRepository: AndyMik90/Auto-Claude
Length of output: 832
🏁 Script executed:
#!/bin/bash
# Get context around line 196-197
sed -n '190,210p' apps/frontend/src/main/__tests__/ipc-handlers.test.tsRepository: AndyMik90/Auto-Claude
Length of output: 700
🏁 Script executed:
#!/bin/bash
# Search for all test cases and where isRunning might be called/asserted
rg -n "expect.*isRunning|isRunning\(" apps/frontend/src/main/__tests__/ipc-handlers.test.tsRepository: AndyMik90/Auto-Claude
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Get a broader view to see test structure and any assertions
sed -n '200,400p' apps/frontend/src/main/__tests__/ipc-handlers.test.ts | head -100Repository: AndyMik90/Auto-Claude
Length of output: 4214
🏁 Script executed:
#!/bin/bash
# Search more broadly for any usage of mockAgentManager.isRunning
rg -n "mockAgentManager.*isRunning|\.isRunning" apps/frontend/src/main/__tests__/ipc-handlers.test.tsRepository: AndyMik90/Auto-Claude
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Get a count of all references to isRunning
rg -c "isRunning" apps/frontend/src/main/__tests__/ipc-handlers.test.tsRepository: AndyMik90/Auto-Claude
Length of output: 66
🏁 Script executed:
#!/bin/bash
# Search through the entire test file to see all uses of isRunning in context
rg -B 3 -A 3 "isRunning" apps/frontend/src/main/__tests__/ipc-handlers.test.tsRepository: AndyMik90/Auto-Claude
Length of output: 470
Remove or document the unused isRunning mock.
The isRunning mock is added to mockAgentManager but is never used in any test assertions. Either remove it if no tests depend on it, or add tests that exercise this functionality. If it's intended for future tests, add a comment explaining why it's needed.
🤖 Prompt for AI Agents
In @apps/frontend/src/main/__tests__/ipc-handlers.test.ts at line 158, The mock
property isRunning on mockAgentManager is unused; either remove it from the mock
and its type (delete the isRunning: ReturnType<typeof vi.fn> declaration and any
assignment in mockAgentManager) or add a test that asserts its behavior (e.g.,
set isRunning.mockReturnValueOnce/true and assert code paths that call
mockAgentManager.isRunning are exercised), or if it’s intentionally reserved for
future tests, add an inline comment next to isRunning in the mock explaining why
it is present and when it will be used; update any related imports or type
annotations to keep the test file consistent.
2 participants
Summary
Context
Issue #668 reports an error when initializing a new project after skipping git setup. The project is created, but initialize fails due to git requirements. This change keeps initialization aligned with the user’s choice.
Testing
Notes
Summary by CodeRabbit
New Features
Behavior Changes
Tests
Localization
✏️ Tip: You can customize this high-level summary in your review settings.