Github issue #1805 - Fix Real-Time Subtask Display via Worktree-Aware FileWatcher#1839
Conversation
…aware FileWatcher path Updated three fileWatcher.watch calls to use worktree paths: - TASK_START handler (line 213) - TASK_UPDATE_STATUS handler when setting to in_progress (line 716) - TASK_START_RECOVERY handler (line 1157) Pattern: Find worktree using findTaskWorktree(), use it if exists, otherwise fall back to main project path. This ensures FileWatcher monitors the correct path where planner creates implementation_plan.json in the isolated worktree. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…er to use worktree-aware FileWatcher path Follow same pattern as TASK_START: - Declare local specsBaseDirForWatcher variable - Use findTaskWorktree() to detect worktree - Construct path inline for both branches (worktree and main project) This ensures FileWatcher monitors the correct path where planner creates implementation_plan.json, enabling real-time subtask display.
No changes needed - subtask-1-1 already updated all three FileWatcher locations including TASK_RECOVER_STUCK (line 1152-1158). Current implementation correctly: - Uses findTaskWorktree() to detect worktree path - Constructs specDirForWatcher from worktree when available - Falls back to main project path when worktree doesn't exist - Follows same pattern as TASK_START and TASK_UPDATE_STATUS handlers This ensures FileWatcher monitors the correct path where the backend writes implementation_plan.json during stuck task recovery restart. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
… tests TypeScript: PASSED (no errors) Tests: 3053/3061 PASSED (99.7%) One pre-existing test failure unrelated to FileWatcher changes Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Summary of ChangesHello @VDT-91, 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 enhances the application's file watching capabilities by introducing Git worktree awareness. The primary goal is to resolve issues where real-time updates for subtasks might not have been correctly displayed when the project was accessed via a Git worktree. By intelligently determining the correct base path for file watching, the system can now reliably track changes in such setups, improving the user experience for developers working with worktrees. Highlights
Changelog
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
|
📝 WalkthroughWalkthroughModified file-watching directory resolution in task execution handlers to prefer git worktree paths when available. When Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request fixes an issue with real-time subtask display by making the file watcher worktree-aware. The changes correctly look for a task's worktree path when determining the spec directory to watch. The implementation is correct, but the logic has been duplicated in three different places within execution-handlers.ts. I've left a comment suggesting to refactor this into a single helper function to improve maintainability and adhere to the DRY principle.
| const specsBaseDir = getSpecsDir(project.autoBuildPath); | ||
| const specDir = path.join( | ||
| project.path, | ||
| specsBaseDir, | ||
| task.specId | ||
| ); | ||
| const worktreePath = findTaskWorktree(project.path, task.specId); | ||
| const specDir = worktreePath | ||
| ? path.join(worktreePath, specsBaseDir, task.specId) | ||
| : path.join(project.path, specsBaseDir, task.specId); |
There was a problem hiding this comment.
This logic to determine the specDir by checking for a worktree is duplicated in three places in this file (here, lines 712-716, and lines 1153-1157). To improve maintainability and adhere to the DRY (Don't Repeat Yourself) principle, this should be extracted into a single helper function.
For example, you could create a function like getSpecDirForWatcher:
function getSpecDirForWatcher(project: Project, task: Task): string {
const specsBaseDir = getSpecsDir(project.autoBuildPath);
const worktreePath = findTaskWorktree(project.path, task.specId);
const basePath = worktreePath || project.path;
return path.join(basePath, specsBaseDir, task.specId);
}And then call it in all three places:
const specDir = getSpecDirForWatcher(project, task);
This makes the code cleaner and avoids potential bugs if this logic needs to be updated in the future.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts (1)
711-734:⚠️ Potential issue | 🔴 CriticalBug: spec existence check and spec creation use the non-worktree-aware
specDir.The file watcher correctly uses the worktree-aware
specDirForWatcher(line 717), but lines 720 and 734 still referencespecDirfrom line 634, which always resolves underproject.path. Ifspec.mdonly exists in the worktree, this will incorrectly trigger spec creation and pass the wrong directory tostartSpecCreation.Compare with the TASK_START handler (lines 216–237), where all paths consistently use the worktree-aware
specDir, and with the TASK_RECOVER_STUCK handler (line 1161), which correctly usesspecDirForWatcher.🐛 Proposed fix: use specDirForWatcher consistently
// Check if spec.md exists - const specFilePath = path.join(specDir, AUTO_BUILD_PATHS.SPEC_FILE); + const specFilePath = path.join(specDirForWatcher, AUTO_BUILD_PATHS.SPEC_FILE); const hasSpec = existsSync(specFilePath); const needsSpecCreation = !hasSpec; const needsImplementation = hasSpec && task.subtasks.length === 0;console.warn('[TASK_UPDATE_STATUS] Starting spec creation for:', task.specId); - agentManager.startSpecCreation(taskId, project.path, taskDescription, specDir, task.metadata, baseBranchForUpdate, project.id); + agentManager.startSpecCreation(taskId, project.path, taskDescription, specDirForWatcher, task.metadata, baseBranchForUpdate, project.id);
…e-subtask-updates-are-sil
AndyMik90
left a comment
There was a problem hiding this comment.
🤖 Auto Claude PR Review
Merge Verdict: 🔴 BLOCKED
🔴 Blocked - 2 critical/high/medium issue(s) must be fixed.
Blocked by 1 critical issues
Risk Assessment
| Factor | Level | Notes |
|---|---|---|
| Complexity | Low | Based on lines changed |
| Security Impact | None | Based on security findings |
| Scope Coherence | Good | Based on structural review |
🚨 Blocking Issues (Must Fix)
- Critical: Incomplete fix: specFilePath and startSpecCreation still use old non-worktree path (apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts:720)
Findings Summary
- Critical: 1 issue(s)
- Medium: 1 issue(s)
- Low: 1 issue(s)
Generated by Auto Claude PR Review
Findings (3 selected of 3 total)
🔴 [0bebf42bdb23] [CRITICAL] Incomplete fix: specFilePath and startSpecCreation still use old non-worktree path
📁 apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts:720
In the TASK_UPDATE_STATUS handler, the PR correctly introduces a worktree-aware specDirForWatcher (lines 712-716) for the file watcher. However, on line 720, specFilePath is constructed from the OLD specDir variable (defined at line 634 as path.join(project.path, specsBaseDir, task.specId)) instead of specDirForWatcher. Similarly, on line 734, agentManager.startSpecCreation() is called with the old specDir instead of specDirForWatcher.
This means:
- The file watcher correctly watches the worktree path ✅
- The spec existence check (
hasSpec) looks in the project root instead of the worktree ❌ - Spec creation is started targeting the project root instead of the worktree ❌
When a worktree exists, this will incorrectly determine that the spec file is missing (because it checks the project root, not the worktree) and potentially trigger unnecessary spec re-creation in the wrong directory.
The recovery handler (Location 3, line 1161) correctly uses specDirForWatcher for the spec file check and for startSpecCreation (line 1172), confirming this is indeed a bug in Location 2. | In the TASK_UPDATE_STATUS handler, the PR correctly creates a worktree-aware specDirForWatcher (line 714) for the file watcher, but the spec existence check on line 720 still uses the old specDir from line 634 which always resolves to project.path.
Compare with the other two locations this PR modifies:
- Location 1 (TASK_START, line 216): Uses worktree-aware
specDir— CORRECT - Location 3 (Recovery, line 1161): Uses worktree-aware
specDirForWatcher— CORRECT - Location 2 (TASK_UPDATE_STATUS, line 720): Uses old
specDirfrom line 634 — INCORRECT
When a worktree exists, specDir (line 634) = <project>/.auto-claude/specs/<specId>/ while specDirForWatcher (line 714) = <worktree>/.auto-claude/specs/<specId>/. The spec.md check on line 720 looks in the main project path, not the worktree where the spec actually lives.
Additionally, line 734 passes the old specDir to startSpecCreation, while location 1 (line 232) and location 3 (line 1172) correctly pass the worktree-aware path. | The PR applies the worktree-aware path fix inconsistently across its three locations. In the TASK_UPDATE_STATUS handler (location 2), the file watcher correctly uses the worktree-aware specDirForWatcher (lines 711-717), but the spec existence check at line 720 still uses the non-worktree-aware specDir (defined at line 634 as path.join(project.path, specsBaseDir, task.specId)). Similarly, startSpecCreation() at line 734 passes the non-worktree-aware specDir.
Compare with the PR's other two locations:
- Location 1 (line 207-216):
specDiritself is worktree-aware; used for both watcher AND spec check ✓ - Location 3 (line 1155-1161):
specDirForWatcheris worktree-aware; used for both watcher AND spec check ✓ - Location 2 (line 711-734): Only the watcher is worktree-aware; spec check and spec creation use
specDirfrom main project path ✗
This means if the spec only exists in the worktree directory, the TASK_UPDATE_STATUS handler will incorrectly report hasSpec = false and attempt to re-create the spec.
Suggested fix:
Replace `specDir` with `specDirForWatcher` on lines 720 and 734, matching the pattern used in the recovery handler:
```typescript
// Line 720: change from specDir to specDirForWatcher
const specFilePath = path.join(specDirForWatcher, AUTO_BUILD_PATHS.SPEC_FILE);
// Line 734: change from specDir to specDirForWatcher
agentManager.startSpecCreation(taskId, project.path, taskDescription, specDirForWatcher, task.metadata, baseBranchForUpdate, project.id);
#### 🟡 [5902ffdc71da] [MEDIUM] Worktree-aware spec dir resolution duplicated 3 times — extract helper
📁 `apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts:208`
The same 5-line worktree-aware spec directory resolution pattern is copy-pasted across three locations in this file (lines 208-212, 712-716, 1153-1157):
```typescript
const specsBaseDir = getSpecsDir(project.autoBuildPath);
const worktreePath = findTaskWorktree(project.path, task.specId);
const specDir = worktreePath
? path.join(worktreePath, specsBaseDir, task.specId)
: path.join(project.path, specsBaseDir, task.specId);
This duplication is what caused the bug in Location 2 — when the pattern was introduced, one call site didn't fully integrate it. A shared helper function (e.g., getWorktreeAwareSpecDir(project, task)) would prevent such inconsistencies and make future changes safer.
Suggested fix:
Extract a helper function in `worktree-paths.ts` or locally in this file:
```typescript
function getWorktreeAwareSpecDir(
projectPath: string,
autoBuildPath: string | undefined,
specId: string
): string {
const specsBaseDir = getSpecsDir(autoBuildPath);
const worktreePath = findTaskWorktree(projectPath, specId);
const basePath = worktreePath || projectPath;
return path.join(basePath, specsBaseDir, specId);
}
Then each call site becomes a single line:
const specDir = getWorktreeAwareSpecDir(project.path, project.autoBuildPath, task.specId);
#### 🔵 [63297b51771e] [LOW] Redundant variable `specsBaseDirForWatcher` duplicates existing `specsBaseDir`
📁 `apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts:712`
At line 712, the PR introduces `const specsBaseDirForWatcher = getSpecsDir(project.autoBuildPath)` which holds exactly the same value as `specsBaseDir` already defined at line 633 in the same scope (`const specsBaseDir = getSpecsDir(project.autoBuildPath)`). The other two locations in the PR (locations 1 and 3) simply use `specsBaseDir` without renaming.
Searched the file: `specsBaseDir` at line 633 and `specsBaseDirForWatcher` at line 712 both call `getSpecsDir(project.autoBuildPath)` with the same argument, and both are in the same handler scope.
**Suggested fix:**
Remove line 712 and reuse the existing specsBaseDir variable on line 714, e.g.:
const specDirForWatcher = worktreePath ? path.join(worktreePath, specsBaseDir, task.specId) : path.join(project.path, specsBaseDir, task.specId);
---
*This review was generated by Auto Claude.*
…e-subtask-updates-are-sil
AndyMik90
left a comment
There was a problem hiding this comment.
🤖 Auto Claude PR Review
🔴 Follow-up Review: Blocked
🔴 Blocked - 3 blocking issue(s) require fixes.
Resolution Status
- ✅ Resolved: 0 previous findings addressed
- ❌ Unresolved: 3 previous findings remain
- 🆕 New Issues: 1 new findings in recent changes
Finding Validation
- 🔍 Dismissed as False Positives: 0 findings were re-investigated and found to be incorrect
- ✓ Confirmed Valid: 3 findings verified as genuine issues
- 👤 Needs Human Review: 0 findings require manual verification
🚨 Blocking Issues
- quality: [UNRESOLVED] Incomplete fix: specFilePath and startSpecCreation still use old non-worktree path
- quality: [UNRESOLVED] Worktree-aware spec dir resolution duplicated 3 times — extract helper
- quality: [FROM COMMENTS] sentry[bot] and AndyMik90 review both flag specFilePath using wrong path
Verdict
All 21 CI checks are passing. However, the CRITICAL finding [0bebf42bdb23] remains unresolved and has been independently confirmed valid by the finding-validator with concrete code evidence. In the TASK_UPDATE_STATUS handler, specFilePath (line 720) and startSpecCreation (line 734) still use the non-worktree-aware specDir (defined at line 634 as path.join(project.path, specsBaseDir, task.specId)) instead of the worktree-aware specDirForWatcher (lines 714-716). This means spec existence checks and spec creation will operate on the wrong directory when a worktree is active. Both TASK_START and TASK_RECOVER_STUCK handlers do this correctly, confirming this is an oversight. The only new commit since last review was a merge from develop which did not address any of the 3 previous findings. The MEDIUM finding (duplicated pattern, no helper) is also confirmed valid and contributed to this bug class. Comments from @sentry[bot] and @AndyMik90 corroborate the critical issue. Fix: replace specDir with specDirForWatcher on lines 720 and 734.
Review Process
Agents invoked: resolution-verifier, comment-analyzer, finding-validator
This is an AI-generated follow-up review using parallel specialist analysis with finding validation.
Findings (4 selected of 4 total)
🔴 [0bebf42bdb23] [CRITICAL] [UNRESOLVED] Incomplete fix: specFilePath and startSpecCreation still use old non-worktree path
📁 apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts:720
In the TASK_UPDATE_STATUS handler, the PR correctly introduces a worktree-aware specDirForWatcher (lines 712-716) for the file watcher. However, on line 720, specFilePath is constructed from the OLD specDir variable (defined at line 634 as path.join(project.path, specsBaseDir, task.specId)) instead of specDirForWatcher. Similarly, on line 734, agentManager.startSpecCreation() is called with the old specDir instead of specDirForWatcher.
This means:
- The file watcher correctly watches the worktree path ✅
- The spec existence check (
hasSpec) looks in the project root instead of the worktree ❌ - Spec creation is started targeting the project root instead of the worktree ❌
When a worktree exists, this will incorrectly determine that the spec file is missing (because it checks the project root, not the worktree) and potentially trigger unnecessary spec re-creation in the wrong directory.
The recovery handler (Location 3, line 1161) correctly uses specDirForWatcher for the spec file check and for startSpecCreation (line 1172), confirming this is indeed a bug in Location 2. | In the TASK_UPDATE_STATUS handler, the PR correctly creates a worktree-aware specDirForWatcher (line 714) for the file watcher, but the spec existence check on line 720 still uses the old specDir from line 634 which always resolves to project.path.
Compare with the other two locations this PR modifies:
- Location 1 (TASK_START, line 216): Uses worktree-aware
specDir— CORRECT - Location 3 (Recovery, line 1161): Uses worktree-aware
specDirForWatcher— CORRECT - Location 2 (TASK_UPDATE_STATUS, line 720): Uses old
specDirfrom line 634 — INCORRECT
When a worktree exists, specDir (line 634) = <project>/.auto-claude/specs/<specId>/ while specDirForWatcher (line 714) = <worktree>/.auto-claude/specs/<specId>/. The spec.md check on line 720 looks in the main project path, not the worktree where the spec actually lives.
Additionally, line 734 passes the old specDir to startSpecCreation, while location 1 (line 232) and location 3 (line 1172) correctly pass the worktree-aware path. | The PR applies the worktree-aware path fix inconsistently across its three locations. In the TASK_UPDATE_STATUS handler (location 2), the file watcher correctly uses the worktree-aware specDirForWatcher (lines 711-717), but the spec existence check at line 720 still uses the non-worktree-aware specDir (defined at line 634 as path.join(project.path, specsBaseDir, task.specId)). Similarly, startSpecCreation() at line 734 passes the non-worktree-aware specDir.
Compare with the PR's other two locations:
- Location 1 (line 207-216):
specDiritself is worktree-aware; used for both watcher AND spec check ✓ - Location 3 (line 1155-1161):
specDirForWatcheris worktree-aware; used for both watcher AND spec check ✓ - Location 2 (line 711-734): Only the watcher is worktree-aware; spec check and spec creation use
specDirfrom main project path ✗
This means if the spec only exists in the worktree directory, the TASK_UPDATE_STATUS handler will incorrectly report hasSpec = false and attempt to re-create the spec.
Resolution note: Line 720: const specFilePath = path.join(specDir, AUTO_BUILD_PATHS.SPEC_FILE); and line 734: agentManager.startSpecCreation(taskId, project.path, taskDescription, specDir, ...) both still use the non-worktree specDir (line 634: path.join(project.path, specsBaseDir, task.specId)) instead of the worktree-aware specDirForWatcher (lines 714-716). The merge from develop did not address this.
Suggested fix:
Replace `specDir` with `specDirForWatcher` on lines 720 and 734, matching the pattern used in the recovery handler:
```typescript
// Line 720: change from specDir to specDirForWatcher
const specFilePath = path.join(specDirForWatcher, AUTO_BUILD_PATHS.SPEC_FILE);
// Line 734: change from specDir to specDirForWatcher
agentManager.startSpecCreation(taskId, project.path, taskDescription, specDirForWatcher, task.metadata, baseBranchForUpdate, project.id);
#### 🟡 [5902ffdc71da] [MEDIUM] [UNRESOLVED] Worktree-aware spec dir resolution duplicated 3 times — extract helper
📁 `apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts:208`
The same 5-line worktree-aware spec directory resolution pattern is copy-pasted across three locations in this file (lines 208-212, 712-716, 1153-1157):
```typescript
const specsBaseDir = getSpecsDir(project.autoBuildPath);
const worktreePath = findTaskWorktree(project.path, task.specId);
const specDir = worktreePath
? path.join(worktreePath, specsBaseDir, task.specId)
: path.join(project.path, specsBaseDir, task.specId);
This duplication is what caused the bug in Location 2 — when the pattern was introduced, one call site didn't fully integrate it. A shared helper function (e.g., getWorktreeAwareSpecDir(project, task)) would prevent such inconsistencies and make future changes safer.
Resolution note: The same 5-line worktree-aware spec directory resolution pattern (getSpecsDir + findTaskWorktree + ternary path.join) remains copy-pasted at lines 208-212, 712-716, and 1153-1157. No helper function was extracted. The merge from develop did not address this.
Suggested fix:
Extract a helper function in `worktree-paths.ts` or locally in this file:
```typescript
function getWorktreeAwareSpecDir(
projectPath: string,
autoBuildPath: string | undefined,
specId: string
): string {
const specsBaseDir = getSpecsDir(autoBuildPath);
const worktreePath = findTaskWorktree(projectPath, specId);
const basePath = worktreePath || projectPath;
return path.join(basePath, specsBaseDir, specId);
}
Then each call site becomes a single line:
const specDir = getWorktreeAwareSpecDir(project.path, project.autoBuildPath, task.specId);
#### 🔵 [63297b51771e] [LOW] [UNRESOLVED] Redundant variable `specsBaseDirForWatcher` duplicates existing `specsBaseDir`
📁 `apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts:712`
At line 712, the PR introduces `const specsBaseDirForWatcher = getSpecsDir(project.autoBuildPath)` which holds exactly the same value as `specsBaseDir` already defined at line 633 in the same scope (`const specsBaseDir = getSpecsDir(project.autoBuildPath)`). The other two locations in the PR (locations 1 and 3) simply use `specsBaseDir` without renaming.
Searched the file: `specsBaseDir` at line 633 and `specsBaseDirForWatcher` at line 712 both call `getSpecsDir(project.autoBuildPath)` with the same argument, and both are in the same handler scope.
Resolution note: Line 633: `const specsBaseDir = getSpecsDir(project.autoBuildPath);` and line 712: `const specsBaseDirForWatcher = getSpecsDir(project.autoBuildPath);` — same pure function call, same scope, different variable names. The merge from develop did not address this.
**Suggested fix:**
Remove line 712 and reuse the existing specsBaseDir variable on line 714, e.g.:
const specDirForWatcher = worktreePath ? path.join(worktreePath, specsBaseDir, task.specId) : path.join(project.path, specsBaseDir, task.specId);
#### 🟠 [CMT-001] [HIGH] [FROM COMMENTS] sentry[bot] and AndyMik90 review both flag specFilePath using wrong path
📁 `apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts:720`
Both @sentry[bot] (bug prediction) and @AndyMik90 (CHANGES_REQUESTED review) independently flag the same critical issue: specFilePath on line 720 uses non-worktree-aware specDir while the file watcher correctly uses worktree-aware specDirForWatcher. This aligns with and corroborates finding 0bebf42bdb23.
---
*This review was generated by Auto Claude.*
Base Branch
developbranch (required for all feature/fix PRs)main(hotfix only - maintainers)Description
Related Issue
Closes #
Type of Change
Area
Commit Message Format
Follow conventional commits:
<type>: <subject>Types: feat, fix, docs, style, refactor, test, chore
Example:
feat: add user authentication systemAI Disclosure
Tool(s) used:
Testing level:
Untested -- AI output not yet verified
Lightly tested -- ran the app / spot-checked key paths
Fully tested -- all tests pass, manually verified behavior
I understand what this PR does and how the underlying code works
Checklist
developbranchPlatform Testing Checklist
CRITICAL: This project supports Windows, macOS, and Linux. Platform-specific bugs are a common source of breakage.
platform/module instead of directprocess.platformchecksfindExecutable()or platform abstractions)If you only have access to one OS: CI now tests on all platforms. Ensure all checks pass before submitting.
CI/Testing Requirements
Screenshots
Feature Toggle
use_feature_nameBreaking Changes
Breaking: Yes / No
Details:
Summary by CodeRabbit