Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion codeframe/core/conductor.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from concurrent.futures import ThreadPoolExecutor, as_completed
from dataclasses import dataclass, field
from datetime import datetime, timezone
from pathlib import Path
from enum import Enum
from typing import Callable, Optional

Expand Down Expand Up @@ -533,6 +534,7 @@ class BatchRun:
stall_timeout_s: int = 300
stall_action: str = "blocker"
concurrency: ConcurrencyConfig = field(default_factory=ConcurrencyConfig)
isolate: bool = True

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if TaskWorktree is imported or used anywhere in conductor.py
rg -n "TaskWorktree|worktrees" codeframe/core/conductor.py

# Check if worktree_path is ever passed as non-None
rg -n "worktree_path=" codeframe/core/conductor.py

Repository: frankbria/codeframe

Length of output: 45


🏁 Script executed:

# Check imports at top of conductor.py
head -50 codeframe/core/conductor.py

# Check if TaskWorktree exists anywhere in the codebase
rg -l "class TaskWorktree" codeframe/

# Check the signature of _execute_parallel and how it calls _execute_task_subprocess
rg -A 10 "def _execute_parallel" codeframe/core/conductor.py | head -30

# Check _execute_group_parallel similarly
rg -A 10 "def _execute_group_parallel" codeframe/core/conductor.py | head -30

Repository: frankbria/codeframe

Length of output: 2523


🏁 Script executed:

# Check if TaskWorktree is defined in worktrees.py
rg -n "class TaskWorktree" codeframe/core/worktrees.py

# Look at the full _execute_parallel implementation
rg -A 50 "def _execute_parallel\(" codeframe/core/conductor.py | head -70

# Look at the full _execute_group_parallel implementation  
rg -A 50 "def _execute_group_parallel\(" codeframe/core/conductor.py | head -70

# Check the _execute_task_subprocess signature
rg -B 2 -A 10 "def _execute_task_subprocess\(" codeframe/core/conductor.py

Repository: frankbria/codeframe

Length of output: 4146


🏁 Script executed:

# Check how _execute_task_subprocess is called within _execute_group_parallel
rg -B 5 -A 15 "def execute_task\(" codeframe/core/conductor.py | head -40

# Check the complete execute_task implementation
rg -A 25 "def execute_task\(task_id: str\)" codeframe/core/conductor.py | head -35

# Check if batch.isolate is ever checked or used
rg -n "batch\.isolate\|isolate\s*==" codeframe/core/conductor.py

# Check where TaskWorktree is actually imported or used
rg -n "TaskWorktree\|from.*worktrees" codeframe/core/conductor.py

Repository: frankbria/codeframe

Length of output: 2029


🏁 Script executed:

# Check if isolate flag is stored in BatchRun but never used
rg -n "self.isolate\|batch.isolate" codeframe/core/conductor.py

# Check where BatchRun is defined and what fields it has
rg -B 5 -A 30 "class BatchRun" codeframe/core/conductor.py | head -50

# Check if worktree_path is used inside _execute_task_subprocess
rg -A 30 "def _execute_task_subprocess" codeframe/core/conductor.py | head -40

Repository: frankbria/codeframe

Length of output: 2331


🏁 Script executed:

# Check how worktree_path is actually used in _execute_task_subprocess
rg -A 50 "def _execute_task_subprocess" codeframe/core/conductor.py | tail -30

# Check if there's any orchestration code that creates TaskWorktree
rg -n "TaskWorktree(" codeframe/core/

# Check the events to see if there's any worktree-related event emission
rg -n "worktree\|WORKTREE" codeframe/core/events.py

Repository: frankbria/codeframe

Length of output: 1113


🏁 Script executed:

# Check where _execute_task_subprocess is called to see if worktree_path is ever passed
rg "worktree_path\s*=" codeframe/core/conductor.py

# Check if batch.isolate is checked anywhere in the file
rg -n "\.isolate" codeframe/core/conductor.py

# Check if TaskWorktree is instantiated anywhere in the codebase
rg "TaskWorktree\(" codeframe/core/

# Look for any worktree-related logic in start_batch or elsewhere
rg -B 3 -A 3 "isolate" codeframe/core/conductor.py

Repository: frankbria/codeframe

Length of output: 638


🏁 Script executed:

# Check the full start_batch function to see if it creates worktrees
rg -A 60 "def start_batch\(" codeframe/core/conductor.py | head -80

# Check if there's any orchestration logic that creates worktrees before calling _execute_parallel
rg -B 5 -A 5 "_execute_parallel\(workspace" codeframe/core/conductor.py

