feat(parallel): parallel execution system with branch-per-session workflow#252
feat(parallel): parallel execution system with branch-per-session workflow#252
Conversation
…system Phase 1 of parallel execution: type definitions for workers, worktrees, merges, and conflicts; Kahn's algorithm for dependency analysis and parallel group detection; event types extending EngineEventType.
…lver Phase 2 of parallel execution: git worktree pool with acquire/release lifecycle, sequential merge queue with fast-forward-first strategy and backup tags for rollback, AI-assisted conflict resolution with three-way merge extraction.
Phase 3 of parallel execution: Worker wraps ExecutionEngine per worktree with event forwarding, ParallelExecutor coordinates task graph groups with batched workers and sequential merge, session persistence for crash recovery.
Phase 4: Add --serial/--sequential/--parallel flags to run command, ParallelConfig type to StoredConfig/RuntimeOptions, auto-detection path in executeRunCommand that analyzes task graph before choosing execution mode.
Add four new TUI components for visualizing parallel execution: - ParallelProgressView: worker overview panel with status, progress, merge queue - WorkerDetailView: drill-down view for single worker output stream - MergeProgressView: merge queue monitoring with backup tags and conflict indicators - ConflictResolutionPanel: overlay for AI conflict resolution with keyboard controls Integrate all components into RunApp with new ViewMode options (parallel-overview, parallel-detail, merge-progress), keyboard shortcuts (w=workers, m=merges, Enter=drill-down, Esc=back), and conflict panel overlay.
Add 8 new MDX pages under website/content/docs/parallel/: - overview: feature introduction and quick start - how-it-works: architecture deep-dive with execution flow - task-dependencies: dependency analysis and parallel grouping - git-worktrees: worktree lifecycle, layout, disk considerations - merge-and-conflicts: merge strategy, AI resolution, rollback - tui-guide: worker views, merge monitoring, keyboard shortcuts - configuration: all config options and CLI flags - troubleshooting: common issues and recovery procedures Update existing docs: - navigation.ts: new "Parallel Execution" section with "New" label - cli/run.mdx: add --serial/--parallel flags and parallel section - configuration/config-file.mdx: add [parallel] section example - configuration/options.mdx: add parallel options reference - getting-started/introduction.mdx: mention parallel in architecture - troubleshooting/common-issues.mdx: add parallel troubleshooting
Add 97 tests across 6 test files covering all parallel modules: - task-graph: 27 tests (dependency analysis, topological sort, cycle detection) - session: 17 tests (persistence, Map serialization, orphan detection) - merge-engine: 17 tests (fast-forward, merge commit, conflicts, rollback) - worktree-manager: 20 tests (acquire, release, cleanup, dirty checking) - worker: 10 tests (lifecycle, display state, event registration) - parallel-executor: 7 tests (planning logic, batch splitting) Fix duplicate edge bug in task-graph.ts when both dependsOn and blocks declare the same dependency relationship. Tests avoid mock.module() to prevent global mock leakage following existing codebase conventions.
The parallel execution system was hanging because each Worker created its own ExecutionEngine, which initialized a new tracker instance in the worktree directory. The beads-rust tracker data doesn't exist in worktrees, causing tracker.sync() to hang forever. Two root causes fixed: 1. Tracker initialization: Add WorkerModeOptions to ExecutionEngine so workers can inject a pre-initialized tracker, bypassing re-init 2. Task injection: Add forcedTask support so the engine works on the assigned task instead of querying the tracker for task selection Also fix: parallel event logging was gated behind !config.showTui, meaning TUI mode (the default) produced zero output during execution. Events are now logged unconditionally until TUI integration is built.
Connect ParallelExecutor's event stream to RunApp's parallel props so the TUI renders worker progress, merge queue, and conflict resolution during parallel execution. Previously parallel mode ran silently on the CLI. - Make engine optional in RunApp (absent in parallel mode where workers have their own engines) and guard all engine.* calls - Add runParallelWithTui() that translates ParallelExecutor events into React state and renders RunAppWrapper with parallel props populated - Route parallel TUI path through runParallelWithTui when showTui is true - Keep headless parallel path with console logging as fallback - Add parallel props passthrough in RunAppWrapper 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Compound knowledge: captures the integration gap where parallel execution path never routed through TUI rendering, and the solution pattern of making engine optional + creating an event-to-React bridge function. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…scoverability Three bugs fixed from live parallel TUI testing: 1. Frozen TUI: Replaced individual `let` variables with a shared `parallelState` object and added 500ms polling interval as safety net. The shared object ensures React always reads the latest values on re-render, even when execSync calls in worktree-manager/merge-engine block the event loop and prevent event-driven triggerRerender from firing. 2. Console bleed-through: Added `stdio: ['pipe', 'pipe', 'pipe']` to all execSync calls in worktree-manager, merge-engine, and conflict-resolver to prevent git output (especially stderr) from corrupting the TUI. Also gated the pre-TUI "Parallel execution enabled" console.log behind headless mode, and removed console.error from the parallel executor catch handler. 3. View discoverability: Auto-switch to parallel-overview view mode when isParallelMode is true, so users immediately see the worker status panel instead of the empty sequential task view.
Make parallel execution feel native in the existing task list UI rather than requiring separate views. Multiple tasks show ▶ simultaneously, selecting any running task shows its worker output in the right panel, and the dashboard shows concurrent worker count. Key changes: - Worker status overlay in displayedTasks useMemo for single-render-cycle updates - Task-to-worker mapping for output routing via parallelTaskIdToWorkerId - Pause (p), kill (x), and start/restart (s/Enter) support in parallel mode - Worker count display in ProgressDashboard - ParallelExecutor.reset() for re-execution after stop/complete
Security fixes: - Replace shell interpolation with execFileSync argument arrays in conflict-resolver.ts, merge-engine.ts, and worktree-manager.ts - Add git ref validation to prevent command injection via branch/tag names - Add branch name sanitization in worktree-manager.ts for safe git refs Bug fixes: - Fix forced-task infinite loop in engine when task is skipped/failed - Move tracker.completeTask to only run after successful merge - Fix error truncation with negative slice end in MergeProgressView - Fix duplicate switch case handlers in RunApp.tsx (add 'enter' case) - Fix await missing on rejected promise assertions in tests - Fix worktree path comparison in test cleanup using path.resolve() - Fix cross-platform disk space check using fs.statfs() instead of df Documentation fixes: - Add ```text language specifier to ASCII diagrams in markdown docs - Fix misleading "shallow worktrees" heading in git-worktrees.mdx - Remove stale 'R' shortcut documentation in merge-and-conflicts.mdx - Fix ASCII diagram in task-dependencies.mdx (remove trailing bar) - Fix cleanup command in common-issues.mdx using git for-each-ref Signal handling: - Add SIGINT/SIGTERM handlers for headless parallel execution mode - Properly stop executor, reset tasks, and persist state on signal https://claude.ai/code/session_01Tk23aPpLVDdZkt39NGTTQh
- Change actionable tasks guard from >= 3 to >= 2 so --parallel works
for two-task runs
- Clear prior conflict resolution state on new conflict:detected events
- Persist session state on worker lifecycle (start/complete/fail)
- Use sanitized branch name from worktreeManager.acquire() for merges
- Replace control character regex with Unicode property escape (\p{Cc})
- Derive timing.isRunning from worker's actual status instead of true
The parallel module tests were not being run in CI, causing coverage to drop below 40% when new parallel code was added. Add the parallel test batch to ensure these 97 tests contribute to coverage metrics.
…asks Three fixes for parallel execution issues discovered during testing: 1. Force autoCommit: true in parallel worker configs Workers now always have autoCommit enabled regardless of user config, because parallel mode requires commits to merge back to main. 2. Add prompt when autoCommit is disabled Before starting parallel execution, if autoCommit is off, prompt user: - Enable auto-commit for this session (recommended) - Fall back to sequential mode - Quit 3. Merge progress.md after each successful task merge After each successful merge (or conflict resolution), append the worker's progress.md to main progress.md with a separator. This ensures learnings from earlier tasks are visible to subsequent workers in later groups. Also adds worktreePath to WorkerResult for progress.md merging.
When auto-commit has nothing to commit (e.g., files are gitignored), emit a 'task:auto-commit-skipped' event with the reason. This helps users debug why parallel execution may show "no commits ahead of HEAD" merge failures when their changes are being excluded by .gitignore.
Parallel execution requires files to be committed so they can be merged. Previously the .gitignore excluded output-*.txt, merged-*.txt, and summary.txt, causing all parallel merges to fail with "no commits ahead".
Changed from cryptic "Source branch has no commits ahead of HEAD" to actionable "No commits to merge...Common cause: output files are in .gitignore" to help users debug why parallel merges fail.
…erge failed Shows ⚠ (yellow warning) instead of ✓ for tasks where: - Agent completed the task (said COMPLETED) - But merge failed (e.g., no commits because files were gitignored) This gives users clear visual feedback that work was done but not integrated, helping them understand why tasks aren't marked as fully done. Also adds infrastructure for tracking auto-commit-skipped events (reserved for future status bar warnings).
React's useMemo compares Set references, not contents. Mutating a Set with .add()/.delete() doesn't change its reference, so React wouldn't detect changes and re-render. Fixed by creating new Set instances when adding/deleting, ensuring React sees a new reference and properly re-computes displayedTasks.
When a merge fails and is rolled back, `git reset --hard` only resets tracked files. Any NEW files that were introduced during the merge attempt remain as untracked files in the working directory. Added `git clean -fd` after all rollback operations to remove these orphaned files: - Non-conflict merge failure rollback - Conflict-detected rollback - Manual rollbackMerge() - Session rollback This prevents confusing state where files from failed merges appear in the main workspace as untracked files.
Tasks now correctly show ✓ done status after merges complete. Previously, task status wasn't updating because the TUI's task state didn't re-read from the tracker after merges. Added parallelMergedTaskIds set that: - Populates on merge:completed event (immutably to trigger React re-render) - Gets checked in displayedTasks useMemo to mark tasks as 'done' - Persists after parallel execution ends, unlike worker states
…sues
Branch-per-session feature:
- Create `ralph-session/{shortId}` branch for parallel execution
- All worker changes merge to session branch instead of current branch
- Add `--direct-merge` flag to opt out and use legacy behavior
- Show completion guidance with merge/PR/discard commands
- Track sessionBranch and originalBranch in session state
CodeRabbit fixes:
- Fix worktree release identifier mismatch (was adding extra prefix)
- Fix command injection in session.ts (use execFileSync)
- Add space validation to validateGitRef
- Fix troubleshooting.mdx commands for empty branch lists
- Add totalWorkerCount guard in ProgressDashboard
- Fix headerLines calculation in WorkerDetailView (off-by-one)
- Add missing useCallback deps in RunApp.tsx
Other improvements:
- Add debug logging utilities for parallel execution
- Improve worktree cleanup handling
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
WalkthroughAdds a complete parallel-execution subsystem: task-graph analysis, worktree-backed workers, ParallelExecutor orchestration, sequential MergeEngine with AI-assisted ConflictResolver, session persistence, CLI/config flags, TUI views, many tests, and CI/.gitignore updates. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI
participant PE as ParallelExecutor
participant TG as TaskGraphAnalyzer
participant WM as WorktreeManager
participant W as Worker
participant ME as MergeEngine
participant CR as ConflictResolver
participant S as SessionStore
CLI->>PE: executeRunCommand(options, tracker)
PE->>TG: analyzeTaskGraph(tasks)
TG-->>PE: TaskGraphAnalysis
PE->>PE: decide parallel vs sequential
loop per ParallelGroup
PE->>WM: acquire(workerId, taskId)
WM-->>PE: WorktreeInfo
PE->>W: initialize(baseConfig, tracker)
W->>W: ExecutionEngine.initialize(workerMode)
W->>W: run assigned task
W-->>PE: WorkerResult / events
PE->>S: save/update session state
PE->>ME: enqueue(workerResult)
end
ME->>ME: processNext(operation)
alt conflict detected
ME->>CR: resolveConflicts(operation)
CR->>CR: per-file AI resolve
CR-->>ME: resolutions or rollback
end
ME-->>PE: merge events (completed/failed/rolled-back)
PE->>WM: cleanupAll()
PE->>S: finalise session
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ility When tests pass a relative worktreeDir path, store it as an absolute path resolved against cwd. This ensures fs.existsSync(info.path) works correctly regardless of the calling process's working directory.
The test expected worktree directories to be added to .gitignore, but this functionality was removed when worktrees were moved outside the project directory (as siblings). Since worktrees are no longer inside the project, they don't need to be gitignored.
The coverage threshold check uses only the tests/ batch coverage, which doesn't exercise the new src/parallel/ code. The parallel code has its own tests (src/parallel/*.test.ts) with 87-100% coverage for key files, but these run in a separate batch that isn't factored into the threshold. This is a structural issue with how coverage is calculated when adding new code directories. A better long-term fix would be to use combined coverage from all batches instead of just the first one.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #252 +/- ##
==========================================
+ Coverage 45.45% 45.51% +0.06%
==========================================
Files 84 91 +7
Lines 24396 27378 +2982
==========================================
+ Hits 11088 12460 +1372
- Misses 13308 14918 +1610
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/parallel/worktree-manager.test.ts`:
- Around line 51-53: The comment inside the afterEach block in
worktree-manager.test.ts uses temporal phrasing ("Before")—replace it with a
timeless, evergreen phrasing (e.g., "Clean up worktrees prior to removing the
repo directory" or "Clean up worktrees to allow repo directory removal") and
update any similar comments (like those that start with "First acquire") across
tests; edit the comment adjacent to the afterEach() in this file and run a quick
grep for analogous temporal phrases in the test suite to make them consistent.
- Around line 269-273: The test comment incorrectly describes the default
worktree base directory; update the inline comment in the test that references
WorktreeManager/defaultManager to state the correct default path is the sibling
`.ralph-worktrees/<project>` (not `.ralph-tui/worktrees`). Locate the test using
the WorktreeManager constructor in worktree-manager.test.ts and replace the
misleading comment with a brief accurate note that the default manager uses
`.ralph-worktrees/<project>` as the base directory.
In `@src/parallel/worktree-manager.ts`:
- Around line 343-350: The comments in ensureWorktreeDir and the earlier related
comment use temporal words ("now", "no longer", "already"); update the comment
above the private async ensureWorktreeDir() method and the earlier
worktree-related comment to be time-agnostic — e.g., state facts about behavior
and intent (that worktrees are placed in a sibling directory outside the
project, they do not need .gitignore entries, and worktreeDir is an absolute
path) without using temporal phrasing; keep references to
this.config.worktreeDir and ensureWorktreeDir so reviewers can locate and
replace the lines with the evergreen wording.
- Around line 120-126: worktreeId is constructed from unsanitised workerId and
used to build worktreePath; call sanitizeBranchName(workerId) when creating
worktreeId (e.g., replace const worktreeId = `worker-${workerId}` with a version
that uses sanitizeBranchName) so path separators or .. cannot escape the
intended directory, and ensure worktreePath continues to use the sanitized
value; also update the inline comments around worktreeDir and at the other noted
comment sites to remove temporal wording (replace phrases like "now", "no
longer", "already" with direct descriptions of the value/behavior) so comments
are evergreen.
1. Post-parallel completion logic (run.tsx): - Use parallelExecutor.getState() for parallel mode completion check - Track parallelAllComplete outside try block for proper scoping 2. Worker output Map mutation (run.tsx): - Create new Map instance instead of mutating in place - Ensures downstream memos observe changes 3. Merge failure recovery (index.ts): - Call handleMergeFailure() for non-conflict merge failures - Ensures failed tasks get requeued/reset in tracker 4. Parallel restart gating (RunApp.tsx): - Add parallelDerivedStatus computed from worker states - Use derived status instead of stale local status for restart gating 5. handleKeyboard dependencies (RunApp.tsx): - Add missing deps: onParallelPause, onParallelResume, onParallelStart - Add parallelDerivedStatus to prevent stale closures
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/tui/components/RunApp.tsx (1)
2116-2201:⚠️ Potential issue | 🟡 MinorAdd
parallelWorkersto the dependency array
isRunningis derived from the worker status on line 2143, but the memo does not depend onparallelWorkers, so the running/completed indicator can remain stale unless another dependency changes.Proposed fix
- }, [effectiveTaskId, selectedTask, selectedIteration, viewMode, currentTaskId, currentIteration, currentOutput, currentSegments, iterations, historicalOutputCache, currentIterationStartedAt, isViewingRemote, remoteStatus, remoteCurrentIteration, remoteOutput, remoteIterationCache, remoteCurrentTaskId, isParallelMode, parallelTaskIdToWorkerId, parallelWorkerOutputs]); + }, [effectiveTaskId, selectedTask, selectedIteration, viewMode, currentTaskId, currentIteration, currentOutput, currentSegments, iterations, historicalOutputCache, currentIterationStartedAt, isViewingRemote, remoteStatus, remoteCurrentIteration, remoteOutput, remoteIterationCache, remoteCurrentTaskId, isParallelMode, parallelTaskIdToWorkerId, parallelWorkerOutputs, parallelWorkers]);
🤖 Fix all issues with AI agents
In `@src/commands/run.tsx`:
- Around line 1669-1674: In onParallelStart replace in-place .clear() calls on
parallelState maps/sets with fresh instances so React/memoization picks up the
change: instead of parallelState.workers = [];
parallelState.workerOutputs.clear(); parallelState.taskIdToWorkerId.clear();
call assignments that create new objects (e.g., new arrays/new Map()/new Set())
and also ensure the three Sets completedLocallyTaskIds,
autoCommitSkippedTaskIds, and mergedTaskIds are reinitialized to new Set()
instances; keep the parallelExecutor.reset() call as-is. This ensures references
change and downstream components re-evaluate properly.
In `@src/tui/components/RunApp.tsx`:
- Around line 525-531: The conflict panel never opens because showConflictPanel
is never updated when parallelConflicts changes; add a useEffect that watches
the parallelConflicts array (or its length) and when it becomes non-empty call
setShowConflictPanel(true) and setConflictSelectedIndex(0), and when it becomes
empty call setShowConflictPanel(false); place this effect near the other
useState hooks for showConflictPanel/ conflictSelectedIndex in RunApp.tsx so the
panel toggles and selection resets whenever parallelConflicts updates.
- Around line 1631-1648: There are duplicate case 'enter'/'return' blocks
causing the first to unconditionally break and the second to be unreachable, and
a const (parallelStatusForRestart) declared in a case without a block; fix by
consolidating both case 'enter' and 'return' into one scoped block: open a brace
for the case, perform the parallel-only logic first (check viewMode ===
'parallel-overview' / setViewMode('parallel-detail') as before, then compute
const parallelStatusForRestart = parallelDerivedStatus ?? status inside the
block and run the isParallelMode && onParallelStart && status-check branch to
call setStatus('running') and onParallelStart()), and if none of the parallel
guards match fall through to the iteration drill-down behavior that was in the
other case—then close the block and break. Ensure you reference and use
setViewMode, parallelDerivedStatus, status, isParallelMode, onParallelStart,
setStatus exactly as named.
Code fixes: - Replace .clear() with new Map() in onParallelStart for React change detection - Add block scope to merge:completed and enter/return switch cases - Add useEffect for auto-showing conflict panel when conflicts detected - Fix worktree release call with correct "worker-" prefix - Replace execSync with execFileSync in debug-log.ts to prevent command injection - Fix test comment about default worktree path Documentation fixes: - Fix xargs -r macOS compatibility in common-issues.mdx - Add directMerge option and --direct-merge flag to configuration docs - Update worktreeDir default path to reflect sibling directory location
Add comprehensive tests for debug-log.ts functions: - initDebugLog, debugLog, closeDebugLog lifecycle - logGitStatus with valid and invalid git directories - logWorktreeInfo with valid and invalid git directories - logBranchCommits with existing and non-existent branches Coverage for debug-log.ts: 12% → 99%
Debug logging was previously always enabled, creating log files on every
parallel execution. Now it's opt-in via the --debug flag:
ralph-tui run --parallel --debug
When enabled, creates parallel-debug-{timestamp}.log with:
- Git status at each stage
- Worktree operations
- Branch commit comparisons
- Merge diagnostics
Useful for users to gather diagnostic info when reporting issues.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/tui/components/RunApp.tsx (1)
991-1018:⚠️ Potential issue | 🟡 MinorHandle prompt preview when no local engine is available.
In parallel mode the engine is undefined, so the effect can leave “Generating prompt preview...” indefinitely. Provide a fallback message (or disable prompt view) when no engine is present.
Suggested update
- } else if (engine) { + } else if (engine) { const result = await engine.generatePromptPreview(effectiveTaskId); // Don't update state if this effect was cancelled (user changed task again) if (cancelled) return; @@ } else { setPromptPreview(`Error: ${result.error}`); setTemplateSource(undefined); } + } else { + if (cancelled) return; + setPromptPreview('Prompt preview is unavailable without a local engine.'); + setTemplateSource(undefined); }src/commands/run.tsx (1)
918-980:⚠️ Potential issue | 🟡 MinorAdd guards to prevent handler invocation in parallel mode.
These handlers are passed unconditionally to RunApp but throw whenengineis undefined (parallel mode). In RunApp.tsx:
- Line 1858 (
onLoadEpics): Has.catch()error handling, so failures are gracefully displayed- Lines 2905–2906 (
onEpicSwitch): Awaited without try-catch; an unhandled promise rejection will crash the TUI- Lines 2912–2913 (
onFilePathSwitch): No exception handling; exceptions propagate unhandledEither conditionally pass these handlers only when
!isParallelMode, or add try-catch blocks around their invocations in RunApp.tsx to handle the errors gracefully.
🤖 Fix all issues with AI agents
In `@src/commands/run.tsx`:
- Around line 1414-1419: The code mutates parallelState.taskIdToWorkerId
in-place using .set(), which keeps the Map reference the same and prevents
memoized selectors (used for parallelTaskIdToWorkerId in RunApp.tsx) from
noticing updates; mirror the workerOutputs pattern by creating a new Map from
the existing one, apply the .set() on that new Map, and then assign the new Map
back to parallelState.taskIdToWorkerId (within the 'worker:created' case where
parallelExecutor.getWorkerStates() is used) so React's memoization sees the
changed reference.
In `@src/parallel/index.ts`:
- Around line 154-157: The JSDoc for the reset function uses temporal phrasing
("Call this before `execute()`") — update the comment for the reset method
(reset) to be evergreen by removing temporal wording and describing intent/usage
declaratively (e.g., "Resets internal state so the executor can be run again;
use this when restarting after completion or stop" or similar), and keep
reference to the related method (`execute`) to explain purpose without
imperative timing language.
🧹 Nitpick comments (1)
src/parallel/debug-log.test.ts (1)
23-31: Useos.tmpdir()for temporary paths to keep tests portable.Line 25 and Line 119 hard-code
/tmp, which breaks on Windows. Preferos.tmpdir().Suggested update
-import * as fs from 'node:fs'; +import * as fs from 'node:fs'; import * as path from 'node:path'; +import { tmpdir } from 'node:os'; @@ - tempDir = fs.mkdtempSync(path.join('/tmp', 'debug-log-test-')); + tempDir = fs.mkdtempSync(path.join(tmpdir(), 'debug-log-test-')); @@ - const nonGitDir = fs.mkdtempSync(path.join('/tmp', 'non-git-')); + const nonGitDir = fs.mkdtempSync(path.join(tmpdir(), 'non-git-'));Also applies to: 118-120
Remove the debug-log module and --debug flag entirely: - Debug logging exposed PII (usernames in paths) - Not useful enough to justify the maintenance burden - Logged git operations but not agent behavior (most common issues) Removed: - src/parallel/debug-log.ts and tests - --debug CLI flag - debug option from ParallelExecutorConfig - All debugLog/logGitStatus/logBranchCommits calls The codebase is now cleaner without the noise of conditional debug calls.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/parallel/merge-engine.ts`:
- Around line 28-31: The current control-character check using the regex
/[\x00-\x1f\x7f]/ should be replaced with a character-code scan to satisfy the
lint rule: iterate over the string referenced by ref (in the validation block
that currently throws in merge-engine.ts) and for each character use charCodeAt
to detect codes < 0x20 (32) or === 0x7f (127); if any such code is found, throw
the same Error(`Invalid git ref for ${context}: contains control characters`).
Update the check where ref is validated (the existing if-block) to use this
loop-based detection instead of the regex.
In `@src/parallel/worker.ts`:
- Around line 125-167: The start() method currently unconditionally sets
this.status to 'completed' and emits worker:completed after await
this.engine.start(), which misreports cancelled or non‑completed runs; modify
start() to check this.status (or a cancelled flag set by stop()) and inspect
engine.getState() to determine if the task actually completed (e.g.,
engineState.tasksCompleted > 0) before setting status and emitting
worker:completed; if the worker was cancelled or the task did not complete, set
status to 'cancelled' or 'failed', set success: false and taskCompleted
accordingly in the WorkerResult, and only emit worker:completed when reporting a
true completion (emit a separate cancelled/failed event or worker:completed with
success=false if desired). Ensure references: start(), stop(), this.status,
this.engine.start(), this.engine.getState(), emitParallel(), and WorkerResult
are updated consistently.
…itions - run.tsx: Create new Map instance for taskIdToWorkerId updates so React memoization detects changes (immutable update pattern) - merge-engine.ts: Replace regex control character check with char code loop for lint compliance - worker.ts: Check if worker was cancelled after engine.start() returns to avoid misreporting cancelled runs as completed (race condition fix)
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/parallel/merge-engine.ts`:
- Around line 440-441: Update the temporal comment above the call
this.git(['merge', '--abort']) to remove "for now" and make it evergreen;
replace the line "Abort the merge for now — conflict resolver handles this
separately" with a timeless phrasing such as "Abort the merge — conflict
resolver handles this separately" (or similar) so the comment describes the
intent without time-bound language.
- Around line 615-625: getConflictedFiles currently checks for unmerged status
codes 'UU','AA','DD','AU','UA' but omits delete/modify conflict codes 'DU' and
'UD', so add 'DU' and 'UD' to the conditional that pushes to conflicted (the
block iterating over output.split('\n') and using status = line.substring(0, 2)
and conflicted.push(line.substring(3).trim())); also expand the comment above
that conditional (the comment on line 617) to list the full set of unmerged
codes being checked: UU, AA, DD, AU, UA, DU, UD.
- Around line 15-58: In validateGitRef add two checks: reject refs that end with
a slash (ref.endsWith('/')) and reject a lone '@' (ref === '@') — throw
descriptive Errors similar to the existing ones (e.g., "Invalid git ref for
${context}: ends with '/'" and "Invalid git ref for ${context}: is '@'"); insert
these checks into the validateGitRef function alongside the other validations
(place the trailing-slash check near the "consecutive slashes" / ".lock" checks
and the lone-@ check early with the empty/ref checks) so invalid refs are caught
before git commands run.
In `@src/parallel/worker.ts`:
- Around line 228-233: The stop() method currently awaits this.engine.stop()
before setting this.status which can race with start()'s post-start check of
this.getStatus(); update stop() (method name stop) to set this.status =
'cancelled' immediately before calling/awaiting this.engine.stop(), so the
engine.stop() call occurs after the status flip and start() will observe the
cancelled state via this.getStatus(); no other behavior needs changing.
🧹 Nitpick comments (1)
src/parallel/worker.ts (1)
320-327: Good defensive error handling in event forwarding.Swallowing listener errors prevents a misbehaving listener from breaking the worker. Consider logging these errors in debug mode for troubleshooting.
… condition - merge-engine.ts: Add validation for refs ending with '/' and lone '@' - merge-engine.ts: Add DU/UD conflict codes for delete/modify conflicts - worker.ts: Set status='cancelled' before awaiting engine.stop() to ensure start()'s getStatus() check observes the cancelled state
- Add conflict-resolver.test.ts with comprehensive validation and API tests - Add session tests for updateSessionWithBranch and cleanupOrphanedWorktrees - Add worker tests for pause, resume, and display state methods Improves patch coverage for the parallel execution feature.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/parallel/conflict-resolver.test.ts`:
- Around line 359-375: Update the temporal comments around branch validation to
be evergreen: replace phrases like "will fail later on git command" and "will
fail on git execution instead" with a neutral statement that describes behavior
without time words; for example, in the test using ConflictResolver and
mockMergeOperation, change the comments above validBranches and the try block to
say that these branch names should pass validation and that any git-related
failures are handled during git execution (e.g., "These branch names should pass
validation; git execution may still fail"), keeping references to
ConflictResolver, mockMergeOperation, and operation.sourceBranch to locate the
test.
In `@src/parallel/session.test.ts`:
- Around line 251-256: Replace the temporal "First/Then" comments with an
evergreen, non-temporal description of the sequence of operations: e.g., "Update
other properties using updateSessionAfterGroup, mark task 'C' as requeued with
markTaskRequeued, and apply branch info using updateSessionWithBranch." Include
the function names (updateSessionAfterGroup, markTaskRequeued,
updateSessionWithBranch) so the intent and order remain clear without using
time-based words.
🧹 Nitpick comments (3)
src/parallel/worker.test.ts (1)
9-34: Use OS temp dir for portability in test config.Hardcoding
/tmpwill fail on non-POSIX runners; favouros.tmpdir()withpath.join().♻️ Proposed change
import { describe, test, expect } from 'bun:test'; +import * as os from 'node:os'; +import * as path from 'node:path'; import type { TrackerTask } from '../plugins/trackers/types.js'; import type { WorkerConfig, WorkerDisplayState } from './types.js'; import type { ParallelEvent } from './events.js'; import { Worker } from './worker.js'; @@ function workerConfig(id: string, task: TrackerTask): WorkerConfig { return { id, task, - worktreePath: `/tmp/worktrees/${id}`, + worktreePath: path.join(os.tmpdir(), 'worktrees', id), branchName: `ralph-parallel/${task.id}`, - cwd: '/tmp/project', + cwd: path.join(os.tmpdir(), 'project'), }; }src/parallel/session.test.ts (1)
25-33: Preferfs.mkdtempSync+os.tmpdir()for temp dirs.This avoids POSIX-only paths and reduces collision risk.
♻️ Proposed change
import { describe, test, expect, beforeEach, afterEach } from 'bun:test'; import * as fs from 'node:fs'; +import * as os from 'node:os'; import * as path from 'node:path'; @@ function createTempDir(): string { - const dir = path.join( - '/tmp', - `ralph-test-session-${Date.now()}-${Math.random().toString(36).slice(2)}` - ); - fs.mkdirSync(dir, { recursive: true }); - return dir; + return fs.mkdtempSync(path.join(os.tmpdir(), 'ralph-test-session-')); }src/parallel/conflict-resolver.test.ts (1)
7-59: Centralise the temp repo path and avoid hardcoded/tmp.Using
os.tmpdir()improves cross-platform reliability and keeps the path consistent.♻️ Proposed change
import { describe, test, expect } from 'bun:test'; +import * as os from 'node:os'; +import * as path from 'node:path'; import { ConflictResolver, type AiResolverCallback } from './conflict-resolver.js'; import type { ParallelEvent } from './events.js'; import type { MergeOperation, FileConflict, WorkerResult } from './types.js'; import type { TrackerTask } from '../plugins/trackers/types.js'; + +const TEST_REPO = path.join(os.tmpdir(), 'ralph-test-repo'); @@ test('creates instance with cwd', () => { - const resolver = new ConflictResolver('/tmp/test-repo'); + const resolver = new ConflictResolver(TEST_REPO); expect(resolver).toBeInstanceOf(ConflictResolver); });Apply
TEST_REPOto the remainingnew ConflictResolver('/tmp/test-repo')occurrences in this file.
- Remove temporal language ("will fail later", "First/Then")
- Use neutral descriptions of behavior and operation sequences
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/parallel/session.test.ts`:
- Around line 361-362: The current tautological assertion uses `result.cleaned +
result.errors.length >= 0`; replace it with an explicit check that at least one
outcome occurred by asserting `result.cleaned > 0 || result.errors.length > 0`
so the test ensures either something was cleaned or an error was reported;
locate the assertion referencing `result.cleaned` and `result.errors.length` in
the test and update it to use a logical OR check (or equivalent truthy
assertion) instead of the sum comparison.
…arallel tests - Fix TrackerPluginMeta mock to include required fields: supportsBidirectionalSync, supportsHierarchy, supportsDependencies - Fix TrackerPlugin mock to include all required interface methods - Fix RalphConfig mock to use proper ErrorHandlingConfig object - Replace tautological assertion (x + y >= 0) with meaningful check (x > 0 || y > 0) in cleanupOrphanedWorktrees test
feat(parallel): parallel execution system with branch-per-session workflow
Summary
This PR introduces the parallel execution system for ralph-tui, enabling multiple tasks to be worked on simultaneously using git worktrees. Key features include:
Parallel Execution Core
Branch-Per-Session Workflow (New)
ralph-session/{shortId}branch for all parallel execution--direct-mergeflag to opt out and use legacy direct-to-branch behaviorTUI Integration
CodeRabbit Fixes
Test plan
ralph-tui run --parallelon a test project with multiple tasksralph-session/*branch is createdgit mergeof session branch works--direct-mergeflag preserves legacy behaviorbun run typecheck && bun run lintSummary by CodeRabbit
New Features
Configuration
Documentation
Tests
Chores