feat(plan): create per-project sandboxes for multi-project plans
CI / helm (pull_request) Successful in 34s
CI / push-validation (pull_request) Successful in 27s
CI / build (pull_request) Successful in 1m17s
CI / lint (pull_request) Successful in 1m36s
CI / quality (pull_request) Successful in 1m37s
CI / typecheck (pull_request) Successful in 1m51s
CI / security (pull_request) Successful in 2m5s
CI / integration_tests (pull_request) Successful in 4m4s
CI / e2e_tests (pull_request) Successful in 4m21s
CI / unit_tests (pull_request) Successful in 8m0s
CI / docker (pull_request) Successful in 1m41s
CI / coverage (pull_request) Successful in 12m36s
CI / status-check (pull_request) Successful in 3s
CI / lint (push) Successful in 50s
CI / helm (push) Successful in 32s
CI / push-validation (push) Successful in 30s
CI / build (push) Successful in 45s
CI / quality (push) Successful in 1m16s
CI / typecheck (push) Successful in 1m19s
CI / security (push) Successful in 1m33s
CI / e2e_tests (push) Successful in 4m8s
CI / integration_tests (push) Successful in 4m56s
CI / unit_tests (push) Successful in 5m58s
CI / docker (push) Successful in 2m17s
CI / benchmark-publish (pull_request) Has been skipped
CI / coverage (push) Successful in 12m21s
CI / status-check (push) Successful in 4s
CI / benchmark-regression (push) Has been skipped
CI / benchmark-regression (pull_request) Failing after 1h12m31s
CI / benchmark-publish (push) Successful in 1h17m21s
CI / helm (pull_request) Successful in 34s
CI / push-validation (pull_request) Successful in 27s
CI / build (pull_request) Successful in 1m17s
CI / lint (pull_request) Successful in 1m36s
CI / quality (pull_request) Successful in 1m37s
CI / typecheck (pull_request) Successful in 1m51s
CI / security (pull_request) Successful in 2m5s
CI / integration_tests (pull_request) Successful in 4m4s
CI / e2e_tests (pull_request) Successful in 4m21s
CI / unit_tests (pull_request) Successful in 8m0s
CI / docker (pull_request) Successful in 1m41s
CI / coverage (pull_request) Successful in 12m36s
CI / status-check (pull_request) Successful in 3s
CI / lint (push) Successful in 50s
CI / helm (push) Successful in 32s
CI / push-validation (push) Successful in 30s
CI / build (push) Successful in 45s
CI / quality (push) Successful in 1m16s
CI / typecheck (push) Successful in 1m19s
CI / security (push) Successful in 1m33s
CI / e2e_tests (push) Successful in 4m8s
CI / integration_tests (push) Successful in 4m56s
CI / unit_tests (push) Successful in 5m58s
CI / docker (push) Successful in 2m17s
CI / benchmark-publish (pull_request) Has been skipped
CI / coverage (push) Successful in 12m21s
CI / status-check (push) Successful in 4s
CI / benchmark-regression (push) Has been skipped
CI / benchmark-regression (pull_request) Failing after 1h12m31s
CI / benchmark-publish (push) Successful in 1h17m21s
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
This commit was merged in pull request #10828.
This commit is contained in:
@@ -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
|
||||
@@ -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}"
|
||||
)
|
||||
@@ -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)
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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/<plan_id>/``
|
||||
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")
|
||||
|
||||
Reference in New Issue
Block a user