- Add includeArchived parameter to loadInsightsSession and propagate to
loadInsightsSessions, deleteSession, and clearSession
- Update all callers in Insights.tsx to pass showArchived through
- Remove projectId from showArchived effect deps to prevent duplicate
loads on project switch (mount effect already handles it)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Return success:false from IPC handlers when bulk operations have failures
- Propagate failedIds through store functions for proper error reporting
- Add session ID validation in paths.ts to prevent path traversal
- Prune selectedIds when sessions list changes to avoid stale selections
- Fix isFirstRun race condition when projectId changes in Insights
- Convert archivedAt to Date in loadSessionById for type consistency
- Fix accessibility: conditional tabIndex based on selection mode
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comprehensive fixes addressing 4 critical and accessibility issues:
**Critical Bug Fixes:**
1. Fixed UI state mismatch when active session is archived/deleted
- Added loadInsightsSession() call after bulk operations
- Ensures frontend stays in sync when backend auto-switches sessions
2. Fixed race condition causing flicker on projectId change
- Added prevProjectId ref to reset skip flag per-projectId
- Prevents duplicate loadInsightsSessions calls when showArchived is true
3. Added comprehensive error handling to all bulk handlers
- Wrapped all async operations in try/catch blocks
- Added detailed error logging with session IDs and context
- Prevents unhandled promise rejections from IPC failures
**Accessibility Improvements:**
4. Fixed selection mode accessibility in SessionItem
- Removed redundant checkbox onCheckedChange to prevent double-toggle
- Made parent row fully interactive with role="checkbox" in selection mode
- Added aria-checked attribute for screen reader support
- Wired row onClick and keyboard handlers to toggle selection
- Users can now activate selection via row click/keyboard or nested checkbox
All changes maintain backward compatibility and improve robustness.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comprehensive code quality improvements addressing all review comments:
**Critical Fixes:**
- Fixed double-toggle bug where checkbox called onToggleSelect twice
- Added accessibility attributes (role, tabIndex, onKeyDown) to interactive div
**Error Handling:**
- Removed error re-throws in async event handlers to prevent unhandled rejections
- Added error logging to empty catch blocks in session-storage.ts
- Added error handling with .catch() for dropdown menu promise rejections
- Added logging for partial failures in bulk operations
**Code Quality:**
- Replaced non-null assertion (!) with explicit null check in session-manager.ts
- Removed redundant async/await wrappers in archive/unarchive handlers
- Removed duplicate loadInsightsSession calls from store functions
- Added skip-first-run guard to prevent double load on component mount
- Added clarifying comment for showArchived useEffect dependency
**Performance:**
- Eliminated redundant IPC calls by removing duplicate session reloads
- Fixed flicker issue caused by double reload with different filters
All changes maintain backward compatibility and improve user experience.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fixed fire-and-forget async operations that were clearing UI state
before operations completed. Changes:
- Updated prop types to return Promise<void> for async callbacks
- Made handleBulkDelete and handleBulkArchive async functions
- Added await for all async callback invocations
- Added try/catch error handling with console logging
- Updated SessionItem prop types for onArchive/onUnarchive
- Made single archive/unarchive arrow functions async
This ensures UI state is only cleared after operations complete
successfully and prevents unhandled promise rejections.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Make handleArchiveSession, handleUnarchiveSession, handleDeleteSessions,
and handleArchiveSessions async, await store calls, then reload sessions
with showArchived flag to preserve archived session visibility.
QA Fix Session: 2
Add showArchived state, import bulk store helpers (deleteSessions, archiveSession,
archiveSessions, unarchiveSession), create handler functions, add useEffect to
reload sessions when showArchived changes, and pass all new props to ChatHistorySidebar.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add deleteSessions, archiveSession, archiveSessions, unarchiveSession helpers
and update loadInsightsSessions to accept optional includeArchived parameter.
Also update ElectronAPI types and browser mocks for new methods.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add handlers for bulk delete, archive, bulk archive, and unarchive
sessions. Update list sessions handler to accept includeArchived param.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: handle empty/greenfield projects in spec creation and prevent stuck planning state (#1426)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: address PR review findings - planning_phase_ended bug, type hints, dedup (#1426)
- Convert planning_phase_ended to instance attribute self._planning_phase_ended
so _run_phases() can mark it True after each end_phase() call, preventing
double-end on exception propagation
- Add Path type annotation to _is_greenfield_project(spec_dir)
- Extract duplicated greenfield detection into _check_and_log_greenfield() helper
- Add TaskLogger and types.ModuleType type hints to _run_phases() signature
- Simplify redundant SystemExit handler with explanatory comment
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: prevent greenfield false positive on missing/corrupt project index
When get_project_index_stats() returns {} (file missing, JSON parse
error, or unrecognized format), _is_greenfield_project() now returns
False instead of incorrectly classifying the project as greenfield.
Also removes unused TYPE_CHECKING import and empty conditional block.
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* fix: clear terminalEventSeen on task restart to prevent stuck-after-planning (#1828)
The terminalEventSeen Set in TaskStateManager was never cleared when a task
was restarted. When spec_runner.py emits PLANNING_COMPLETE, the taskId is
added to terminalEventSeen. If the subsequent coding process (run.py) fails,
handleProcessExited() returns early because terminalEventSeen.has(taskId)
is true, silently swallowing the PROCESS_EXITED event. The XState actor
never transitions, leaving the task permanently stuck in 'coding' state.
Additionally, lastSequenceByTask from the old process would cause events
from a new process (starting at sequence 0) to be dropped as duplicates.
Fix: Add prepareForRestart(taskId) method that clears both terminalEventSeen
and lastSequenceByTask without stopping the XState actor. Call it in all 4
locations where a new agent process is started:
- TASK_START handler
- TASK_STOP handler (so subsequent restart works)
- TASK_UPDATE_STATUS auto-start path
- TASK_RECOVER_STUCK auto-restart path
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: add prepareForRestart to TASK_REVIEW rejection path
Add missing prepareForRestart(taskId) call before startQAProcess() in the
TASK_REVIEW rejection handler. This is the 5th location where a new agent
process is started for an existing task, but was missed in the original fix.
Without this, if the QA fixer process crashes after a review rejection,
terminalEventSeen would cause handleProcessExited() to swallow the exit
event, leaving the task permanently stuck.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* fix: watch worktree path for implementation_plan.json changes (#1805)
The FileWatcher was always watching the main project's spec directory for
implementation_plan.json changes. When tasks run in a worktree, the backend
writes the plan file to the worktree directory instead, so the watcher never
detected changes and subtask progress was never sent to the UI.
Changes:
- Add getSpecDirForWatcher() helper that checks worktree path first
- Update all 3 file watcher setup locations (TASK_START, TASK_UPDATE_STATUS
auto-start, TASK_RECOVER_STUCK auto-restart) to use worktree-aware paths
- Add re-watch logic in execution-progress handler: when a worktree appears
after task start, automatically switch the watcher to the worktree path
- Add worktree fallback in exit handler for reading final plan state
- Add getWatchedSpecDir() method to FileWatcher for path comparison
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: address PR review findings - naming consistency, async error handling (#1805)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: address PR review findings for file watcher race condition and error handling
- Add pendingWatches guard in FileWatcher.watch() to prevent overlapping async
calls from creating duplicate watchers (CodeRabbit critical finding)
- Add .catch() to all three fire-and-forget fileWatcher.watch() calls in
execution-handlers.ts to prevent unhandled promise rejections
- Remove shadowed specsBaseDir re-declaration in autoRestart block, reusing
the outer variable from the same TASK_RECOVER_STUCK handler scope
* fix: address PR review findings for file-watcher race conditions and variable shadowing
- Change pendingWatches from Set<string> to Map<string, string> (taskId->specDir)
so re-watch calls with a different specDir are allowed through instead of silently dropped
- Add cancelledWatches Set to coordinate unwatch() with in-flight watch() calls,
preventing watcher leaks when unwatch() runs during watch()'s await points
- Add .catch() handler to fileWatcher.unwatch() call in agent-events-handlers exit handler,
consistent with the .catch() pattern used for all watch() calls
- Remove shadowed const mainSpecDir re-declaration inside autoRestart block in
execution-handlers.ts, using the outer variable from the enclosing try block instead
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: resolve FileWatcher race conditions and unhandled promise rejections
- Add supersession check in watch() after awaiting existing watcher close
to prevent a later concurrent call from having its watcher overwritten
- Return early in unwatch() when a watch() is in-flight to prevent
double-closing the same FSWatcher
- Cancel in-flight watch() calls in unwatchAll() by marking their taskIds
in cancelledWatches before closing existing watchers
- Add .catch() to fileWatcher.unwatch() calls in TASK_STOP and
TASK_RECOVER_STUCK handlers to surface errors instead of silently dropping them
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: resolve concurrent watch() race conditions in FileWatcher
- Make finally block conditional so superseding watch() calls are not
wiped out by the superseded call cleaning up pendingWatches
- Delete watcher from map before awaiting close() to prevent concurrent
calls from double-closing the same FSWatcher reference
- Make cancelledWatches cleanup conditional on the call still owning the
pendingWatches entry, preventing premature flag removal for concurrent calls
- Fix misleading comment about mainSpecDir declaration scope
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: resolve PR review findings for FileWatcher dead code, missing guards, and test coverage
- Remove dead code in finally block: after delete(), has() is always
false so the inner if was always true; simplify to a single delete +
cancelledWatches.delete call (Finding 1)
- Add implementation_plan.json existence check in getSpecDirForWatcher
before preferring the worktree path, so the watcher is started in
the correct directory even when the plan file hasn't been written yet
(Finding 2)
- Clear pendingWatches in unwatchAll() so in-flight watch() calls can
no longer register new watchers after a full teardown (Finding 3)
- Also clear cancelledWatches in unwatchAll() since in-flight calls bail
via the supersession check and won't clean up the flags themselves
- Add comprehensive concurrency tests for FileWatcher covering
deduplication, supersession, cancellation, and unwatchAll behaviour
(Finding 4)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: use path.join() in file-watcher tests for cross-platform compatibility
Replace hardcoded forward-slash strings in getWatchedSpecDir assertions with
path.join() so expected values match on Windows (backslash) and Unix (forward
slash) alike.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: remove duplicate specDir declaration after rebase
The rebase on origin/develop introduced a duplicate `const specDir` declaration
that caused TypeScript and Biome CI failures. The variable was already declared
earlier in the same scope with the same value.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* fix: resolve Claude CLI not found on Windows - PATH merge, prompt size cap, and cwd (#1661)
Three root causes addressed:
1. PATH overwrite: pythonEnv.PATH was overwriting the augmented PATH (with npm
globals) in spawn env. Now merges PATH entries instead, prepending
python-specific paths (pywin32_system32) while preserving all augmented entries.
2. System prompt size: On Windows, SDK passes system_prompt as --system-prompt
CLI arg. Large CLAUDE.md files exceed CreateProcessW's 32,768 char limit,
causing misleading "Claude Code not found" error. Now caps CLAUDE.md content
on Windows to stay under the limit.
3. Cross-drive cwd: Agent processes were spawned with autoBuildSource as cwd.
On Windows with cross-drive setups, this caused file access issues. Now uses
projectPath as cwd since all script paths are absolute.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: address PR review findings - constants, logging, CI fixes (#1661)
- Extract magic number 24000 into WINDOWS_MAX_SYSTEM_PROMPT_CHARS constant
(set to 20000 for more conservative ~12KB CLI headroom)
- Extract truncation suffix into WINDOWS_TRUNCATION_MESSAGE constant
- Fix double-print when truncation occurs: only print "included in system
prompt" when CLAUDE.md was NOT truncated (was_truncated flag)
- Fix CI test failures: update subprocess-spawn tests to expect projectPath
as cwd instead of autoBuildSource (matches the #1661 CWD change)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: normalize PATH key casing and fix truncation budget on Windows
- Normalize env objects to a single uppercase 'PATH' key before merging
to prevent duplicate PATH keys on Windows where process.env has 'Path'
and getAugmentedEnv() writes 'PATH'. Without this, Object.keys().find()
returns 'Path' first (insertion order), discarding augmented entries,
and the final spread produces both 'Path' and 'PATH' keys.
Follows the same pattern used in python-env-manager.ts. (#1661)
- Subtract WINDOWS_TRUNCATION_MESSAGE length from the truncation budget
so the final system prompt stays within WINDOWS_MAX_SYSTEM_PROMPT_CHARS.
Addresses PR #1843 review findings NEW-001, NEW-002, NEW-003.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: address PR review findings for Windows PATH key casing and truncation budget
- Finding 1 (MEDIUM): Prefer 'PATH' key directly when present in env to avoid
insertion-order bug where Object.keys().find() returned 'Path' first on Windows
- Finding 2 (MEDIUM): Normalization block (delete stale cased key, write 'PATH')
already in place from previous commit; Finding 1 fix ensures envPathKey resolves
correctly so normalization fires only when truly needed
- Finding 3 (LOW): Subtract header template overhead from max_claude_md_chars to
prevent ~44-char overshoot in Windows command-line truncation budget (#1661)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: remove stale 'Path' key after PATH normalization on Windows
When getAugmentedEnv() spreads process.env on Windows, the resulting object
contains both 'Path' (from process.env spread) and 'PATH' (explicitly written
by getAugmentedEnv). The prior normalization block only removed non-'PATH' keys
when 'PATH' was absent, leaving the stale 'Path' key when both coexisted.
Add a cleanup loop to delete all case-variant PATH keys that differ from
'PATH' after the main normalization, ensuring the child process inherits a
single canonical 'PATH' entry with the fully-augmented value. (#1661)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* refactor: extract shared PATH normalization utilities and add unit tests
- Extract normalizeEnvPathKey() and mergePythonEnvPath() into env-utils.ts
as shared, exported helpers to eliminate duplicated PATH key case-normalization
logic across agent-process.ts and python-env-manager.ts (Finding 3)
- Add PATH normalization call in agent-queue.ts spawnIdeationProcess and
spawnRoadmapProcess to fix the same Windows PATH duplicate-key issue that
was fixed in agent-process.ts (#1661) (Finding 1)
- Add comprehensive unit tests for normalizeEnvPathKey() and mergePythonEnvPath()
covering Windows-style 'Path' key renaming, duplicate key removal, PATH
deduplication across merge, and Unix separator support (Finding 2)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* fix: handle planning phase crash and resume recovery (#1562)
When spec creation crashes, the task gets stuck in "planning" state
forever because the backend never emits PLANNING_FAILED to the frontend
XState machine. Clicking Resume then also crashes because the resume
logic transitions to "coding" state, but there are no subtasks yet.
Root causes and fixes:
1. Backend orchestrator (orchestrator.py):
- Wrap run() in try/except to emit PLANNING_FAILED on unhandled exceptions
- Add _emit_planning_failed() calls at every early return path
- Fix spec_dir tracking after rename_spec_dir_from_requirements()
2. XState machine (task-machine.ts):
- Add PLANNING_STARTED transitions from error and human_review states
- This allows tasks that crashed during planning to resume back to planning
3. Execution handlers (execution-handlers.ts):
- Detect error state with 0 subtasks and send PLANNING_STARTED (not USER_RESUMED)
- Check actual implementation_plan.json for subtasks instead of task.subtasks.length
- Handles both with-actor and without-actor (app restart) code paths
Closes#1562
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: address PR review findings - reliable subtask check, spec_dir rename, empty except (#1562)
- Move planHasSubtasks calculation (reads implementation_plan.json) before
XState handling so the crash-during-planning check uses the reliable
file-based check instead of task.subtasks.length
- Change rename_spec_dir_from_requirements to return the new Path directly
instead of a bool, eliminating brittle directory scanning in orchestrator
- Add descriptive comment to empty except clause to satisfy code scanning
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: update tests for rename_spec_dir_from_requirements return type change (#1562)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: address PR review findings for planning crash resume
- Update phase_executor.spec_dir and spec_validator after directory rename
so subsequent phases don't use stale paths (critical bug flagged by
sentry, coderabbitai, and Auto Claude review)
- Fix TASK_UPDATE_STATUS handler to use file-based plan check instead of
unreliable task.subtasks.length (same #1562 bug fixed in TASK_START)
- Replace manual subtask counting with existing checkSubtasksCompletion helper
- Use safeReadFileSync instead of existsSync+readFileSync (TOCTOU fix)
- Add self.validator update to backward-compat _rename_spec_dir_from_requirements
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
- Remove redundant conditional in setGenerationStatus (NEW-001)
- Add comprehensive test coverage for catch-up logic (NEW-002)
- Pass persisted context to getOrCreateGenerationActor on reload (CMT-001)
- Add reverse-direction assertions for state name arrays (FNEW-003)
- Fix stale line reference in comment (NEW-003)
- Use precise no-op guard in updateFeatureLinkedSpec (NEW-004)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add vite/client types reference to fix import.meta.hot TS errors
- Add unit tests for mapGenerationStateToPhase/mapFeatureStateToStatus
ensuring all machine states map correctly without silent fallthrough
- Restore persisted state in getOrCreateGenerationActor matching the
feature actor pattern with resolveState()
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Preserve 'exited' status in setClaudeMode(false) instead of overwriting to 'running'
- Add isResumingPhase guard to SWAP_RESUME_COMPLETE for consistency with other swap events
- Clean up stale migratedSessionFlags when resumeClaudeAsync finds no terminal
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace conditional assertions with direct assertions since
sys.modules patching deterministically makes available=True.
The else-branch was dead code with a trivially-passing assertion.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add catch-up logic for 'error' phase in setGenerationStatus so
GENERATION_ERROR is not silently dropped when actor is in idle/complete
- Fix progress updates being dropped for empty string message by using
!== undefined instead of truthy check on status.message
- Improve compile-time assertion comments explaining both-direction sync
enforcement strategy (forward via type assertion, reverse via switch
exhaustiveness in map functions)
- Replace unnecessary dynamic import of resetActors in test afterEach
with static import
- Document that backward transitions are intentionally unsupported in
the forward-only generation pipeline
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The onTerminalExit handler used store.updateTerminal() to reset isClaudeMode,
which is a plain Zustand setter that bypasses XState notification. Replace with
store.setClaudeMode() which properly checks XState state before sending events.
Since setTerminalStatus('exited') already sends SHELL_EXITED to XState (handling
the claude_active -> exited transition), the setClaudeMode(false) call here only
updates Zustand - its XState guard correctly skips sending CLAUDE_EXITED since
the machine is already in 'exited' state.
Fixes: NCR-R7-001 (HIGH) - Claude exit via updateTerminal bypasses XState machine
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add defense-in-depth count and size validation in InsightsExecutor
(NEW-005)
- Make image analysis unsupported warning more prominent with icon and
background styling (REVIEW-001/CMT-001)
- Add inline notice on sent messages that images were not analyzed
(CMT-001)
- Add i18n keys for image not-analyzed notice (en + fr)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Reuse SKIP_DIRS from context.constants instead of duplicating exclusion list
- Fix exception types in write error handlers (TypeError/ValueError, not JSONDecodeError)
- Add warning log when path validation bypassed due to exhausted retries
- Use existing safeReadFileSync helper for attempt_history reads
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace raw json.dump with write_json_atomic when writing
implementation_plan.json in mark_subtask_stuck() to prevent file
corruption, consistent with 8+ other call sites in the codebase.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Avoid hard-asserting status['available'] is True, which depends on
sys.modules patching behavior. Instead check the key exists and branch
on its value, consistent with neighboring tests in the same class.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The fallback safety net timer was only cancelled in the TASK_START handler,
but not in the TASK_UPDATE_STATUS auto-start path or TASK_RECOVER_STUCK
auto-restart path. This meant a stale timer could incorrectly stop a
newly restarted task if it was restarted within the 500ms window via
drag-to-in-progress or recovery auto-restart.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add missing debug log when venv symlink health check fails and the
recreate fallback succeeds, matching the Python backend's behavior
for consistent debug output.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The cleanup timer debug log was computing pendingDelete.size - 1 before
the actual delete call, logging an incorrect remaining count. Moved both
delete calls before the debug log so it reports the accurate size.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Extract duplicated stuck-subtask loading into _load_stuck_subtask_ids()
- Write stuck reason to actual_output instead of notes field for
consistency with the QA reviewer's expectations
- Clarify progress message to mention terminal states (completed/failed/stuck)
- Update test assertions to match actual_output field
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When a coding agent marks a subtask as "stuck", QA validation would never
start because is_build_complete() requires ALL subtasks to have status
"completed". This creates a deadlock: coder exits (no more subtasks to
work on), but QA never triggers.
Changes:
- Add is_build_ready_for_qa() that considers builds ready when all
subtasks reach a terminal state (completed, failed, or stuck)
- Update mark_subtask_stuck() to also set status="failed" in
implementation_plan.json, keeping plan file in sync with reality
- Reorder QA loop to check human feedback before build completeness,
so QA_FIX_REQUEST.md bypasses the build gate as intended
- Replace is_build_complete() with is_build_ready_for_qa() in
should_run_qa() and CLI qa commands
- Add 20 new tests covering is_build_ready_for_qa() edge cases and
mark_subtask_stuck() plan update behavior
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Extract _score_and_select() from duplicated candidate scoring blocks
- Use _find_correct_path_indexed() in _auto_correct_subtask_files()
with a shared file index built once for all missing files
- Use find_subtask_in_plan() helper instead of inline nested loop
- Add more dirs to _EXCLUDE_DIRS (.idea, .vscode, vendor, target, out)
- Narrow exception handlers from (OSError, JSONDecodeError) to OSError
for write_json_atomic (JSON errors can't occur on write)
- Fix type annotation for missing_entries list
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add functions for building a file index, finding correct paths using fuzzy matching, and auto-correcting subtask file paths.
- Introduce directory exclusion logic to optimize file searching.
- Enhance validation of file paths in implementation plans to support better error recovery.
- Add comprehensive tests for the new functionalities, covering various matching scenarios and edge cases.
This update improves the robustness of the coder pipeline by enabling it to automatically correct file paths based on existing project structure, thus enhancing overall stability and user experience.
Add test_get_graphiti_status_no_graph_backend to verify error handling when
graphiti_core imports successfully but neither real_ladybug nor kuzu are
available. This addresses CodeRabbit's recommendation to test the error path
in config.py lines 645-650.
Addresses CodeRabbit review comment on PR #1834.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add nested try-except in config.py for clearer error messages when graph DB backend is missing
- Mock imports in test_config.py to make test environment-independent
- Ensure test passes regardless of whether graphiti_core/real_ladybug/kuzu are installed
Resolves CodeRabbit and Gemini review comments on PR #1834.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove FalkorDB docker service reference from project_index.json (docker-compose.yml no longer exists)
- Correct line number reference in test_config.py comment (line 644 not 641)
Code review findings - no functional changes, just metadata cleanup.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
## Problem
The memory system was still checking for FalkorDB imports in `config.py`,
causing it to always report as unavailable and fall back to file-based
storage, despite LadybugDB being the configured and installed database.
Error in logs:
```
Graphiti packages not installed: falkordb is required for FalkorDri...
```
## Root Cause
In `get_graphiti_status()` at line 638, the code tried to import:
```python
from graphiti_core.driver.falkordb_driver import FalkorDriver
```
This import failed because FalkorDB was removed when migrating to LadybugDB.
## Changes Made
### Priority 1 — Critical Bug Fix
**File**: `apps/backend/integrations/graphiti/config.py` (lines 634-646)
- Replaced FalkorDB import check with LadybugDB/kuzu import check
- Now tries `real_ladybug` first (Python 3.12+), falls back to `kuzu`
- Removed unreachable pragma: no cover comment (line now executes)
### Priority 2 — Test Infrastructure Updates
1. **`conftest.py`** (lines 84-103)
- Renamed fixture: `mock_falkor_driver` → `mock_kuzu_driver`
- Updated docstring and patch path to reference KuzuDriver
2. **`test_config.py`** (lines 1056-1070, 1092-1094)
- Updated test to reflect new behavior: `available=True` when packages
installed, even with embedder validation errors (embedder is optional)
- Updated comment from "falkordb" to "LadybugDB/kuzu"
3. **`test_memory.py`** (lines 267, 278)
- Updated variable name: `mock_falkordb_driver` → `mock_kuzu_driver`
- Updated sys.modules patch path to use kuzu_driver instead of falkordb_driver
### Priority 3 — Documentation Updates
4. **`test_memory_facade.py`** (line 163)
- Updated comment: "remote FalkorDB" → "remote database"
5. **`spec_runner.py`** (line 139)
- Updated example: "FalkorDB" → "LadybugDB"
## Testing
All 670 graphiti tests pass:
```
apps/backend/.venv/bin/pytest apps/backend/integrations/graphiti/tests/ -v
========== 670 passed, 6 skipped, 112 deselected, 4 warnings in 2.10s ==========
```
## Impact
- Memory system now correctly detects LadybugDB as available
- No more false negatives causing fallback to file-based storage
- All existing functionality preserved
- No breaking changes
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fixed all 3 findings from the latest auto-claude review:
1. [NCR-NEW-001] MEDIUM: Fallback safety net timeout race condition
- Store setTimeout timer reference in a Map keyed by taskId
- Export cancelFallbackTimer() function to clear pending timers
- Call cancelFallbackTimer() at start of TASK_START handler
- Prevents stale timer from incorrectly stopping newly restarted tasks
- Clean up timer reference after it fires
2. [CMT-NEW-001] LOW: Duplicate hasPlan detection logic
- Replace inline hasPlan logic in execution-handlers.ts TASK_STOP
- Use shared hasPlanWithSubtasks() utility from plan-file-utils.ts
- Eliminates code duplication and ensures consistent behavior
3. [CMT-001] LOW: Misleading async keyword
- Remove async from setTimeout callback in agent-events-handlers.ts
- No await expressions exist in the callback
- All operations (getCurrentState, hasPlanWithSubtasks) are synchronous
All changes maintain backward compatibility and fix the race condition
where restarting a task within 500ms could be incorrectly stopped by
the fallback timer from the previous process exit.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Extract hasPlan logic into shared hasPlanWithSubtasks() utility in plan-file-utils.ts
to eliminate duplication and centralize plan validation logic
- Make setTimeout callback async to avoid blocking readFileSync in event loop
- Replace hardcoded terminal status check with cleaner .includes() pattern
- Add status gate for final phase updates to prevent UI flicker when failed
phase arrives after task has transitioned to human_review
- Import and use hasPlanWithSubtasks in agent-events-handlers.ts
All review comments addressed:
- Gemini: Critical/Medium - forceTransition method, magic number (already fixed in previous commit)
- Gemini: Medium - hardcoded status list (now uses .includes)
- CodeRabbit: Trivial - extract XSTATE_ACTIVE_STATES (already fixed in previous commit)
- CodeRabbit: Minor - derive hasPlan from file check (now uses shared utility)
- CodeRabbit: Minor - prevent UI flicker from final phase updates (now gates on status)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Resolves NEW-001 and CMT-001 from the follow-up review:
- Extract XSTATE_ACTIVE_STATES to shared constant in task-state-utils.ts
- Export XSTATE_ACTIVE_STATES from state-machines index
- Replace hardcoded hasPlan: true with dynamic plan file check
- Pattern matches TASK_STOP handler (execution-handlers.ts lines 299-310)
- Tasks stuck in 'planning' state now correctly route to backlog, not human_review
- Improved logging shows hasPlan value for debugging
This ensures the fallback safety net routes tasks to the correct Kanban column
based on whether a plan file exists with subtasks.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add named constant STUCK_TASK_FALLBACK_TIMEOUT_MS for magic number
- Use XState getCurrentState() instead of cached task status to avoid stale cache issues
- Check XState active states directly (planning, coding, qa_review, qa_fixing)
- Improve logging to show actual XState state name
- Add inline comments explaining the stale cache avoidance strategy
This resolves all 3 blocking issues from the Auto Claude review:
1. CRITICAL: forceTransition() method - fixed by using handleUiEvent()
2. MEDIUM: Stale closure - fixed by checking XState directly
3. MEDIUM: Magic number - fixed by using named constant
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The 500ms fallback safety net was calling `taskStateManager.forceTransition()`
which doesn't exist, causing TypeScript compilation errors.
Fixed to:
- Use `handleUiEvent()` with `USER_STOPPED` event (proper XState transition)
- Look up both task and project fresh (avoid stale closure references)
- Add null check for `checkProject` before proceeding
This ensures the fallback actually works when XState fails to transition
tasks out of in_progress after process exit.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When an agent process exits with incomplete/stuck subtasks, the task gets
stuck in the Kanban UI — it spins forever, can't be stopped, and can't be
dragged back to planning.
This fix addresses 5 specific state synchronization gaps between the backend
process lifecycle and the frontend XState state machine:
1. Reset execution progress on terminal state transitions (task-store.ts)
- When tasks reach terminal states (human_review, error, done, pr_created),
execution progress is now reset to idle
- Prevents stuck tasks from showing stale progress indicators in UI
2. Propagate final phase updates even after XState settles (agent-events-handlers.ts)
- Final 'complete' or 'failed' phase updates are now sent to renderer
- Previously these were silently dropped, causing UI to never show completion
3. Add fallback exit handler to force state transition (agent-events-handlers.ts)
- If task remains in_progress 500ms after process exit, force to human_review
- Safety net for when XState fails to properly handle PROCESS_EXITED event
4. Disable dragging for in_progress tasks (SortableTaskCard.tsx)
- Prevents users from dragging tasks that are currently running or stuck
- Adds disabled flag to useSortable hook when status is 'in_progress'
5. Allow stop button for stuck tasks (TaskCard.tsx)
- Users can now force-stop stuck tasks using the stop button
- Removed the isStuck check that prevented stopping stuck tasks
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
AndyMik90