# Verify TaskWorktree is not imported at all in conductor.py
head -30 codeframe/core/conductor.py | grep -i "import\|from"

Repository: frankbria/codeframe

Length of output: 3757


The isolate flag is stored but never used to create worktrees.

The BatchRun.isolate field and start_batch(isolate=) parameter are added, but the parallel execution paths (_execute_parallel, _execute_group_parallel) never check this flag. Additionally, TaskWorktree (defined in codeframe/core/worktrees.py) is not imported or instantiated in conductor.py, and worktree_path is never passed a non-None value when calling _execute_task_subprocess().

The infrastructure exists—_execute_task_subprocess() correctly handles worktree_path via cwd=str(worktree_path) if worktree_path else workspace.repo_path—but the orchestration logic that would create worktrees when isolate=True and pass their paths to subprocesses is missing. Is this deferred to a follow-up PR, or should issue #418 be considered incomplete?

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@codeframe/core/conductor.py` at line 537, The BatchRun.isolate flag is set
but never used when running parallel batches; update the orchestration so that
when BatchRun.isolate (and the start_batch(isolate=) parameter) is True you
create per-task worktrees and pass their paths into _execute_task_subprocess via
worktree_path. Specifically: import TaskWorktree from codeframe.core.worktrees,
create a TaskWorktree for each task in _execute_parallel and
_execute_group_parallel when isolate is True, ensure the created worktree path
is passed to _execute_task_subprocess (which already uses cwd=str(worktree_path)
if provided), and clean up or close the TaskWorktree after the subprocess
completes; keep the non-isolate path unchanged so existing behavior is
preserved.



def start_batch(
Expand All @@ -548,6 +550,7 @@ def start_batch(
stall_timeout_s: int = 300,
stall_action: str = "blocker",
concurrency_by_status: Optional[dict[str, int]] = None,
isolate: bool = True,
) -> BatchRun:
"""Start a batch execution of multiple tasks.

Expand Down Expand Up @@ -602,6 +605,7 @@ def start_batch(
stall_timeout_s=stall_timeout_s,
stall_action=stall_action,
concurrency=concurrency,
isolate=isolate,
)

# Save to database
Expand Down Expand Up @@ -1882,6 +1886,7 @@ def _execute_task_subprocess(
engine: str = "react",
stall_timeout_s: int = 300,
stall_action: str = "blocker",
worktree_path: Optional[Path] = None,
) -> str:
"""Execute a single task via subprocess.

Expand Down Expand Up @@ -1912,7 +1917,7 @@ def _execute_task_subprocess(
# Use Popen instead of run for process tracking
process = subprocess.Popen(
cmd,
cwd=workspace.repo_path,
cwd=str(worktree_path) if worktree_path else workspace.repo_path,
stdout=None, # Let output flow to terminal
stderr=None,
text=True,
Expand Down
175 changes: 175 additions & 0 deletions codeframe/core/worktrees.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
"""Worktree-per-task isolation for parallel batch execution.

Creates isolated git worktrees so parallel agents don't modify files in the
same working directory. Each task gets its own branch and working tree,
then merges back to the base branch on completion.

Lifecycle:
1. create(workspace_path, task_id) → worktree path
2. Agent runs with cwd set to worktree
3. merge_back(workspace_path, task_id) → MergeResult
4. cleanup(workspace_path, task_id)
"""

from __future__ import annotations

import logging
import subprocess
from dataclasses import dataclass
from pathlib import Path
from typing import Optional

logger = logging.getLogger(__name__)

WORKTREE_DIR = ".codeframe/worktrees"


@dataclass
class MergeResult:
"""Result from merging a worktree branch back to base."""

task_id: str
success: bool
conflict_details: str
merge_commit: Optional[str]


class TaskWorktree:
"""Manages git worktrees for isolated parallel task execution."""

def create(
self,
workspace_path: Path,
task_id: str,
base_branch: str = "main",
) -> Path:
"""Create an isolated worktree for a task.

Args:
workspace_path: Root of the git repository
task_id: Task identifier (used for branch and directory name)
base_branch: Branch to base the worktree on

Returns:
Path to the created worktree directory

Raises:
subprocess.CalledProcessError: If git worktree creation fails
"""
worktree_path = workspace_path / WORKTREE_DIR / task_id
worktree_path.parent.mkdir(parents=True, exist_ok=True)
branch_name = f"cf/{task_id}"

subprocess.run(
["git", "worktree", "add", str(worktree_path), "-b", branch_name, base_branch],
cwd=str(workspace_path),
capture_output=True,
text=True,
check=True,
)

logger.info("Created worktree for %s at %s", task_id, worktree_path)
return worktree_path
Comment thread
coderabbitai[bot] marked this conversation as resolved.

def merge_back(
self,
workspace_path: Path,
task_id: str,
base_branch: str = "main",
) -> MergeResult:
"""Merge worktree branch back to base branch.

Args:
workspace_path: Root of the git repository
task_id: Task identifier
base_branch: Branch to merge into

Returns:
MergeResult with success status and optional conflict details
"""
branch_name = f"cf/{task_id}"

# Checkout base branch
subprocess.run(
["git", "checkout", base_branch],
cwd=str(workspace_path),
capture_output=True,
text=True,
check=True,
)
Comment thread
coderabbitai[bot] marked this conversation as resolved.

# Attempt merge
result = subprocess.run(
["git", "merge", branch_name, "--no-ff", "-m", f"Merge {branch_name} into {base_branch}"],
cwd=str(workspace_path),
capture_output=True,
text=True,
)

if result.returncode == 0:
# Get merge commit hash
head = subprocess.run(
["git", "rev-parse", "HEAD"],
cwd=str(workspace_path),
capture_output=True,
text=True,
)
merge_commit = head.stdout.strip() if head.returncode == 0 else None

logger.info("Merged %s back to %s", branch_name, base_branch)
return MergeResult(
task_id=task_id,
success=True,
conflict_details="",
merge_commit=merge_commit,
)
else:
# Merge conflict — abort and report
conflict_output = result.stdout + result.stderr
subprocess.run(
["git", "merge", "--abort"],
cwd=str(workspace_path),
capture_output=True,
)

logger.warning("Merge conflict for %s: %s", branch_name, conflict_output[:200])
return MergeResult(
task_id=task_id,
success=False,
conflict_details=conflict_output[:2000],
merge_commit=None,
)

def cleanup(
self,
workspace_path: Path,
task_id: str,
) -> None:
"""Remove worktree and delete task branch.

Never raises — cleanup failures are logged as warnings.
"""
worktree_path = workspace_path / WORKTREE_DIR / task_id
branch_name = f"cf/{task_id}"

# Remove worktree
try:
subprocess.run(
["git", "worktree", "remove", str(worktree_path), "--force"],
cwd=str(workspace_path),
capture_output=True,
text=True,
)
except Exception as exc:
logger.warning("Failed to remove worktree for %s: %s", task_id, exc)

# Delete branch
try:
subprocess.run(
["git", "branch", "-D", branch_name],
cwd=str(workspace_path),
capture_output=True,
text=True,
)
except Exception as exc:
Comment on lines +156 to +174

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Git command failures are silently ignored in cleanup.

The docstring promises "cleanup failures are logged as warnings," but subprocess.run without check=True won't raise on non-zero exit codes. The try/except only catches Python-level exceptions (e.g., git not found), not git command failures. If the worktree removal or branch deletion fails, nothing is logged.

🐛 Proposed fix to log git command failures
         # Remove worktree
         try:
-            subprocess.run(
+            result = subprocess.run(
                 ["git", "worktree", "remove", str(worktree_path), "--force"],
                 cwd=str(workspace_path),
                 capture_output=True,
                 text=True,
             )
+            if result.returncode != 0:
+                logger.warning("Failed to remove worktree for %s: %s", task_id, result.stderr)
         except Exception as exc:
             logger.warning("Failed to remove worktree for %s: %s", task_id, exc)

         # Delete branch
         try:
-            subprocess.run(
+            result = subprocess.run(
                 ["git", "branch", "-D", branch_name],
                 cwd=str(workspace_path),
                 capture_output=True,
                 text=True,
             )
+            if result.returncode != 0:
+                logger.warning("Failed to delete branch %s: %s", branch_name, result.stderr)
         except Exception as exc:
             logger.warning("Failed to delete branch %s: %s", branch_name, exc)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@codeframe/core/worktrees.py` around lines 156 - 174, The cleanup currently
calls subprocess.run for git commands without check=True so non-zero git exits
are not raised or logged; update the two subprocess.run invocations (the ["git",
"worktree", "remove", ...] call and the ["git", "branch", "-D", ...] call in
this module) to detect failures and log them as warnings: either pass check=True
so CalledProcessError is raised and caught by the existing except block, or
capture the CompletedProcess result and on non-zero returncode call
logger.warning with a descriptive message that includes task_id/branch_name and
the process's stdout/stderr; ensure you still catch and log other Exceptions as
before.

logger.warning("Failed to delete branch %s: %s", branch_name, exc)
Loading
Loading