- Add missing vi.mock for cli-tool-manager and sentry in runner-env test
(unmocked getToolInfo caused filesystem lookups timing out on Windows CI)
- Loop HTML tag stripping to handle nested tag fragments (CodeQL #5077)
- Decode & entity last to prevent double-unescaping (CodeQL #5076)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The Claude CLI emits message types like rate_limit_event that the SDK's
message_parser doesn't recognize, causing MessageParseError to kill the
entire agent session stream. This adds two layers of defense:
1. Monkey-patch SDK's parse_message to convert unknown types into safe
SystemMessage objects instead of raising
2. safe_receive_messages() wrapper around receive_response() that filters
patched messages and catches stream-level errors gracefully
Applied to all agent consumers: session, qa_reviewer, qa_fixer, and
agent_runner. Also bumps minimum SDK to >=0.1.39.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Surface review errors in UI instead of silently falling back to "Not Reviewed"
- Thread reviewError from store through hook → GitHubPRs → PRDetail → ReviewStatusTree
- Fix error payload to include prNumber so store updates correct PR key
- Use CLI tool manager (getToolInfo) instead of `which gh` in validateGitHubModule
so bundled Electron apps can find gh via Homebrew/augmented PATH
- Pass GITHUB_CLI_PATH in subprocess env via getRunnerEnv
- Use resolved gh path for `gh auth status` check
- Add Sentry breadcrumbs and error capture for gh CLI resolution diagnostics
- Add i18n keys for retryReview (en + fr)
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>
- 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>
- Use setClaudeMode() instead of updateTerminal() in onTerminalClaudeExit
handler to properly send CLAUDE_EXITED to XState machine
- Remove premature migratedSessionFlags cleanup from destroy() since
resumeClaudeAsync already consumes flags and killAll() clears the map
- Add swap phase ordering guards (isCapturingPhase, isMigratingPhase,
isRecreatingPhase) to prevent out-of-order swap events
- Add YOLO mode flag to deprecated sync resumeClaude for consistency
- Fix isBusy preservation heuristic by using dedicated updateClaudeSessionId
action for self-transitions in claude_active state
- Add debugLog when setPendingClaudeResume silently drops request due to
missing claudeSessionId
- Handle CLAUDE_BUSY events in claude_starting and pending_resume states
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit addresses all 10 findings from the latest Auto Claude PR review:
**BLOCKING FIXES (3 MEDIUM severity):**
1. NCR-NEW-004: Add user notification when session migration fails
- Added i18n key 'terminal:swap.migrationFailed' (EN + FR)
- Hook now shows toast notification when isClaudeMode && sessionId && !sessionMigrated
- Users are now informed when their Claude session is lost during profile switch
2. NEW-001: Add idle-state guard to setClaudeSessionId
- setClaudeSessionId now checks if XState machine is in 'idle' state
- Sends SHELL_READY first before CLAUDE_ACTIVE, matching setClaudeMode pattern
- Prevents XState/Zustand desync during profile change flow
3. NEW-003: Correct fire-and-forget IPC comment
- Removed incorrect claim about onTerminalPendingResume fallback
- Updated comment to accurately describe actual failure behavior
- Documents that errors are logged but no event is emitted back to renderer
**LOW SEVERITY FIXES (5):**
4. CMT-002: Remove EEXIST dead code
- copyFile() without COPYFILE_EXCL never throws EEXIST
- Removed unreachable catch branch at lines 136-141
- Added comment documenting silent overwrite behavior
5. NEW-004-STORE: Add state check for CLAUDE_EXITED event
- setClaudeMode now checks machine state before sending CLAUDE_EXITED
- Only sends event if machine is in 'claude_starting' or 'claude_active'
- Prevents XState/Zustand desync when event is dropped
6. NEW-006: Preserve isBusy during CLAUDE_ACTIVE self-transitions
- setClaudeSessionId action now checks if it's a self-transition
- Preserves existing isBusy state during self-transitions
- Prevents late session ID updates from incorrectly clearing busy indicator
7. CMT-NEW-001: Add swap state assertions to RESET tests
- Updated RESET test to include swapTargetProfileId and swapPhase
- Tests now verify all 6 context fields are cleared (not just 4)
- Comprehensive test coverage for resetContext action
**REMAINING LOW SEVERITY (acknowledged but not blocking):**
8. CMT-NEW-002: Swap phase events lack ordering guards
- Acknowledged: ordering currently guaranteed by single caller
- Guards would be defensive but not essential for current implementation
- Can be addressed in future refactoring if needed
9. CMT-NEW-003: Dual Zustand/XState state updates lack architectural docs
- Acknowledged: inline comments exist per method
- Architectural-level documentation could be added separately
- Does not block merge as per-method comments are clear
All tests passing (3271 passed, 6 skipped). TypeScript compiles cleanly.
Biome linting clean for all changed files.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Three improvements for production code quality:
1. Use path alias for consistency
- Change terminal-store.ts line 7 to use @shared/* instead of relative import
- Matches project conventions (CLAUDE.md path alias guidelines)
2. Prevent XState/Zustand state divergence in setPendingClaudeResume
- Only set pendingClaudeResume flag in Zustand if RESUME_REQUESTED was sent to XState
- When claudeSessionId is missing, skip both XState event and Zustand update
- Prevents UI showing "resume pending" when state machine doesn't know about it
3. Clean up migratedSessionFlags on individual terminal destroy
- Previously only cleared on killAll(), entries could linger if terminal closed before resume
- Now removes entry in destroy() if terminal has claudeSessionId
- Prevents Map from accumulating stale session flags
All tests pass (128/128 files, 3271/3277 tests).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Address three blocking review findings:
1. NEW-008: setClaudeMode now preserves claudeSessionId when dispatching CLAUDE_ACTIVE
- Include terminal's current claudeSessionId in the event to prevent XState
setClaudeSessionId action from overwriting it to undefined
- Fixes hasActiveSession guard for profile swaps
2. NCR-001: setPendingClaudeResume preserves claudeSessionId in RESUME_COMPLETE event
- Include terminal's claudeSessionId when sending RESUME_COMPLETE to XState
- Prevents machine from losing session ID on resume completion
3. NEW-002: Remove ineffective try/catch around fire-and-forget IPC call
- resumeClaudeInTerminal uses ipcRenderer.send() which returns void immediately
- Removed try/catch and await that could never catch main process errors
- Added comment documenting fire-and-forget nature and error handling via events
All tests pass (52/52).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add setError action to claude_active CLAUDE_EXITED transition to preserve error messages
- Fix missing await on resumeClaudeInTerminal call with proper error handling fallback
- Improve TypeScript type safety in test helper using Partial<TerminalContext>
- Add test coverage for CLAUDE_EXITED error preservation from claude_active state
All tests pass (52/52).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Convert migrateSession() from sync to async fs operations (mkdir,
copyFile, cp, unlink from fs/promises) to avoid blocking the Electron
main process during session migration
- Remove dead TerminalSwapState/TerminalSwapPhase types and swapState
field from TerminalProcess (no longer referenced after prior cleanup)
- Remove dead swapState logic from activateDeferredResume
- Add migratedSessionFlags.clear() to killAll() to prevent memory leaks
- Add CLAUDE_ACTIVE handler to pending_resume XState state (fixes race
where Claude becomes active before RESUME_COMPLETE fires)
- Add test coverage for CLAUDE_ACTIVE in pending_resume state
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Security:
- Move YOLO mode (dangerouslySkipPermissions) to server-side storage
instead of accepting from renderer via IPC. Main process stores flag
during profile migration and restores it when resume is called.
XState machine fixes:
- Add CLAUDE_ACTIVE self-transition in claude_active state so
setClaudeSessionId can update context.claudeSessionId (was silently
dropped, blocking hasActiveSession guard)
- Add SHELL_EXITED dispatch in setTerminalStatus for 'exited' status
so XState machine reaches its exited state
Dead code removal:
- Remove unused swapProfileAndResume method (migration handled directly
in IPC handler) and its now-unused migrateSession import
- Remove unused deriveTerminalStateFromMachine function and SnapshotFrom
import
Resource cleanup:
- clearAllTerminals now disposes terminalBufferManager entries and
xtermCallbacks alongside XState actors
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Make pending_resume state reachable by adding RESUME_REQUESTED transitions
from shell_ready and claude_active states
- Add CLAUDE_ACTIVE transition from shell_ready for direct activation path
- Wire SHELL_READY dispatch in setTerminalStatus and setClaudeMode to
prevent XState machine from being stuck in idle
- Remove dead TERMINAL_SWAP_PROGRESS IPC send (no listener existed)
- Remove unused isSwapping guard from terminal machine
- Remove ineffective try/catch around fire-and-forget ipcRenderer.send()
- Preserve dangerouslySkipPermissions (YOLO mode) through profile swap
terminal recreation flow via resume options
- Align swap phase naming: capturing_session → capturing (matches XState)
- Fix French translation accents: démarrer, changé, à
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add module-level terminalActors Map and helper functions (getOrCreateTerminalActor,
sendTerminalMachineEvent, deriveTerminalStateFromMachine) for XState actor management.
Store actions (setClaudeMode, setClaudeSessionId, setClaudeBusy, setPendingClaudeResume)
now forward corresponding events to the XState machine. Actors are cleaned up on
terminal removal and clearAllTerminals.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace manual "Run: claude --resume" message with automatic resume call via
resumeClaudeInTerminal IPC. Updated preload API and IPC types to pass
migratedSession option. YOLO mode is preserved automatically via the main
process terminal object.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add optional `options` parameter with `migratedSession` flag to resumeClaudeAsync()
- When migratedSession is true, log post-swap resume context and skip sessionId deprecation warning
- Preserve dangerouslySkipPermissions flag from terminal's stored state during resume
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add XState v5 state machine for terminal lifecycle with states: idle,
shell_ready, claude_starting, claude_active, swapping, pending_resume,
exited. Context tracks claudeSessionId, profileId, swap state, isBusy,
and error. Includes guards (hasActiveSession, isSwapping) and assign
actions for all context updates.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Resolves all 9 findings from auto-claude review (2026-02-16):
HIGH severity (1):
- NEW-001: Added catch-up transition for 'analyzing' state when setting status to 'generating'
The actor now properly transitions through 'discovering' before reaching 'generating'
MEDIUM severity (3):
- NEW-002: Fixed updateFeatureLinkedSpec to respect XState state machine
Removed fallback that bypassed XState for 'done' features. Now skips store write
when LINK_SPEC event is silently ignored (no linkedSpecId in context)
- NEW-003: Wired up HMR cleanup and test actor reset
Added import.meta.hot.dispose handler to reset actors during HMR
Added resetActors() call in test afterEach to prevent test pollution
- NEW-004: Added comprehensive catch-up logic for 'complete' phase
The actor now properly transitions through all intermediate states
(analyzing → discovering → generating) before accepting GENERATION_COMPLETE
LOW severity (1):
- CMT-001: Clarified test describe block title
Changed from "same-status transition is no-op" to "redundant status transitions"
to distinguish true no-ops (ignored events) from self-transitions (MARK_DONE in done)
All XState catch-up transitions now handle out-of-order status messages correctly,
preventing the UI from showing incorrect generation/feature status.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fix all CodeRabbit review comments and Biome warnings:
- Replace non-null assertions with explicit null checks in roadmap-store.ts (lines 548, 553, 559, 565)
- Replace non-null assertion with proper type guards in test (line 396)
- Use @shared/state-machines path alias instead of relative imports
- Prune stale feature actors in setRoadmap when roadmap changes
- Add lastActivityAt timestamp tracking to generation machine context
- Update deriveGenerationStatus to use persisted lastActivityAt from context
- Fix misleading test title "ignore MARK_DONE" → "self-transition"
- Add test verifying progress preservation on GENERATION_ERROR
- Remove progress reset in setError action to preserve progress on errors
- Add compile-time assertions to ensure state name arrays stay in sync with XState machines
All tests pass (77/77). TypeScript compilation successful. Zero Biome warnings in modified files.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fixes 6 issues identified in follow-up review (2 HIGH, 1 MEDIUM, 3 LOW):
1. HIGH [65226bd9539f]: Generation can now restart from complete/error states
- Added RESET + START_GENERATION logic in 'analyzing' case
2. HIGH [282536242c43]: Phase restoration fixed for discovering/generating
- Drive actor through intermediate transitions to reach target state
- Handle idle and complete/error states before sending phase events
3. MEDIUM [1b74aa0cd0f1]: Progress value clamping added to updateProgress
- Clamp progress to 0-100 range using Math.min/max
4. LOW [c833788d76ce]: setError now resets progress to 0 on error
- Consistent error state with progress reset
5. LOW [686d19af55d5]: Document unused SETTLED_STATES constants
- Added comments explaining future public API use
6. LOW [605d439c4efd]: Document getState() outside set() pattern
- Explain XState actors as external side effects
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Replace deprecated String.prototype.substr with substring
- Add resetActors() helper for test cleanup and HMR
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Move deleteFeature actor cleanup outside set() for consistency
- Skip no-op store writes when XState silently ignores events
- Use 'as const' on assign action string literals for type narrowing
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Use TaskOutcome/RoadmapFeatureStatus types in feature machine context
instead of loose strings, eliminating unsafe casts in roadmap-store
- Add TASK_COMPLETED/DELETED/ARCHIVED handlers to under_review state,
fixing a behavioral regression where linked task events were silently
ignored
- Move XState actor side effects outside Zustand set() callbacks in
updateFeatureStatus, markFeatureDoneBySpecId, updateFeatureLinkedSpec
- Clean up stale feature actors in setRoadmap to prevent memory leaks
when switching projects or loading new roadmaps
- Consolidate duplicate imports from shared/state-machines
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
AndyMik90