feat(isolation): re-enable worktree isolation with real merge-back (#787) (#867)

* feat(isolation): re-enable worktree isolation with real merge-back (#787)

Re-enables --isolation worktree for the single-run path (cf work start) with
auto-commit + merge-back, reversing the #714 interim hard-reject.

- worktrees: TaskWorktree.auto_commit() stages+commits worktree changes
- sandbox/context: rebased_workspace() keeps state on main / code on worktree;
  ExecutionContext gains merge_back/preserve; validate_isolation no longer
  rejects WORKTREE (CLOUD still raises); create_execution_context builds a real
  worktree (not registered so orphan cleanup can't nuke a preserved branch)
- builtin adapters (#715): react/plan run against workspace_path
- verification_wrapper (#716): gates + quick-fixes run against the worktree
- runtime: merge-back on success; conflict -> blocker + preserve; fail -> preserve
- cli: work start accepts worktree; work batch run rejects it (subprocess can't
  reach the gitignored .codeframe DB from a worktree) with a batch-specific message

Tests: new test_worktree_isolation.py (file-on-base after success, conflict->blocker
+preserve, fail->preserve, #715/#716 threading); updated sandbox/exit-code tests
from the #714 disable contract to the #787 enable contract.

* fix(isolation): harden worktree merge-back per review (#787)

Addresses defects found by opencode/GLM + CI review bots:

- auto_commit fails loud (critical): git add/commit now raise on failure instead
  of reporting success — a failed stage/commit preserves the branch rather than
  merging nothing then deleting it (the #714 silent-data-loss class).
- Preserved-branch collision (major): create_execution_context moved inside
  execute_agent's try + a clear ValueError when a leftover cf/<task> branch or
  worktree exists — a retry becomes a handled FAILED (preserved), not a stranded
  IN_PROGRESS run.
- engine_stats miscount (major): re-sync agent_status to the final state so a
  merge-back conflict is recorded BLOCKED, not COMPLETED.
- Concurrent merge-back (major): serialize the checkout+merge on the shared main
  tree with a cross-process flock (_main_tree_lock).
- Dirty-tree checkout failure now routes through the merge-back error -> blocker
  + preserve (try/except around merge_back()).

Tests: auto_commit-raises regression, preserved-branch-collision, registry-empty
assertion; fixed mock-workspace path mismatches in test_builtin /
test_verification_wrapper / test_engine_registry_extended (rebased_workspace calls
dataclasses.replace, which needs workspace_path==repo_path for the mock no-op).

Author: frankbria

codeframe/cli/app.py

@@ -2489,12 +2489,12 @@ def work_start(
 
         task = matching[0]
 
-        # Reject unsupported isolation first (#714): worktree would silently
-        # discard agent work. Fails closed before any run is created.
+        # Reject unsupported isolation first (before any run is created). WORKTREE
+        # is now accepted for this single-run path (#787); CLOUD still raises.
         from codeframe.core.sandbox.context import IsolationLevel, validate_isolation
         try:
             validate_isolation(IsolationLevel(isolation))
-        except ValueError as exc:
+        except (ValueError, NotImplementedError) as exc:
             console.print(f"[red]Error:[/red] {exc}")
             raise typer.Exit(1)
 
@@ -3800,7 +3800,6 @@ def batch_run(
         codeframe work batch run --all-ready --engine plan
         codeframe work batch run task1 task2 --dry-run
         codeframe work batch run task1 task2 --retry 2
-        codeframe work batch run --all-ready --isolation worktree
     """
     from codeframe.core.workspace import get_workspace
     from codeframe.core import tasks as tasks_module, conductor
@@ -3859,12 +3858,23 @@ def batch_run(
             console.print("[red]Error:[/red] Specify task IDs or use --all-ready/--all-blocked")
             raise typer.Exit(1)
 
-        # Reject unsupported isolation up front (#714) — before the API-key
-        # check and before any run is created (worktree would discard work).
+        # Reject unsupported isolation up front — before the API-key check and
+        # before any batch is created. CLOUD is not implemented; WORKTREE is
+        # enabled only for the single-run path (`cf work start`, #787) — the
+        # batch subprocess path can't reach the gitignored .codeframe DB from a
+        # worktree, so it stays rejected here until that's solved.
         from codeframe.core.sandbox.context import IsolationLevel, validate_isolation
+        if IsolationLevel(isolation) == IsolationLevel.WORKTREE:
+            console.print(
+                "[red]Error:[/red] worktree isolation is not yet supported for "
+                "batch runs (subprocess workers can't reach the workspace state "
+                "DB in a worktree). Use it with a single task: "
+                "`cf work start <task> --execute --isolation worktree`."
+            )
+            raise typer.Exit(1)
         try:
             validate_isolation(IsolationLevel(isolation))
-        except ValueError as exc:
+        except (ValueError, NotImplementedError) as exc:
             console.print(f"[red]Error:[/red] {exc}")
             raise typer.Exit(1)
 

codeframe/core/adapters/builtin.py

@@ -74,15 +74,21 @@ def run(
     ) -> AgentResult:
         """Run the ReactAgent with stall retry, map AgentStatus to AgentResult."""
         from codeframe.core.react_agent import ReactAgent
+        from codeframe.core.sandbox.context import rebased_workspace
         from codeframe.core.stall_detector import StallDetectedError
 
+        # #715: when isolated in a worktree, run against workspace_path so code
+        # I/O and gates land in the worktree; task/blocker state stays on the
+        # main-repo DB (rebased_workspace keeps state_dir unchanged).
+        run_workspace = rebased_workspace(self._workspace, workspace_path)
+
         def _bridge_event(event_type: str, data: dict) -> None:
             if on_event:
                 on_event(AgentEvent(type=event_type, data=data))
 
         def _build_agent() -> ReactAgent:
             kwargs: dict = {
-                "workspace": self._workspace,
+                "workspace": run_workspace,
                 "llm_provider": self._llm_provider,
                 "stall_timeout_s": self._stall_timeout_s,
                 "event_publisher": self._event_publisher,
@@ -179,14 +185,18 @@ def run(
     ) -> AgentResult:
         """Run the plan-based Agent with supervisor retry, map to AgentResult."""
         from codeframe.core.agent import Agent, AgentStatus
+        from codeframe.core.sandbox.context import rebased_workspace
+
+        # #715: run against the worktree path when isolated (state stays on main).
+        run_workspace = rebased_workspace(self._workspace, workspace_path)
 
         def _bridge_event(event_type: str, data: dict) -> None:
             if on_event:
                 on_event(AgentEvent(type=event_type, data=data))
 
         def _build_agent() -> Agent:
             return Agent(
-                workspace=self._workspace,
+                workspace=run_workspace,
                 llm_provider=self._llm_provider,
                 dry_run=self._dry_run,
                 on_event=_bridge_event,

codeframe/core/adapters/verification_wrapper.py

@@ -53,6 +53,10 @@ def __init__(
     ) -> None:
         self._inner = inner
         self._workspace = workspace
+        # Workspace that verification gates + quick fixes run against. Rebased to
+        # the worktree in run() when isolated (#716); blocker creation still uses
+        # self._workspace so blockers land in the main-repo state DB.
+        self._verify_workspace = workspace
         self._max_correction_rounds = max_correction_rounds
         self._gate_names = gate_names  # None = use default gates
         self._verbose = verbose
@@ -70,6 +74,10 @@ def run(
         on_event: Callable[[AgentEvent], None] | None = None,
     ) -> AgentResult:
         """Run the inner adapter, then verify with gates. Self-correct on failure."""
+        from codeframe.core.sandbox.context import rebased_workspace
+
+        # #716: verify against the worktree when isolated (no-op for NONE).
+        self._verify_workspace = rebased_workspace(self._workspace, workspace_path)
 
         # Initial run
         result = self._inner.run(task_id, prompt, workspace_path, on_event)
@@ -90,7 +98,7 @@ def run(
                 ))
 
             gate_result = run_gates(
-                self._workspace,
+                self._verify_workspace,
                 gates=self._gate_names,
                 verbose=self._verbose,
             )
@@ -160,7 +168,7 @@ def run(
 
         # Final gate check after all correction rounds
         gate_result = run_gates(
-            self._workspace,
+            self._verify_workspace,
             gates=self._gate_names,
             verbose=self._verbose,
         )
@@ -179,11 +187,11 @@ def _try_quick_fix(self, error_summary: str) -> bool:
 
         Returns True if a fix was successfully applied.
         """
-        fix = find_quick_fix(error_summary, repo_path=self._workspace.repo_path)
+        fix = find_quick_fix(error_summary, repo_path=self._verify_workspace.repo_path)
         if fix is None:
             return False
 
-        success, msg = apply_quick_fix(fix, self._workspace.repo_path)
+        success, msg = apply_quick_fix(fix, self._verify_workspace.repo_path)
         if success:
             self._verbose_print(f"[VerificationWrapper] Quick fix: {msg}")
         return success

codeframe/core/conductor.py

@@ -1701,9 +1701,9 @@ def _execute_parallel(
         CycleDetectedError: If circular dependencies are detected
     """
     # Clean up orphaned worktrees from crashed workers on previous runs.
-    # Legacy-record path only (#714): worktree isolation is disabled, so this
-    # fires just for batches persisted before the fix — harmless best-effort
-    # cleanup of pre-existing debris.
+    # Batch worktree isolation stays rejected at the CLI (#787 enables it only
+    # for the single-run path), so this fires just for legacy batches persisted
+    # with worktree isolation — harmless best-effort cleanup of pre-existing debris.
     from codeframe.core.sandbox.context import IsolationLevel as _IL
     if batch.isolation == _IL.WORKTREE.value:
         from codeframe.core.worktrees import WorktreeRegistry

codeframe/core/runtime.py

@@ -710,14 +710,27 @@ def execute_agent(
     import time as _time_mod
     _perf_start_ms = int(_time_mod.monotonic() * 1000)
 
-    # Create execution context (handles isolation; NONE is a no-op)
-    from codeframe.core.sandbox.context import IsolationLevel, create_execution_context
-    exec_ctx = create_execution_context(
-        run.task_id, IsolationLevel(isolation), workspace.repo_path
+    from codeframe.core.sandbox.context import (
+        ExecutionContext,
+        IsolationLevel,
+        create_execution_context,
     )
-    effective_repo_path = exec_ctx.workspace_path
+    # Execution context is created INSIDE the try (#787): building a worktree runs
+    # git, which can fail (e.g. a preserved cf/<task_id> branch from a prior run).
+    # Creating it here would let that exception escape execute_agent and strand the
+    # run IN_PROGRESS; inside the try it becomes a handled FAILED with preservation.
+    exec_ctx: "ExecutionContext | None" = None
+    effective_repo_path = workspace.repo_path
+    # Worktree isolation (#787): tracks whether agent work was merged back so the
+    # finally block cleans up (merged) vs preserves the branch (failed/conflict).
+    worktree_merged = False
 
     try:
+        exec_ctx = create_execution_context(
+            run.task_id, IsolationLevel(isolation), workspace.repo_path
+        )
+        effective_repo_path = exec_ctx.workspace_path
+
         # Execute before_task hook (aborts on failure)
         if env_config and hook_ctx:
             try:
@@ -830,6 +843,61 @@ def on_adapter_event(event: AdapterEvent) -> None:
             )
             state.blocker = blocker_obj
 
+        # Worktree isolation (#787): on a successful run, auto-commit + merge the
+        # task branch back to base. A merge conflict (or an auto-commit failure)
+        # becomes a blocker and the branch is preserved (downgrade COMPLETED →
+        # BLOCKED so the run reflects it before the status transition below).
+        # worktree_merged is only set True on a clean merge, so any other outcome
+        # — including an exception — routes the finally block to preserve().
+        # Runs here only when exec_ctx has a worktree (merge_back is None for NONE).
+        if exec_ctx.merge_back is not None and state.status == AgentStatus.COMPLETED:
+            from codeframe.core import blockers as blockers_mod
+            try:
+                merge_result = exec_ctx.merge_back()
+            except Exception as merge_exc:
+                # Auto-commit / merge raised (e.g. commit hook rejection, git
+                # error). Never proceed to cleanup — that would discard the
+                # agent's uncommitted work (the #714 class of bug).
+                block_q = (
+                    f"Worktree merge-back failed for cf/{run.task_id}: {merge_exc}. "
+                    "The branch and worktree have been preserved for recovery."
+                )
+                blocker_obj = blockers_mod.create(
+                    workspace, task_id=run.task_id, question=block_q,
+                )
+                state = AgentState(status=AgentStatus.BLOCKED)
+                state.blocker = blocker_obj
+                run_logger.error(
+                    LogCategory.BLOCKER,
+                    f"Worktree merge-back error for {run.task_id}",
+                    {"error": str(merge_exc)[:500]},
+                )
+            else:
+                if not merge_result.success:
+                    conflict_q = (
+                        f"Worktree merge-back conflicted merging cf/{run.task_id} "
+                        "into the base branch. Resolve manually — the branch and "
+                        "worktree have been preserved for recovery.\n\n"
+                        f"{merge_result.conflict_details[:1000]}"
+                    )
+                    blocker_obj = blockers_mod.create(
+                        workspace, task_id=run.task_id, question=conflict_q,
+                    )
+                    state = AgentState(status=AgentStatus.BLOCKED)
+                    state.blocker = blocker_obj
+                    run_logger.warning(
+                        LogCategory.BLOCKER,
+                        f"Worktree merge-back conflict for {run.task_id}",
+                        {"conflict": merge_result.conflict_details[:500]},
+                    )
+                else:
+                    worktree_merged = True
+
+        # Re-sync agent_status to the final state: merge-back may have downgraded
+        # a COMPLETED run to BLOCKED, and engine_stats.record_run below keys off
+        # agent_status — without this a conflicted run is miscounted as COMPLETED.
+        agent_status = state.status
+
         # Log final status
         if state.status == AgentStatus.COMPLETED:
             run_logger.info(LogCategory.STATE_CHANGE, "Agent completed successfully")
@@ -928,8 +996,17 @@ def on_adapter_event(event: AdapterEvent) -> None:
     finally:
         # Always close the output logger to ensure file is properly flushed
         output_logger.close()
-        # Clean up execution context (no-op for NONE, removes worktree for WORKTREE)
-        exec_ctx.cleanup()
+        # Clean up execution context. For NONE this is a harmless no-op. For a
+        # WORKTREE run: remove the worktree + branch only when work was merged
+        # back; otherwise preserve them for recovery (failure/blocked/conflict/
+        # exception — never silently discard agent work, the #714 bug).
+        # exec_ctx is None when create_execution_context itself failed (e.g. a
+        # preserved branch collision) — nothing to clean up or preserve here.
+        if exec_ctx is not None:
+            if exec_ctx.merge_back is not None and not worktree_merged:
+                exec_ctx.preserve()
+            else:
+                exec_ctx.cleanup()
 
 
 def _event_type_to_category(event_type: str):