From f0923e08ba61b5f352bd3cc87f9d12e1881e95fe Mon Sep 17 00:00:00 2001 From: Hamza Khyari Date: Wed, 22 Apr 2026 12:43:50 +0000 Subject: [PATCH] feat(plan): create per-project sandboxes for multi-project plans MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per spec §19310-19312, each resource gets its own sandbox and Apply commits each sandbox separately. When a plan is linked to multiple projects with git-checkout resources: - _create_sandbox_for_plan creates a worktree for EACH resource (not just the first), returning a list of _SandboxInfo objects - LLM output is written to the first resource's worktree (primary) - _route_sandbox_files_to_worktrees moves files that belong to other resources into their worktrees by matching against each resource's git ls-files output - Each worktree is committed independently - _apply_sandbox_changes merges each resource's worktree separately, showing per-resource Apply Summary and Sandbox Cleanup panels - Partial apply: if one resource's merge fails, others still proceed Single-resource plans are fully backward compatible — same behavior as before. ISSUES CLOSED: #7270 --- features/multi_project_sandbox.feature | 63 +++ features/steps/multi_project_sandbox_steps.py | 374 ++++++++++++++++++ .../steps/plan_cli_coverage_boost_steps.py | 2 +- .../plan_lifecycle_commands_coverage_steps.py | 4 +- ...tdd_plan_execute_phase_processing_steps.py | 4 +- src/cleveragents/cli/commands/plan.py | 269 ++++++++++--- 6 files changed, 650 insertions(+), 66 deletions(-) create mode 100644 features/multi_project_sandbox.feature create mode 100644 features/steps/multi_project_sandbox_steps.py diff --git a/features/multi_project_sandbox.feature b/features/multi_project_sandbox.feature new file mode 100644 index 000000000..fe0a85b22 --- /dev/null +++ b/features/multi_project_sandbox.feature @@ -0,0 +1,63 @@ +@multi-project-sandbox +Feature: Per-resource sandboxes for multi-project plans (#7270) + Per spec §19310-19312, each resource gets its own sandbox and + Apply commits each sandbox separately. + + Scenario: Single-resource plan creates one sandbox for mps + Given a temp git project "alpha" for mps + And a mocked plan service linking project "alpha" for mps + When I call _create_sandbox_for_plan for mps + Then sandbox_infos should have 1 entry for mps + And sandbox_root should be a directory for mps + + Scenario: Multi-resource plan creates sandboxes for each resource for mps + Given a temp git project "alpha" for mps + And a temp git project "beta" for mps + And a mocked plan service linking projects "alpha" and "beta" for mps + When I call _create_sandbox_for_plan for mps + Then sandbox_infos should have 2 entries for mps + And each sandbox_info should have a different sandbox_path for mps + + Scenario: Route files moves file to correct worktree for mps + Given a temp git project named "alpha" containing "src/app.py" for mps + And a temp git project named "beta" containing "src/api.py" for mps + And a mocked plan service linking projects "alpha" and "beta" for mps + And sandbox_infos for both projects for mps + And a file "src/api.py" exists in the primary sandbox for mps + When I call _route_sandbox_files_to_worktrees for mps + Then "src/api.py" should exist in the beta sandbox for mps + And "src/api.py" should not exist in the alpha sandbox for mps + + Scenario: Route files preserves primary file when both projects share path for mps + Given a temp git project named "alpha" containing "README.md" for mps + And a temp git project named "beta" containing "README.md" for mps + And a mocked plan service linking projects "alpha" and "beta" for mps + And sandbox_infos for both projects for mps + And the file "README.md" in the primary sandbox is overwritten with "ROUTED_CONTENT" for mps + When I call _route_sandbox_files_to_worktrees for mps + Then "README.md" in the alpha sandbox should contain "ROUTED_CONTENT" for mps + And "README.md" in the beta sandbox should not contain "ROUTED_CONTENT" for mps + + Scenario: Route files is a no-op for single resource for mps + Given a temp git project named "alpha" containing "src/app.py" for mps + And sandbox_infos with only one entry for mps + And a file "src/app.py" exists in the primary sandbox for mps + When I call _route_sandbox_files_to_worktrees for mps + Then "src/app.py" should still exist in the alpha sandbox for mps + + Scenario: Apply merges multiple worktrees separately for mps + Given a temp git project "alpha" with a worktree branch for mps + And a temp git project "beta" with a worktree branch for mps + And a mocked plan service linking projects "alpha" and "beta" for mps + When I call _apply_sandbox_changes for mps + Then both projects should have the merged changes for mps + And the console output should contain "Apply Summary" for mps + + Scenario: Partial apply continues when one merge fails for mps + Given a temp git project "alpha" with a worktree branch for mps + And a temp git project "beta" with a conflicting worktree branch for mps + And a mocked plan service linking projects "alpha" and "beta" for mps + When I call _apply_sandbox_changes for mps + Then alpha should have the merged changes for mps + And beta should have the original content for mps + And _apply_sandbox_changes should return False for mps diff --git a/features/steps/multi_project_sandbox_steps.py b/features/steps/multi_project_sandbox_steps.py new file mode 100644 index 000000000..47225fd7f --- /dev/null +++ b/features/steps/multi_project_sandbox_steps.py @@ -0,0 +1,374 @@ +"""Steps for multi_project_sandbox.feature.""" + +from __future__ import annotations + +import os +import shutil +import subprocess +import tempfile +from io import StringIO +from pathlib import Path +from unittest.mock import MagicMock, patch + +from behave import given, then, when + +_PLAN_ID = "01TESTMULTIPROJ000000000000" + + +def _git(args: list[str], cwd: str) -> subprocess.CompletedProcess[str]: + return subprocess.run( + ["git", *args], + cwd=cwd, + capture_output=True, + text=True, + check=True, + timeout=10, + ) + + +def _init_git_repo(path: str) -> None: + _git(["init", "-q", "-b", "main"], path) + _git(["config", "user.name", "T"], path) + _git(["config", "user.email", "t@t"], path) + _git(["config", "commit.gpgsign", "false"], path) + + +# ── Given ────────────────────────────────────────────── + + +@given('a temp git project "{name}" for mps') +def step_create_project(context: object, name: str) -> None: + if not hasattr(context, "mps_projects"): + context.mps_projects = {} + d = tempfile.mkdtemp(prefix=f"mps-{name}-") + context.add_cleanup(shutil.rmtree, d, True) + _init_git_repo(d) + Path(d, "README.md").write_text(f"# {name}\n") + _git(["add", "."], d) + _git(["commit", "-q", "-m", "init"], d) + context.mps_projects[name] = d + + +@given('a temp git project named "{name}" containing "{filename}" for mps') +def step_create_project_with_file(context: object, name: str, filename: str) -> None: + if not hasattr(context, "mps_projects"): + context.mps_projects = {} + d = tempfile.mkdtemp(prefix=f"mps-{name}-") + context.add_cleanup(shutil.rmtree, d, True) + _init_git_repo(d) + fpath = os.path.join(d, filename) + os.makedirs(os.path.dirname(fpath), exist_ok=True) + Path(fpath).write_text(f"# {name} {filename}\n") + _git(["add", "."], d) + _git(["commit", "-q", "-m", "init"], d) + context.mps_projects[name] = d + + +@given('a temp git project "{name}" with a worktree branch for mps') +def step_create_project_with_worktree(context: object, name: str) -> None: + if not hasattr(context, "mps_projects"): + context.mps_projects = {} + d = tempfile.mkdtemp(prefix=f"mps-{name}-") + context.add_cleanup(shutil.rmtree, d, True) + _init_git_repo(d) + Path(d, f"{name}.py").write_text(f"# original {name}\n") + _git(["add", "."], d) + _git(["commit", "-q", "-m", "init"], d) + + branch = f"cleveragents/plan-{_PLAN_ID}" + wt_dir = tempfile.mkdtemp(prefix=f"mps-wt-{name}-") + context.add_cleanup(shutil.rmtree, wt_dir, True) + _git(["worktree", "add", "-b", branch, wt_dir, "HEAD"], d) + Path(wt_dir, f"{name}.py").write_text(f"# fixed {name}\n") + _git(["add", "."], wt_dir) + _git(["commit", "-q", "-m", f"fix {name}"], wt_dir) + + context.mps_projects[name] = d + + +@given('a temp git project "{name}" with a conflicting worktree branch for mps') +def step_create_project_with_conflict(context: object, name: str) -> None: + if not hasattr(context, "mps_projects"): + context.mps_projects = {} + d = tempfile.mkdtemp(prefix=f"mps-{name}-") + context.add_cleanup(shutil.rmtree, d, True) + _init_git_repo(d) + Path(d, f"{name}.py").write_text(f"# original {name}\n") + _git(["add", "."], d) + _git(["commit", "-q", "-m", "init"], d) + + branch = f"cleveragents/plan-{_PLAN_ID}" + wt_dir = tempfile.mkdtemp(prefix=f"mps-wt-{name}-") + context.add_cleanup(shutil.rmtree, wt_dir, True) + _git(["worktree", "add", "-b", branch, wt_dir, "HEAD"], d) + Path(wt_dir, f"{name}.py").write_text(f"# branch {name}\n") + _git(["add", "."], wt_dir) + _git(["commit", "-q", "-m", f"branch {name}"], wt_dir) + + # Create conflict on main + Path(d, f"{name}.py").write_text(f"# main {name}\n") + _git(["add", "."], d) + _git(["commit", "-q", "-m", f"main {name}"], d) + + context.mps_projects[name] = d + + +def _build_mocks(context: object, project_names: list[str]) -> tuple: + """Build mock service + container for the given projects.""" + links = [] + resources = {} + for name in project_names: + rid = f"res-mps-{name}" + mock_lr = MagicMock() + mock_lr.resource_id = rid + links.append((name, mock_lr)) + + mock_resource = MagicMock() + mock_resource.resource_type_name = "git-checkout" + mock_resource.location = context.mps_projects[name] + mock_resource.resource_id = rid + resources[rid] = mock_resource + + mock_plan = MagicMock() + mock_plan.project_links = [MagicMock(project_name=name) for name, _ in links] + + mock_service = MagicMock() + mock_service.get_plan.return_value = mock_plan + + mock_projects = {} + for name, lr in links: + mock_proj = MagicMock() + mock_proj.linked_resources = [lr] + mock_projects[name] = mock_proj + + mock_project_repo = MagicMock() + mock_project_repo.get.side_effect = lambda n: mock_projects.get(n) + + mock_resource_registry = MagicMock() + mock_resource_registry.show_resource.side_effect = lambda rid: resources[rid] + + mock_container = MagicMock() + mock_container.namespaced_project_repo.return_value = mock_project_repo + mock_container.resource_registry_service.return_value = mock_resource_registry + + context.mps_service = mock_service + context.mps_container = mock_container + return mock_service, mock_container + + +@given('a mocked plan service linking project "{name}" for mps') +def step_mock_single(context: object, name: str) -> None: + _build_mocks(context, [name]) + + +@given('a mocked plan service linking projects "{a}" and "{b}" for mps') +def step_mock_multi(context: object, a: str, b: str) -> None: + _build_mocks(context, [a, b]) + + +@given("sandbox_infos for both projects for mps") +def step_create_sandbox_infos(context: object) -> None: + from cleveragents.cli.commands.plan import _create_sandbox_for_plan + + with patch( + "cleveragents.application.container.get_container", + return_value=context.mps_container, + ): + context.mps_sandbox_root, context.mps_sandbox_infos = _create_sandbox_for_plan( + _PLAN_ID, context.mps_service + ) + for info in context.mps_sandbox_infos: + context.add_cleanup(info.sandbox_obj.cleanup) + + +@given("sandbox_infos with only one entry for mps") +def step_create_single_sandbox_info(context: object) -> None: + names = list(context.mps_projects.keys()) + _build_mocks(context, [names[0]]) + + from cleveragents.cli.commands.plan import _create_sandbox_for_plan + + with patch( + "cleveragents.application.container.get_container", + return_value=context.mps_container, + ): + context.mps_sandbox_root, context.mps_sandbox_infos = _create_sandbox_for_plan( + _PLAN_ID, context.mps_service + ) + for info in context.mps_sandbox_infos: + context.add_cleanup(info.sandbox_obj.cleanup) + + +@given('a file "{filename}" exists in the primary sandbox for mps') +def step_write_file_to_primary(context: object, filename: str) -> None: + primary = context.mps_sandbox_infos[0] + fpath = os.path.join(primary.sandbox_path, filename) + os.makedirs(os.path.dirname(fpath), exist_ok=True) + Path(fpath).write_text("# routed content\n") + + +# ── When ─────────────────────────────────────────────── + + +@when("I call _create_sandbox_for_plan for mps") +def step_call_create(context: object) -> None: + from cleveragents.cli.commands.plan import _create_sandbox_for_plan + + with patch( + "cleveragents.application.container.get_container", + return_value=context.mps_container, + ): + context.mps_sandbox_root, context.mps_sandbox_infos = _create_sandbox_for_plan( + _PLAN_ID, context.mps_service + ) + for info in context.mps_sandbox_infos: + context.add_cleanup(info.sandbox_obj.cleanup) + + +@when("I call _route_sandbox_files_to_worktrees for mps") +def step_call_route(context: object) -> None: + from cleveragents.cli.commands.plan import _route_sandbox_files_to_worktrees + + _route_sandbox_files_to_worktrees(context.mps_sandbox_infos) + + +@when("I call _apply_sandbox_changes for mps") +def step_call_apply(context: object) -> None: + from rich.console import Console + + from cleveragents.cli.commands.plan import _apply_sandbox_changes + + output = StringIO() + console = Console(file=output, width=200) + + with patch( + "cleveragents.application.container.get_container", + return_value=context.mps_container, + ): + context.mps_apply_result = _apply_sandbox_changes( + _PLAN_ID, + context.mps_service, + console, + ) + context.mps_console_output = output.getvalue() + + +# ── Then ─────────────────────────────────────────────── + + +@then("sandbox_infos should have {count:d} entry for mps") +@then("sandbox_infos should have {count:d} entries for mps") +def step_check_count(context: object, count: int) -> None: + assert len(context.mps_sandbox_infos) == count, ( + f"Expected {count} sandbox_infos, got {len(context.mps_sandbox_infos)}" + ) + + +@then("sandbox_root should be a directory for mps") +def step_check_root_dir(context: object) -> None: + assert os.path.isdir(context.mps_sandbox_root), ( + f"sandbox_root is not a directory: {context.mps_sandbox_root}" + ) + + +@then("each sandbox_info should have a different sandbox_path for mps") +def step_check_unique_paths(context: object) -> None: + paths = [info.sandbox_path for info in context.mps_sandbox_infos] + assert len(paths) == len(set(paths)), f"Duplicate sandbox paths: {paths}" + + +@then('"{filename}" should exist in the beta sandbox for mps') +def step_file_in_beta(context: object, filename: str) -> None: + beta_info = context.mps_sandbox_infos[1] + fpath = os.path.join(beta_info.sandbox_path, filename) + assert os.path.isfile(fpath), f"{filename} not found in beta sandbox" + + +@then('"{filename}" should not exist in the alpha sandbox for mps') +def step_file_not_in_alpha(context: object, filename: str) -> None: + alpha_info = context.mps_sandbox_infos[0] + fpath = os.path.join(alpha_info.sandbox_path, filename) + assert not os.path.isfile(fpath), f"{filename} still in alpha sandbox" + + +@then('"{filename}" should still exist in the alpha sandbox for mps') +def step_file_still_in_alpha(context: object, filename: str) -> None: + alpha_info = context.mps_sandbox_infos[0] + fpath = os.path.join(alpha_info.sandbox_path, filename) + assert os.path.isfile(fpath), f"{filename} not found in alpha sandbox" + + +@then("both projects should have the merged changes for mps") +def step_both_merged(context: object) -> None: + for name, path in context.mps_projects.items(): + content = Path(path, f"{name}.py").read_text() + assert "fixed" in content, f"Project {name} not merged: {content}" + + +@then("alpha should have the merged changes for mps") +def step_alpha_merged(context: object) -> None: + path = context.mps_projects["alpha"] + content = Path(path, "alpha.py").read_text() + assert "fixed" in content, f"Alpha not merged: {content}" + + +@given( + 'the file "{filename}" in the primary sandbox is overwritten with ' + '"{content}" for mps' +) +def step_overwrite_primary_file(context: object, filename: str, content: str) -> None: + primary = context.mps_sandbox_infos[0] + fpath = os.path.join(primary.sandbox_path, filename) + Path(fpath).write_text(content + "\n") + + +@then('"{filename}" in the alpha sandbox should contain "{text}" for mps') +def step_alpha_file_contains(context: object, filename: str, text: str) -> None: + alpha_info = context.mps_sandbox_infos[0] + content = Path(alpha_info.sandbox_path, filename).read_text() + assert text in content, ( + f"Expected '{text}' in alpha's {filename} but got: {content}" + ) + + +@then('"{filename}" in the beta sandbox should not contain "{text}" for mps') +def step_beta_file_not_contains(context: object, filename: str, text: str) -> None: + beta_info = context.mps_sandbox_infos[1] + fpath = os.path.join(beta_info.sandbox_path, filename) + if not os.path.isfile(fpath): + return # File doesn't exist in beta — that's fine + content = Path(fpath).read_text() + assert text not in content, ( + f"'{text}' should not be in beta's {filename} but found: {content}" + ) + + +@then('"{filename}" should not exist in the beta sandbox for mps') +def step_file_not_in_beta(context: object, filename: str) -> None: + beta_info = context.mps_sandbox_infos[1] + fpath = os.path.join(beta_info.sandbox_path, filename) + assert not os.path.isfile(fpath), f"{filename} found in beta sandbox" + + +@then('the console output should contain "Apply Summary" for mps') +def step_console_has_apply_summary(context: object) -> None: + output = context.mps_console_output + assert "Apply Summary" in output, ( + f"Expected 'Apply Summary' in console output but got:\n{output[:500]}" + ) + + +@then("beta should have the original content for mps") +def step_beta_unchanged(context: object) -> None: + path = context.mps_projects["beta"] + content = Path(path, "beta.py").read_text() + assert "original" in content or "main" in content, ( + f"Expected beta to be unchanged but got: {content}" + ) + + +@then("_apply_sandbox_changes should return False for mps") +def step_apply_returns_false(context: object) -> None: + assert context.mps_apply_result is False, ( + f"Expected False but got {context.mps_apply_result}" + ) diff --git a/features/steps/plan_cli_coverage_boost_steps.py b/features/steps/plan_cli_coverage_boost_steps.py index 2318c9584..f95820626 100644 --- a/features/steps/plan_cli_coverage_boost_steps.py +++ b/features/steps/plan_cli_coverage_boost_steps.py @@ -243,7 +243,7 @@ def step_mocked_lifecycle_service_for_coverage(context) -> None: # Mock _create_sandbox_for_plan for execute tests create_sandbox_patcher = patch( "cleveragents.cli.commands.plan._create_sandbox_for_plan", - return_value=(None, None), + return_value=(None, []), ) create_sandbox_patcher.start() context._cleanup_handlers.append(create_sandbox_patcher.stop) diff --git a/features/steps/plan_lifecycle_commands_coverage_steps.py b/features/steps/plan_lifecycle_commands_coverage_steps.py index e43195e54..465d5c555 100644 --- a/features/steps/plan_lifecycle_commands_coverage_steps.py +++ b/features/steps/plan_lifecycle_commands_coverage_steps.py @@ -669,7 +669,7 @@ def step_invoke_execute_plan_queued(context): ), patch( "cleveragents.cli.commands.plan._create_sandbox_for_plan", - return_value=(None, None), + return_value=(None, []), ), patch( "cleveragents.application.services.plan_executor.PlanExecutor", @@ -723,7 +723,7 @@ def step_invoke_execute_plan_auto_progressed(context): ), patch( "cleveragents.cli.commands.plan._create_sandbox_for_plan", - return_value=(None, None), + return_value=(None, []), ), patch( "cleveragents.application.services.plan_executor.PlanExecutor", diff --git a/features/steps/tdd_plan_execute_phase_processing_steps.py b/features/steps/tdd_plan_execute_phase_processing_steps.py index 2c7a92b78..bb1fc47ea 100644 --- a/features/steps/tdd_plan_execute_phase_processing_steps.py +++ b/features/steps/tdd_plan_execute_phase_processing_steps.py @@ -177,7 +177,7 @@ def step_invoke_cli_execute(context: Context) -> None: ), patch( "cleveragents.cli.commands.plan._create_sandbox_for_plan", - return_value=(None, None), + return_value=(None, []), ), patch( "cleveragents.cli.commands.plan._get_plan_executor", @@ -368,7 +368,7 @@ def step_invoke_cli_execute_no_plan_id(context: Context) -> None: ), patch( "cleveragents.cli.commands.plan._create_sandbox_for_plan", - return_value=(None, None), + return_value=(None, []), ), patch( "cleveragents.cli.commands.plan._get_plan_executor", diff --git a/src/cleveragents/cli/commands/plan.py b/src/cleveragents/cli/commands/plan.py index eeec06751..2ad8c188f 100644 --- a/src/cleveragents/cli/commands/plan.py +++ b/src/cleveragents/cli/commands/plan.py @@ -1420,33 +1420,57 @@ def _cleanup_sandbox_for_plan( ): continue - if GitWorktreeSandbox.cleanup_stale(resource.location, plan_id): - return # Cleaned up — done + GitWorktreeSandbox.cleanup_stale(resource.location, plan_id) + + +class _SandboxInfo: + """Metadata for a per-resource sandbox.""" + + __slots__ = ("project_name", "resource_location", "sandbox_obj", "sandbox_path") + + def __init__( + self, + sandbox_path: str, + sandbox_obj: Any, + resource_location: str, + project_name: str, + ) -> None: + self.sandbox_path = sandbox_path + self.sandbox_obj = sandbox_obj + self.resource_location = resource_location + self.project_name = project_name def _create_sandbox_for_plan( plan_id: str, service: PlanLifecycleService, -) -> tuple[str | None, Any]: - """Create a git worktree sandbox for a plan's linked project. +) -> tuple[str | None, list[_SandboxInfo]]: + """Create per-resource git worktree sandboxes for a plan. + + Per spec §19310, each resource gets its own sandbox. A parent + directory is created under ``.cleveragents/sandbox//`` + with per-resource subdirectories named by resource ID. Returns: - A ``(sandbox_root, sandbox_object)`` tuple. When the plan's - project has a git-checkout resource, *sandbox_object* is a - :class:`GitWorktreeSandbox` and *sandbox_root* is the worktree - path. Otherwise falls back to a flat directory under - ``.cleveragents/sandbox/`` and *sandbox_object* is ``None``. + A ``(parent_sandbox_root, sandbox_infos)`` tuple. + *parent_sandbox_root* is the parent directory containing all + per-resource subdirectories (passed to ``PlanExecutor`` as + ``sandbox_root``). *sandbox_infos* is a list of + :class:`_SandboxInfo` objects, one per git-checkout resource. + When no git resources are found, falls back to a flat directory + and returns an empty list. """ from cleveragents.application.container import get_container - from cleveragents.infrastructure.sandbox.git_worktree import ( - GitWorktreeSandbox, - ) container = get_container() plan = service.get_plan(plan_id) project_names = [pl.project_name for pl in getattr(plan, "project_links", [])] - # Try to find a git-checkout resource for the first linked project + sandboxes: list[_SandboxInfo] = [] + # Track processed repo paths to avoid cleanup_stale destroying + # a sandbox we just created for the same repo (M1 fix). + processed_repos: set[str] = set() + for project_name in project_names: try: project = container.namespaced_project_repo().get(project_name) @@ -1454,10 +1478,10 @@ def _create_sandbox_for_plan( continue if project is None: continue - for lr in getattr(project, "linked_resources", []): + for linked_resource in getattr(project, "linked_resources", []): try: resource = container.resource_registry_service().show_resource( - lr.resource_id, + linked_resource.resource_id, ) except Exception: continue @@ -1466,6 +1490,11 @@ def _create_sandbox_for_plan( and resource.location and os.path.isdir(os.path.join(resource.location, ".git")) ): + repo_abs = os.path.realpath(resource.location) + if repo_abs in processed_repos: + continue # M1: skip duplicate repos + processed_repos.add(repo_abs) + GitWorktreeSandbox.cleanup_stale( resource.location, plan_id, @@ -1474,13 +1503,34 @@ def _create_sandbox_for_plan( resource_id=resource.resource_id, original_path=resource.location, ) - ctx = sandbox.create(plan_id) - return ctx.sandbox_path, sandbox + try: + ctx = sandbox.create(plan_id) + except Exception: + # M3: cleanup already-created sandboxes on failure + for prev in sandboxes: + prev.sandbox_obj.cleanup() + raise + sandboxes.append( + _SandboxInfo( + sandbox_path=ctx.sandbox_path, + sandbox_obj=sandbox, + resource_location=resource.location, + project_name=project_name, + ) + ) + + if sandboxes: + # Always use the first resource's worktree as sandbox_root + # (backward compatible — PlanExecutor and LLMExecuteActor + # write all FILE: blocks here). For multi-resource plans, + # _route_sandbox_files_to_worktrees() redistributes files + # to the correct worktrees after execute completes. + return sandboxes[0].sandbox_path, sandboxes # Fallback: flat directory sandbox flat_root = os.path.join(os.getcwd(), ".cleveragents", "sandbox") os.makedirs(flat_root, exist_ok=True) - return flat_root, None + return flat_root, [] def _apply_sandbox_changes( @@ -1500,7 +1550,7 @@ def _apply_sandbox_changes( ``True`` if changes were applied successfully, ``False`` if the merge failed (conflict, timeout, etc.). - Spec reference: ``specification.md`` §13241-13276. + Spec reference: ``specification.md`` §19310-19313, §13241-13276. """ import subprocess @@ -1511,7 +1561,11 @@ def _apply_sandbox_changes( project_names = [pl.project_name for pl in getattr(plan, "project_links", [])] branch_name = f"cleveragents/plan-{plan_id}" - # Try git worktree merge for each linked git-checkout resource + # Try git worktree merge for each linked git-checkout resource. + # Per spec §19312: Apply commits each sandbox separately. + merged_count = 0 + merge_failed = False + for project_name in project_names: try: project = container.namespaced_project_repo().get(project_name) @@ -1565,7 +1619,6 @@ def _apply_sandbox_changes( # Count changed files from diff --stat stat_lines = (diff_stat.stdout or "").strip().splitlines() - # Last line is summary; file lines above it artifact_count = max(0, len(stat_lines) - 1) if stat_lines else 0 # Parse insertions/deletions from --shortstat @@ -1600,18 +1653,16 @@ def _apply_sandbox_changes( ) except subprocess.TimeoutExpired: console.print( - "[red]Merge timed out.[/red]\n" - "[yellow]Run 'git merge --abort' manually " - "to clean up the repository.[/yellow]" + f"[red]Merge timed out for {project_name}.[/red]\n" + "[yellow]Run 'git merge --abort' manually.[/yellow]" ) - return False + merge_failed = True + continue except subprocess.CalledProcessError as merge_err: - # Git writes conflict info to stdout, not stderr detail = (merge_err.stdout or merge_err.stderr or "").strip() if not detail: detail = "Unknown merge error" - console.print(f"[red]Merge failed:[/red] {detail}") - # Abort the merge to leave the repo in a clean state + console.print(f"[red]Merge failed for {project_name}:[/red] {detail}") try: abort_result = subprocess.run( ["git", "merge", "--abort"], @@ -1623,15 +1674,14 @@ def _apply_sandbox_changes( except subprocess.TimeoutExpired: console.print( "[red]Merge abort timed out.[/red]\n" - "[yellow]Run 'git merge --abort' manually " - "to clean up the repository.[/yellow]" + "[yellow]Run 'git merge --abort' manually.[/yellow]" ) - return False + merge_failed = True + continue if abort_result.returncode == 0: console.print( - "[yellow]Merge aborted — project is unchanged. " - "Resolve conflicts manually or re-run " - "the plan.[/yellow]" + f"[yellow]{project_name}: merge aborted — " + "project is unchanged.[/yellow]" ) else: abort_err = ( @@ -1641,14 +1691,15 @@ def _apply_sandbox_changes( abort_err = "Unknown error" console.print( f"[red]Merge abort failed:[/red] {abort_err}\n" - "[yellow]Run 'git merge --abort' manually " - "to clean up the repository.[/yellow]" + "[yellow]Run 'git merge --abort' manually.[/yellow]" ) - return False + merge_failed = True + continue + merged_count += 1 applied_at = datetime.now().strftime("%Y-%m-%d %H:%M") - # ── Apply Summary panel (spec §13241-13247) ── + # ── Per-resource Apply Summary panel (spec §13241) ── summary = ( f"[cyan]Plan:[/cyan] {plan_id}\n" f"[blue]Artifacts:[/blue] {artifact_count} file(s) updated\n" @@ -1663,7 +1714,6 @@ def _apply_sandbox_changes( worktree_removed = False branch_deleted = False - # Find and remove the worktree directory wt_list = subprocess.run( ["git", "worktree", "list", "--porcelain"], cwd=repo_path, @@ -1678,7 +1728,13 @@ def _apply_sandbox_changes( if part.startswith("worktree "): wt_path = part.split("worktree ", 1)[1] subprocess.run( - ["git", "worktree", "remove", "--force", wt_path], + [ + "git", + "worktree", + "remove", + "--force", + wt_path, + ], cwd=repo_path, capture_output=True, check=False, @@ -1686,7 +1742,6 @@ def _apply_sandbox_changes( ) worktree_removed = True - # Delete the branch del_result = subprocess.run( ["git", "branch", "-D", branch_name], cwd=repo_path, @@ -1712,18 +1767,25 @@ def _apply_sandbox_changes( ) console.print(Panel(cleanup_text, title="Sandbox Cleanup", expand=False)) - # ── Next Steps panel (spec §13271-13274) ── - console.print( - Panel( - "- Review git diff\n- Commit changes", - title="Next Steps", - expand=False, - ) + # Show footer after all resources are processed + if merged_count > 0: + console.print( + Panel( + "- Review git diff\n- Commit changes", + title="Next Steps", + expand=False, ) + ) + console.print("[green]✓ OK[/green] Changes applied") + if merge_failed: + console.print( + "[yellow]Some resources failed to merge. See errors above.[/yellow]" + ) + return False + return True - # ── Footer (spec §13276) ── - console.print("[green]✓ OK[/green] Changes applied") - return True # Done — merged successfully + if merge_failed: + return False # Fallback: flat file copy from .cleveragents/sandbox/ sandbox_root = os.path.join(os.getcwd(), ".cleveragents", "sandbox") @@ -1764,6 +1826,85 @@ def _apply_sandbox_changes( return failed_count == 0 +def _route_sandbox_files_to_worktrees( + sandbox_infos: list[_SandboxInfo], +) -> None: + """Route files from the primary sandbox to per-resource worktrees. + + When a plan has multiple git-checkout resources, the LLM writes all + ``FILE:`` blocks to the first resource's worktree. This function + moves files that belong to other resources into their respective + worktrees by matching file paths against each resource's known + file list (via ``git ls-files``). + + Per spec §19310: each resource gets its own sandbox. + """ + import subprocess + + if len(sandbox_infos) <= 1: + return # Single resource — nothing to route + + primary = sandbox_infos[0] + + # Build file list for the primary resource so we never move + # files that belong to it (C1 fix: prevents data loss when + # projects share the same relative path, e.g. README.md). + try: + primary_result = subprocess.run( + ["git", "ls-files", "--cached", "--others", "--exclude-standard"], + cwd=primary.resource_location, + capture_output=True, + text=True, + check=True, + timeout=30, + ) + primary_files: set[str] = { + f.strip() for f in primary_result.stdout.splitlines() if f.strip() + } + except Exception: + # Cannot determine primary file list — skip routing entirely + # to avoid data loss (M-NEW-2: empty set would move all files). + return + + # Build file lists for non-primary resources + resource_files: dict[int, set[str]] = {} + for idx, info in enumerate(sandbox_infos[1:], start=1): + try: + result = subprocess.run( + ["git", "ls-files", "--cached", "--others", "--exclude-standard"], + cwd=info.resource_location, + capture_output=True, + text=True, + check=True, + timeout=30, + ) + resource_files[idx] = { + f.strip() for f in result.stdout.splitlines() if f.strip() + } + except Exception: + resource_files[idx] = set() + + # Walk the primary sandbox and move files that belong elsewhere. + # Only move a file if it matches a secondary resource's file list + # AND does NOT exist in the primary resource's file list. + for dirpath, _dirnames, filenames in os.walk(primary.sandbox_path): + for fname in filenames: + full_path = os.path.join(dirpath, fname) + rel_path = os.path.relpath(full_path, primary.sandbox_path) + + # Never move files that belong to the primary resource + if rel_path in primary_files: + continue + + for idx, known_files in resource_files.items(): + if rel_path in known_files: + target = sandbox_infos[idx] + dst = os.path.join(target.sandbox_path, rel_path) + os.makedirs(os.path.dirname(dst), exist_ok=True) + shutil.move(full_path, dst) + break + + def _commit_worktree_changes(worktree_path: str, plan_id: str) -> None: """Stage and commit LLM output in the worktree branch. @@ -2465,6 +2606,7 @@ def execute_plan( PreflightRejection, ) + sandbox_infos: list[_SandboxInfo] = [] try: from cleveragents.domain.models.core.plan import ( PlanPhase, @@ -2529,9 +2671,9 @@ def execute_plan( pre.execution_environment = execution_environment.lower() service._commit_plan(pre) - # Create sandbox for this plan (git worktree or flat fallback) - # and build the executor with the sandbox path. - sandbox_root, sandbox_obj = _create_sandbox_for_plan(plan_id, service) + # Create per-resource sandboxes (spec §19310) and build the + # executor with the sandbox path. + sandbox_root, sandbox_infos = _create_sandbox_for_plan(plan_id, service) executor = _get_plan_executor( lifecycle_service=service, sandbox_root=sandbox_root, @@ -2605,13 +2747,11 @@ def execute_plan( executor.run_execute(plan_id) plan = service.get_plan(plan_id) - # Stage and commit LLM-generated files in the worktree - # branch WITHOUT merging — the merge happens at apply time. - if sandbox_obj is not None and sandbox_obj.context is not None: - _commit_worktree_changes( - sandbox_obj.context.sandbox_path, - plan_id, - ) + # Route files to correct per-resource worktrees (spec §19310) + # then commit each worktree branch. + _route_sandbox_files_to_worktrees(sandbox_infos) + for sinfo in sandbox_infos: + _commit_worktree_changes(sinfo.sandbox_path, plan_id) # Notify A2A facade for protocol bookkeeping. # Use plan.status (read-only) instead of plan.execute (transition) @@ -2667,6 +2807,13 @@ def execute_plan( except Exception as e: console.print(f"[red]Unexpected error:[/red] {e}") raise typer.Abort() from e + finally: + # M4: cleanup sandboxes on any failure path. + # GitWorktreeSandbox.cleanup() is idempotent — safe to call + # even after a successful apply (which already cleaned up). + for _sinfo in sandbox_infos: + with contextlib.suppress(Exception): + _sinfo.sandbox_obj.cleanup() @app.command("apply")