feat: add real-time task sync for external changes #544
Conversation
Watch the specs directory for changes made outside the app (CLI, AI agents, etc.) and update task views in real-time without requiring manual refresh. Features: - File watcher using chokidar monitors specs directory - Surgical Zustand store updates (no UI flash) - Debounced updates (400ms) to handle rapid changes - Opt-in via Settings > Agent Settings > "Auto-Sync External Changes" - Detects: new tasks, removed tasks, and task modifications Implementation: - Extended FileWatcher class with specs directory watching - Added IPC channels for spec events (added/removed/updated) - Added TASK_GET handler for fetching individual tasks - Surgical store handlers in task-store.ts - Settings toggle with dynamic watcher start/stop 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds an opt-in filesystem watcher for project spec directories with debounced spec lifecycle events (added/removed/updated), IPC plumbing and Task get-by-spec APIs, renderer store handlers to surgically update tasks, plus assorted tooling refactors (Homebrew Python utility) and PR worktree support in the backend. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Settings UI
participant Main as Main Process (settings-handlers)
participant FileWatcher as FileWatcher (main)
participant FS as File System
participant IPCMain as IPC (main)
participant Preload as Preload API
participant Renderer as Renderer
participant TaskStore as Task Store
participant ProjectStore as ProjectStore
UI->>Main: SETTINGS_SAVE(toggle watchFilesystemForExternalChanges)
alt enable
Main->>FileWatcher: watchSpecsDirectory(projectId, specsPath)
FileWatcher->>FS: start watching spec dirs & key JSON files
else disable
Main->>FileWatcher: unwatchAllSpecsDirectories()
end
Note over FS,FileWatcher: External file change (e.g., task_metadata.json)
FS->>FileWatcher: file event
FileWatcher->>FileWatcher: debounce 400ms, classify event
FileWatcher->>IPCMain: emit spec-added/spec-removed/spec-updated
IPCMain->>Preload: forward TASK_SPEC_* events
Preload->>Renderer: invoke onSpec* callbacks
Renderer->>TaskStore: handleSpecAdded/Removed/Updated
alt need spec data
TaskStore->>IPCMain: TASK_GET(projectId, specId)
IPCMain->>ProjectStore: ProjectStore.getTask(projectId, specId)
ProjectStore-->>IPCMain: Task|null
IPCMain-->>TaskStore: Task|null
TaskStore->>Renderer: update UI state
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Thanks for your first PR!
A maintainer will review it soon. Please make sure:
- Your branch is synced with
develop - CI checks pass
- You've followed our contribution guide
Welcome to the Auto Claude community!
Summary of ChangesHello @greghughespdx, 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 introduces a significant enhancement to the application's task management by enabling real-time synchronization of tasks. Previously, users had to manually refresh the application to see changes made to task files by external tools like the CLI or AI agents. This PR addresses that by integrating a robust file watcher that monitors the 'specs' directory. When changes are detected—whether a new task is added, an existing one is removed, or its metadata is updated—the application's UI is automatically and efficiently refreshed. The implementation uses debouncing to consolidate rapid file system events and ensures a seamless user experience through surgical updates to the task store. This feature is configurable and can be toggled on or off via a new setting in the application. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a real-time task synchronization feature, which is a great enhancement for user experience when working with external tools. The implementation is well-structured, using chokidar for file watching, IPC for communication, and surgical Zustand store updates. The feature is also thoughtfully made opt-in. My review includes a couple of suggestions to improve performance and maintainability. One is a high-severity suggestion to optimize how individual tasks are fetched to avoid loading all tasks on every update. The other is a medium-severity suggestion to refactor some duplicated code in the file watcher for better maintainability. Overall, this is a solid contribution.
| const tasks = projectStore.getTasks(projectId); | ||
| const task = tasks.find(t => t.specId === specId); |
There was a problem hiding this comment.
The TASK_GET handler currently loads all tasks for a project using projectStore.getTasks(projectId) and then filters them. This is inefficient, especially for projects with a large number of tasks, as it reads all task files from disk for every single task update event. To improve performance, I recommend creating a more targeted method in projectStore, like getTask(projectId, specId), which would load only the necessary files for the specified task.
| watcher.on('change', (filePath: string) => { | ||
| const fileName = path.basename(filePath); | ||
| // Only watch for our target files | ||
| if (fileName !== 'implementation_plan.json' && fileName !== 'task_metadata.json') { | ||
| return; | ||
| } | ||
|
|
||
| // Extract specId from path: specsPath/specId/filename.json | ||
| const relativePath = path.relative(specsPath, filePath); | ||
| const parts = relativePath.split(path.sep); | ||
| if (parts.length !== 2) { | ||
| return; // Not a file directly inside a spec folder | ||
| } | ||
|
|
||
| const specId = parts[0]; | ||
| this.debouncedSpecUpdate(projectId, specId); | ||
| }); | ||
|
|
||
| // Also handle file additions (new JSON files in existing spec folders) | ||
| watcher.on('add', (filePath: string) => { | ||
| const fileName = path.basename(filePath); | ||
| if (fileName !== 'implementation_plan.json' && fileName !== 'task_metadata.json') { | ||
| return; | ||
| } | ||
|
|
||
| const relativePath = path.relative(specsPath, filePath); | ||
| const parts = relativePath.split(path.sep); | ||
| if (parts.length !== 2) { | ||
| return; | ||
| } | ||
|
|
||
| const specId = parts[0]; | ||
| this.debouncedSpecUpdate(projectId, specId); | ||
| }); |
There was a problem hiding this comment.
The logic for handling change and add events for files is duplicated. This could be refactored into a single helper function to improve maintainability and reduce code repetition. A single function can handle both events, making the code cleaner and easier to manage.
const handleFileEvent = (filePath: string) => {
const fileName = path.basename(filePath);
// Only watch for our target files
if (fileName !== 'implementation_plan.json' && fileName !== 'task_metadata.json') {
return;
}
// Extract specId from path: specsPath/specId/filename.json
const relativePath = path.relative(specsPath, filePath);
const parts = relativePath.split(path.sep);
if (parts.length !== 2) {
return; // Not a file directly inside a spec folder
}
const specId = parts[0];
this.debouncedSpecUpdate(projectId, specId);
};
watcher.on('change', handleFileEvent);
watcher.on('add', handleFileEvent);![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/frontend/package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.json
📒 Files selected for processing (14)
apps/frontend/src/main/file-watcher.tsapps/frontend/src/main/ipc-handlers/agent-events-handlers.tsapps/frontend/src/main/ipc-handlers/settings-handlers.tsapps/frontend/src/main/ipc-handlers/task/crud-handlers.tsapps/frontend/src/preload/api/task-api.tsapps/frontend/src/renderer/components/settings/AdvancedSettings.tsxapps/frontend/src/renderer/components/settings/GeneralSettings.tsxapps/frontend/src/renderer/hooks/useIpc.tsapps/frontend/src/renderer/stores/task-store.tsapps/frontend/src/shared/constants/config.tsapps/frontend/src/shared/constants/ipc.tsapps/frontend/src/shared/i18n/locales/en/settings.jsonapps/frontend/src/shared/types/ipc.tsapps/frontend/src/shared/types/settings.ts
🧰 Additional context used
📓 Path-based instructions (4)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/main/ipc-handlers/settings-handlers.tsapps/frontend/src/renderer/hooks/useIpc.tsapps/frontend/src/shared/types/settings.tsapps/frontend/src/renderer/components/settings/AdvancedSettings.tsxapps/frontend/src/shared/types/ipc.tsapps/frontend/src/main/ipc-handlers/task/crud-handlers.tsapps/frontend/src/renderer/stores/task-store.tsapps/frontend/src/preload/api/task-api.tsapps/frontend/src/main/file-watcher.tsapps/frontend/src/shared/constants/config.tsapps/frontend/src/main/ipc-handlers/agent-events-handlers.tsapps/frontend/src/shared/constants/ipc.tsapps/frontend/src/renderer/components/settings/GeneralSettings.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/main/ipc-handlers/settings-handlers.tsapps/frontend/src/renderer/hooks/useIpc.tsapps/frontend/src/shared/types/settings.tsapps/frontend/src/renderer/components/settings/AdvancedSettings.tsxapps/frontend/src/shared/types/ipc.tsapps/frontend/src/main/ipc-handlers/task/crud-handlers.tsapps/frontend/src/renderer/stores/task-store.tsapps/frontend/src/preload/api/task-api.tsapps/frontend/src/main/file-watcher.tsapps/frontend/src/shared/constants/config.tsapps/frontend/src/main/ipc-handlers/agent-events-handlers.tsapps/frontend/src/shared/constants/ipc.tsapps/frontend/src/renderer/components/settings/GeneralSettings.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/main/ipc-handlers/settings-handlers.tsapps/frontend/src/renderer/hooks/useIpc.tsapps/frontend/src/shared/types/settings.tsapps/frontend/src/renderer/components/settings/AdvancedSettings.tsxapps/frontend/src/shared/types/ipc.tsapps/frontend/src/main/ipc-handlers/task/crud-handlers.tsapps/frontend/src/renderer/stores/task-store.tsapps/frontend/src/preload/api/task-api.tsapps/frontend/src/main/file-watcher.tsapps/frontend/src/shared/constants/config.tsapps/frontend/src/main/ipc-handlers/agent-events-handlers.tsapps/frontend/src/shared/constants/ipc.tsapps/frontend/src/renderer/components/settings/GeneralSettings.tsx
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/settings.json
🧠 Learnings (2)
📚 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/settings.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/settings.json
🧬 Code graph analysis (6)
apps/frontend/src/main/ipc-handlers/settings-handlers.ts (3)
apps/frontend/src/main/project-store.ts (1)
projectStore(785-785)apps/frontend/src/main/file-watcher.ts (1)
fileWatcher(303-303)apps/frontend/src/shared/constants/config.ts (1)
getSpecsDir(106-109)
apps/frontend/src/renderer/hooks/useIpc.ts (1)
apps/frontend/src/renderer/stores/task-store.ts (3)
handleSpecAdded(560-585)handleSpecRemoved(591-596)handleSpecUpdated(602-625)
apps/frontend/src/main/ipc-handlers/task/crud-handlers.ts (6)
apps/frontend/src/main/settings-utils.ts (1)
readSettingsFile(29-43)apps/frontend/src/shared/types/settings.ts (1)
AppSettings(217-270)apps/frontend/src/shared/constants/config.ts (1)
DEFAULT_APP_SETTINGS(19-54)apps/frontend/src/main/project-store.ts (1)
projectStore(785-785)apps/frontend/src/main/file-watcher.ts (1)
fileWatcher(303-303)apps/frontend/src/shared/constants/ipc.ts (1)
IPC_CHANNELS(6-488)
apps/frontend/src/preload/api/task-api.ts (2)
apps/frontend/src/shared/types/task.ts (1)
Task(237-257)apps/frontend/src/shared/constants/ipc.ts (1)
IPC_CHANNELS(6-488)
apps/frontend/src/main/ipc-handlers/agent-events-handlers.ts (1)
apps/frontend/src/main/file-watcher.ts (1)
fileWatcher(303-303)
apps/frontend/src/renderer/components/settings/GeneralSettings.tsx (1)
apps/frontend/src/renderer/components/settings/utils/hookProxyFactory.ts (1)
settings(15-15)
🔇 Additional comments (27)
apps/frontend/src/shared/types/settings.ts (1)
268-269: LGTM! Well-defined optional property.The new
watchFilesystemForExternalChangesproperty is appropriately typed as an optional boolean, which aligns with the opt-in nature of this feature. The descriptive comment clearly documents its purpose.apps/frontend/src/renderer/components/settings/GeneralSettings.tsx (1)
166-182: LGTM! Excellent adherence to React and i18n patterns.This implementation correctly follows established patterns:
- Uses
useTranslation()hook with proper translation keys (no hardcoded strings)- Properly binds to
settings.watchFilesystemForExternalChangeswith safe?? falsedefault- Matches the structure and style of adjacent settings (e.g.,
autoNameTerminals)- Correctly propagates changes via
onSettingsChangeAs per coding guidelines for i18n usage and React patterns.
apps/frontend/src/main/ipc-handlers/agent-events-handlers.ts (1)
203-230: LGTM! Event handlers follow established patterns and the complete event flow is properly implemented.The three new spec lifecycle event handlers are well-implemented:
- Consistent with existing event handler patterns in this file
- Proper null-safety checks for
mainWindow- Helpful console logging for debugging
- Clear section comment for code organization
The event signatures are logical and correctly mapped:
spec-addedincludesspecPath(useful for new specs) → forwarded viaTASK_SPEC_ADDEDspec-removedandspec-updatedonly needprojectIdandspecId→ forwarded viaTASK_SPEC_REMOVEDandTASK_SPEC_UPDATEDThe complete event flow is properly implemented: fileWatcher emits events → agent-events-handlers forwards via IPC → preload API listeners in task-api.ts → task-store processes renderer-side updates.
apps/frontend/src/shared/constants/config.ts (1)
51-53: LGTM!The new
watchFilesystemForExternalChangessetting is correctly added with a sensible default offalsefor this opt-in feature. The inline comment clearly documents the purpose.apps/frontend/src/main/ipc-handlers/settings-handlers.ts (2)
6-6: LGTM!Import additions are correct and necessary for the new filesystem watching functionality.
17-18: LGTM!Correct imports for
fileWatchersingleton andprojectStoreto enable watcher lifecycle management.apps/frontend/src/renderer/hooks/useIpc.ts (3)
2-2: LGTM!Correct import of the spec event handlers from task-store.
169-189: LGTM!The spec event listeners follow the established pattern in this file. The async handlers (
handleSpecAdded,handleSpecUpdated) are intentionally called without await for fire-and-forget behavior, which is appropriate here since error handling is done within the handlers themselves.The unused
_specPathparameter is correctly prefixed with underscore.
204-206: LGTM!Cleanup functions properly added to prevent memory leaks when the component unmounts.
apps/frontend/src/shared/types/ipc.ts (2)
150-150: LGTM!The
getTaskmethod signature correctly returnsIPCResult<Task | null>to handle both found and not-found cases.
666-675: LGTM!The spec event listener types are well-defined and follow the established pattern for IPC event listeners in this file. The cleanup function return type
() => voidis consistent with other listeners.apps/frontend/src/shared/constants/ipc.ts (1)
51-56: LGTM!New IPC channel constants are well-organized and follow the existing naming conventions. The placement in the Task events section with a clear subsection comment for single task operations improves code organization.
apps/frontend/src/main/ipc-handlers/task/crud-handlers.ts (3)
2-3: LGTM!Necessary imports for the new functionality including
DEFAULT_APP_SETTINGS,AppSettings, andgetSpecsDir.
10-11: LGTM!Imports for
fileWatchersingleton andreadSettingsFileutility.
55-76: LGTM!The
TASK_GEThandler is clean and follows the established pattern. Returning{ success: true, data: null }for not-found cases is appropriate since a missing task isn't an error condition in this context (the spec may have been deleted).apps/frontend/src/renderer/stores/task-store.ts (5)
22-22: LGTM!New action added to the
TaskStateinterface for spec-based task removal.
167-179: LGTM!Clean implementation that correctly:
- Returns early if task not found (no-op)
- Clears
selectedTaskIdwhen the removed task was selected- Uses immutable filter pattern consistent with the store
552-585: Well-designed surgical update with appropriate fallback.The
handleSpecAddedimplementation correctly:
- Attempts a surgical fetch of just the new task
- Checks for duplicates before adding
- Updates existing task if already present (handles race conditions)
- Falls back to full reload only on error
This is a robust pattern for handling external file changes.
587-596: LGTM!Simple and correct. The unused
projectIdparameter maintains API consistency with other handlers and enables future enhancements (e.g., project-scoped validation).
598-625: LGTM!The
handleSpecUpdatedimplementation correctly:
- Skips processing if task isn't in the store (avoids unnecessary IPC)
- Performs surgical update without full reload
- Doesn't need a fallback since we're only updating existing tasks
The lack of fallback (unlike
handleSpecAdded) is appropriate here since update failures for existing tasks are less critical than missing new tasks entirely.apps/frontend/src/main/file-watcher.ts (4)
13-24: LGTM!The new interfaces are well-structured for tracking per-project specs watchers and debounce state.
32-34: LGTM!The new class properties are appropriate. The 400ms debounce is a reasonable choice for handling rapid filesystem changes.
146-244: LGTM!The
watchSpecsDirectoryimplementation is well-designed:
- Proper depth limiting (
depth: 1) ensures only spec folders and their direct children are watched.- Immediate emission for directory add/remove events, with debouncing for file changes, is the correct approach.
- Path validation correctly filters for direct children only.
250-267: LGTM!The debounce implementation is correct with proper timeout cleanup and composite key for per-spec deduplication.
apps/frontend/src/preload/api/task-api.ts (3)
17-82: LGTM!The interface additions are well-typed and consistent with existing patterns:
getTaskcorrectly returnsIPCResult<Task | null>for when a task may not exist.- Spec event listeners follow the established pattern of returning cleanup functions.
89-91: LGTM!The
getTaskimplementation is straightforward and follows the existing pattern for IPC invoke calls.
292-342: LGTM!The spec event listener implementations are well-structured:
- Follow the established pattern from existing event listeners in this file.
- Properly return cleanup functions using
removeListenerto prevent memory leaks.- Handler signatures correctly match the expected IPC event parameters.
| // Handle filesystem watcher setting change | ||
| if (settings.watchFilesystemForExternalChanges !== undefined) { | ||
| const wasEnabled = currentSettings.watchFilesystemForExternalChanges ?? false; | ||
| const isNowEnabled = settings.watchFilesystemForExternalChanges; | ||
|
|
||
| if (!wasEnabled && isNowEnabled) { | ||
| // Setting was turned ON - start watching all active projects | ||
| console.log('[SETTINGS_SAVE] Filesystem watching enabled, starting watchers for all projects'); | ||
| const projects = projectStore.getProjects(); | ||
| for (const project of projects) { | ||
| if (!fileWatcher.isWatchingSpecs(project.id)) { | ||
| const specsBaseDir = getSpecsDir(project.autoBuildPath); | ||
| const specsPath = path.join(project.path, specsBaseDir); | ||
| fileWatcher.watchSpecsDirectory(project.id, specsPath).catch((err) => { | ||
| console.error(`[SETTINGS_SAVE] Failed to start specs watcher for project ${project.id}:`, err); | ||
| }); | ||
| } | ||
| } | ||
| } else if (wasEnabled && !isNowEnabled) { | ||
| // Setting was turned OFF - stop all watchers | ||
| console.log('[SETTINGS_SAVE] Filesystem watching disabled, stopping all watchers'); | ||
| fileWatcher.unwatchAllSpecsDirectories().catch((err) => { | ||
| console.error('[SETTINGS_SAVE] Failed to stop specs watchers:', err); | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Implementation is correct, but consider aggregating errors for user feedback.
The logic correctly handles the ON/OFF transitions for the filesystem watching feature. However, when enabling watchers fails for some projects, errors are only logged to console. Consider aggregating failures and including them in the response so the UI can inform the user which projects failed to start watching.
Current approach is acceptable for an opt-in feature, but worth considering for improved UX.
🤖 Prompt for AI Agents
In apps/frontend/src/main/ipc-handlers/settings-handlers.ts around lines
204-229, when enabling filesystem watchers failures are only logged to console;
collect any errors encountered while starting per-project watchers into an array
(including project id and error message), await all watch attempts (e.g.,
Promise.allSettled or sequential try/catch), and add that aggregated error list
to the handler's response payload so the UI can surface which projects failed to
start watching; do the same for unwatchAllSpecsDirectories failures (include its
error in the response) and ensure the saved-settings result schema is extended
to carry optional watcherErrors.
| /** | ||
| * Check if filesystem watching is enabled in settings | ||
| */ | ||
| function isFilesystemWatchingEnabled(): boolean { | ||
| const savedSettings = readSettingsFile() as Partial<AppSettings> | undefined; | ||
| const settings = { ...DEFAULT_APP_SETTINGS, ...savedSettings }; | ||
| return settings.watchFilesystemForExternalChanges ?? false; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider caching or memoizing the settings read.
The helper reads settings from disk on every call. While this isn't a hot path (called only during TASK_LIST), consider whether the settings could be cached or accessed via a centralized settings manager to avoid repeated disk I/O.
Current implementation is functionally correct and acceptable for the use case.
🤖 Prompt for AI Agents
In apps/frontend/src/main/ipc-handlers/task/crud-handlers.ts around lines 13 to
20, the helper currently reads the settings file on every call which causes
repeated disk I/O; change it to read once and cache the parsed settings in
memory (e.g., a module-scoped variable) and return the cached value on
subsequent calls, and ensure you provide a means to invalidate or refresh the
cache when settings are updated (or alternatively fetch from the app’s
centralized settings manager if available) so the cached settings remain
consistent.
|
|
||
| // Start watching the specs directory for this project (if enabled in settings) | ||
| const project = projectStore.getProject(projectId); | ||
| if (project && isFilesystemWatchingEnabled()) { | ||
| const specsBaseDir = getSpecsDir(project.autoBuildPath); | ||
| const specsPath = path.join(project.path, specsBaseDir); | ||
|
|
||
| // Only start watching if not already watching | ||
| if (!fileWatcher.isWatchingSpecs(projectId)) { | ||
| console.log(`[IPC] TASK_LIST starting specs watcher for project ${projectId}`); | ||
| fileWatcher.watchSpecsDirectory(projectId, specsPath).catch((err) => { | ||
| console.error(`[IPC] Failed to start specs watcher for project ${projectId}:`, err); | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Side effect in TASK_LIST is acceptable but worth documenting.
Initializing the specs watcher as a side effect of TASK_LIST is a pragmatic approach that ensures watching starts when the user views tasks. The guard conditions (isFilesystemWatchingEnabled() and !fileWatcher.isWatchingSpecs(projectId)) prevent unnecessary operations.
Consider adding a brief comment explaining why the watcher is started here (e.g., "// Lazily start watcher when tasks are first viewed").
🤖 Prompt for AI Agents
In apps/frontend/src/main/ipc-handlers/task/crud-handlers.ts around lines 35 to
49, add a brief explanatory comment above the block that starts the specs
watcher to document the intentional side effect: state that the watcher is
started lazily when TASK_LIST runs so watchers are only created when the user
views tasks (and that existing guards prevent duplicate or unwanted watching).
Keep the comment short, reference the guards isFilesystemWatchingEnabled() and
fileWatcher.isWatchingSpecs(projectId), and mention it’s deliberate to avoid
moving logic elsewhere.
| "watchFilesystem": "Auto-Sync External Changes", | ||
| "watchFilesystemDescription": "Automatically update task views when tasks are created or modified outside the app (e.g., via CLI or AI agents)" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if French translations exist for watchFilesystem keys
if [ -f "apps/frontend/src/shared/i18n/locales/fr/settings.json" ]; then
echo "French settings file found. Checking for watchFilesystem keys..."
cat apps/frontend/src/shared/i18n/locales/fr/settings.json | grep -A 1 "watchFilesystem"
echo ""
echo "Full context around line 103-104 in French file:"
# Get approximate line count to find similar location
wc -l apps/frontend/src/shared/i18n/locales/fr/settings.json
else
echo "French settings file not found"
exit 1
fiRepository: AndyMik90/Auto-Claude
Length of output: 238
🏁 Script executed:
#!/bin/bash
# Check English settings file for the keys
sed -n '100,110p' apps/frontend/src/shared/i18n/locales/en/settings.jsonRepository: AndyMik90/Auto-Claude
Length of output: 680
Add French translations for the new watch filesystem keys.
The watchFilesystem and watchFilesystemDescription keys are missing from apps/frontend/src/shared/i18n/locales/fr/settings.json. Per the coding guidelines, translation keys must be added to all supported language files (minimum: en/.json and fr/.json).
🤖 Prompt for AI Agents
In apps/frontend/src/shared/i18n/locales/fr/settings.json around where lines
103–104 map to in en, the keys "watchFilesystem" and
"watchFilesystemDescription" are missing; add them with French translations,
e.g. "watchFilesystem": "Synchronisation automatique des changements externes"
and "watchFilesystemDescription": "Met à jour automatiquement les vues de tâches
lorsque des tâches sont créées ou modifiées en dehors de l'application (par ex.
via CLI ou agents IA)", and ensure proper JSON syntax (quotes, commas) to match
surrounding entries.
- Extract duplicate file event handlers into handleSpecFileEvent helper - Add optimized getTask() method to ProjectStore for single-task loading - Add loadTaskFromSpecFolder() private method to avoid loading all tasks - Update TASK_GET handler to use efficient single-task loader 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
MikeeBuilds
added
feature
Review Feedback AcknowledgedThanks for the review suggestions! Key points noted: High Priority
Medium Priority
Low Priority
These are valid improvements for a follow-up PR. The current implementation is functional for the MVP. |
|
) * Enhance Python detection to find versioned Homebrew installations Fixes issue where users with Python 3.9.6 at /usr/bin/python3 would get "Auto Claude requires Python 3.10 or higher" error even when they had newer Python versions installed via Homebrew. Changes: - Updated findHomebrewPython() to check for versioned Python installations - Now searches for python3.13, python3.12, python3.11, python3.10 in addition to generic python3 - Validates each found Python to ensure it meets version requirements - Checks both Apple Silicon (/opt/homebrew/bin) and Intel Mac (/usr/local/bin) locations This ensures the app automatically finds and uses the latest compatible Python version instead of falling back to the potentially outdated system Python. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]> * Update apps/frontend/src/main/python-detector.ts Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Address PR review findings for Python detection enhancement Addresses all review findings from Auto Claude PR Review: 1. [HIGH] Align version ordering between python-detector.ts and cli-tool-manager.ts - Both now use consistent order: versioned first (3.13→3.10), then generic python3 - This ensures different parts of the app use the same Python version - Added validation in cli-tool-manager.ts (was missing before) 2. [MEDIUM] Add try/catch around validatePythonVersion calls - Wrapped validation in try/catch to handle timeouts and permission errors - Follows same pattern as findPythonCommand() - Ensures graceful fallback to next candidate on validation failure 3. [LOW] Add debug logging for Python detection - Added console.log for successful detection with version info - Added console.warn for rejected candidates with reason - Added logging when no valid Python found - Improves troubleshooting of user Python detection issues 4. [LOW] Document maintenance requirement for version list - Added JSDoc note about updating list for new Python releases - Added TODO comment for Python 3.14+ updates - Applied to both files for consistency Additional improvements: - Fixed bug in cli-tool-manager.ts that returned first found Python without validation - Both detection systems now validate Python version requirements (3.10+) - Consistent logging format between both detection systems 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]> * Add Python 3.14 support to version detection Python 3.14 was released, so adding it to the detection lists: - Updated pythonNames arrays in both python-detector.ts and cli-tool-manager.ts - Added python3.14 to SAFE_PYTHON_COMMANDS set - Updated JSDoc comments to reflect Python 3.14 support - Removed TODO about Python 3.14 (now implemented) This ensures the app can detect and use Python 3.14 installations. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]> * Refactor: Extract shared Homebrew Python detection logic Eliminated code duplication by extracting shared Python detection logic into a reusable utility module. Changes: - Created apps/frontend/src/main/utils/homebrew-python.ts - Exported findHomebrewPython() utility function - Accepts validation function and log prefix as parameters - Contains all shared detection logic (version list, validation, logging) - Updated python-detector.ts - Removed duplicate findHomebrewPython() implementation (45 lines) - Now imports and delegates to shared utility - Maintains identical behavior and error semantics - Updated cli-tool-manager.ts - Removed duplicate findHomebrewPython() implementation (52 lines) - Now imports and delegates to shared utility - Maintains identical behavior and error semantics Benefits: - Single source of truth for Homebrew Python detection - Easier to maintain (update version list in one place) - Consistent behavior across the application - Reduced code duplication (~90 lines eliminated) The refactored code maintains 100% backward compatibility with identical return values, logging behavior, and error handling. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]> --------- Co-authored-by: Claude Sonnet 4.5 <[email protected]> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Andy <[email protected]>
…ik90#532) PR review agents were reading files from the current checkout branch (e.g., develop) instead of the actual PR branch when using Read/Grep/Glob tools. This caused incorrect review findings. The fix creates a temporary detached worktree at the PR head commit for each review, ensuring agents read from the correct branch state: - Add head_sha/base_sha fields to PRContext dataclass - Create worktree at PR commit before spawning specialist agents - Use worktree path as project_dir for SDK client - Cleanup worktree after review with fallback chain - Add startup cleanup for orphaned worktrees from crashed runs Worktrees are stored in .auto-claude/pr-review-worktrees/ (already gitignored) to avoid /tmp filesystem boundary issues and support concurrent reviews. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.5 <[email protected]>
Addresses PR feedback - prevents memory leaks by clearing pending setTimeout callbacks when unwatchSpecsDirectory() or unwatchAllSpecsDirectories() is called. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
apps/backend/runners/github/context_gatherer.pyapps/backend/runners/github/services/parallel_orchestrator_reviewer.pyapps/frontend/src/main/cli-tool-manager.tsapps/frontend/src/main/file-watcher.tsapps/frontend/src/main/python-detector.tsapps/frontend/src/main/utils/homebrew-python.ts
🧰 Additional context used
📓 Path-based instructions (4)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/main/python-detector.tsapps/frontend/src/main/utils/homebrew-python.tsapps/frontend/src/main/cli-tool-manager.tsapps/frontend/src/main/file-watcher.ts
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/main/python-detector.tsapps/frontend/src/main/utils/homebrew-python.tsapps/frontend/src/main/cli-tool-manager.tsapps/frontend/src/main/file-watcher.ts
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/main/python-detector.tsapps/frontend/src/main/utils/homebrew-python.tsapps/frontend/src/main/cli-tool-manager.tsapps/frontend/src/main/file-watcher.ts
apps/backend/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
apps/backend/**/*.py: Always use the Claude Agent SDK (claude-agent-sdkpackage) for all AI interactions, never use the Anthropic API directly
Use thecreate_client()function fromapps/backend/core/client.pyto instantiate Claude SDK clients, not directClaudeSDKClientinitialization
Files:
apps/backend/runners/github/context_gatherer.pyapps/backend/runners/github/services/parallel_orchestrator_reviewer.py
⚙️ CodeRabbit configuration file
apps/backend/**/*.py: Focus on Python best practices, type hints, and async patterns.
Check for proper error handling and security considerations.
Verify compatibility with Python 3.12+.
Files:
apps/backend/runners/github/context_gatherer.pyapps/backend/runners/github/services/parallel_orchestrator_reviewer.py
🧠 Learnings (1)
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Use git worktrees for isolated agent builds with branches following the naming pattern `auto-claude/{spec-name}`
Applied to files:
apps/backend/runners/github/services/parallel_orchestrator_reviewer.py
🧬 Code graph analysis (2)
apps/frontend/src/main/python-detector.ts (2)
apps/frontend/src/main/cli-tool-manager.ts (1)
findHomebrewPython(739-744)apps/frontend/src/main/utils/homebrew-python.ts (1)
findHomebrewPython(33-77)
apps/frontend/src/main/utils/homebrew-python.ts (1)
apps/frontend/src/main/cli-tool-manager.ts (1)
findHomebrewPython(739-744)
🔇 Additional comments (18)
apps/backend/runners/github/context_gatherer.py (2)
204-206: LGTM! Clean dataclass field additions.The new
head_shaandbase_shafields are well-designed:
- Proper type hints with sensible defaults (empty strings)
- Clear documentation explaining their purpose (worktree creation for PR review isolation)
- Non-breaking change due to default values
- Consistent naming convention
297-298: LGTM! Safe field population.The new fields are populated correctly:
- Uses
.get()with appropriate fallback to empty strings- Keys (
headRefOid,baseRefOid) are already included in the metadata fetch (lines 312-313)- Consistent with the existing pattern for extracting PR metadata
- SHAs are validated before use in git commands elsewhere (lines 338-347)
apps/frontend/src/main/python-detector.ts (2)
5-5: LGTM! Clean delegation to shared utility.The refactoring successfully centralizes Homebrew Python detection logic while maintaining the existing function signature. The import alias prevents naming conflicts, and the delegation properly passes the validation callback and log prefix.
Also applies to: 38-43
301-310: Python 3.14 is already released and available. Python 3.14.0 was released on October 7, 2025, with bugfix releases (3.14.1, 3.14.2) already available. The addition ofpython3.14to the safe commands list is justified and correct. The comment reminder to update when new Python versions are released is helpful for future maintenance.apps/frontend/src/main/cli-tool-manager.ts (1)
30-30: LGTM! Consistent delegation pattern.The refactoring mirrors the approach in
python-detector.ts, properly delegating to the shared utility. The arrow function correctly preserves thethiscontext for thevalidatePythoninstance method, and the log prefix appropriately identifies the source module.Also applies to: 735-743
apps/frontend/src/main/utils/homebrew-python.ts (2)
15-19: LGTM! Well-defined validation interface.The
PythonValidationinterface provides a clear contract for validation results. The optionalversionfield and requiredmessagefield enable both success and failure scenarios to be communicated effectively.
33-77: LGTM! Solid implementation with good error handling.The function implements a sensible detection strategy:
- Prioritizes newer Python versions for better compatibility
- Checks architecture-specific Homebrew paths (Apple Silicon and Intel)
- Validates each candidate before returning
- Handles errors gracefully without blocking other candidates
- Provides comprehensive logging for debugging
The same verification about Python 3.14 availability applies here as in the other files.
apps/backend/runners/github/services/parallel_orchestrator_reviewer.py (3)
23-25: LGTM: Clean imports and constant definition.The new imports (shutil, subprocess, uuid) are appropriate for worktree management, and the project-local worktree directory follows good conventions.
Also applies to: 66-67
166-197: LGTM: Robust cleanup with fallback chain.The cleanup implementation correctly handles edge cases with defensive checks, git worktree remove as primary method, shutil.rmtree fallback, and appropriate error logging without raising exceptions (suitable for cleanup paths).
707-710: LGTM: Proper cleanup in finally block.The finally block ensures worktree cleanup occurs even on errors, preventing resource leaks. This aligns with the PR objectives to address memory leak concerns.
apps/frontend/src/main/file-watcher.ts (8)
13-24: LGTM! Well-structured interfaces.The new interfaces are cleanly designed and properly typed to support specs directory watching and debounced updates.
28-34: LGTM! Clean state management.The state management additions are well-organized, properly scoped, and the 400ms debounce window is appropriate for coalescing rapid file system events.
203-224: Excellent refactoring! Duplication issue resolved.The
handleSpecFileEventhelper function elegantly addresses the previous review feedback about duplicated logic for handlingchangeandaddevents. The implementation is clean and maintainable.
237-254: LGTM! Correct debounce implementation.The debounce logic properly clears existing timeouts before scheduling new ones, preventing memory leaks and ensuring only the last update fires after the debounce window.
259-268: Excellent fix! Memory leak concern resolved.The method now properly clears pending debounced updates before closing the watcher (line 264), completely addressing the previous review feedback about potential memory leaks from pending
spec-updatedevents.
273-280: LGTM! Clean helper implementation.The helper method correctly clears all pending debounced updates for a specific project, ensuring no dangling timers remain after unwatching.
285-287: LGTM!Simple and correct query method consistent with the existing
isWatchingpattern.
292-306: Excellent implementation! Bulk cleanup properly handled.The method correctly clears all pending debounced updates before closing watchers (lines 293-297), fully addressing the previous review feedback. The parallel watcher closure with
Promise.allis efficient.
| subprocess.run( | ||
| ["git", "fetch", "origin", head_sha], | ||
| cwd=self.project_dir, | ||
| capture_output=True, | ||
| timeout=60, | ||
| ) |
There was a problem hiding this comment.
Add error handling for git fetch command.
The git fetch operation doesn't verify success. If the fetch fails (network issues, invalid SHA, permissions), the subsequent git worktree add will fail with a less informative error message.
🔎 Proposed fix
- subprocess.run(
+ fetch_result = subprocess.run(
["git", "fetch", "origin", head_sha],
cwd=self.project_dir,
capture_output=True,
+ text=True,
timeout=60,
)
+
+ if fetch_result.returncode != 0:
+ raise RuntimeError(
+ f"Failed to fetch commit {head_sha}: {fetch_result.stderr}"
+ )🤖 Prompt for AI Agents
In apps/backend/runners/github/services/parallel_orchestrator_reviewer.py around
lines 144 to 149, the git fetch subprocess call does not check for failure;
change the invocation to detect errors (either pass check=True or inspect the
CompletedProcess.returncode), capture and log stderr/stdout on failure, and
raise or return a clear exception so callers get a meaningful message (include
head_sha and project_dir in the log). Ensure timeout remains and include the
subprocess output in the logged/raised error to aid debugging.
| result = subprocess.run( | ||
| ["git", "worktree", "add", "--detach", str(worktree_path), head_sha], | ||
| cwd=self.project_dir, | ||
| capture_output=True, | ||
| text=True, | ||
| ) |
There was a problem hiding this comment.
Add timeout to git worktree add command.
The git worktree add command lacks a timeout, which could cause the review process to hang indefinitely if the operation stalls.
🔎 Proposed fix
result = subprocess.run(
["git", "worktree", "add", "--detach", str(worktree_path), head_sha],
cwd=self.project_dir,
capture_output=True,
text=True,
+ timeout=60,
)🤖 Prompt for AI Agents
In apps/backend/runners/github/services/parallel_orchestrator_reviewer.py around
lines 152 to 157, the subprocess.run call invoking "git worktree add" has no
timeout and can hang; add a timeout argument (e.g., timeout=60 or a configurable
constant), wrap the call in a try/except catching subprocess.TimeoutExpired, log
an error including the timeout and command output, and abort/return failure in
that branch so the reviewer process does not hang indefinitely.
| registered = { | ||
| Path(line.split(" ", 1)[1]) | ||
| for line in result.stdout.split("\n") | ||
| if line.startswith("worktree ") | ||
| } |
There was a problem hiding this comment.
Add defensive parsing for git worktree list output.
Line 213 assumes each line starting with "worktree " has a space and path component. If the git output is malformed or the format changes, this will raise an IndexError.
🔎 Proposed fix
registered = {
- Path(line.split(" ", 1)[1])
+ Path(parts[1])
for line in result.stdout.split("\n")
- if line.startswith("worktree ")
+ if line.startswith("worktree ") and (parts := line.split(" ", 1)) and len(parts) == 2
}🤖 Prompt for AI Agents
In apps/backend/runners/github/services/parallel_orchestrator_reviewer.py around
lines 212 to 216, the current comprehension assumes every line that starts with
"worktree " contains a space and a valid path and may raise IndexError on
malformed git output; change the parsing to be defensive by iterating lines,
trimming whitespace, skipping empty lines, verifying the line starts with
"worktree " and that splitting with maxsplit=1 yields a non-empty path component
(or use str.partition(" ") and check the third part), and only then construct
Path(path); optionally log or ignore malformed entries rather than letting an
exception propagate.
| # Create temporary worktree at PR head commit for isolated review | ||
| # This ensures agents read from the correct PR state, not the current checkout | ||
| head_sha = context.head_sha or context.head_branch | ||
| if not head_sha: | ||
| logger.warning( | ||
| "[ParallelOrchestrator] No head_sha available, using current checkout" | ||
| ) | ||
| # Fallback to original behavior if no SHA available | ||
| project_root = ( | ||
| self.project_dir.parent.parent | ||
| if self.project_dir.name == "backend" | ||
| else self.project_dir | ||
| ) | ||
| else: | ||
| try: | ||
| worktree_path = self._create_pr_worktree( | ||
| head_sha, context.pr_number | ||
| ) | ||
| project_root = worktree_path | ||
| except RuntimeError as e: | ||
| logger.warning( | ||
| f"[ParallelOrchestrator] Worktree creation failed, " | ||
| f"using current checkout: {e}" | ||
| ) | ||
| # Fallback to original behavior if worktree creation fails | ||
| project_root = ( | ||
| self.project_dir.parent.parent | ||
| if self.project_dir.name == "backend" | ||
| else self.project_dir | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Refactor duplicate project_root calculation logic.
The project_root fallback calculation appears twice (lines 543-546 and 560-563) with identical logic. This violates the DRY principle and makes the code harder to maintain.
🔎 Proposed refactor
+ # Calculate fallback project root
+ def get_fallback_project_root() -> Path:
+ return (
+ self.project_dir.parent.parent
+ if self.project_dir.name == "backend"
+ else self.project_dir
+ )
+
# Create temporary worktree at PR head commit for isolated review
# This ensures agents read from the correct PR state, not the current checkout
head_sha = context.head_sha or context.head_branch
if not head_sha:
logger.warning(
"[ParallelOrchestrator] No head_sha available, using current checkout"
)
- # Fallback to original behavior if no SHA available
- project_root = (
- self.project_dir.parent.parent
- if self.project_dir.name == "backend"
- else self.project_dir
- )
+ project_root = get_fallback_project_root()
else:
try:
worktree_path = self._create_pr_worktree(
head_sha, context.pr_number
)
project_root = worktree_path
except RuntimeError as e:
logger.warning(
f"[ParallelOrchestrator] Worktree creation failed, "
f"using current checkout: {e}"
)
- # Fallback to original behavior if worktree creation fails
- project_root = (
- self.project_dir.parent.parent
- if self.project_dir.name == "backend"
- else self.project_dir
- )
+ project_root = get_fallback_project_root()📝 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.
| # Create temporary worktree at PR head commit for isolated review | |
| # This ensures agents read from the correct PR state, not the current checkout | |
| head_sha = context.head_sha or context.head_branch | |
| if not head_sha: | |
| logger.warning( | |
| "[ParallelOrchestrator] No head_sha available, using current checkout" | |
| ) | |
| # Fallback to original behavior if no SHA available | |
| project_root = ( | |
| self.project_dir.parent.parent | |
| if self.project_dir.name == "backend" | |
| else self.project_dir | |
| ) | |
| else: | |
| try: | |
| worktree_path = self._create_pr_worktree( | |
| head_sha, context.pr_number | |
| ) | |
| project_root = worktree_path | |
| except RuntimeError as e: | |
| logger.warning( | |
| f"[ParallelOrchestrator] Worktree creation failed, " | |
| f"using current checkout: {e}" | |
| ) | |
| # Fallback to original behavior if worktree creation fails | |
| project_root = ( | |
| self.project_dir.parent.parent | |
| if self.project_dir.name == "backend" | |
| else self.project_dir | |
| ) | |
| # Calculate fallback project root | |
| def get_fallback_project_root() -> Path: | |
| return ( | |
| self.project_dir.parent.parent | |
| if self.project_dir.name == "backend" | |
| else self.project_dir | |
| ) | |
| # Create temporary worktree at PR head commit for isolated review | |
| # This ensures agents read from the correct PR state, not the current checkout | |
| head_sha = context.head_sha or context.head_branch | |
| if not head_sha: | |
| logger.warning( | |
| "[ParallelOrchestrator] No head_sha available, using current checkout" | |
| ) | |
| project_root = get_fallback_project_root() | |
| else: | |
| try: | |
| worktree_path = self._create_pr_worktree( | |
| head_sha, context.pr_number | |
| ) | |
| project_root = worktree_path | |
| except RuntimeError as e: | |
| logger.warning( | |
| f"[ParallelOrchestrator] Worktree creation failed, " | |
| f"using current checkout: {e}" | |
| ) | |
| project_root = get_fallback_project_root() |
🤖 Prompt for AI Agents
In apps/backend/runners/github/services/parallel_orchestrator_reviewer.py around
lines 535 to 564, the fallback project_root calculation is duplicated in both
the head_sha-missing branch and the worktree-creation-exception branch; extract
that fallback computation into a single variable or small helper (e.g.,
compute_fallback_project_root or fallback_project_root =
(self.project_dir.parent.parent if self.project_dir.name == "backend" else
self.project_dir)) defined before the if/else, then use that variable in both
places so the logic is only defined once and behavior remains identical.
| async watchSpecsDirectory(projectId: string, specsPath: string): Promise<void> { | ||
| // Stop any existing watcher for this project | ||
| await this.unwatchSpecsDirectory(projectId); | ||
|
|
||
| // Check if specs directory exists | ||
| if (!existsSync(specsPath)) { | ||
| console.log(`[FileWatcher] Specs directory not found, skipping watch: ${specsPath}`); | ||
| return; | ||
| } | ||
|
|
||
| console.log(`[FileWatcher] Starting specs directory watcher for project ${projectId}: ${specsPath}`); | ||
|
|
||
| // Watch for new directories and file changes inside spec folders | ||
| const watcher = chokidar.watch(specsPath, { | ||
| persistent: true, | ||
| ignoreInitial: true, | ||
| depth: 1, // Watch spec folders and their direct children (JSON files) | ||
| awaitWriteFinish: { | ||
| stabilityThreshold: 300, | ||
| pollInterval: 100 | ||
| } | ||
| }); | ||
|
|
||
| // Store watcher info | ||
| this.specsWatchers.set(projectId, { | ||
| projectId, | ||
| watcher, | ||
| specsPath | ||
| }); | ||
|
|
||
| // Handle new spec directory added | ||
| watcher.on('addDir', (dirPath: string) => { | ||
| // Only emit for direct children of specs directory | ||
| const relativePath = path.relative(specsPath, dirPath); | ||
| if (relativePath && !relativePath.includes(path.sep)) { | ||
| const specId = path.basename(dirPath); | ||
| console.log(`[FileWatcher] Spec directory added: ${specId} in project ${projectId}`); | ||
| this.emit('spec-added', projectId, specId, dirPath); | ||
| } | ||
| }); | ||
|
|
||
| // Handle spec directory removed | ||
| watcher.on('unlinkDir', (dirPath: string) => { | ||
| // Only emit for direct children of specs directory | ||
| const relativePath = path.relative(specsPath, dirPath); | ||
| if (relativePath && !relativePath.includes(path.sep)) { | ||
| const specId = path.basename(dirPath); | ||
| console.log(`[FileWatcher] Spec directory removed: ${specId} in project ${projectId}`); | ||
| this.emit('spec-removed', projectId, specId); | ||
| } | ||
| }); | ||
|
|
||
| // Helper to handle file change/add events for spec files | ||
| const handleSpecFileEvent = (filePath: string) => { | ||
| const fileName = path.basename(filePath); | ||
| // Only watch for our target files | ||
| if (fileName !== 'implementation_plan.json' && fileName !== 'task_metadata.json') { | ||
| return; | ||
| } | ||
|
|
||
| // Extract specId from path: specsPath/specId/filename.json | ||
| const relativePath = path.relative(specsPath, filePath); | ||
| const parts = relativePath.split(path.sep); | ||
| if (parts.length !== 2) { | ||
| return; // Not a file directly inside a spec folder | ||
| } | ||
|
|
||
| const specId = parts[0]; | ||
| this.debouncedSpecUpdate(projectId, specId); | ||
| }; | ||
|
|
||
| // Handle file changes and additions inside spec folders | ||
| watcher.on('change', handleSpecFileEvent); | ||
| watcher.on('add', handleSpecFileEvent); | ||
|
|
||
| // Handle errors | ||
| watcher.on('error', (error: unknown) => { | ||
| const message = error instanceof Error ? error.message : String(error); | ||
| console.error(`[FileWatcher] Specs watcher error for project ${projectId}:`, message); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider emitting error events for consistency.
The specs watching implementation is solid, but error handling differs from the existing task watching pattern. The watch() method emits 'error' events (lines 47, 83), while watchSpecsDirectory() uses console.error (line 230) without emitting events.
For consistency and to allow consuming code to handle errors uniformly, consider emitting error events for both directory-not-found (line 157) and watcher errors (line 230).
🔎 Optional: align error handling with existing pattern
// Check if specs directory exists
if (!existsSync(specsPath)) {
- console.log(`[FileWatcher] Specs directory not found, skipping watch: ${specsPath}`);
+ this.emit('specs-error', projectId, `Specs directory not found: ${specsPath}`);
return;
} // Handle errors
watcher.on('error', (error: unknown) => {
const message = error instanceof Error ? error.message : String(error);
console.error(`[FileWatcher] Specs watcher error for project ${projectId}:`, message);
+ this.emit('specs-error', projectId, message);
});🤖 Prompt for AI Agents
In apps/frontend/src/main/file-watcher.ts around lines 151 to 231, the specs
watcher currently logs errors to the console instead of emitting them like the
existing task watcher; update error handling so it emits an 'error' event for
consistency: when the specs directory does not exist (line ~157) create and emit
an Error with context (projectId and specsPath) before returning, and in the
watcher.on('error') handler (line ~230) emit the same error event (e.g.,
this.emit('error', projectId, error)) in addition to any existing console
logging so consumers can handle errors uniformly.
|
Closing to resubmit from proper feature branch (feat/real-time-task-sync) instead of develop |
6 participants
Watches the specs directory for changes made outside the app (CLI, AI agents, etc.) and smoothly update task views in real-time without requiring manual refresh.
Features:
Implementation:
🤖 Generated with Claude Code
Base Branch
developbranch (required for all feature/fix PRs)main(hotfix only - maintainers)Description
Adds real-time sync for tasks created or modified outside the app (via CLI, AI agents, etc.) so users don't need to refresh manually. Added Settings feature to allow the user to toggle the feature on or off in Settings/Agents
Related Issue
N/A
Type of Change
Area
Commit Message Format
(format was followed in commit)
Checklist
developbranchCI/Testing Requirements
Screenshots
Feature Toggle
use_feature_nameBreaking Changes
Breaking: No
This is my first ever PR - it feels good to make one - thanks for building Auto-Claude!
Summary by CodeRabbit
New Features
Settings
Behavior
Localization
✏️ Tip: You can customize this high-level summary in your review settings.