fix(ui): persist staged task state across app restarts #800
Conversation
Previously, when a task was staged and the app restarted, the UI showed the staging interface again instead of recognizing the task was already staged. This happened because the condition order checked worktree existence before checking the stagedInMainProject flag. Changes: - Fix condition priority in TaskReview.tsx to check stagedInMainProject before worktreeStatus.exists - Add 'Mark Done Only' button to mark task complete without deleting worktree - Add 'Review Again' button to clear staged state and re-show staging UI - Add TASK_CLEAR_STAGED_STATE IPC handler to reset staged flags in implementation plan files - Add handleReviewAgain callback in useTaskDetail hook
github-actions
bot
added
area/frontend
📝 WalkthroughWalkthroughAdds IPC Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as TaskReview UI
participant Hook as useTaskDetail
participant Preload as Preload/API
participant Main as Main IPC
participant FS as implementation_plan.json
User->>UI: Click "Review Again"
UI->>Hook: handleReviewAgain()
Hook->>Hook: clear local staged & preview state
Hook->>Preload: clearStagedState(taskId)
Preload->>Main: IPC TASK_CLEAR_STAGED_STATE
Main->>FS: read main spec/implementation_plan.json
Main->>FS: update main plan (clear staged flags, update timestamp)
Main->>FS: try read/write worktree plan (best-effort)
Main->>Main: invalidate project task cache
Main-->>Preload: { cleared: true }
Preload-->>Hook: { cleared: true }
Hook->>Hook: reload worktree status & diff
Hook-->>UI: updated state
UI-->>User: refreshed review view
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (1)apps/frontend/src/shared/i18n/locales/**/*.json📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (4)📓 Common learnings📚 Learning: 2025-12-30T16:38:36.314ZApplied to files:
📚 Learning: 2025-12-30T16:38:36.314ZApplied to files:
📚 Learning: 2026-01-04T23:59:48.743ZApplied to files:
⏰ 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). (3)
🔇 Additional comments (2)
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 @AndyMik90, 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 addresses a critical UI bug where the application failed to correctly persist the staged state of a task across restarts, leading to user confusion. Previously, the UI would incorrectly display the staging interface even for tasks that were already staged, because it prioritized the existence of a worktree over the 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
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
apps/frontend/src/renderer/components/task-detail/task-review/WorkspaceMessages.tsx
Fixed
Show fixed
Hide fixed
![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 new 'Review Again' feature for tasks, allowing users to clear a task's staged state and re-evaluate it. This involves adding a new TASK_CLEAR_STAGED_STATE IPC channel and handler in the Electron main process, which removes stagedInMainProject and stagedAt flags from the task's implementation plan in both the main project and its worktree, and invalidates the task cache. On the frontend, the TaskAPI and ElectronAPI interfaces are updated, a handleReviewAgain function is added to reset UI state and reload worktree information, and the TaskReview and StagedInProjectMessage components are modified to display the 'Review Again' button and handle its logic. The StagedInProjectMessage component was also refactored to offer more granular options for marking tasks as done, including a 'Mark Done Only' option that retains the worktree. Review comments suggest improving the frontend's handleReviewAgain by explicitly reloading tasks after clearing the staged state to prevent UI inconsistencies, and refactoring the backend IPC handler to extract duplicated file manipulation logic into a helper function and consistently import fs/promises.
| const handleReviewAgain = async () => { | ||
| if (!onReviewAgain) return; | ||
|
|
||
| setIsResetting(true); | ||
| setError(null); | ||
|
|
||
| try { | ||
| // Clear the staged flag via IPC | ||
| const result = await window.electronAPI.clearStagedState(task.id); | ||
|
|
||
| if (!result.success) { | ||
| setError(result.error || 'Failed to reset staged state'); | ||
| return; | ||
| } | ||
|
|
||
| // Trigger re-render by calling parent callback | ||
| onReviewAgain(); | ||
| } catch (err) { | ||
| console.error('Error resetting staged state:', err); | ||
| setError(err instanceof Error ? err.message : 'Failed to reset staged state'); | ||
| } finally { | ||
| setIsResetting(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
There's a potential race condition here. After clearStagedState succeeds, it invalidates the backend cache, but the UI's task object isn't immediately updated. onReviewAgain() refreshes local UI state but might do so with a stale task prop. To ensure the UI reflects the change correctly, you should explicitly reload the tasks for the project after the IPC call succeeds. This will update the store and provide the component with fresh data.
You'll need to import loadTasks from ../../../stores/task-store.
const handleReviewAgain = async () => {
if (!onReviewAgain) return;
setIsResetting(true);
setError(null);
try {
// Clear the staged flag via IPC
const result = await window.electronAPI.clearStagedState(task.id);
if (!result.success) {
setError(result.error || 'Failed to reset staged state');
return;
}
// Trigger re-render by calling parent callback
onReviewAgain();
// Force a reload of tasks to get the updated `stagedInMainProject` flag
await loadTasks(task.projectId);
} catch (err) {
console.error('Error resetting staged state:', err);
setError(err instanceof Error ? err.message : 'Failed to reset staged state');
} finally {
setIsResetting(false);
}
};
| // Read, update, and write the plan file | ||
| const { promises: fsPromises } = require('fs'); | ||
| const planContent = await fsPromises.readFile(planPath, 'utf-8'); | ||
| const plan = JSON.parse(planContent); | ||
|
|
||
| // Clear the staged state flags | ||
| delete plan.stagedInMainProject; | ||
| delete plan.stagedAt; | ||
| plan.updated_at = new Date().toISOString(); | ||
|
|
||
| await fsPromises.writeFile(planPath, JSON.stringify(plan, null, 2)); | ||
|
|
||
| // Also update worktree plan if it exists | ||
| const worktreePath = findTaskWorktree(project.path, task.specId); | ||
| if (worktreePath) { | ||
| const worktreePlanPath = path.join(worktreePath, specsBaseDir, task.specId, AUTO_BUILD_PATHS.IMPLEMENTATION_PLAN); | ||
| if (existsSync(worktreePlanPath)) { | ||
| try { | ||
| const worktreePlanContent = await fsPromises.readFile(worktreePlanPath, 'utf-8'); | ||
| const worktreePlan = JSON.parse(worktreePlanContent); | ||
| delete worktreePlan.stagedInMainProject; | ||
| delete worktreePlan.stagedAt; | ||
| worktreePlan.updated_at = new Date().toISOString(); | ||
| await fsPromises.writeFile(worktreePlanPath, JSON.stringify(worktreePlan, null, 2)); | ||
| } catch (e) { | ||
| // Non-fatal - worktree plan update is best-effort | ||
| console.warn('[CLEAR_STAGED_STATE] Failed to update worktree plan:', e); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This block has a couple of areas for improvement:
- Code Duplication: The logic to read, update, and write the plan file is repeated for the main project and the worktree. This can be extracted into a helper function to improve readability and maintainability.
- Module Import:
fs/promisesis being required inline. For consistency, it's better to import it at the top of the file with the otherfsmodules.
Here's an example of how you could define a helper function for this:
async function clearStagedFlagsInPlan(planPath: string) {
const planContent = await fsPromises.readFile(planPath, 'utf-8');
const plan = JSON.parse(planContent);
delete plan.stagedInMainProject;
delete plan.stagedAt;
plan.updated_at = new Date().toISOString();
await fsPromises.writeFile(planPath, JSON.stringify(plan, null, 2));
}![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @apps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.ts:
- Around line 241-270: The handler handleReviewAgain currently swallows reload
errors (only console.error); add a user-visible error path by introducing an
error state (e.g., worktreeReloadError via useState) or by returning a
success/failure boolean from handleReviewAgain so callers can react; update the
catch block to setWorktreeReloadError(err) (or set the error return value) and
ensure finally still calls setIsLoadingWorktree(false); also clear the error at
the start of the function (setWorktreeReloadError(null)) so repeated calls reset
the UI; keep existing updates to setWorktreeStatus and setWorktreeDiff on
success.
In
@apps/frontend/src/renderer/components/task-detail/task-review/WorkspaceMessages.tsx:
- Line 91: The prop worktreeStatus in the WorkspaceMessages component is
declared in the Props interface and destructured but never used; either remove
the prop declaration and its destructuring from the WorkspaceMessages component
(and any related imports/types like WorktreeStatus) if no worktree details are
needed, or use worktreeStatus in the render logic where hasWorktree is checked —
e.g., replace/augment the hasWorktree conditional in the WorkspaceMessages
function to read fields from worktreeStatus (branch, commit count, etc.) and
display them. Update the Props interface and the component parameter list
accordingly to keep types consistent.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
apps/frontend/src/main/ipc-handlers/task/worktree-handlers.tsapps/frontend/src/preload/api/task-api.tsapps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsxapps/frontend/src/renderer/components/task-detail/TaskReview.tsxapps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.tsapps/frontend/src/renderer/components/task-detail/task-review/WorkspaceMessages.tsxapps/frontend/src/renderer/lib/mocks/workspace-mock.tsapps/frontend/src/shared/constants/ipc.tsapps/frontend/src/shared/types/ipc.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/ipc.tsapps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.tsapps/frontend/src/preload/api/task-api.tsapps/frontend/src/shared/constants/ipc.tsapps/frontend/src/renderer/lib/mocks/workspace-mock.tsapps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsxapps/frontend/src/renderer/components/task-detail/TaskReview.tsxapps/frontend/src/main/ipc-handlers/task/worktree-handlers.tsapps/frontend/src/renderer/components/task-detail/task-review/WorkspaceMessages.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/shared/types/ipc.tsapps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.tsapps/frontend/src/preload/api/task-api.tsapps/frontend/src/shared/constants/ipc.tsapps/frontend/src/renderer/lib/mocks/workspace-mock.tsapps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsxapps/frontend/src/renderer/components/task-detail/TaskReview.tsxapps/frontend/src/main/ipc-handlers/task/worktree-handlers.tsapps/frontend/src/renderer/components/task-detail/task-review/WorkspaceMessages.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/shared/types/ipc.tsapps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.tsapps/frontend/src/preload/api/task-api.tsapps/frontend/src/shared/constants/ipc.tsapps/frontend/src/renderer/lib/mocks/workspace-mock.tsapps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsxapps/frontend/src/renderer/components/task-detail/TaskReview.tsxapps/frontend/src/main/ipc-handlers/task/worktree-handlers.tsapps/frontend/src/renderer/components/task-detail/task-review/WorkspaceMessages.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/task-detail/TaskDetailModal.tsxapps/frontend/src/renderer/components/task-detail/TaskReview.tsxapps/frontend/src/renderer/components/task-detail/task-review/WorkspaceMessages.tsx
🧬 Code graph analysis (2)
apps/frontend/src/preload/api/task-api.ts (1)
apps/frontend/src/shared/constants/ipc.ts (1)
IPC_CHANNELS(6-516)
apps/frontend/src/renderer/components/task-detail/task-review/WorkspaceMessages.tsx (4)
apps/frontend/src/shared/types/task.ts (1)
WorktreeStatus(298-307)apps/frontend/src/renderer/stores/task-store.ts (1)
persistTaskStatus(330-351).design-system/src/components/Button.tsx (1)
Button(10-44).design-system/src/lib/icons.ts (2)
Check(21-21)RotateCcw(31-31)
⏰ 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). (3)
- GitHub Check: Cursor Bugbot
- GitHub Check: CodeQL (python)
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (13)
apps/frontend/src/shared/constants/ipc.ts (1)
44-44: LGTM! IPC channel constant follows established conventions.The new
TASK_CLEAR_STAGED_STATEconstant is correctly placed in the worktree management section and follows the existing naming pattern.apps/frontend/src/renderer/lib/mocks/workspace-mock.ts (1)
65-68: LGTM! Mock implementation is consistent.The
clearStagedStatemock follows the same pattern as other workspace mock functions and returns the appropriate success structure for testing.apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsx (1)
445-445: LGTM! Prop wiring follows existing patterns.The
onReviewAgainprop is correctly passed from the state hook to the TaskReview component, following the same pattern as other review action handlers.apps/frontend/src/shared/types/ipc.ts (1)
171-171: LGTM! Type definition is well-structured.The
clearStagedStatemethod signature is consistent with other workspace management operations and correctly typed withIPCResult<{ cleared: boolean }>.apps/frontend/src/preload/api/task-api.ts (2)
54-54: LGTM! API method correctly added to interface.The
clearStagedStatemethod is properly defined in the TaskAPI interface with the correct type signature.
146-147: LGTM! Implementation follows established patterns.The
clearStagedStateimplementation correctly usesipcRenderer.invokewith the appropriate IPC channel, consistent with other workspace management operations.apps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.ts (1)
352-352: LGTM!The
handleReviewAgainhandler is properly exposed in the hook's return value, following the established pattern for other handlers.apps/frontend/src/renderer/components/task-detail/TaskReview.tsx (2)
103-141: LGTM! Well-structured conditional rendering with clear priority.The updated priority order correctly addresses the bug where staging UI reappeared after restart:
- Loading state
- Fresh staging success (this session)
- Already staged (persisted state) - this was the missing check
- Worktree exists (show staging UI)
- No workspace
The comments clearly document each case, improving maintainability.
45-45: LGTM!The new optional
onReviewAgainprop follows the established pattern for callback props in this component.apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts (2)
2544-2604: LGTM! Well-implemented IPC handler for clearing staged state.The handler correctly:
- Validates task/project existence before proceeding
- Uses async I/O for file operations
- Clears the staged flags (
stagedInMainProject,stagedAt) and updates timestamp- Updates both main and worktree plans (worktree is best-effort)
- Invalidates the tasks cache to ensure UI consistency
- Has proper error handling with informative messages
2557-2559: Minor: Consider EAFP pattern for consistency.The handler uses
existsSync(LBYL) before reading the plan file, while other parts of this file use EAFP (try/catch with ENOENT check). This is fine since the error is handled, but the worktree plan path at line 2577 uses the same pattern which is appropriate for best-effort operations.apps/frontend/src/renderer/components/task-detail/task-review/WorkspaceMessages.tsx (2)
146-169: LGTM! handleReviewAgain implementation is correct.The handler properly:
- Guards against missing callback
- Manages loading state (
isResetting)- Calls IPC to clear persisted staged state
- Handles errors and displays them to the user
- Calls parent callback only on success to trigger UI refresh
190-234: LGTM! Button state management is well-implemented.All action buttons are properly disabled during any operation (
isDeleting || isMarkingDone || isResetting), preventing race conditions from multiple concurrent actions.
| // Handle "Review Again" - clears staged state and reloads worktree info | ||
| const handleReviewAgain = useCallback(async () => { | ||
| // Clear staged success state if it was set in this session | ||
| setStagedSuccess(null); | ||
| setStagedProjectPath(undefined); | ||
| setSuggestedCommitMessage(undefined); | ||
|
|
||
| // Reset merge preview to force re-check | ||
| setMergePreview(null); | ||
| hasLoadedPreviewRef.current = null; | ||
|
|
||
| // Reload worktree status | ||
| setIsLoadingWorktree(true); | ||
| try { | ||
| const [statusResult, diffResult] = await Promise.all([ | ||
| window.electronAPI.getWorktreeStatus(task.id), | ||
| window.electronAPI.getWorktreeDiff(task.id) | ||
| ]); | ||
| if (statusResult.success && statusResult.data) { | ||
| setWorktreeStatus(statusResult.data); | ||
| } | ||
| if (diffResult.success && diffResult.data) { | ||
| setWorktreeDiff(diffResult.data); | ||
| } | ||
| } catch (err) { | ||
| console.error('Failed to reload worktree info:', err); | ||
| } finally { | ||
| setIsLoadingWorktree(false); | ||
| } | ||
| }, [task.id]); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider surfacing reload errors to the UI.
The handleReviewAgain handler logs errors to console but doesn't expose them to the user. If the worktree status/diff reload fails, the user has no indication of the failure.
Consider adding an error state or returning a success/failure indicator:
♻️ Optional: Surface errors to calling component
// Handle "Review Again" - clears staged state and reloads worktree info
- const handleReviewAgain = useCallback(async () => {
+ const handleReviewAgain = useCallback(async (): Promise<{ success: boolean; error?: string }> => {
// Clear staged success state if it was set in this session
setStagedSuccess(null);
setStagedProjectPath(undefined);
setSuggestedCommitMessage(undefined);
// Reset merge preview to force re-check
setMergePreview(null);
hasLoadedPreviewRef.current = null;
// Reload worktree status
setIsLoadingWorktree(true);
try {
const [statusResult, diffResult] = await Promise.all([
window.electronAPI.getWorktreeStatus(task.id),
window.electronAPI.getWorktreeDiff(task.id)
]);
if (statusResult.success && statusResult.data) {
setWorktreeStatus(statusResult.data);
}
if (diffResult.success && diffResult.data) {
setWorktreeDiff(diffResult.data);
}
+ return { success: true };
} catch (err) {
console.error('Failed to reload worktree info:', err);
+ return { success: false, error: err instanceof Error ? err.message : 'Failed to reload' };
} finally {
setIsLoadingWorktree(false);
}
}, [task.id]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Handle "Review Again" - clears staged state and reloads worktree info | |
| const handleReviewAgain = useCallback(async () => { | |
| // Clear staged success state if it was set in this session | |
| setStagedSuccess(null); | |
| setStagedProjectPath(undefined); | |
| setSuggestedCommitMessage(undefined); | |
| // Reset merge preview to force re-check | |
| setMergePreview(null); | |
| hasLoadedPreviewRef.current = null; | |
| // Reload worktree status | |
| setIsLoadingWorktree(true); | |
| try { | |
| const [statusResult, diffResult] = await Promise.all([ | |
| window.electronAPI.getWorktreeStatus(task.id), | |
| window.electronAPI.getWorktreeDiff(task.id) | |
| ]); | |
| if (statusResult.success && statusResult.data) { | |
| setWorktreeStatus(statusResult.data); | |
| } | |
| if (diffResult.success && diffResult.data) { | |
| setWorktreeDiff(diffResult.data); | |
| } | |
| } catch (err) { | |
| console.error('Failed to reload worktree info:', err); | |
| } finally { | |
| setIsLoadingWorktree(false); | |
| } | |
| }, [task.id]); | |
| // Handle "Review Again" - clears staged state and reloads worktree info | |
| const handleReviewAgain = useCallback(async (): Promise<{ success: boolean; error?: string }> => { | |
| // Clear staged success state if it was set in this session | |
| setStagedSuccess(null); | |
| setStagedProjectPath(undefined); | |
| setSuggestedCommitMessage(undefined); | |
| // Reset merge preview to force re-check | |
| setMergePreview(null); | |
| hasLoadedPreviewRef.current = null; | |
| // Reload worktree status | |
| setIsLoadingWorktree(true); | |
| try { | |
| const [statusResult, diffResult] = await Promise.all([ | |
| window.electronAPI.getWorktreeStatus(task.id), | |
| window.electronAPI.getWorktreeDiff(task.id) | |
| ]); | |
| if (statusResult.success && statusResult.data) { | |
| setWorktreeStatus(statusResult.data); | |
| } | |
| if (diffResult.success && diffResult.data) { | |
| setWorktreeDiff(diffResult.data); | |
| } | |
| return { success: true }; | |
| } catch (err) { | |
| console.error('Failed to reload worktree info:', err); | |
| return { success: false, error: err instanceof Error ? err.message : 'Failed to reload' }; | |
| } finally { | |
| setIsLoadingWorktree(false); | |
| } | |
| }, [task.id]); |
🤖 Prompt for AI Agents
In @apps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.ts
around lines 241 - 270, The handler handleReviewAgain currently swallows reload
errors (only console.error); add a user-visible error path by introducing an
error state (e.g., worktreeReloadError via useState) or by returning a
success/failure boolean from handleReviewAgain so callers can react; update the
catch block to setWorktreeReloadError(err) (or set the error return value) and
ensure finally still calls setIsLoadingWorktree(false); also clear the error at
the start of the function (setWorktreeReloadError(null)) so repeated calls reset
the UI; keep existing updates to setWorktreeStatus and setWorktreeDiff on
success.
apps/frontend/src/renderer/components/task-detail/task-review/WorkspaceMessages.tsx
Outdated
Show resolved
Hide resolved
| <> | ||
| <Check className="h-4 w-4 mr-2" /> | ||
| Mark Done Only | ||
| </> |
There was a problem hiding this comment.
New user-facing strings should use i18n translation keys.
This PR introduces new hardcoded user-facing strings that should use the useTranslation() hook per the coding guidelines. New strings include:
- "Mark Done Only" (line 255)
- "Review Again" (line 278)
- "Marking done..." (line 224, 249)
- "Resetting..." (line 273)
As per coding guidelines for apps/frontend/src/**/*.{ts,tsx}: "Use useTranslation() hook with namespace prefixes for accessing translation strings."
🌐 Example i18n fix for new strings
+import { useTranslation } from 'react-i18next';
export function StagedInProjectMessage({ task, projectPath, hasWorktree = false, worktreeStatus, onClose, onReviewAgain }: StagedInProjectMessageProps) {
+ const { t } = useTranslation('taskReview');
const [isDeleting, setIsDeleting] = useState(false);
// ... rest of state
// In the JSX:
- Mark Done Only
+ {t('actions.markDoneOnly')}
- Review Again
+ {t('actions.reviewAgain')}
- Marking done...
+ {t('actions.markingDone')}
- Resetting...
+ {t('actions.resetting')}Also applies to: 276-279
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
apps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.ts
Outdated
Show resolved
Hide resolved
| } finally { | ||
| setIsLoadingWorktree(false); | ||
| } | ||
| }, [task.id]); |
There was a problem hiding this comment.
Missing workspaceError reset in handleReviewAgain causes stale errors
Low Severity
The handleReviewAgain function resets several state variables (stagedSuccess, stagedProjectPath, suggestedCommitMessage, mergePreview) but doesn't reset workspaceError. The original worktree load effect at line 124 clears workspaceError before reloading, but handleReviewAgain omits this. If a user has a merge/discard error and then clicks "Review Again", the stale error message would continue to display in the WorkspaceStatus component even though the state is being refreshed.
When dragging a task to the 'done' column, if the task has a worktree:
- Shows a confirmation dialog asking about worktree cleanup
- Staged tasks: Can 'Keep Worktree' or 'Delete Worktree & Mark Done'
- Non-staged tasks: Must delete worktree or cancel (to prevent losing work)
Also fixes a race condition where discardWorktree sent 'backlog' status
before persistTaskStatus('done') could execute, causing task to briefly
appear in Done then jump back to Planning.
Added skipStatusChange parameter to discardWorktree IPC to prevent this.
github-actions
bot
added
🔄 Checking
github-actions
bot
added
🔄 Checking
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts:
- Around line 2565-2594: The duplicated read-update-write logic for clearing
staged flags should be extracted into a helper (clearStagedFlagsInPlan) and
switch to EAFP to avoid TOCTOU: implement async clearStagedFlagsInPlan(planPath,
fsPromises) that reads, JSON.parses, deletes stagedInMainProject/stagedAt,
updates updated_at, writes back and returns true; catch errors and return false
for ENOENT but rethrow others; replace the inline main-plan logic with a call to
clearStagedFlagsInPlan(planPath, fsPromises) (treat false as missing required
plan error) and replace the worktree branch to build worktreePlanPath (using
findTaskWorktree, specsBaseDir, task.specId,
AUTO_BUILD_PATHS.IMPLEMENTATION_PLAN) and call
clearStagedFlagsInPlan(worktreePlanPath, fsPromises) inside a try/catch so
failures remain best-effort.
- Around line 2561-2563: Replace the existsSync check plus subsequent readFile
usage with an EAFP-style try/catch: remove the existsSync(planPath) branch and
instead attempt to read the file (the code that uses planPath/readFile in this
handler), catch errors, and if err.code === 'ENOENT' return { success: false,
error: 'Implementation plan not found' }; rethrow or return a generic failure
for other errors. Follow the same pattern used by updatePlanWithRetry() for
error handling and avoid the TOCTOU race.
In @apps/frontend/src/renderer/components/KanbanBoard.tsx:
- Around line 506-514: Dragging a card to "done" can appear to complete then
snap back because the handler returns early when checkTaskHasWorktree(task.id)
is true; update the UI to reflect a pending state before returning by setting a
short-lived visual indicator or optimistic pending status (e.g., call
setPendingDoneTask(task) and set a "pending" flag on that task or global state
like setWorktreeDialogOpen(true) plus setTaskPending(task.id)) so the card shows
an explicit "awaiting confirmation" state while the worktree dialog is open;
ensure you use the existing symbols checkTaskHasWorktree, setPendingDoneTask,
setWorktreeDialogOpen and the task.id/targetStatus logic so the drag UX no
longer looks like an unexpected rollback.
- Around line 580-635: The noted translation keys used in KanbanBoard
(kanban.worktreeCleanupTitle, kanban.worktreeCleanupStaged,
kanban.worktreeCleanupNotStaged, kanban.keepWorktree, kanban.deleteWorktree) are
missing from the tasks locale files; add these keys to both the English and
French tasks locale JSONs with the appropriate localized strings (English values
should match the current fallback strings like "Worktree Cleanup", "This task
has been staged and has a worktree. Would you like to clean up the worktree?",
"This task has a worktree with changes that have not been merged. Delete the
worktree to mark as done, or cancel to review the changes first.", "Keep
Worktree", "Delete Worktree & Mark Done"; provide equivalent French translations
for each key) so the UI uses proper i18n instead of fallback text.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
apps/frontend/src/main/ipc-handlers/task/worktree-handlers.tsapps/frontend/src/preload/api/task-api.tsapps/frontend/src/renderer/components/KanbanBoard.tsxapps/frontend/src/shared/types/ipc.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/KanbanBoard.tsxapps/frontend/src/shared/types/ipc.tsapps/frontend/src/main/ipc-handlers/task/worktree-handlers.tsapps/frontend/src/preload/api/task-api.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/KanbanBoard.tsxapps/frontend/src/shared/types/ipc.tsapps/frontend/src/main/ipc-handlers/task/worktree-handlers.tsapps/frontend/src/preload/api/task-api.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/KanbanBoard.tsxapps/frontend/src/shared/types/ipc.tsapps/frontend/src/main/ipc-handlers/task/worktree-handlers.tsapps/frontend/src/preload/api/task-api.ts
🧠 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/KanbanBoard.tsx
🧬 Code graph analysis (4)
apps/frontend/src/renderer/components/KanbanBoard.tsx (3)
apps/frontend/src/shared/types/task.ts (2)
Task(238-258)TaskStatus(8-8)apps/frontend/src/renderer/stores/task-store.ts (1)
persistTaskStatus(330-351)apps/frontend/src/shared/constants/task.ts (1)
TASK_STATUS_COLUMNS(11-17)
apps/frontend/src/shared/types/ipc.ts (1)
apps/frontend/src/shared/types/task.ts (1)
WorktreeDiscardResult(402-405)
apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts (2)
apps/frontend/src/shared/constants/config.ts (2)
getSpecsDir(106-109)AUTO_BUILD_PATHS(83-100)apps/frontend/src/main/worktree-paths.ts (1)
findTaskWorktree(39-49)
apps/frontend/src/preload/api/task-api.ts (2)
apps/frontend/src/__mocks__/electron.ts (1)
ipcRenderer(53-61)apps/frontend/src/shared/constants/ipc.ts (1)
IPC_CHANNELS(6-516)
🪛 GitHub Check: CodeQL
apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts
[failure] 2575-2575: Potential file system race condition
The file may have changed since it was checked.
[failure] 2588-2588: Potential file system race condition
The file may have changed since it was checked.
⏰ 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: Cursor Bugbot
🔇 Additional comments (7)
apps/frontend/src/shared/types/ipc.ts (1)
170-171: LGTM!The type signatures are correctly defined and consistent with the handler implementations. The optional
skipStatusChangeparameter and the newclearStagedStatemethod are properly typed.apps/frontend/src/preload/api/task-api.ts (2)
53-54: LGTM!The interface correctly declares both the updated
discardWorktreesignature with the optionalskipStatusChangeparameter and the newclearStagedStatemethod.
143-147: LGTM!The implementation correctly passes the
skipStatusChangeparameter through to the IPC channel and properly wiresclearStagedStatetoTASK_CLEAR_STAGED_STATE. The pattern is consistent with other IPC methods in this file.apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts (3)
2278-2278: LGTM!The
skipStatusChangeparameter is correctly added and allows the caller to prevent the automatic status change to 'backlog' when the worktree is discarded. This fixes the race condition mentioned in the PR objectives wherediscardWorktreecould send a status change beforepersistTaskStatus('done')ran.
2321-2328: LGTM!The conditional emission of
TASK_STATUS_CHANGEbased onskipStatusChangecorrectly prevents the transient status flip race condition when the caller intends to set a different status immediately after discarding.
2566-2566: Moverequire('fs')to top-level import.The
fs/promisesmodule is required inline. For consistency with the rest of the file and better performance (avoiding repeated require calls), import it at the top of the file alongside the existingfsimports.Proposed change
At the top of the file (around line 5), add:
import { promises as fsPromises } from 'fs';Then remove the inline require at line 2566.
Likely an incorrect or invalid review comment.
apps/frontend/src/renderer/components/KanbanBoard.tsx (1)
477-478: AsynchandleDragEndis compatible with@dnd-kit/core— no race condition risk.While
@dnd-kit/core'sonDragEnddoes not await async handlers, this implementation is safe because the drag state is cleared synchronously at lines 479-480 before any async work executes. Each drag event is handled independently, and the async worktree validation at line 507 does not block the drag library's internal logic. Rapid successive drags do not create race conditions since each event's synchronous state cleanup is isolated.
| if (targetStatus === 'done') { | ||
| const hasWorktree = await checkTaskHasWorktree(task.id); | ||
|
|
||
| if (hasWorktree) { | ||
| // Show dialog asking about worktree cleanup | ||
| setPendingDoneTask(task); | ||
| setWorktreeDialogOpen(true); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Potential UX issue: drag appears to complete but task stays in place.
When moving to "done" triggers the worktree dialog, the function returns early at line 513 without updating the task status. The user sees the drag complete visually (card snaps back), then the dialog appears. This could be confusing.
Consider either:
- Adding a brief visual indicator that an action is pending, or
- Documenting this as expected behavior in the dialog description
This is minor since the dialog explains the situation, but worth noting for UX polish.
🤖 Prompt for AI Agents
In @apps/frontend/src/renderer/components/KanbanBoard.tsx around lines 506 -
514, Dragging a card to "done" can appear to complete then snap back because the
handler returns early when checkTaskHasWorktree(task.id) is true; update the UI
to reflect a pending state before returning by setting a short-lived visual
indicator or optimistic pending status (e.g., call setPendingDoneTask(task) and
set a "pending" flag on that task or global state like
setWorktreeDialogOpen(true) plus setTaskPending(task.id)) so the card shows an
explicit "awaiting confirmation" state while the worktree dialog is open; ensure
you use the existing symbols checkTaskHasWorktree, setPendingDoneTask,
setWorktreeDialogOpen and the task.id/targetStatus logic so the drag UX no
longer looks like an unexpected rollback.
| {t('kanban.worktreeCleanupTitle', 'Worktree Cleanup')} | ||
| </AlertDialogTitle> | ||
| <AlertDialogDescription asChild> | ||
| <div className="text-left space-y-2"> | ||
| {pendingDoneTask?.stagedInMainProject ? ( | ||
| <p> | ||
| {t('kanban.worktreeCleanupStaged', 'This task has been staged and has a worktree. Would you like to clean up the worktree?')} | ||
| </p> | ||
| ) : ( | ||
| <p> | ||
| {t('kanban.worktreeCleanupNotStaged', 'This task has a worktree with changes that have not been merged. Delete the worktree to mark as done, or cancel to review the changes first.')} | ||
| </p> | ||
| )} | ||
| {pendingDoneTask && ( | ||
| <p className="text-sm font-medium text-foreground/80 bg-muted/50 rounded px-2 py-1.5"> | ||
| {pendingDoneTask.title} | ||
| </p> | ||
| )} | ||
| </div> | ||
| </AlertDialogDescription> | ||
| </AlertDialogHeader> | ||
| <AlertDialogFooter className="flex-col sm:flex-row gap-2"> | ||
| <Button | ||
| variant="outline" | ||
| onClick={() => { | ||
| setWorktreeDialogOpen(false); | ||
| setPendingDoneTask(null); | ||
| }} | ||
| disabled={isCleaningUp} | ||
| > | ||
| {t('common:cancel', 'Cancel')} | ||
| </Button> | ||
| {/* Only show "Keep Worktree" option if task is staged */} | ||
| {pendingDoneTask?.stagedInMainProject && ( | ||
| <Button | ||
| variant="secondary" | ||
| onClick={() => pendingDoneTask && handleMoveToDone(pendingDoneTask, false)} | ||
| disabled={isCleaningUp} | ||
| className="gap-2" | ||
| > | ||
| <FolderCheck className="h-4 w-4" /> | ||
| {t('kanban.keepWorktree', 'Keep Worktree')} | ||
| </Button> | ||
| )} | ||
| <Button | ||
| variant={pendingDoneTask?.stagedInMainProject ? 'default' : 'destructive'} | ||
| onClick={() => pendingDoneTask && handleMoveToDone(pendingDoneTask, true)} | ||
| disabled={isCleaningUp} | ||
| className="gap-2" | ||
| > | ||
| {isCleaningUp ? ( | ||
| <Loader2 className="h-4 w-4 animate-spin" /> | ||
| ) : ( | ||
| <Trash2 className="h-4 w-4" /> | ||
| )} | ||
| {t('kanban.deleteWorktree', 'Delete Worktree & Mark Done')} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the translation files in the repository
find . -type f -name "*.json" | grep -E "(locale|i18n|translation|lang)" | head -20Repository: AndyMik90/Auto-Claude
Length of output: 1198
🏁 Script executed:
# Let's search for kanban keys in the translation files
echo "=== Searching for kanban keys in translation files ==="
rg "kanban|worktreeCleanup|keepWorktree|deleteWorktree" apps/frontend/src/shared/i18n/locales/ --type json
echo ""
echo "=== Checking if kanban.json file exists ==="
ls -la apps/frontend/src/shared/i18n/locales/en/ | grep -i kanban
ls -la apps/frontend/src/shared/i18n/locales/fr/ | grep -i kanban
echo ""
echo "=== All JSON files in en directory ==="
ls apps/frontend/src/shared/i18n/locales/en/
echo ""
echo "=== Checking common.json for cancel key ==="
rg "cancel" apps/frontend/src/shared/i18n/locales/en/common.json -C2Repository: AndyMik90/Auto-Claude
Length of output: 1029
🏁 Script executed:
# Let's examine the kanban object in tasks.json to see what keys are defined
echo "=== Content of kanban object in en/tasks.json ==="
cat apps/frontend/src/shared/i18n/locales/en/tasks.json | jq '.kanban' 2>/dev/null || cat apps/frontend/src/shared/i18n/locales/en/tasks.json | grep -A 50 '"kanban"'
echo ""
echo "=== Content of kanban object in fr/tasks.json ==="
cat apps/frontend/src/shared/i18n/locales/fr/tasks.json | jq '.kanban' 2>/dev/null || cat apps/frontend/src/shared/i18n/locales/fr/tasks.json | grep -A 50 '"kanban"'Repository: AndyMik90/Auto-Claude
Length of output: 1797
🏁 Script executed:
# Let's verify that these keys are definitely missing and count the files that need updates
echo "=== Verifying missing keys in en/tasks.json ==="
grep -c "worktreeCleanupTitle\|worktreeCleanupStaged\|worktreeCleanupNotStaged\|keepWorktree\|deleteWorktree" apps/frontend/src/shared/i18n/locales/en/tasks.json || echo "No matches found (confirmed missing)"
echo ""
echo "=== Verifying missing keys in fr/tasks.json ==="
grep -c "worktreeCleanupTitle\|worktreeCleanupStaged\|worktreeCleanupNotStaged\|keepWorktree\|deleteWorktree" apps/frontend/src/shared/i18n/locales/fr/tasks.json || echo "No matches found (confirmed missing)"
echo ""
echo "=== Confirming common:cancel key exists ==="
grep -c "\"cancel\"" apps/frontend/src/shared/i18n/locales/en/common.jsonRepository: AndyMik90/Auto-Claude
Length of output: 286
Add missing kanban i18n translation keys to both en and fr language files.
The code uses translation keys (kanban.worktreeCleanupTitle, kanban.worktreeCleanupStaged, kanban.worktreeCleanupNotStaged, kanban.keepWorktree, kanban.deleteWorktree) that are not defined in the translation files. These keys must be added to both apps/frontend/src/shared/i18n/locales/en/tasks.json and apps/frontend/src/shared/i18n/locales/fr/tasks.json to ensure proper localization instead of relying on fallback strings.
🤖 Prompt for AI Agents
In @apps/frontend/src/renderer/components/KanbanBoard.tsx around lines 580 -
635, The noted translation keys used in KanbanBoard
(kanban.worktreeCleanupTitle, kanban.worktreeCleanupStaged,
kanban.worktreeCleanupNotStaged, kanban.keepWorktree, kanban.deleteWorktree) are
missing from the tasks locale files; add these keys to both the English and
French tasks locale JSONs with the appropriate localized strings (English values
should match the current fallback strings like "Worktree Cleanup", "This task
has been staged and has a worktree. Would you like to clean up the worktree?",
"This task has a worktree with changes that have not been merged. Delete the
worktree to mark as done, or cancel to review the changes first.", "Keep
Worktree", "Delete Worktree & Mark Done"; provide equivalent French translations
for each key) so the UI uses proper i18n instead of fallback text.
| } | ||
|
|
||
| // Normal status change | ||
| persistTaskStatus(activeTaskId, targetStatus); |
There was a problem hiding this comment.
Rapid drags to done column can lose tasks
Low Severity
Making handleDragEnd async introduces a race condition when dragging tasks to the "done" column. If a user quickly drags two tasks with worktrees to "done", both async checkTaskHasWorktree calls run concurrently. When they complete, each call sets setPendingDoneTask and setWorktreeDialogOpen, causing the second task to overwrite the first in pendingDoneTask. The dialog displays only the last task, and the first task's drag is silently lost - it never gets moved to "done" and the user receives no indication that their action was ignored.
| setWorktreeDialogOpen(false); | ||
| setPendingDoneTask(null); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Ignoring status persist failure after worktree deletion
Medium Severity
In handleMoveToDone, the return value of persistTaskStatus is ignored. Since persistTaskStatus returns false on failure (rather than throwing), if the status persist fails after the worktree was successfully deleted, the dialog closes without showing any error. The worktree is gone, local UI state shows the task as done, but the file on disk has the original status. On app restart, the task reappears in its original column but the worktree data is permanently lost.
…pp restarts Integrates fix/task-state-tracking branch changes: - Fixed condition priority in TaskReview to check stagedInMainProject before worktreeStatus.exists - Added "Mark Done Only" button to mark task complete without deleting worktree - Added "Review Again" button to clear staged state and allow re-staging - Added TASK_CLEAR_STAGED_STATE IPC handler to reset staged flags in plan files - Added worktree cleanup dialog when dragging task to Done column in Kanban - Preserved PR creation functionality alongside new Review Again feature Resolves state persistence issues across app restarts. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Document the 4-layer flip-flop prevention architecture: - Layer 1: Log parsing (agent-events.ts) - Layer 2: IPC dispatch validation (agent-events-handlers.ts) - Layer 3: Plan update guards (task-store.ts) - Layer 4: Batching bypass (useIpc.ts) Clarifies that these are intentional defense-in-depth layers, not code duplication. Related: ACS-51, ACS-55, ACS-71, PR AndyMik90#800, PR AndyMik90#824 Co-Authored-By: Claude Opus 4.5 <[email protected]>
Resolved conflicts by keeping both features: - Task state tracking (clearStagedState, onReviewAgain, skipStatusChange) - PR creation (createWorktreePR, showPRDialog, onCreatePR) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
github-actions
bot
added
🔄 Checking
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @apps/frontend/src/renderer/components/KanbanBoard.tsx:
- Around line 500-501: The code uses TASK_STATUS_COLUMNS.includes(overId as
TaskStatus) which causes a TypeScript error because TaskStatus can include
values (like 'pr_created') not present in the TASK_STATUS_COLUMNS tuple; replace
that runtime check with the existing type guard isValidDropColumn(overId) so
TypeScript narrows the type before assigning targetStatus (i.e., call
isValidDropColumn(overId) and only then set targetStatus = overId as TaskStatus
or let the guard narrow the type directly), referencing TASK_STATUS_COLUMNS,
TaskStatus, isValidDropColumn, targetStatus and overId.
- Around line 583-649: The new Kanban worktree dialog strings
(kanban.worktreeCleanupTitle, kanban.worktreeCleanupStaged,
kanban.worktreeCleanupNotStaged, kanban.keepWorktree, kanban.deleteWorktree) are
missing from the locale files—add these keys under the "kanban" section in the
English and French tasks.json locale files with appropriate translations (use
the current fallback strings as the values if needed), so the AlertDialog text
in KanbanBoard.tsx displays localized content.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
apps/frontend/src/main/ipc-handlers/task/worktree-handlers.tsapps/frontend/src/preload/api/task-api.tsapps/frontend/src/renderer/components/KanbanBoard.tsxapps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsxapps/frontend/src/renderer/components/task-detail/TaskReview.tsxapps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.tsapps/frontend/src/renderer/lib/mocks/workspace-mock.tsapps/frontend/src/shared/constants/ipc.tsapps/frontend/src/shared/types/ipc.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/lib/mocks/workspace-mock.tsapps/frontend/src/shared/constants/ipc.tsapps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsxapps/frontend/src/renderer/components/KanbanBoard.tsxapps/frontend/src/renderer/components/task-detail/TaskReview.tsxapps/frontend/src/preload/api/task-api.tsapps/frontend/src/main/ipc-handlers/task/worktree-handlers.tsapps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.tsapps/frontend/src/shared/types/ipc.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/lib/mocks/workspace-mock.tsapps/frontend/src/shared/constants/ipc.tsapps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsxapps/frontend/src/renderer/components/KanbanBoard.tsxapps/frontend/src/renderer/components/task-detail/TaskReview.tsxapps/frontend/src/preload/api/task-api.tsapps/frontend/src/main/ipc-handlers/task/worktree-handlers.tsapps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.tsapps/frontend/src/shared/types/ipc.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/lib/mocks/workspace-mock.tsapps/frontend/src/shared/constants/ipc.tsapps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsxapps/frontend/src/renderer/components/KanbanBoard.tsxapps/frontend/src/renderer/components/task-detail/TaskReview.tsxapps/frontend/src/preload/api/task-api.tsapps/frontend/src/main/ipc-handlers/task/worktree-handlers.tsapps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.tsapps/frontend/src/shared/types/ipc.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:48.743Z
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/task-detail/TaskDetailModal.tsxapps/frontend/src/renderer/components/KanbanBoard.tsxapps/frontend/src/renderer/components/task-detail/TaskReview.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/KanbanBoard.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/KanbanBoard.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 : Store translation strings in namespace-organized JSON files at `apps/frontend/src/shared/i18n/locales/{lang}/*.json` for each supported language
Applied to files:
apps/frontend/src/renderer/components/KanbanBoard.tsx
🧬 Code graph analysis (2)
apps/frontend/src/renderer/components/task-detail/TaskReview.tsx (2)
apps/frontend/src/renderer/components/task-detail/task-review/index.ts (2)
LoadingMessage(16-16)StagedInProjectMessage(16-16)apps/frontend/src/renderer/components/task-detail/task-review/WorkspaceMessages.tsx (2)
LoadingMessage(14-23)StagedInProjectMessage(99-297)
apps/frontend/src/shared/types/ipc.ts (1)
apps/frontend/src/shared/types/task.ts (1)
WorktreeDiscardResult(403-406)
🪛 GitHub Actions: CI
apps/frontend/src/renderer/components/KanbanBoard.tsx
[error] 500-500: TS2345: Argument of type 'TaskStatus' is not assignable to parameter of type '"backlog" | "in_progress" | "ai_review" | "human_review" | "done"'. (npm run typecheck: tsc --noEmit)
⏰ 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). (3)
- GitHub Check: Cursor Bugbot
- GitHub Check: CodeQL (python)
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (14)
apps/frontend/src/shared/constants/ipc.ts (1)
45-45: LGTM! New IPC channel constant is properly defined.The
TASK_CLEAR_STAGED_STATEconstant follows the project's naming conventions and is appropriately placed within the workspace management section.apps/frontend/src/renderer/lib/mocks/workspace-mock.ts (1)
73-76: LGTM! Mock implementation is consistent.The
clearStagedStatemock follows the established pattern in this file and returns the expected structure{ success: true, data: { cleared: true } }that matches the IPC type definition.apps/frontend/src/shared/types/ipc.ts (2)
173-173: LGTM! Optional parameter added correctly.The
skipStatusChangeoptional parameter is properly typed and maintains backward compatibility with existing callers that don't pass this parameter.
174-174: IPC handler is properly implemented.The
TASK_CLEAR_STAGED_STATEhandler is correctly implemented inapps/frontend/src/main/ipc-handlers/task/worktree-handlers.tswith the proper type signature and error handling. No action required.apps/frontend/src/preload/api/task-api.ts (2)
55-56: LGTM! Interface definitions are consistent.The
TaskAPIinterface updates match theElectronAPIinterface definitions and maintain type consistency across the IPC boundary.
146-150: LGTM! IPC implementations are correct.Both method implementations properly forward parameters to their respective IPC channels and maintain correct return types. The implementation follows established patterns in this file.
apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsx (1)
529-529: Prop wiring is complete and correctly typed.The
TaskReviewcomponent accepts theonReviewAgainprop (defined asonReviewAgain?: () => voidinTaskReviewProps), and theuseTaskDetailhook properly exportshandleReviewAgainas auseCallbackthat clears staged state and reloads worktree information for task re-review.apps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.ts (2)
285-314: LGTM -handleReviewAgainhandler implementation is correct.The handler properly clears local UI state and reloads worktree status. The separation of concerns is appropriate:
WorkspaceMessages.tsxhandles clearing the persisted state via IPC (clearStagedState), while this hook handles resetting React state and refreshing the UI.
476-476: LGTM - Handler correctly exposed in hook's public API.apps/frontend/src/renderer/components/task-detail/TaskReview.tsx (1)
113-129: Correct fix for the persistence bug - condition order now respects persisted state.The rendering priority is now correctly ordered:
- Loading state
- Fresh staging success (current session)
- Persisted staged state (
task.stagedInMainProject) - this is the key fix- Worktree exists but not staged
- No workspace
This ensures that after app restart, tasks that were previously staged will show
StagedInProjectMessageinstead of incorrectly showing the staging UI again.apps/frontend/src/renderer/components/KanbanBoard.tsx (2)
456-485: LGTM - Worktree check and move-to-done logic is correctly implemented.Key points:
checkTaskHasWorktreesafely handles IPC errors by returningfalsehandleMoveToDonecorrectly usesskipStatusChange: trueto prevent the handler from emitting 'backlog' status, then explicitly sets 'done'- Failure to delete worktree is logged but non-blocking - appropriate since the task can still be marked done
515-528: Good UX decision - deferring to confirmation dialog when worktree exists.The early return after
setPendingDoneTaskandsetWorktreeDialogOpen(true)correctly prevents automatic status change, giving the user a chance to decide about worktree cleanup.apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts (2)
2591-2641: LGTM -skipStatusChangeparameter correctly prevents unwanted status emission.The optional parameter maintains backward compatibility while enabling the KanbanBoard's "Delete Worktree & Mark Done" flow to avoid the intermediate 'backlog' status.
2851-2915: LGTM -TASK_CLEAR_STAGED_STATEhandler implementation is correct.The handler properly:
- Validates task and plan existence
- Removes
stagedInMainProjectandstagedAtflags usingdeleteoperator- Updates worktree plan as best-effort (non-fatal on failure)
- Invalidates the tasks cache to ensure UI refresh picks up the changes
The separation between main plan (must succeed) and worktree plan (best-effort) is appropriate.
|
|
||
| {/* Worktree cleanup confirmation dialog */} | ||
| <AlertDialog open={worktreeDialogOpen} onOpenChange={setWorktreeDialogOpen}> | ||
| <AlertDialogContent className="max-w-md"> | ||
| <AlertDialogHeader> | ||
| <AlertDialogTitle className="flex items-center gap-2"> | ||
| <FolderCheck className="h-5 w-5 text-primary" /> | ||
| {t('kanban.worktreeCleanupTitle', 'Worktree Cleanup')} | ||
| </AlertDialogTitle> | ||
| <AlertDialogDescription asChild> | ||
| <div className="text-left space-y-2"> | ||
| {pendingDoneTask?.stagedInMainProject ? ( | ||
| <p> | ||
| {t('kanban.worktreeCleanupStaged', 'This task has been staged and has a worktree. Would you like to clean up the worktree?')} | ||
| </p> | ||
| ) : ( | ||
| <p> | ||
| {t('kanban.worktreeCleanupNotStaged', 'This task has a worktree with changes that have not been merged. Delete the worktree to mark as done, or cancel to review the changes first.')} | ||
| </p> | ||
| )} | ||
| {pendingDoneTask && ( | ||
| <p className="text-sm font-medium text-foreground/80 bg-muted/50 rounded px-2 py-1.5"> | ||
| {pendingDoneTask.title} | ||
| </p> | ||
| )} | ||
| </div> | ||
| </AlertDialogDescription> | ||
| </AlertDialogHeader> | ||
| <AlertDialogFooter className="flex-col sm:flex-row gap-2"> | ||
| <Button | ||
| variant="outline" | ||
| onClick={() => { | ||
| setWorktreeDialogOpen(false); | ||
| setPendingDoneTask(null); | ||
| }} | ||
| disabled={isCleaningUp} | ||
| > | ||
| {t('common:cancel', 'Cancel')} | ||
| </Button> | ||
| {/* Only show "Keep Worktree" option if task is staged */} | ||
| {pendingDoneTask?.stagedInMainProject && ( | ||
| <Button | ||
| variant="secondary" | ||
| onClick={() => pendingDoneTask && handleMoveToDone(pendingDoneTask, false)} | ||
| disabled={isCleaningUp} | ||
| className="gap-2" | ||
| > | ||
| <FolderCheck className="h-4 w-4" /> | ||
| {t('kanban.keepWorktree', 'Keep Worktree')} | ||
| </Button> | ||
| )} | ||
| <Button | ||
| variant={pendingDoneTask?.stagedInMainProject ? 'default' : 'destructive'} | ||
| onClick={() => pendingDoneTask && handleMoveToDone(pendingDoneTask, true)} | ||
| disabled={isCleaningUp} | ||
| className="gap-2" | ||
| > | ||
| {isCleaningUp ? ( | ||
| <Loader2 className="h-4 w-4 animate-spin" /> | ||
| ) : ( | ||
| <Trash2 className="h-4 w-4" /> | ||
| )} | ||
| {t('kanban.deleteWorktree', 'Delete Worktree & Mark Done')} | ||
| </Button> | ||
| </AlertDialogFooter> | ||
| </AlertDialogContent> | ||
| </AlertDialog> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the locale file structure
find apps/frontend/src/shared/i18n/locales -type f -name "*.json" | head -20Repository: AndyMik90/Auto-Claude
Length of output: 1158
🏁 Script executed:
# Search for worktreeCleanup keys in the codebase
rg "worktreeCleanup|keepWorktree|deleteWorktree" apps/frontend/src/shared/i18n/locales/Repository: AndyMik90/Auto-Claude
Length of output: 47
🏁 Script executed:
# Check the structure of en locale files
ls -la apps/frontend/src/shared/i18n/locales/en/Repository: AndyMik90/Auto-Claude
Length of output: 815
🏁 Script executed:
# Search for any kanban keys in existing locale files
rg "kanban" apps/frontend/src/shared/i18n/locales/en/Repository: AndyMik90/Auto-Claude
Length of output: 217
🏁 Script executed:
# Check tasks.json and dialogs.json for structure
cat apps/frontend/src/shared/i18n/locales/en/tasks.json | head -40Repository: AndyMik90/Auto-Claude
Length of output: 1012
🏁 Script executed:
# Get the full kanban section from tasks.json
rg -A 50 '"kanban"' apps/frontend/src/shared/i18n/locales/en/tasks.jsonRepository: AndyMik90/Auto-Claude
Length of output: 1672
🏁 Script executed:
# Check fr locale for kanban keys
rg -A 50 '"kanban"' apps/frontend/src/shared/i18n/locales/fr/tasks.jsonRepository: AndyMik90/Auto-Claude
Length of output: 1875
Add translation keys for the worktree cleanup dialog to locale files.
The code introduces five new user-facing strings without adding translation keys to the locale files:
kanban.worktreeCleanupTitlekanban.worktreeCleanupStagedkanban.worktreeCleanupNotStagedkanban.keepWorktreekanban.deleteWorktree
These keys must be added to both apps/frontend/src/shared/i18n/locales/en/tasks.json and apps/frontend/src/shared/i18n/locales/fr/tasks.json within the kanban section. Using fallback strings is not sufficient—per project requirements, new user-facing text must have proper translation entries in all required language files.
🤖 Prompt for AI Agents
In @apps/frontend/src/renderer/components/KanbanBoard.tsx around lines 583 -
649, The new Kanban worktree dialog strings (kanban.worktreeCleanupTitle,
kanban.worktreeCleanupStaged, kanban.worktreeCleanupNotStaged,
kanban.keepWorktree, kanban.deleteWorktree) are missing from the locale
files—add these keys under the "kanban" section in the English and French
tasks.json locale files with appropriate translations (use the current fallback
strings as the values if needed), so the AlertDialog text in KanbanBoard.tsx
displays localized content.
AndyMik90
left a comment
AndyMik90
left a comment
There was a problem hiding this comment.
🤖 Auto Claude PR Review
🔴 Follow-up Review: Blocked
🔴 Blocked - 2 CI check(s) failing. Fix CI before merge.
Resolution Status
- ✅ Resolved: 0 previous findings addressed
- ❌ Unresolved: 0 previous findings remain
- 🆕 New Issues: 4 new findings in recent changes
🚨 Blocking Issues
- quality: TypeScript type error: includes() incompatible with TaskStatus
- quality: Missing i18n translation keys for worktree cleanup dialog
- quality: Missing error handling in handleMoveToDone
Verdict
BLOCKED due to 2 failing CI checks (CodeQL, test-frontend). The test-frontend failure is likely caused by the TypeScript type error at line 500 in KanbanBoard.tsx where TASK_STATUS_COLUMNS.includes() is used instead of the existing isValidDropColumn() type guard. This is a critical issue that must be fixed. Additionally, missing i18n translation keys (medium) and error handling gaps (medium) should be addressed. Fix the TypeScript error first to unblock CI, then address the i18n and error handling issues.
Review Process
Agents invoked: new-code-reviewer, comment-analyzer
This is an AI-generated follow-up review using parallel specialist analysis with finding validation.
Findings (4 selected of 4 total)
🔴 [NEW-001] [CRITICAL] TypeScript type error: includes() incompatible with TaskStatus
📁 apps/frontend/src/renderer/components/KanbanBoard.tsx:500
Line 500 uses TASK_STATUS_COLUMNS.includes(overId as TaskStatus) which causes a TypeScript error because TaskStatus includes 'pr_created' which is not in the TASK_STATUS_COLUMNS tuple. The existing type guard isValidDropColumn() at lines 42-45 should be used instead. This is likely causing the test-frontend CI failure.
Suggested fix:
Replace with: if (isValidDropColumn(overId)) { targetStatus = overId; }
🔵 [NEW-002] [LOW] TOCTOU race condition in clearStagedState handler
📁 apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts:2868
The clearStagedState handler uses existsSync() to check for the plan file, then reads it with fsPromises.readFile(). This creates a time-of-check-to-time-of-use race condition. While agents disagreed on severity (one marked as false positive), the pattern diverges from codebase conventions that use EAFP pattern with try/catch.
Suggested fix:
Use EAFP pattern: wrap readFile in try/catch and check for ENOENT error code
🟡 [NEW-003] [MEDIUM] Missing i18n translation keys for worktree cleanup dialog
📁 apps/frontend/src/renderer/components/KanbanBoard.tsx:590
The new worktree cleanup dialog uses translation keys (kanban.worktreeCleanupTitle, kanban.worktreeCleanupStaged, kanban.worktreeCleanupNotStaged, kanban.keepWorktree, kanban.deleteWorktree) that do not exist in any locale files. Verified via grep that no locale files contain these keys.
Suggested fix:
Add missing keys to apps/frontend/src/shared/i18n/locales/en/tasks.json and all other locale files
🟡 [NEW-004] [MEDIUM] Missing error handling in handleMoveToDone
📁 apps/frontend/src/renderer/components/KanbanBoard.tsx:466
The handleMoveToDone function logs errors from discardWorktree but continues execution, and has no error handling for persistTaskStatus failure. This could leave tasks in an inconsistent state.
Suggested fix:
Consider aborting the operation if worktree deletion fails, or add try/catch around persistTaskStatus with user feedback
This review was generated by Auto Claude.
…nd i18n - Fix TypeScript error in KanbanBoard by using isValidDropColumn type guard instead of incorrect includes() cast with TaskStatus - Fix TOCTOU race condition in clearStagedState handler by using EAFP pattern (try/catch) instead of existsSync before read/write - Fix task data refresh in handleReviewAgain by calling loadTasks after clearing staged state to reflect updated task data - Add workspaceError reset in handleReviewAgain - Add missing i18n translation keys for kanban worktree cleanup dialog (en/fr: worktreeCleanupTitle, worktreeCleanupStaged, worktreeCleanupNotStaged, keepWorktree, deleteWorktree) - Remove unused Trash2 import and WorktreeStatus type import - Remove unused worktreeStatus prop from StagedInProjectMessage 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
github-actions
bot
added
🔄 Checking
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/frontend/src/renderer/components/KanbanBoard.tsx (1)
534-547: Fix hardcoded strings - i18n violation.Lines 544 contain hardcoded English strings
'Refreshing...'and'Refresh Tasks'that should be localized using thet()function.As per coding guidelines, all user-facing text in the frontend must use i18n translation keys instead of hardcoded strings.
🌍 Proposed fix
Add the translation keys to
apps/frontend/src/shared/i18n/locales/en/tasks.jsonandfr/tasks.json:# In both en/tasks.json and fr/tasks.json under an appropriate section (e.g., "actions"): + "refreshing": "Refreshing...", + "refreshTasks": "Refresh Tasks"Then update the component:
- {isRefreshing ? 'Refreshing...' : 'Refresh Tasks'} + {isRefreshing ? t('actions.refreshing') : t('actions.refreshTasks')}apps/frontend/src/renderer/components/task-detail/task-review/WorkspaceMessages.tsx (1)
189-293: Critical: New user-facing strings violate i18n requirements.Multiple new hardcoded user-facing strings have been introduced without i18n translation keys. Per coding guidelines, all user-facing text in frontend files must use i18n with the
useTranslation()hook.New hardcoded strings that need translation:
- Button labels: "Delete Worktree & Mark Done", "Mark as Done", "Mark Done Only", "Review Again"
- Loading states: "Cleaning up...", "Marking done...", "Resetting..."
- Helper text and error messages throughout the UI
🌐 Recommended fix: Add i18n support
- Add translation keys to language files (e.g.,
apps/frontend/src/shared/i18n/locales/en/tasks.jsonandfr/tasks.json):{ "taskReview": { "deleteWorktreeAndMarkDone": "Delete Worktree & Mark Done", "markAsDone": "Mark as Done", "markDoneOnly": "Mark Done Only", "reviewAgain": "Review Again", "cleaningUp": "Cleaning up...", "markingDone": "Marking done...", "resetting": "Resetting...", "keepWorktreeHint": "\"Delete Worktree & Mark Done\" cleans up the isolated workspace. \"Mark Done Only\" keeps it for reference." } }
- Import and use the
useTranslationhook:+import { useTranslation } from 'react-i18next'; + export function StagedInProjectMessage({ task, projectPath, hasWorktree = false, onClose, onReviewAgain }: StagedInProjectMessageProps) { + const { t } = useTranslation(); const [isDeleting, setIsDeleting] = useState(false); // ... rest of component
- Replace hardcoded strings with translation calls:
- Delete Worktree & Mark Done + {t('tasks:taskReview.deleteWorktreeAndMarkDone')}Repeat for all user-facing strings.
As per coding guidelines and learnings, based on learnings pre-existing i18n issues can be deferred, but new violations introduced in this PR should be addressed.
🤖 Fix all issues with AI agents
In @apps/frontend/src/renderer/components/KanbanBoard.tsx:
- Around line 466-485: The cleanup path in handleMoveToDone swallows worktree
deletion failures and still marks the task done without user-visible feedback;
update the error branch after calling
window.electronAPI.discardWorktree(task.id, true) to surface the failure to the
user (e.g., call a toast warning or set an error state) while continuing to call
persistTaskStatus(task.id, 'done'), and ensure existing cleanup finally-block
behavior (setIsCleaningUp(false), setWorktreeDialogOpen(false),
setPendingDoneTask(null)) remains unchanged so UI state is consistent.
In @apps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.ts:
- Around line 285-323: Update the inline comment in the handleReviewAgain
callback to explicitly state the call order: note that clearStagedState IPC was
already invoked by the parent WorkspaceMessages component before this callback
runs, and that loadTasks(selectedProject.id) is reloading task data from the
store to reflect the cleared staged state; reference handleReviewAgain,
clearStagedState, WorkspaceMessages, and loadTasks in the comment to make the
dependency and ordering clear.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
apps/frontend/src/main/ipc-handlers/task/worktree-handlers.tsapps/frontend/src/renderer/components/KanbanBoard.tsxapps/frontend/src/renderer/components/task-detail/TaskReview.tsxapps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.tsapps/frontend/src/renderer/components/task-detail/task-review/WorkspaceMessages.tsxapps/frontend/src/shared/i18n/locales/en/tasks.jsonapps/frontend/src/shared/i18n/locales/fr/tasks.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/tasks.jsonapps/frontend/src/shared/i18n/locales/fr/tasks.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/task-detail/TaskReview.tsxapps/frontend/src/renderer/components/task-detail/task-review/WorkspaceMessages.tsxapps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.tsapps/frontend/src/renderer/components/KanbanBoard.tsxapps/frontend/src/main/ipc-handlers/task/worktree-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/task-detail/TaskReview.tsxapps/frontend/src/renderer/components/task-detail/task-review/WorkspaceMessages.tsxapps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.tsapps/frontend/src/renderer/components/KanbanBoard.tsxapps/frontend/src/main/ipc-handlers/task/worktree-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/task-detail/TaskReview.tsxapps/frontend/src/renderer/components/task-detail/task-review/WorkspaceMessages.tsxapps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.tsapps/frontend/src/renderer/components/KanbanBoard.tsxapps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts
🧠 Learnings (7)
📓 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:48.743Z
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/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/shared/i18n/locales/en/tasks.jsonapps/frontend/src/renderer/components/task-detail/task-review/WorkspaceMessages.tsxapps/frontend/src/renderer/components/KanbanBoard.tsxapps/frontend/src/shared/i18n/locales/fr/tasks.json
📚 Learning: 2026-01-04T23:59:48.743Z
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:48.743Z
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.
Applied to files:
apps/frontend/src/shared/i18n/locales/en/tasks.jsonapps/frontend/src/shared/i18n/locales/fr/tasks.json
📚 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 : Store translation strings in namespace-organized JSON files at `apps/frontend/src/shared/i18n/locales/{lang}/*.json` for each supported language
Applied to files:
apps/frontend/src/shared/i18n/locales/en/tasks.jsonapps/frontend/src/renderer/components/task-detail/task-review/WorkspaceMessages.tsxapps/frontend/src/renderer/components/KanbanBoard.tsxapps/frontend/src/shared/i18n/locales/fr/tasks.json
📚 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/task-detail/TaskReview.tsxapps/frontend/src/renderer/components/task-detail/task-review/WorkspaceMessages.tsxapps/frontend/src/renderer/components/KanbanBoard.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/task-detail/task-review/WorkspaceMessages.tsxapps/frontend/src/renderer/components/KanbanBoard.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/task-detail/task-review/WorkspaceMessages.tsx
🧬 Code graph analysis (4)
apps/frontend/src/renderer/components/task-detail/TaskReview.tsx (2)
apps/frontend/src/renderer/components/task-detail/task-review/WorkspaceMessages.tsx (2)
LoadingMessage(14-23)StagedInProjectMessage(98-296)apps/frontend/src/renderer/components/task-detail/task-review/index.ts (2)
LoadingMessage(16-16)StagedInProjectMessage(16-16)
apps/frontend/src/renderer/components/task-detail/task-review/WorkspaceMessages.tsx (3)
apps/frontend/src/renderer/components/settings/utils/hookProxyFactory.ts (2)
setError(19-19)error(18-18)apps/frontend/src/renderer/stores/task-store.ts (1)
persistTaskStatus(457-478).design-system/src/lib/icons.ts (2)
Check(21-21)RotateCcw(31-31)
apps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.ts (1)
apps/frontend/src/renderer/stores/task-store.ts (1)
loadTasks(373-390)
apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts (4)
apps/frontend/src/shared/types/task.ts (1)
WorktreeDiscardResult(403-406)apps/frontend/src/shared/constants/ipc.ts (1)
IPC_CHANNELS(6-517)apps/frontend/src/shared/constants/config.ts (1)
getSpecsDir(106-109)apps/frontend/src/main/worktree-paths.ts (1)
findTaskWorktree(39-49)
⏰ 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). (4)
- GitHub Check: CodeQL
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: CodeQL (python)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (9)
apps/frontend/src/shared/i18n/locales/fr/tasks.json (1)
79-84: LGTM! French localization additions are correct.The new worktree-related translation keys are properly added and the French translations are grammatically correct. This follows the project's i18n guidelines by adding keys to both language files.
apps/frontend/src/shared/i18n/locales/en/tasks.json (1)
79-84: LGTM! English localization additions are clear and well-structured.The new worktree cleanup keys are properly organized under the kanban namespace and provide clear user-facing text for the new functionality.
apps/frontend/src/renderer/components/task-detail/TaskReview.tsx (1)
113-152: Excellent fix! Condition reordering correctly resolves the staging UI bug.The new priority chain properly checks persisted state (
task.stagedInMainProject) before worktree existence (worktreeStatus?.exists). This ensures that once a task is staged, the staging UI won't reappear after app restart, which was the core bug described in the PR objectives.The logic flow is now:
- Show loading if checking worktree
- Show nothing if fresh staging just completed (StagedSuccessMessage shown above)
- Show "already staged" message if persisted flag is set (even if worktree still exists)
- Show staging UI if worktree exists but not yet staged
- Show "no workspace" message otherwise
apps/frontend/src/renderer/components/KanbanBoard.tsx (1)
347-350: LGTM! Worktree cleanup dialog implementation is well-designed.The new worktree cleanup flow is properly implemented:
- Checks for worktree existence before showing the dialog
- Differentiates between staged and non-staged tasks in the UI
- Only shows "Keep Worktree" option when the task is already staged
- Proper loading states and error handling in the dialog
The integration with the drag-and-drop flow is seamless and provides good UX for worktree management.
Also applies to: 455-463, 584-649
apps/frontend/src/renderer/components/task-detail/task-review/WorkspaceMessages.tsx (3)
1-1: LGTM: Type definition and icon import.The addition of
RotateCcwicon andonReviewAgaincallback prop are correctly typed and align with the new "Review Again" functionality.Also applies to: 92-92
100-143: LGTM: State management and handleMarkDoneOnly logic.The new state variables (
isMarkingDone,isResetting) and thehandleMarkDoneOnlyfunction are correctly implemented with proper error handling and state management.
145-168: LGTM: Review again handler implementation.The
handleReviewAgainfunction correctly calls the IPC handler to clear staged state, handles errors appropriately, and triggers the parent callback to refresh the UI.apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts (2)
2591-2591: LGTM: Optional status change suppression.The addition of the
skipStatusChangeparameter toTASK_WORKTREE_DISCARDis well-designed, allowing callers to suppress the automatic status change when they need to set a different status themselves. This prevents race conditions and maintains backward compatibility through the optional parameter.Also applies to: 2634-2641
2851-2925: LGTM: TASK_CLEAR_STAGED_STATE handler implementation.The new handler is well-implemented with several good practices:
- Uses EAFP pattern (try/catch) to avoid TOCTOU race conditions
- Updates both main project and worktree plans (best-effort for worktree)
- Invalidates task cache to ensure UI reflects the cleared state
- Comprehensive error handling with specific ENOENT checks
- Uses async/await with fsPromises for non-blocking I/O
The implementation correctly clears the staged flags and triggers a cache invalidation to ensure the UI state is refreshed.
| const handleMoveToDone = async (task: Task, deleteWorktree: boolean) => { | ||
| setIsCleaningUp(true); | ||
| try { | ||
| if (deleteWorktree) { | ||
| // Delete worktree first, skip automatic status change to backlog | ||
| // since we're about to set status to 'done' | ||
| const result = await window.electronAPI.discardWorktree(task.id, true); | ||
| if (!result.success) { | ||
| console.error('Failed to delete worktree:', result.error); | ||
| // Continue anyway - user can clean up manually | ||
| } | ||
| } | ||
| // Mark as done | ||
| await persistTaskStatus(task.id, 'done'); | ||
| } finally { | ||
| setIsCleaningUp(false); | ||
| setWorktreeDialogOpen(false); | ||
| setPendingDoneTask(null); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider improving error handling clarity.
The error handling in handleMoveToDone logs the failure but continues to mark the task as done anyway (line 479). While the comment "Continue anyway - user can clean up manually" explains this behavior, it might be better to inform the user of the failure explicitly.
Consider adding a toast notification or updating the UI to show that worktree deletion failed but the task was still marked as done. This provides better user feedback about the partial success.
Example approach:
if (!result.success) {
console.error('Failed to delete worktree:', result.error);
// Show toast or set error state to inform user
toast.warning('Task marked as done, but worktree deletion failed. You may need to clean it up manually.');
}🤖 Prompt for AI Agents
In @apps/frontend/src/renderer/components/KanbanBoard.tsx around lines 466 -
485, The cleanup path in handleMoveToDone swallows worktree deletion failures
and still marks the task done without user-visible feedback; update the error
branch after calling window.electronAPI.discardWorktree(task.id, true) to
surface the failure to the user (e.g., call a toast warning or set an error
state) while continuing to call persistTaskStatus(task.id, 'done'), and ensure
existing cleanup finally-block behavior (setIsCleaningUp(false),
setWorktreeDialogOpen(false), setPendingDoneTask(null)) remains unchanged so UI
state is consistent.
| // Handle "Review Again" - clears staged state and reloads worktree info | ||
| const handleReviewAgain = useCallback(async () => { | ||
| // Clear staged success state if it was set in this session | ||
| setStagedSuccess(null); | ||
| setStagedProjectPath(undefined); | ||
| setSuggestedCommitMessage(undefined); | ||
|
|
||
| // Reset merge preview to force re-check | ||
| setMergePreview(null); | ||
| hasLoadedPreviewRef.current = null; | ||
|
|
||
| // Reset workspace error state | ||
| setWorkspaceError(null); | ||
|
|
||
| // Reload worktree status | ||
| setIsLoadingWorktree(true); | ||
| try { | ||
| const [statusResult, diffResult] = await Promise.all([ | ||
| window.electronAPI.getWorktreeStatus(task.id), | ||
| window.electronAPI.getWorktreeDiff(task.id) | ||
| ]); | ||
| if (statusResult.success && statusResult.data) { | ||
| setWorktreeStatus(statusResult.data); | ||
| } | ||
| if (diffResult.success && diffResult.data) { | ||
| setWorktreeDiff(diffResult.data); | ||
| } | ||
|
|
||
| // Reload task data from store to reflect cleared staged state | ||
| // (clearStagedState IPC already invalidated the cache) | ||
| if (selectedProject) { | ||
| await loadTasks(selectedProject.id); | ||
| } | ||
| } catch (err) { | ||
| console.error('Failed to reload worktree info:', err); | ||
| } finally { | ||
| setIsLoadingWorktree(false); | ||
| } | ||
| }, [task.id, selectedProject]); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
LGTM! The handleReviewAgain implementation correctly refreshes UI state.
The function properly clears staged state UI elements and reloads worktree information. The comment on line 314 indicates that the IPC call to clearStagedState happens before this callback (in the WorkspaceMessages component), which is confirmed by the code snippets provided.
One minor suggestion: Consider making the comment on lines 313-314 more explicit about the call order, e.g., "Reload task data from store to reflect cleared staged state (clearStagedState IPC was already called by the parent component before invoking this callback)."
🤖 Prompt for AI Agents
In @apps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.ts
around lines 285 - 323, Update the inline comment in the handleReviewAgain
callback to explicitly state the call order: note that clearStagedState IPC was
already invoked by the parent WorkspaceMessages component before this callback
runs, and that loadTasks(selectedProject.id) is reloading task data from the
store to reflect the cleared staged state; reference handleReviewAgain,
clearStagedState, WorkspaceMessages, and loadTasks in the comment to make the
dependency and ordering clear.
| export function StagedInProjectMessage({ task, projectPath, hasWorktree = false, onClose, onReviewAgain }: StagedInProjectMessageProps) { | ||
| const [isDeleting, setIsDeleting] = useState(false); | ||
| const [isMarkingDone, setIsMarkingDone] = useState(false); | ||
| const [isResetting, setIsResetting] = useState(false); |
There was a problem hiding this comment.
Missing skipStatusChange causes task status race condition
Medium Severity
The handleDeleteWorktreeAndMarkDone function calls discardWorktree(task.id) without passing skipStatusChange=true. Since the handler sends a TASK_STATUS_CHANGE event with status 'backlog' when skipStatusChange is not true, and then persistTaskStatus(task.id, 'done') is immediately called, there's a race condition. The task status will briefly change to 'backlog' before being set to 'done', causing UI flickering and unnecessary state transitions. The KanbanBoard.tsx correctly passes true for the same use case at line 472.
github-actions
bot
added
🔄 Checking
AndyMik90
left a comment
AndyMik90
left a comment
There was a problem hiding this comment.
✅ Auto Claude Review - APPROVED
Status: Ready to Merge
Summary: ## 🟡 Follow-up Review: Merge With Changes
🟡 Can merge - Minor suggestions noted, no blockers.
Resolution Status
- ✅ Resolved: 2 previous findings addressed
- ❌ Unresolved: 2 previous findings remain
- 🆕 New Issues: 2 new findings in recent changes
Finding Validation
- 🔍 Dismissed as False Positives: 1 findings were re-investigated and found to be incorrect
- ✓ Confirmed Valid: 1 findings verified as genuine issues
- 👤 Needs Human Review: 0 findings require manual verification
Verdict
CI Status: ✅ All 17 checks passing - no blockers.
Previous Issues:
- ✅ NEW-004 RESOLVED: The handleMoveToDone error handling issue that triggered the previous "Needs Revision" verdict is now properly fixed with a try-finally block.
⚠️ NCR-001 PARTIALLY_RESOLVED: JSON.parse errors are caught and handled, just with a generic message.⚠️ NCR-002 CONFIRMED VALID (LOW): Race condition exists but is a theoretical edge case with graceful handling.- 🚫 CMT-001 DISMISSED: Pre-existing i18n strings are out of scope for this PR.
New Findings: 2 low-severity code quality improvements identified. The JSON.parse and UI refresh issues are valid suggestions but don't block merge since:
- Errors ARE handled gracefully (success:false returned)
- No data corruption or security issues
- Issues are about improved error messages and edge case UX
Verdict Rationale: The original blocking issue (NEW-004) is resolved. Remaining findings are low-severity improvements that enhance debugging experience and edge case handling. The code is functional and safe - these suggestions can be addressed in a follow-up or accepted as-is.
Review Process
Agents invoked: resolution-verifier, new-code-reviewer, finding-validator
This is an AI-generated follow-up review using parallel specialist analysis with finding validation.
💡 Suggestions (3)
These are non-blocking suggestions for consideration:
🔵 [NCR-002] [LOW] [UNRESOLVED] Minor race condition between worktree check and dialog display
📁 apps/frontend/src/renderer/components/KanbanBoard.tsx:517
In handleDragEnd, there's an async gap between checking if a task has a worktree (line 517) and displaying the dialog (line 521-522). During this time, another action could delete the worktree or change task status. The pendingDoneTask could become stale by the time user clicks a dialog button.
Resolution note: if (targetStatus === 'done') {
const hasWorktree = await checkTaskHasWorktree(task.id);
if (hasWorktree) {
setPendingDoneTask(task);
setWorktreeDialogOpen(true);
return;
}
}
Suggested fix:
Minor issue given single-user context. For robustness, handleMoveToDone could re-verify worktree exists before deletion.
🔵 [NCR-FU-001] [LOW] JSON.parse without specific error handling for malformed JSON
📁 apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts:2884
The clearStagedState handler parses implementation_plan.json with JSON.parse() inside a try-catch, but the error message is generic ('Failed to clear staged state'). If the file contains invalid JSON, users won't get a clear error about the JSON being corrupted. The error IS handled gracefully (returns success:false), but a more specific message would improve debugging.
Suggested fix:
Wrap JSON.parse in a specific try-catch: try { const plan = JSON.parse(planContent); } catch (parseErr) { return { success: false, error: 'Implementation plan file contains invalid JSON' }; }
🔵 [NCR-FU-002] [LOW] handleReviewAgain doesn't refresh UI if IPC returns error
📁 apps/frontend/src/renderer/components/task-detail/task-review/WorkspaceMessages.tsx:153
When clearStagedState IPC returns success=false, the error is displayed but onReviewAgain() is not called. This leaves the UI in a potentially stale state if partial state changes occurred before the error.
Suggested fix:
Consider calling onReviewAgain() even on failure to ensure UI consistency, or verify no partial state changes can occur.
This automated review found no blocking issues. The PR can be safely merged.
Generated by Auto Claude
* fix(ui): persist staged task state across app restarts
Previously, when a task was staged and the app restarted, the UI showed
the staging interface again instead of recognizing the task was already
staged. This happened because the condition order checked worktree
existence before checking the stagedInMainProject flag.
Changes:
- Fix condition priority in TaskReview.tsx to check stagedInMainProject
before worktreeStatus.exists
- Add 'Mark Done Only' button to mark task complete without deleting
worktree
- Add 'Review Again' button to clear staged state and re-show staging UI
- Add TASK_CLEAR_STAGED_STATE IPC handler to reset staged flags in
implementation plan files
- Add handleReviewAgain callback in useTaskDetail hook
* feat(ui): add worktree cleanup dialog when marking task as done
When dragging a task to the 'done' column, if the task has a worktree:
- Shows a confirmation dialog asking about worktree cleanup
- Staged tasks: Can 'Keep Worktree' or 'Delete Worktree & Mark Done'
- Non-staged tasks: Must delete worktree or cancel (to prevent losing work)
Also fixes a race condition where discardWorktree sent 'backlog' status
before persistTaskStatus('done') could execute, causing task to briefly
appear in Done then jump back to Planning.
Added skipStatusChange parameter to discardWorktree IPC to prevent this.
* fix(frontend): Address PR AndyMik90#800 feedback - type errors, TOCTOU race, and i18n
- Fix TypeScript error in KanbanBoard by using isValidDropColumn type guard
instead of incorrect includes() cast with TaskStatus
- Fix TOCTOU race condition in clearStagedState handler by using EAFP
pattern (try/catch) instead of existsSync before read/write
- Fix task data refresh in handleReviewAgain by calling loadTasks after
clearing staged state to reflect updated task data
- Add workspaceError reset in handleReviewAgain
- Add missing i18n translation keys for kanban worktree cleanup dialog
(en/fr: worktreeCleanupTitle, worktreeCleanupStaged, worktreeCleanupNotStaged,
keepWorktree, deleteWorktree)
- Remove unused Trash2 import and WorktreeStatus type import
- Remove unused worktreeStatus prop from StagedInProjectMessage
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <[email protected]>
---------
Co-authored-by: Claude Opus 4.5 <[email protected]>
Summary
Fixes a bug where the staging UI was shown again after app restart even when a task had already been staged. The persisted
stagedInMainProjectflag was being ignored because the condition order in the UI prioritized worktree existence over staged state.Problem
When a user staged changes for a task:
stagedInMainProjectandstagedAtflags were correctly saved toimplementation_plan.jsonworktreeStatus?.existswas checked beforetask.stagedInMainProjectSolution
TaskReview.tsx- now checksstagedInMainProjectbeforeworktreeStatus?.existsTASK_CLEAR_STAGED_STATEIPC handler - resets staged flags in plan filesChanges
UI Layer
TaskReview.tsx- Fixed condition order, addedonReviewAgainpropWorkspaceMessages.tsx- Added new props, "Mark Done Only" and "Review Again" buttonsuseTaskDetail.ts- AddedhandleReviewAgaincallbackTaskDetailModal.tsx- PassedonReviewAgainto TaskReviewIPC Layer
ipc.ts(constants) - AddedTASK_CLEAR_STAGED_STATEchannelipc.ts(types) - AddedclearStagedStatetypetask-api.ts- Added preload bindingworktree-handlers.ts- Implemented handler to clear staged stateMocks
workspace-mock.ts- Added mock for browser testingTesting
Notes
The worktree is intentionally NOT deleted after staging (only when user explicitly clicks "Delete Worktree & Mark Done"). This allows users to keep the worktree for reference while having the staged state properly tracked.
Summary by CodeRabbit
New Features
Bug Fixes
Localization
✏️ Tip: You can customize this high-level summary in your review settings.