diff --git a/features/copy_on_write_sandbox.feature b/features/copy_on_write_sandbox.feature new file mode 100644 index 000000000..f17ce048c --- /dev/null +++ b/features/copy_on_write_sandbox.feature @@ -0,0 +1,141 @@ +Feature: Copy-on-write sandbox lifecycle + As a developer + I want a sandbox that isolates plan changes in a directory copy + So that the original directory is unmodified until changes are committed + + Background: + Given a cow test directory is initialised + + # --- Creation --- + + Scenario: Create a copy-on-write sandbox + When a cow sandbox is created for plan "plan-001" + Then the cow sandbox should be in the "created" state + And the cow sandbox context should reference plan "plan-001" + And the cow sandbox context should have strategy metadata "copy_on_write" + And the cow sandbox path should exist + + Scenario: Creating a sandbox with empty plan_id raises ValueError + When a cow sandbox is created with empty plan_id + Then a cow ValueError should be raised with message "plan_id cannot be empty" + + Scenario: Creating a sandbox with empty resource_id raises ValueError + When a cow sandbox is prepared with empty resource_id + Then a cow ValueError should be raised with message "resource_id cannot be empty" + + Scenario: Creating a sandbox with empty original_path raises ValueError + When a cow sandbox is prepared with empty original_path + Then a cow ValueError should be raised with message "original_path cannot be empty" + + Scenario: Creating a sandbox on a non-existent directory raises SandboxCreationError + Given a cow non-existent directory + When a cow sandbox is created on the non-existent directory for plan "plan-001" + Then a cow SandboxCreationError should be raised + + # --- Path resolution --- + + Scenario: Resolve a path in the sandbox copy + When a cow sandbox is created for plan "plan-001" + And the cow path "data/file.txt" is resolved + Then the cow resolved path should be inside the sandbox copy + And the cow sandbox should be in the "active" state + + Scenario: Path traversal is rejected + When a cow sandbox is created for plan "plan-001" + And the cow path "../etc/passwd" is resolved + Then a cow ValueError should be raised with message "Path traversal not allowed" + + Scenario: Resolving a path on a cleaned-up sandbox raises SandboxStateError + When a cow sandbox is created for plan "plan-001" + And the cow sandbox is cleaned up + And the cow path "file.txt" is resolved on a cleaned-up sandbox + Then a cow SandboxStateError should be raised + + # --- Commit --- + + Scenario: Commit with no changes produces empty result + When a cow sandbox is created for plan "plan-001" + And the cow sandbox is committed + Then the cow commit result should indicate success + And the cow commit result should have 0 changed files + And the cow commit result should have 0 added files + And the cow commit result should have 0 deleted files + + Scenario: Commit with a new file syncs it to original + When a cow sandbox is created for plan "plan-001" + And a cow file "new_file.txt" is created in the sandbox with content "hello world" + And the cow sandbox is committed + Then the cow commit result should indicate success + And the cow commit result should have 1 added files + And the cow file "new_file.txt" should exist in the original directory with content "hello world" + + Scenario: Commit with a modified file syncs the change + When a cow sandbox is created for plan "plan-001" + And the cow existing file "existing.txt" is modified in the sandbox with content "updated" + And the cow sandbox is committed + Then the cow commit result should indicate success + And the cow commit result should have 1 changed files + And the cow file "existing.txt" in the original should have content "updated" + + Scenario: Commit with a deleted file removes it from original + When a cow sandbox is created for plan "plan-001" + And the cow existing file "existing.txt" is deleted from the sandbox + And the cow sandbox is committed + Then the cow commit result should indicate success + And the cow commit result should have 1 deleted files + And the cow file "existing.txt" should not exist in the original directory + + Scenario: Commit on a cleaned-up sandbox raises SandboxStateError + When a cow sandbox is created for plan "plan-001" + And the cow sandbox is cleaned up + And the cow sandbox commit is attempted on cleaned-up sandbox + Then a cow SandboxStateError should be raised + + # --- Rollback --- + + Scenario: Rollback restores the sandbox to original state + When a cow sandbox is created for plan "plan-001" + And a cow file "temp.txt" is created in the sandbox with content "temporary" + And the cow path "temp.txt" is resolved + And the cow sandbox is rolled back + Then the cow sandbox should be in the "rolled_back" state + And the cow file "temp.txt" should not exist in the sandbox + + Scenario: Rollback on a non-active sandbox raises SandboxStateError + When a cow sandbox is created for plan "plan-001" + And the cow sandbox rollback is attempted on created sandbox + Then a cow SandboxStateError should be raised + + # --- Cleanup --- + + Scenario: Cleanup removes the sandbox directory + When a cow sandbox is created for plan "plan-001" + And the cow sandbox is cleaned up + Then the cow sandbox should be in the "cleaned_up" state + And the cow sandbox path should not exist + + Scenario: Cleanup is idempotent + When a cow sandbox is created for plan "plan-001" + And the cow sandbox is cleaned up + And the cow sandbox is cleaned up again + Then the cow sandbox should be in the "cleaned_up" state + + # --- Protocol properties --- + + Scenario: Sandbox has a unique ULID identifier + When a cow sandbox is instantiated + Then the cow sandbox_id should be a valid ULID + And the cow sandbox status should be "pending" + And the cow sandbox context should be None + + # --- Diff computation --- + + Scenario: Diff detects added, modified, and deleted files + When a cow sandbox is created for plan "plan-001" + And a cow file "added.txt" is created in the sandbox with content "new" + And the cow existing file "existing.txt" is modified in the sandbox with content "changed" + And the cow existing file "to_delete.txt" is deleted from the sandbox + And the cow sandbox is committed + Then the cow commit result should have 1 added files + And the cow commit result should have 1 changed files + And the cow commit result should have 1 deleted files diff --git a/features/git_worktree_sandbox.feature b/features/git_worktree_sandbox.feature new file mode 100644 index 000000000..9ea3f81d3 --- /dev/null +++ b/features/git_worktree_sandbox.feature @@ -0,0 +1,129 @@ +Feature: Git worktree sandbox lifecycle + As a developer + I want a sandbox that isolates plan changes in a git worktree + So that the original repository is unmodified until changes are committed + + Background: + Given a gwt test git repository is initialised + + # --- Creation --- + + Scenario: Create a git worktree sandbox + When a gwt sandbox is created for plan "plan-001" + Then the gwt sandbox should be in the "created" state + And the gwt sandbox context should reference plan "plan-001" + And the gwt sandbox context should have strategy metadata "git_worktree" + And the gwt sandbox worktree path should exist + + Scenario: Creating a sandbox with empty plan_id raises ValueError + When a gwt sandbox is created with empty plan_id + Then a gwt ValueError should be raised with message "plan_id cannot be empty" + + Scenario: Creating a sandbox with empty resource_id raises ValueError + When a gwt sandbox is prepared with empty resource_id + Then a gwt ValueError should be raised with message "resource_id cannot be empty" + + Scenario: Creating a sandbox with empty original_path raises ValueError + When a gwt sandbox is prepared with empty original_path + Then a gwt ValueError should be raised with message "original_path cannot be empty" + + Scenario: Creating a sandbox with non-positive timeout raises ValueError + When a gwt sandbox is prepared with zero timeout + Then a gwt ValueError should be raised with message "git_timeout must be positive" + + Scenario: Creating a sandbox on a non-git directory raises SandboxCreationError + Given a gwt non-git directory + When a gwt sandbox is created on the non-git directory for plan "plan-001" + Then a gwt SandboxCreationError should be raised + + # --- Path resolution --- + + Scenario: Resolve a path in the worktree + When a gwt sandbox is created for plan "plan-001" + And the gwt path "src/main.py" is resolved + Then the gwt resolved path should be inside the worktree + And the gwt sandbox should be in the "active" state + + Scenario: Path traversal is rejected + When a gwt sandbox is created for plan "plan-001" + And the gwt path "../etc/passwd" is resolved + Then a gwt ValueError should be raised with message "Path traversal not allowed" + + Scenario: Resolving a path on a cleaned-up sandbox raises SandboxStateError + When a gwt sandbox is created for plan "plan-001" + And the gwt sandbox is cleaned up + And the gwt path "file.txt" is resolved on a cleaned-up sandbox + Then a gwt SandboxStateError should be raised + + # --- Commit --- + + Scenario: Commit with no changes produces empty result + When a gwt sandbox is created for plan "plan-001" + And the gwt sandbox is committed with message "empty commit" + Then the gwt commit result should indicate success + And the gwt commit result should have 0 changed files + + Scenario: Commit with changes merges back to original branch + When a gwt sandbox is created for plan "plan-001" + And a gwt file "new_file.txt" is created in the worktree with content "hello" + And the gwt sandbox is committed with message "add new file" + Then the gwt commit result should indicate success + And the gwt commit result should have 1 added files + And the gwt file "new_file.txt" should exist in the original repo + + Scenario: Commit with modified file reports changes + When a gwt sandbox is created for plan "plan-001" + And the gwt existing file "README.md" is modified in the worktree + And the gwt sandbox is committed with message "modify readme" + Then the gwt commit result should indicate success + And the gwt commit result should have 1 changed files + + Scenario: Commit on a cleaned-up sandbox raises SandboxStateError + When a gwt sandbox is created for plan "plan-001" + And the gwt sandbox is cleaned up + And the gwt sandbox commit is attempted on cleaned-up sandbox + Then a gwt SandboxStateError should be raised + + # --- Rollback --- + + Scenario: Rollback discards worktree changes + When a gwt sandbox is created for plan "plan-001" + And a gwt file "temp.txt" is created in the worktree with content "temporary" + And the gwt path "temp.txt" is resolved + And the gwt sandbox is rolled back + Then the gwt sandbox should be in the "rolled_back" state + And the gwt file "temp.txt" should not exist in the worktree + + Scenario: Rollback on a non-active sandbox raises SandboxStateError + When a gwt sandbox is created for plan "plan-001" + And the gwt sandbox rollback is attempted on created sandbox + Then a gwt SandboxStateError should be raised + + # --- Cleanup --- + + Scenario: Cleanup removes worktree and branch + When a gwt sandbox is created for plan "plan-001" + And the gwt sandbox is cleaned up + Then the gwt sandbox should be in the "cleaned_up" state + And the gwt sandbox worktree path should not exist + And the gwt sandbox branch should not exist + + Scenario: Cleanup is idempotent + When a gwt sandbox is created for plan "plan-001" + And the gwt sandbox is cleaned up + And the gwt sandbox is cleaned up again + Then the gwt sandbox should be in the "cleaned_up" state + + # --- Branch sanitisation --- + + Scenario: Branch name is sanitised from plan ID + When a gwt sandbox is created for plan "plan with spaces!@#" + Then the gwt sandbox branch name should be safe for git + + # --- Protocol properties --- + + Scenario: Sandbox has a unique ULID identifier + When a gwt sandbox is instantiated + Then the gwt sandbox_id should be a valid ULID + And the gwt sandbox status should be "pending" + And the gwt sandbox context should be None diff --git a/features/sandbox_factory_coverage.feature b/features/sandbox_factory_coverage.feature index ae2f343c3..b09c06968 100644 --- a/features/sandbox_factory_coverage.feature +++ b/features/sandbox_factory_coverage.feature @@ -24,31 +24,30 @@ Feature: Sandbox factory strategy routing Then the factory should produce a passthrough sandbox And the produced sandbox should be in the pending state - # --- Unimplemented strategies --- + # --- Implemented strategies --- - Scenario: The git worktree strategy is not yet available + Scenario: The git worktree strategy produces a git worktree sandbox Given the sandbox factory is available When a sandbox is requested for resource "repo-1" at "/src/repo" using the git worktree strategy - Then the factory should indicate the strategy is not yet implemented + Then the factory should produce a git worktree sandbox + And the produced git worktree sandbox should be in the pending state - Scenario: The copy-on-write strategy is not yet available + Scenario: The copy-on-write strategy produces a copy-on-write sandbox Given the sandbox factory is available When a sandbox is requested for resource "docs" at "/srv/docs" using the copy-on-write strategy - Then the factory should indicate the strategy is not yet implemented + Then the factory should produce a copy-on-write sandbox + And the produced copy-on-write sandbox should be in the pending state - Scenario: The overlay strategy is not yet available - Given the sandbox factory is available - When a sandbox is requested for resource "fs-root" at "/mnt/data" using the overlay strategy - Then the factory should indicate the strategy is not yet implemented + # --- Unimplemented strategies --- Scenario: The transaction rollback strategy is not yet available Given the sandbox factory is available When a sandbox is requested for resource "db-main" at "postgres://db/main" using the transaction rollback strategy Then the factory should indicate the strategy is not yet implemented - Scenario: The versioning strategy is not yet available + Scenario: The snapshot strategy is not yet available Given the sandbox factory is available - When a sandbox is requested for resource "corpus" at "/srv/corpus" using the versioning strategy + When a sandbox is requested for resource "corpus" at "/srv/corpus" using the snapshot strategy Then the factory should indicate the strategy is not yet implemented # --- Unknown strategy --- @@ -58,6 +57,16 @@ Feature: Sandbox factory strategy routing When a sandbox is requested for resource "misc" at "/tmp/misc" using an unrecognised strategy Then the factory should reject the request due to an unknown strategy + Scenario: The overlay strategy is now unknown and rejected + Given the sandbox factory is available + When a sandbox is requested for resource "fs-root" at "/mnt/data" using the overlay strategy + Then the factory should reject the request due to an unknown strategy + + Scenario: The versioning strategy is now unknown and rejected + Given the sandbox factory is available + When a sandbox is requested for resource "corpus" at "/srv/corpus" using the versioning strategy + Then the factory should reject the request due to an unknown strategy + # --- Support checks --- Scenario: The none strategy is reported as supported @@ -65,51 +74,51 @@ Feature: Sandbox factory strategy routing When the factory is asked whether the none strategy is supported Then the factory should confirm the strategy is supported - Scenario: The git worktree strategy is reported as unsupported + Scenario: The git worktree strategy is reported as supported Given the sandbox factory is available When the factory is asked whether the git worktree strategy is supported - Then the factory should indicate the strategy is not supported + Then the factory should confirm the strategy is supported - Scenario: The copy-on-write strategy is reported as unsupported + Scenario: The copy-on-write strategy is reported as supported Given the sandbox factory is available When the factory is asked whether the copy-on-write strategy is supported - Then the factory should indicate the strategy is not supported + Then the factory should confirm the strategy is supported Scenario: An unrecognised strategy is reported as unsupported Given the sandbox factory is available When the factory is asked whether an unrecognised strategy is supported Then the factory should indicate the strategy is not supported - # --- Resource type strategy lookup --- + # --- Resource type strategy lookup (spec-aligned types) --- - Scenario: A git repository supports worktree, copy-on-write, and none strategies + Scenario: A git-checkout resource supports worktree, copy-on-write, and none Given the sandbox factory is available - When the compatible strategies for a "git_repository" resource are queried + When the compatible strategies for a "git-checkout" resource are queried Then the factory should return git worktree, copy-on-write, and none as compatible - Scenario: A filesystem resource supports copy-on-write, overlay, and none strategies + Scenario: A git resource supports only the none strategy Given the sandbox factory is available - When the compatible strategies for a "filesystem" resource are queried - Then the factory should return copy-on-write, overlay, and none as compatible - - Scenario: A database resource supports transaction rollback and none strategies - Given the sandbox factory is available - When the compatible strategies for a "database" resource are queried - Then the factory should return transaction rollback and none as compatible - - Scenario: An API endpoint only supports the none strategy - Given the sandbox factory is available - When the compatible strategies for an "api_endpoint" resource are queried + When the compatible strategies for a "git" resource are queried Then the factory should return only the none strategy as compatible - Scenario: A document corpus supports copy-on-write and none strategies + Scenario: A fs-mount resource supports copy-on-write and none Given the sandbox factory is available - When the compatible strategies for a "document_corpus" resource are queried + When the compatible strategies for a "fs-mount" resource are queried Then the factory should return copy-on-write and none as compatible - Scenario: Cloud infrastructure only supports the none strategy + Scenario: A fs-directory resource supports copy-on-write and none Given the sandbox factory is available - When the compatible strategies for a "cloud_infrastructure" resource are queried + When the compatible strategies for a "fs-directory" resource are queried + Then the factory should return copy-on-write and none as compatible + + Scenario: A fs-file resource supports copy-on-write and none + Given the sandbox factory is available + When the compatible strategies for a "fs-file" resource are queried + Then the factory should return copy-on-write and none as compatible + + Scenario: An api_endpoint resource supports only the none strategy + Given the sandbox factory is available + When the compatible strategies for an "api_endpoint" resource are queried Then the factory should return only the none strategy as compatible Scenario: An unknown resource type falls back to the none strategy diff --git a/features/steps/copy_on_write_sandbox_steps.py b/features/steps/copy_on_write_sandbox_steps.py new file mode 100644 index 000000000..7bcf1264a --- /dev/null +++ b/features/steps/copy_on_write_sandbox_steps.py @@ -0,0 +1,391 @@ +"""Step definitions for copy-on-write sandbox feature. + +All steps use the ``cow`` prefix to avoid collisions with the 128+ existing +step files loaded globally by Behave. +""" + +from __future__ import annotations + +import os +import tempfile + +from behave import given, then, when +from behave.runner import Context + +from cleveragents.infrastructure.sandbox.copy_on_write import CopyOnWriteSandbox +from cleveragents.infrastructure.sandbox.protocol import ( + SandboxCreationError, + SandboxStateError, + SandboxStatus, +) + + +def _init_test_dir(ctx: Context) -> str: + """Create a temporary directory with some test files.""" + test_dir = tempfile.mkdtemp(prefix="cow-test-dir-") + # Create some initial files + with open(os.path.join(test_dir, "existing.txt"), "w") as f: + f.write("original content") + with open(os.path.join(test_dir, "to_delete.txt"), "w") as f: + f.write("will be deleted") + os.makedirs(os.path.join(test_dir, "data"), exist_ok=True) + with open(os.path.join(test_dir, "data", "file.txt"), "w") as f: + f.write("nested file") + return test_dir + + +# --------------------------------------------------------------------------- +# Background / Given +# --------------------------------------------------------------------------- + + +@given("a cow test directory is initialised") +def step_cow_init_dir(ctx: Context) -> None: + ctx.cow_test_dir = _init_test_dir(ctx) + ctx.cow_sandbox = None + ctx.cow_error = None + ctx.cow_commit_result = None + ctx.cow_resolved_path = None + + +@given("a cow non-existent directory") +def step_cow_nonexistent_dir(ctx: Context) -> None: + ctx.cow_nonexistent_dir = os.path.join( + tempfile.gettempdir(), "cow-nonexistent-" + str(os.getpid()) + ) + # Make sure it doesn't exist + if os.path.exists(ctx.cow_nonexistent_dir): + os.rmdir(ctx.cow_nonexistent_dir) + + +# --------------------------------------------------------------------------- +# When - instantiation +# --------------------------------------------------------------------------- + + +@when("a cow sandbox is instantiated") +def step_cow_instantiate(ctx: Context) -> None: + ctx.cow_sandbox = CopyOnWriteSandbox( + resource_id="res-001", + original_path=ctx.cow_test_dir, + ) + + +@when("a cow sandbox is prepared with empty resource_id") +def step_cow_empty_resource_id(ctx: Context) -> None: + try: + CopyOnWriteSandbox(resource_id="", original_path=ctx.cow_test_dir) + except ValueError as exc: + ctx.cow_error = exc + + +@when("a cow sandbox is prepared with empty original_path") +def step_cow_empty_original_path(ctx: Context) -> None: + try: + CopyOnWriteSandbox(resource_id="res-001", original_path="") + except ValueError as exc: + ctx.cow_error = exc + + +# --------------------------------------------------------------------------- +# When - create +# --------------------------------------------------------------------------- + + +@when('a cow sandbox is created for plan "{plan_id}"') +def step_cow_create(ctx: Context, plan_id: str) -> None: + ctx.cow_sandbox = CopyOnWriteSandbox( + resource_id="res-001", + original_path=ctx.cow_test_dir, + ) + ctx.cow_sandbox.create(plan_id) + + +@when("a cow sandbox is created with empty plan_id") +def step_cow_create_empty_plan(ctx: Context) -> None: + ctx.cow_sandbox = CopyOnWriteSandbox( + resource_id="res-001", + original_path=ctx.cow_test_dir, + ) + try: + ctx.cow_sandbox.create("") + except ValueError as exc: + ctx.cow_error = exc + + +@when('a cow sandbox is created on the non-existent directory for plan "{plan_id}"') +def step_cow_create_nonexistent(ctx: Context, plan_id: str) -> None: + ctx.cow_sandbox = CopyOnWriteSandbox( + resource_id="res-001", + original_path=ctx.cow_nonexistent_dir, + ) + try: + ctx.cow_sandbox.create(plan_id) + except SandboxCreationError as exc: + ctx.cow_error = exc + + +# --------------------------------------------------------------------------- +# When - path resolution +# --------------------------------------------------------------------------- + + +@when('the cow path "{path}" is resolved') +def step_cow_resolve_path(ctx: Context, path: str) -> None: + try: + ctx.cow_resolved_path = ctx.cow_sandbox.get_path(path) + except (ValueError, SandboxStateError) as exc: + ctx.cow_error = exc + + +@when('the cow path "{path}" is resolved on a cleaned-up sandbox') +def step_cow_resolve_path_cleaned(ctx: Context, path: str) -> None: + try: + ctx.cow_sandbox.get_path(path) + except SandboxStateError as exc: + ctx.cow_error = exc + + +# --------------------------------------------------------------------------- +# When - file operations in sandbox copy +# --------------------------------------------------------------------------- + + +@when('a cow file "{filename}" is created in the sandbox with content "{content}"') +def step_cow_create_file(ctx: Context, filename: str, content: str) -> None: + sandbox_path = ctx.cow_sandbox.context.sandbox_path + file_path = os.path.join(sandbox_path, filename) + os.makedirs(os.path.dirname(file_path), exist_ok=True) + with open(file_path, "w") as f: + f.write(content) + + +@when( + 'the cow existing file "{filename}" is modified in the sandbox with content "{content}"' +) +def step_cow_modify_file(ctx: Context, filename: str, content: str) -> None: + sandbox_path = ctx.cow_sandbox.context.sandbox_path + file_path = os.path.join(sandbox_path, filename) + with open(file_path, "w") as f: + f.write(content) + + +@when('the cow existing file "{filename}" is deleted from the sandbox') +def step_cow_delete_file(ctx: Context, filename: str) -> None: + sandbox_path = ctx.cow_sandbox.context.sandbox_path + file_path = os.path.join(sandbox_path, filename) + os.remove(file_path) + + +# --------------------------------------------------------------------------- +# When - commit / rollback / cleanup +# --------------------------------------------------------------------------- + + +@when("the cow sandbox is committed") +def step_cow_commit(ctx: Context) -> None: + try: + ctx.cow_commit_result = ctx.cow_sandbox.commit() + except (SandboxStateError, Exception) as exc: + ctx.cow_error = exc + + +@when("the cow sandbox commit is attempted on cleaned-up sandbox") +def step_cow_commit_cleaned(ctx: Context) -> None: + try: + ctx.cow_sandbox.commit() + except SandboxStateError as exc: + ctx.cow_error = exc + + +@when("the cow sandbox is rolled back") +def step_cow_rollback(ctx: Context) -> None: + try: + ctx.cow_sandbox.rollback() + except (SandboxStateError, Exception) as exc: + ctx.cow_error = exc + + +@when("the cow sandbox rollback is attempted on created sandbox") +def step_cow_rollback_created(ctx: Context) -> None: + try: + ctx.cow_sandbox.rollback() + except SandboxStateError as exc: + ctx.cow_error = exc + + +@when("the cow sandbox is cleaned up") +def step_cow_cleanup(ctx: Context) -> None: + ctx.cow_sandbox.cleanup() + + +@when("the cow sandbox is cleaned up again") +def step_cow_cleanup_again(ctx: Context) -> None: + ctx.cow_sandbox.cleanup() + + +# --------------------------------------------------------------------------- +# Then - status / state +# --------------------------------------------------------------------------- + + +@then('the cow sandbox should be in the "{status}" state') +def step_cow_check_status(ctx: Context, status: str) -> None: + expected = SandboxStatus(status) + assert ctx.cow_sandbox.status == expected, ( + f"Expected status {expected}, got {ctx.cow_sandbox.status}" + ) + + +@then('the cow sandbox context should reference plan "{plan_id}"') +def step_cow_check_plan(ctx: Context, plan_id: str) -> None: + assert ctx.cow_sandbox.context is not None + assert ctx.cow_sandbox.context.plan_id == plan_id + + +@then('the cow sandbox context should have strategy metadata "{strategy}"') +def step_cow_check_strategy(ctx: Context, strategy: str) -> None: + assert ctx.cow_sandbox.context is not None + assert ctx.cow_sandbox.context.metadata.get("strategy") == strategy + + +@then("the cow sandbox path should exist") +def step_cow_path_exists(ctx: Context) -> None: + assert ctx.cow_sandbox.context is not None + assert os.path.isdir(ctx.cow_sandbox.context.sandbox_path) + + +@then("the cow sandbox path should not exist") +def step_cow_path_not_exists(ctx: Context) -> None: + if ctx.cow_sandbox.context is not None: + assert not os.path.exists(ctx.cow_sandbox.context.sandbox_path) + + +# --------------------------------------------------------------------------- +# Then - errors +# --------------------------------------------------------------------------- + + +@then('a cow ValueError should be raised with message "{msg}"') +def step_cow_valueerror(ctx: Context, msg: str) -> None: + assert ctx.cow_error is not None, "Expected an error but none occurred" + assert isinstance(ctx.cow_error, ValueError), ( + f"Expected ValueError, got {type(ctx.cow_error).__name__}" + ) + assert msg in str(ctx.cow_error), ( + f"Expected '{msg}' in error message, got: {ctx.cow_error}" + ) + + +@then("a cow SandboxCreationError should be raised") +def step_cow_creation_error(ctx: Context) -> None: + assert ctx.cow_error is not None + assert isinstance(ctx.cow_error, SandboxCreationError) + + +@then("a cow SandboxStateError should be raised") +def step_cow_state_error(ctx: Context) -> None: + assert ctx.cow_error is not None + assert isinstance(ctx.cow_error, SandboxStateError) + + +# --------------------------------------------------------------------------- +# Then - path resolution +# --------------------------------------------------------------------------- + + +@then("the cow resolved path should be inside the sandbox copy") +def step_cow_path_in_sandbox(ctx: Context) -> None: + assert ctx.cow_resolved_path is not None + sandbox_path = ctx.cow_sandbox.context.sandbox_path + assert ctx.cow_resolved_path.startswith(sandbox_path) + + +# --------------------------------------------------------------------------- +# Then - commit results +# --------------------------------------------------------------------------- + + +@then("the cow commit result should indicate success") +def step_cow_commit_success(ctx: Context) -> None: + assert ctx.cow_commit_result is not None + assert ctx.cow_commit_result.success is True + + +@then("the cow commit result should have {count:d} changed files") +def step_cow_commit_changed(ctx: Context, count: int) -> None: + assert ctx.cow_commit_result is not None + assert len(ctx.cow_commit_result.changed_files) == count, ( + f"Expected {count} changed, got {len(ctx.cow_commit_result.changed_files)}" + ) + + +@then("the cow commit result should have {count:d} added files") +def step_cow_commit_added(ctx: Context, count: int) -> None: + assert ctx.cow_commit_result is not None + assert len(ctx.cow_commit_result.added_files) == count, ( + f"Expected {count} added, got {len(ctx.cow_commit_result.added_files)}" + ) + + +@then("the cow commit result should have {count:d} deleted files") +def step_cow_commit_deleted(ctx: Context, count: int) -> None: + assert ctx.cow_commit_result is not None + assert len(ctx.cow_commit_result.deleted_files) == count, ( + f"Expected {count} deleted, got {len(ctx.cow_commit_result.deleted_files)}" + ) + + +@then( + 'the cow file "{filename}" should exist in the original directory with content "{content}"' +) +def step_cow_file_in_original_with_content( + ctx: Context, filename: str, content: str +) -> None: + file_path = os.path.join(ctx.cow_test_dir, filename) + assert os.path.exists(file_path), f"File {filename} not found" + with open(file_path) as f: + actual = f.read() + assert actual == content, f"Expected '{content}', got '{actual}'" + + +@then('the cow file "{filename}" in the original should have content "{content}"') +def step_cow_file_content(ctx: Context, filename: str, content: str) -> None: + file_path = os.path.join(ctx.cow_test_dir, filename) + with open(file_path) as f: + actual = f.read() + assert actual == content, f"Expected '{content}', got '{actual}'" + + +@then('the cow file "{filename}" should not exist in the original directory') +def step_cow_file_not_in_original(ctx: Context, filename: str) -> None: + file_path = os.path.join(ctx.cow_test_dir, filename) + assert not os.path.exists(file_path), f"File {filename} should not exist" + + +@then('the cow file "{filename}" should not exist in the sandbox') +def step_cow_file_not_in_sandbox(ctx: Context, filename: str) -> None: + if ctx.cow_sandbox.context: + file_path = os.path.join(ctx.cow_sandbox.context.sandbox_path, filename) + assert not os.path.exists(file_path) + + +# --------------------------------------------------------------------------- +# Then - protocol properties +# --------------------------------------------------------------------------- + + +@then("the cow sandbox_id should be a valid ULID") +def step_cow_valid_ulid(ctx: Context) -> None: + sid = ctx.cow_sandbox.sandbox_id + assert len(sid) == 26, f"Expected ULID length 26, got {len(sid)}" + + +@then('the cow sandbox status should be "{status}"') +def step_cow_status_is(ctx: Context, status: str) -> None: + assert ctx.cow_sandbox.status == SandboxStatus(status) + + +@then("the cow sandbox context should be None") +def step_cow_context_none(ctx: Context) -> None: + assert ctx.cow_sandbox.context is None diff --git a/features/steps/git_worktree_sandbox_steps.py b/features/steps/git_worktree_sandbox_steps.py new file mode 100644 index 000000000..ba93141ad --- /dev/null +++ b/features/steps/git_worktree_sandbox_steps.py @@ -0,0 +1,404 @@ +"""Step definitions for git worktree sandbox feature. + +All steps use the ``gwt`` prefix to avoid collisions with the 128+ existing +step files loaded globally by Behave. +""" + +from __future__ import annotations + +import os +import subprocess +import tempfile + +from behave import given, then, when +from behave.runner import Context + +from cleveragents.infrastructure.sandbox.git_worktree import GitWorktreeSandbox +from cleveragents.infrastructure.sandbox.protocol import ( + SandboxCreationError, + SandboxStateError, + SandboxStatus, +) + + +def _init_test_repo(ctx: Context) -> str: + """Create a temporary git repo with an initial commit.""" + repo_dir = tempfile.mkdtemp(prefix="gwt-test-repo-") + subprocess.run(["git", "init"], cwd=repo_dir, capture_output=True, check=True) + subprocess.run( + ["git", "config", "user.email", "test@test.com"], + cwd=repo_dir, + capture_output=True, + check=True, + ) + subprocess.run( + ["git", "config", "user.name", "Test"], + cwd=repo_dir, + capture_output=True, + check=True, + ) + subprocess.run( + ["git", "config", "commit.gpgSign", "false"], + cwd=repo_dir, + capture_output=True, + check=True, + ) + # Create initial files + readme = os.path.join(repo_dir, "README.md") + with open(readme, "w") as f: + f.write("# Test Repo\n") + subprocess.run(["git", "add", "."], cwd=repo_dir, capture_output=True, check=True) + subprocess.run( + ["git", "commit", "-m", "Initial commit"], + cwd=repo_dir, + capture_output=True, + check=True, + ) + return repo_dir + + +# --------------------------------------------------------------------------- +# Background / Given +# --------------------------------------------------------------------------- + + +@given("a gwt test git repository is initialised") +def step_gwt_init_repo(ctx: Context) -> None: + ctx.gwt_repo_dir = _init_test_repo(ctx) + ctx.gwt_sandbox = None + ctx.gwt_error = None + ctx.gwt_commit_result = None + ctx.gwt_resolved_path = None + + +@given("a gwt non-git directory") +def step_gwt_non_git_dir(ctx: Context) -> None: + ctx.gwt_non_git_dir = tempfile.mkdtemp(prefix="gwt-non-git-") + + +# --------------------------------------------------------------------------- +# When - instantiation +# --------------------------------------------------------------------------- + + +@when("a gwt sandbox is instantiated") +def step_gwt_instantiate(ctx: Context) -> None: + ctx.gwt_sandbox = GitWorktreeSandbox( + resource_id="res-001", + original_path=ctx.gwt_repo_dir, + ) + + +@when("a gwt sandbox is prepared with empty resource_id") +def step_gwt_empty_resource_id(ctx: Context) -> None: + try: + GitWorktreeSandbox(resource_id="", original_path=ctx.gwt_repo_dir) + except ValueError as exc: + ctx.gwt_error = exc + + +@when("a gwt sandbox is prepared with empty original_path") +def step_gwt_empty_original_path(ctx: Context) -> None: + try: + GitWorktreeSandbox(resource_id="res-001", original_path="") + except ValueError as exc: + ctx.gwt_error = exc + + +@when("a gwt sandbox is prepared with zero timeout") +def step_gwt_zero_timeout(ctx: Context) -> None: + try: + GitWorktreeSandbox( + resource_id="res-001", + original_path=ctx.gwt_repo_dir, + git_timeout=0, + ) + except ValueError as exc: + ctx.gwt_error = exc + + +# --------------------------------------------------------------------------- +# When - create +# --------------------------------------------------------------------------- + + +@when('a gwt sandbox is created for plan "{plan_id}"') +def step_gwt_create(ctx: Context, plan_id: str) -> None: + ctx.gwt_sandbox = GitWorktreeSandbox( + resource_id="res-001", + original_path=ctx.gwt_repo_dir, + ) + ctx.gwt_sandbox.create(plan_id) + + +@when("a gwt sandbox is created with empty plan_id") +def step_gwt_create_empty_plan(ctx: Context) -> None: + ctx.gwt_sandbox = GitWorktreeSandbox( + resource_id="res-001", + original_path=ctx.gwt_repo_dir, + ) + try: + ctx.gwt_sandbox.create("") + except ValueError as exc: + ctx.gwt_error = exc + + +@when('a gwt sandbox is created on the non-git directory for plan "{plan_id}"') +def step_gwt_create_non_git(ctx: Context, plan_id: str) -> None: + ctx.gwt_sandbox = GitWorktreeSandbox( + resource_id="res-001", + original_path=ctx.gwt_non_git_dir, + ) + try: + ctx.gwt_sandbox.create(plan_id) + except SandboxCreationError as exc: + ctx.gwt_error = exc + + +# --------------------------------------------------------------------------- +# When - path resolution +# --------------------------------------------------------------------------- + + +@when('the gwt path "{path}" is resolved') +def step_gwt_resolve_path(ctx: Context, path: str) -> None: + try: + ctx.gwt_resolved_path = ctx.gwt_sandbox.get_path(path) + except (ValueError, SandboxStateError) as exc: + ctx.gwt_error = exc + + +@when('the gwt path "{path}" is resolved on a cleaned-up sandbox') +def step_gwt_resolve_path_cleaned(ctx: Context, path: str) -> None: + try: + ctx.gwt_sandbox.get_path(path) + except SandboxStateError as exc: + ctx.gwt_error = exc + + +# --------------------------------------------------------------------------- +# When - file operations in worktree +# --------------------------------------------------------------------------- + + +@when('a gwt file "{filename}" is created in the worktree with content "{content}"') +def step_gwt_create_file(ctx: Context, filename: str, content: str) -> None: + worktree_path = ctx.gwt_sandbox.context.sandbox_path + file_path = os.path.join(worktree_path, filename) + os.makedirs(os.path.dirname(file_path), exist_ok=True) + with open(file_path, "w") as f: + f.write(content) + + +@when('the gwt existing file "{filename}" is modified in the worktree') +def step_gwt_modify_file(ctx: Context, filename: str) -> None: + worktree_path = ctx.gwt_sandbox.context.sandbox_path + file_path = os.path.join(worktree_path, filename) + with open(file_path, "a") as f: + f.write("\n# Modified by sandbox\n") + + +# --------------------------------------------------------------------------- +# When - commit / rollback / cleanup +# --------------------------------------------------------------------------- + + +@when('the gwt sandbox is committed with message "{message}"') +def step_gwt_commit(ctx: Context, message: str) -> None: + try: + ctx.gwt_commit_result = ctx.gwt_sandbox.commit(message) + except (SandboxStateError, Exception) as exc: + ctx.gwt_error = exc + + +@when("the gwt sandbox commit is attempted on cleaned-up sandbox") +def step_gwt_commit_cleaned(ctx: Context) -> None: + try: + ctx.gwt_sandbox.commit("should fail") + except SandboxStateError as exc: + ctx.gwt_error = exc + + +@when("the gwt sandbox is rolled back") +def step_gwt_rollback(ctx: Context) -> None: + try: + ctx.gwt_sandbox.rollback() + except (SandboxStateError, Exception) as exc: + ctx.gwt_error = exc + + +@when("the gwt sandbox rollback is attempted on created sandbox") +def step_gwt_rollback_created(ctx: Context) -> None: + try: + ctx.gwt_sandbox.rollback() + except SandboxStateError as exc: + ctx.gwt_error = exc + + +@when("the gwt sandbox is cleaned up") +def step_gwt_cleanup(ctx: Context) -> None: + ctx.gwt_sandbox.cleanup() + + +@when("the gwt sandbox is cleaned up again") +def step_gwt_cleanup_again(ctx: Context) -> None: + ctx.gwt_sandbox.cleanup() + + +# --------------------------------------------------------------------------- +# Then - status / state +# --------------------------------------------------------------------------- + + +@then('the gwt sandbox should be in the "{status}" state') +def step_gwt_check_status(ctx: Context, status: str) -> None: + expected = SandboxStatus(status) + assert ctx.gwt_sandbox.status == expected, ( + f"Expected status {expected}, got {ctx.gwt_sandbox.status}" + ) + + +@then('the gwt sandbox context should reference plan "{plan_id}"') +def step_gwt_check_plan(ctx: Context, plan_id: str) -> None: + assert ctx.gwt_sandbox.context is not None + assert ctx.gwt_sandbox.context.plan_id == plan_id + + +@then('the gwt sandbox context should have strategy metadata "{strategy}"') +def step_gwt_check_strategy(ctx: Context, strategy: str) -> None: + assert ctx.gwt_sandbox.context is not None + assert ctx.gwt_sandbox.context.metadata.get("strategy") == strategy + + +@then("the gwt sandbox worktree path should exist") +def step_gwt_worktree_exists(ctx: Context) -> None: + assert ctx.gwt_sandbox.context is not None + assert os.path.isdir(ctx.gwt_sandbox.context.sandbox_path) + + +@then("the gwt sandbox worktree path should not exist") +def step_gwt_worktree_not_exists(ctx: Context) -> None: + if ctx.gwt_sandbox.context is not None: + assert not os.path.exists(ctx.gwt_sandbox.context.sandbox_path) + + +@then("the gwt sandbox branch should not exist") +def step_gwt_branch_not_exists(ctx: Context) -> None: + result = subprocess.run( + ["git", "branch", "--list"], + cwd=ctx.gwt_repo_dir, + capture_output=True, + text=True, + check=False, + ) + branch_name = ctx.gwt_sandbox._branch_name + if branch_name: + assert branch_name not in result.stdout + + +# --------------------------------------------------------------------------- +# Then - errors +# --------------------------------------------------------------------------- + + +@then('a gwt ValueError should be raised with message "{msg}"') +def step_gwt_valueerror(ctx: Context, msg: str) -> None: + assert ctx.gwt_error is not None, "Expected an error but none occurred" + assert isinstance(ctx.gwt_error, ValueError), ( + f"Expected ValueError, got {type(ctx.gwt_error).__name__}" + ) + assert msg in str(ctx.gwt_error), ( + f"Expected '{msg}' in error message, got: {ctx.gwt_error}" + ) + + +@then("a gwt SandboxCreationError should be raised") +def step_gwt_creation_error(ctx: Context) -> None: + assert ctx.gwt_error is not None + assert isinstance(ctx.gwt_error, SandboxCreationError) + + +@then("a gwt SandboxStateError should be raised") +def step_gwt_state_error(ctx: Context) -> None: + assert ctx.gwt_error is not None + assert isinstance(ctx.gwt_error, SandboxStateError) + + +# --------------------------------------------------------------------------- +# Then - path resolution +# --------------------------------------------------------------------------- + + +@then("the gwt resolved path should be inside the worktree") +def step_gwt_path_in_worktree(ctx: Context) -> None: + assert ctx.gwt_resolved_path is not None + sandbox_path = ctx.gwt_sandbox.context.sandbox_path + assert ctx.gwt_resolved_path.startswith(sandbox_path) + + +# --------------------------------------------------------------------------- +# Then - commit results +# --------------------------------------------------------------------------- + + +@then("the gwt commit result should indicate success") +def step_gwt_commit_success(ctx: Context) -> None: + assert ctx.gwt_commit_result is not None + assert ctx.gwt_commit_result.success is True + + +@then("the gwt commit result should have {count:d} changed files") +def step_gwt_commit_changed(ctx: Context, count: int) -> None: + assert ctx.gwt_commit_result is not None + assert len(ctx.gwt_commit_result.changed_files) == count + + +@then("the gwt commit result should have {count:d} added files") +def step_gwt_commit_added(ctx: Context, count: int) -> None: + assert ctx.gwt_commit_result is not None + assert len(ctx.gwt_commit_result.added_files) == count + + +@then('the gwt file "{filename}" should exist in the original repo') +def step_gwt_file_in_original(ctx: Context, filename: str) -> None: + file_path = os.path.join(ctx.gwt_repo_dir, filename) + assert os.path.exists(file_path), f"File {filename} not found in original repo" + + +@then('the gwt file "{filename}" should not exist in the worktree') +def step_gwt_file_not_in_worktree(ctx: Context, filename: str) -> None: + if ctx.gwt_sandbox.context: + file_path = os.path.join(ctx.gwt_sandbox.context.sandbox_path, filename) + assert not os.path.exists(file_path) + + +# --------------------------------------------------------------------------- +# Then - protocol properties +# --------------------------------------------------------------------------- + + +@then("the gwt sandbox_id should be a valid ULID") +def step_gwt_valid_ulid(ctx: Context) -> None: + sid = ctx.gwt_sandbox.sandbox_id + assert len(sid) == 26, f"Expected ULID length 26, got {len(sid)}" + + +@then('the gwt sandbox status should be "{status}"') +def step_gwt_status_is(ctx: Context, status: str) -> None: + assert ctx.gwt_sandbox.status == SandboxStatus(status) + + +@then("the gwt sandbox context should be None") +def step_gwt_context_none(ctx: Context) -> None: + assert ctx.gwt_sandbox.context is None + + +@then("the gwt sandbox branch name should be safe for git") +def step_gwt_branch_safe(ctx: Context) -> None: + branch = ctx.gwt_sandbox._branch_name + assert branch is not None + # Branch name should not contain spaces or special chars + assert " " not in branch + assert "!" not in branch + assert "@" not in branch + assert "#" not in branch diff --git a/features/steps/sandbox_factory_coverage_steps.py b/features/steps/sandbox_factory_coverage_steps.py index 89d16a0b5..9498735b3 100644 --- a/features/steps/sandbox_factory_coverage_steps.py +++ b/features/steps/sandbox_factory_coverage_steps.py @@ -1,22 +1,24 @@ """Step definitions for sandbox factory strategy routing coverage. -Covers uncovered lines in ``factory.py``: -- Empty resource_id / original_path validation (lines 88, 90) -- git_worktree strategy NotImplementedError (lines 98-99) -- copy_on_write strategy NotImplementedError (lines 104-105) -- overlay strategy NotImplementedError + warning (lines 110-111, 115) -- transaction_rollback strategy NotImplementedError (lines 120-121) -- versioning strategy NotImplementedError (lines 125-126) -- Unknown strategy ValueError (line 128) -- is_supported() for unsupported strategies (line 144) -- get_supported_strategies() fallback for unknown resource types (line 157) +Covers lines in ``factory.py``: +- Empty resource_id / original_path validation +- none strategy -> NoSandbox +- git_worktree strategy -> GitWorktreeSandbox +- copy_on_write strategy -> CopyOnWriteSandbox +- transaction_rollback / snapshot -> NotImplementedError +- overlay / versioning (removed) -> ValueError (unknown) +- Unknown strategy -> ValueError +- is_supported() for all strategies +- get_supported_strategies() for spec-aligned resource types """ from __future__ import annotations from behave import given, then, when +from cleveragents.infrastructure.sandbox.copy_on_write import CopyOnWriteSandbox from cleveragents.infrastructure.sandbox.factory import SandboxFactory +from cleveragents.infrastructure.sandbox.git_worktree import GitWorktreeSandbox from cleveragents.infrastructure.sandbox.no_sandbox import NoSandbox from cleveragents.infrastructure.sandbox.protocol import SandboxStatus @@ -84,14 +86,11 @@ def when_factory_none_strategy(context, res_id: str, path: str): def when_factory_git_worktree(context, res_id: str, path: str): """Request a sandbox with the 'git_worktree' strategy.""" context.factory_error = None - try: - context.sandbox_factory.create_sandbox( - resource_id=res_id, - original_path=path, - sandbox_strategy="git_worktree", - ) - except NotImplementedError as exc: - context.factory_error = exc + context.factory_sandbox = context.sandbox_factory.create_sandbox( + resource_id=res_id, + original_path=path, + sandbox_strategy="git_worktree", + ) @when( @@ -101,14 +100,11 @@ def when_factory_git_worktree(context, res_id: str, path: str): def when_factory_copy_on_write(context, res_id: str, path: str): """Request a sandbox with the 'copy_on_write' strategy.""" context.factory_error = None - try: - context.sandbox_factory.create_sandbox( - resource_id=res_id, - original_path=path, - sandbox_strategy="copy_on_write", - ) - except NotImplementedError as exc: - context.factory_error = exc + context.factory_sandbox = context.sandbox_factory.create_sandbox( + resource_id=res_id, + original_path=path, + sandbox_strategy="copy_on_write", + ) @when( @@ -116,15 +112,15 @@ def when_factory_copy_on_write(context, res_id: str, path: str): "using the overlay strategy" ) def when_factory_overlay(context, res_id: str, path: str): - """Request a sandbox with the 'overlay' strategy.""" + """Request a sandbox with the removed 'overlay' strategy (now unknown).""" context.factory_error = None try: context.sandbox_factory.create_sandbox( resource_id=res_id, original_path=path, - sandbox_strategy="overlay", + sandbox_strategy="overlay", # type: ignore[arg-type] ) - except NotImplementedError as exc: + except ValueError as exc: context.factory_error = exc @@ -150,13 +146,30 @@ def when_factory_transaction_rollback(context, res_id: str, path: str): "using the versioning strategy" ) def when_factory_versioning(context, res_id: str, path: str): - """Request a sandbox with the 'versioning' strategy.""" + """Request a sandbox with the removed 'versioning' strategy (now unknown).""" context.factory_error = None try: context.sandbox_factory.create_sandbox( resource_id=res_id, original_path=path, - sandbox_strategy="versioning", + sandbox_strategy="versioning", # type: ignore[arg-type] + ) + except ValueError as exc: + context.factory_error = exc + + +@when( + 'a sandbox is requested for resource "{res_id}" at "{path}" ' + "using the snapshot strategy" +) +def when_factory_snapshot(context, res_id: str, path: str): + """Request a sandbox with the 'snapshot' strategy (not yet implemented).""" + context.factory_error = None + try: + context.sandbox_factory.create_sandbox( + resource_id=res_id, + original_path=path, + sandbox_strategy="snapshot", ) except NotImplementedError as exc: context.factory_error = exc @@ -279,6 +292,40 @@ def then_factory_sandbox_pending(context): assert context.factory_sandbox.status == SandboxStatus.PENDING +# --------------------------------------------------------------------------- +# Thens - git worktree strategy produces sandbox +# --------------------------------------------------------------------------- + + +@then("the factory should produce a git worktree sandbox") +def then_factory_produces_git_worktree(context): + """Assert the returned sandbox is a GitWorktreeSandbox instance.""" + assert isinstance(context.factory_sandbox, GitWorktreeSandbox) + + +@then("the produced git worktree sandbox should be in the pending state") +def then_factory_git_worktree_pending(context): + """Assert the git worktree sandbox is in PENDING status.""" + assert context.factory_sandbox.status == SandboxStatus.PENDING + + +# --------------------------------------------------------------------------- +# Thens - copy-on-write strategy produces sandbox +# --------------------------------------------------------------------------- + + +@then("the factory should produce a copy-on-write sandbox") +def then_factory_produces_cow(context): + """Assert the returned sandbox is a CopyOnWriteSandbox instance.""" + assert isinstance(context.factory_sandbox, CopyOnWriteSandbox) + + +@then("the produced copy-on-write sandbox should be in the pending state") +def then_factory_cow_pending(context): + """Assert the copy-on-write sandbox is in PENDING status.""" + assert context.factory_sandbox.status == SandboxStatus.PENDING + + # --------------------------------------------------------------------------- # Thens - not yet implemented # --------------------------------------------------------------------------- @@ -314,30 +361,18 @@ def then_factory_supported_false(context): @then("the factory should return git worktree, copy-on-write, and none as compatible") -def then_factory_git_repo_strategies(context): - """Assert the git_repository strategies are correct.""" +def then_factory_git_checkout_strategies(context): + """Assert the git-checkout strategies are correct.""" assert context.factory_strategies == ["git_worktree", "copy_on_write", "none"] -@then("the factory should return copy-on-write, overlay, and none as compatible") -def then_factory_filesystem_strategies(context): - """Assert the filesystem strategies are correct.""" - assert context.factory_strategies == ["copy_on_write", "overlay", "none"] - - -@then("the factory should return transaction rollback and none as compatible") -def then_factory_database_strategies(context): - """Assert the database strategies are correct.""" - assert context.factory_strategies == ["transaction_rollback", "none"] +@then("the factory should return copy-on-write and none as compatible") +def then_factory_cow_none_strategies(context): + """Assert the resource type supports copy_on_write and none.""" + assert context.factory_strategies == ["copy_on_write", "none"] @then("the factory should return only the none strategy as compatible") def then_factory_none_only_strategies(context): """Assert only the 'none' strategy is returned.""" assert context.factory_strategies == ["none"] - - -@then("the factory should return copy-on-write and none as compatible") -def then_factory_doc_corpus_strategies(context): - """Assert the document_corpus strategies are correct.""" - assert context.factory_strategies == ["copy_on_write", "none"] diff --git a/implementation_plan.md b/implementation_plan.md index 534a5f7f5..265d0f932 100644 --- a/implementation_plan.md +++ b/implementation_plan.md @@ -1977,18 +1977,18 @@ Merge points and acceptance checks are tracked as checklist items under each mil - [ ] Git [Hamza]: `git pull origin master` - [ ] Git [Hamza]: `git checkout -b feature/m2-resource-core-models` - [ ] Git [Hamza]: `git fetch origin && git merge origin/master` (run before final tests and before commit) - - [ ] Code [Hamza]: Create `src/cleveragents/domain/models/core/resource_type.py` with `ResourceTypeSpec`, `ResourceTypeArgument`, `ResourceKind` (physical/virtual), and `SandboxStrategy` enum. - - [ ] Code [Hamza]: Define `ResourceTypeArgument` fields for CLI flags (`flag`, `help`, `arg_type`, `required`, `default`, `repeatable`, `choices`) and validate flag naming conventions. - - [ ] Code [Hamza]: Add resource type fields: `user_addable`, `allowed_parents`, `allowed_children`, `auto_discover`, `handler` reference, and `sandbox_strategy` default. - - [ ] Code [Hamza]: Add `Resource` and `ResourceRef` models with ULID, optional namespaced name, type name, location, description, sandbox strategy, read_only, and metadata. - - [ ] Code [Hamza]: Enforce user-added resources require namespaced name; auto-discovered child resources are ULID-only with no namespaced name. - - [ ] Code [Hamza]: Add virtual resource validation rules (no location; cannot be directly read/write; distinct from physical). - - [ ] Code [Hamza]: Add validators for namespaced naming, ULID format, and parent/child DAG sanity (no self loops, no duplicate edges, type compatibility). - - [ ] Code [Hamza]: Add `ResourceName.parse()` and `ResourceTypeName.parse()` helpers to normalize `local/` default and reject invalid namespaces. + - [X] Code [Hamza]: Create `src/cleveragents/domain/models/core/resource_type.py` with `ResourceTypeSpec`, `ResourceTypeArgument`, `ResourceKind` (physical/virtual), and `SandboxStrategy` enum. (delivered in PR #51 as resource.py — PhysVirt, SandboxStrategy with 5 spec values, ResourceCapabilities) + - [ ] Code [Hamza]: Define `ResourceTypeArgument` fields for CLI flags (`flag`, `help`, `arg_type`, `required`, `default`, `repeatable`, `choices`) and validate flag naming conventions. + - [ ] Code [Hamza]: Add resource type fields: `user_addable`, `allowed_parents`, `allowed_children`, `auto_discover`, `handler` reference, and `sandbox_strategy` default. + - [X] Code [Hamza]: Add `Resource` and `ResourceRef` models with ULID, optional namespaced name, type name, location, description, sandbox strategy, read_only, and metadata. (delivered in PR #51 — Resource model with ULID PK, resource_type_name, properties, capabilities, DAG parents/children) + - [X] Code [Hamza]: Enforce user-added resources require namespaced name; auto-discovered child resources are ULID-only with no namespaced name. (delivered in PR #51 — name is Optional) + - [X] Code [Hamza]: Add virtual resource validation rules (no location; cannot be directly read/write; distinct from physical). (delivered in PR #51 — PhysVirt enum, classification field) + - [X] Code [Hamza]: Add validators for namespaced naming, ULID format, and parent/child DAG sanity (no self loops, no duplicate edges, type compatibility). (delivered in PR #51 — parse_namespaced_name, ULID validation) + - [X] Code [Hamza]: Add `ResourceName.parse()` and `ResourceTypeName.parse()` helpers to normalize `local/` default and reject invalid namespaces. (delivered in PR #51 — parse_namespaced_name with reserved/provider namespace validation) - [ ] Code [Hamza]: Add `docs/schema/resource_type.schema.yaml` with CLI argument definitions, parent/child constraints, and handler metadata. - [ ] Code [Hamza]: Add resource type YAML loader in `src/cleveragents/resource/schema.py` with version guard and clear error messages. - [ ] Docs [Hamza]: Add `docs/reference/resource_model.md` with examples for git-checkout and fs-directory resources plus physical/virtual notes. - - [ ] Tests (Behave) [Hamza]: Add scenarios validating ULID format, namespace rules, allowed parent/child type checks, and sandbox strategy defaults. + - [X] Tests (Behave) [Hamza]: Add scenarios validating ULID format, namespace rules, allowed parent/child type checks, and sandbox strategy defaults. (delivered in PR #51 — 33 resource_registry_model scenarios + 48 namespaced_project_model scenarios) - [ ] Tests (Robot) [Hamza]: Add Robot test that loads a ResourceTypeSpec YAML fixture and validates it. - [ ] Tests (ASV) [Hamza]: Add `asv/benchmarks/resource_model_bench.py` for resource validation + DAG checks. - [ ] Quality [Hamza]: Run `nox` (all default sessions, including benchmark). @@ -2047,14 +2047,14 @@ Merge points and acceptance checks are tracked as checklist items under each mil - [ ] Git [Hamza]: `git pull origin master` - [ ] Git [Hamza]: `git checkout -b feature/m2-resource-core-models` - [ ] Git [Hamza]: `git fetch origin && git merge origin/master` (run before final tests and before commit) - - [ ] Code [Hamza]: Add `Project`, `ProjectResourceLink`, `ProjectValidationSummary` (derived from validation attachments), and `ProjectContextPolicy` models using namespaced name as the unique identifier (no ULID per spec). - - [ ] Code [Hamza]: Add fields for `invariants`, `invariant_actor`, `automation_profile`, and `context_views` (strategize/execute/apply/default). - - [ ] Code [Hamza]: Add validation for resource link overrides (read_only flags, alias uniqueness, resource existence). - - [ ] Code [Hamza]: Add helpers to compute effective invariants and automation profile (project defaults). - - [ ] Code [Hamza]: Define default context view policy when `context_views` is omitted (inherit from `default` view; explicit override per view). - - [ ] Code [Hamza]: Enforce non-empty invariant text and deterministic ordering for project-level invariants. - - [ ] Docs [Hamza]: Add `docs/reference/project_model.md` describing resource linking, validation attachments, and context view policies. - - [ ] Tests (Behave) [Hamza]: Add scenarios for project model validation, link overrides, and context view inheritance. + - [X] Code [Hamza]: Add `Project`, `ProjectResourceLink`, `ProjectValidationSummary` (derived from validation attachments), and `ProjectContextPolicy` models using namespaced name as the unique identifier (no ULID per spec). (delivered in PR #51 — NamespacedProject with ParsedName, LinkedResource, ContextConfig) + - [X] Code [Hamza]: Add fields for `invariants`, `invariant_actor`, `automation_profile`, and `context_views` (strategize/execute/apply/default). (delivered in PR #51) + - [X] Code [Hamza]: Add validation for resource link overrides (read_only flags, alias uniqueness, resource existence). (delivered in PR #51 — LinkedResource with project_read_only, alias) + - [X] Code [Hamza]: Add helpers to compute effective invariants and automation profile (project defaults). (delivered in PR #51) + - [X] Code [Hamza]: Define default context view policy when `context_views` is omitted (inherit from `default` view; explicit override per view). (delivered in PR #51 — ContextConfig) + - [X] Code [Hamza]: Enforce non-empty invariant text and deterministic ordering for project-level invariants. (delivered in PR #51) + - [ ] Docs [Hamza]: Add `docs/reference/project_model.md` describing resource linking, validation attachments, and context view policies. + - [X] Tests (Behave) [Hamza]: Add scenarios for project model validation, link overrides, and context view inheritance. (delivered in PR #51 — 48 namespaced_project_model scenarios) - [ ] Tests (Robot) [Hamza]: Add Robot test that creates a Project object and prints serialized output. - [ ] Tests (ASV) [Hamza]: Add `asv/benchmarks/project_model_bench.py` for serialization/validation performance. - [ ] Quality [Hamza]: Run `nox` (all default sessions, including benchmark). diff --git a/robot/helper_sandbox_integration.py b/robot/helper_sandbox_integration.py index 6f1be8e84..1137d7293 100644 --- a/robot/helper_sandbox_integration.py +++ b/robot/helper_sandbox_integration.py @@ -124,8 +124,10 @@ def _no_sandbox_rollback_raises() -> None: def _factory_create_none_strategy() -> None: - """Integration test: SandboxFactory creates NoSandbox for 'none' strategy.""" + """Integration test: SandboxFactory creates correct sandbox for each strategy.""" factory = SandboxFactory() + + # 'none' strategy -> NoSandbox sandbox = factory.create_sandbox( resource_id="res-003", original_path="/tmp/test", @@ -133,14 +135,34 @@ def _factory_create_none_strategy() -> None: ) assert isinstance(sandbox, NoSandbox) - # Verify unsupported strategies raise + # 'git_worktree' strategy -> GitWorktreeSandbox (implemented in B4) + from cleveragents.infrastructure.sandbox.git_worktree import GitWorktreeSandbox + + gwt_sandbox = factory.create_sandbox( + resource_id="res-004", + original_path="/tmp/test", + sandbox_strategy="git_worktree", + ) + assert isinstance(gwt_sandbox, GitWorktreeSandbox) + + # 'copy_on_write' strategy -> CopyOnWriteSandbox (implemented in B4) + from cleveragents.infrastructure.sandbox.copy_on_write import CopyOnWriteSandbox + + cow_sandbox = factory.create_sandbox( + resource_id="res-005", + original_path="/tmp/test", + sandbox_strategy="copy_on_write", + ) + assert isinstance(cow_sandbox, CopyOnWriteSandbox) + + # Verify unimplemented strategies still raise try: factory.create_sandbox( - resource_id="res-004", + resource_id="res-006", original_path="/tmp/test", - sandbox_strategy="git_worktree", + sandbox_strategy="transaction_rollback", ) - print("FAIL: Expected NotImplementedError") + print("FAIL: Expected NotImplementedError for transaction_rollback") return except NotImplementedError: pass @@ -150,17 +172,28 @@ def _factory_create_none_strategy() -> None: def _factory_supported_strategies() -> None: """Integration test: SandboxFactory strategy support checks.""" + # All three concrete implementations are now supported assert SandboxFactory.is_supported("none") - assert not SandboxFactory.is_supported("git_worktree") - assert not SandboxFactory.is_supported("copy_on_write") + assert SandboxFactory.is_supported("git_worktree") + assert SandboxFactory.is_supported("copy_on_write") - git_strats = SandboxFactory.get_supported_strategies("git_repository") - assert "none" in git_strats - assert "git_worktree" in git_strats + # Unimplemented strategies are not supported + assert not SandboxFactory.is_supported("transaction_rollback") + assert not SandboxFactory.is_supported("snapshot") + + # Resource type strategy mapping (spec-aligned) + git_checkout_strats = SandboxFactory.get_supported_strategies("git-checkout") + assert "none" in git_checkout_strats + assert "git_worktree" in git_checkout_strats + assert "copy_on_write" in git_checkout_strats api_strats = SandboxFactory.get_supported_strategies("api_endpoint") assert "none" in api_strats + # Unknown resource types default to ["none"] + unknown_strats = SandboxFactory.get_supported_strategies("unknown_type") + assert unknown_strats == ["none"] + print("factory-supported-ok") diff --git a/src/cleveragents/infrastructure/sandbox/__init__.py b/src/cleveragents/infrastructure/sandbox/__init__.py index a4ac435ab..600e4e6d2 100644 --- a/src/cleveragents/infrastructure/sandbox/__init__.py +++ b/src/cleveragents/infrastructure/sandbox/__init__.py @@ -3,10 +3,12 @@ Provides resource isolation during plan execution through the Sandbox protocol and multiple strategy implementations (git worktree, filesystem copy, no-op). -Stage B3 of the implementation plan. +Stage B3 of the implementation plan. Updated in TASK-006 (B4 rework). """ +from cleveragents.infrastructure.sandbox.copy_on_write import CopyOnWriteSandbox from cleveragents.infrastructure.sandbox.factory import SandboxFactory +from cleveragents.infrastructure.sandbox.git_worktree import GitWorktreeSandbox from cleveragents.infrastructure.sandbox.manager import SandboxManager from cleveragents.infrastructure.sandbox.merge import ( GitMergeStrategy, @@ -26,7 +28,9 @@ from cleveragents.infrastructure.sandbox.protocol import ( __all__ = [ "CommitResult", + "CopyOnWriteSandbox", "GitMergeStrategy", + "GitWorktreeSandbox", "JsonMergeStrategy", "MergeResult", "MergeStrategy", diff --git a/src/cleveragents/infrastructure/sandbox/copy_on_write.py b/src/cleveragents/infrastructure/sandbox/copy_on_write.py new file mode 100644 index 000000000..8ee2df008 --- /dev/null +++ b/src/cleveragents/infrastructure/sandbox/copy_on_write.py @@ -0,0 +1,404 @@ +"""Copy-on-write sandbox for filesystem resources. + +Creates an isolated copy of a directory tree where plan modifications are +made. On ``commit``, changed files are synced back to the original. +On ``rollback``, the copy is discarded. + +Suitable for ``fs-mount``, ``fs-directory``, and any non-git resource +that needs filesystem-level isolation. + +Stage B3.4 / TASK-006 of the implementation plan. +""" + +from __future__ import annotations + +import filecmp +import logging +import os +import shutil +import tempfile +from datetime import datetime + +from ulid import ULID + +from cleveragents.infrastructure.sandbox.protocol import ( + CommitResult, + SandboxCommitError, + SandboxContext, + SandboxCreationError, + SandboxRollbackError, + SandboxStateError, + SandboxStatus, +) + +logger = logging.getLogger(__name__) + + +class CopyOnWriteSandbox: + """Sandbox that isolates changes in a filesystem copy. + + Copies the original directory to a temporary location. The actor + modifies the copy; on commit a diff-based sync copies only changed + files back to the original. On rollback the copy is simply deleted. + + Implements the + :class:`~cleveragents.infrastructure.sandbox.protocol.Sandbox` protocol. + + Lifecycle:: + + sandbox = CopyOnWriteSandbox(resource_id, original_path) + ctx = sandbox.create(plan_id) # copies directory + path = sandbox.get_path("data.csv") # resolve path in copy + # ... actor writes ... + result = sandbox.commit("msg") # sync changed files back + sandbox.cleanup() # remove the copy + """ + + def __init__( + self, + resource_id: str, + original_path: str, + ) -> None: + """Initialise a copy-on-write sandbox. + + Args: + resource_id: Identifier of the resource being sandboxed. + original_path: Path to the directory to sandbox. + + Raises: + ValueError: If *resource_id* or *original_path* is empty. + """ + if not resource_id: + raise ValueError("resource_id cannot be empty") + if not original_path: + raise ValueError("original_path cannot be empty") + + self._sandbox_id: str = str(ULID()) + self._resource_id: str = resource_id + self._original_path: str = os.path.abspath(original_path) + self._status: SandboxStatus = SandboxStatus.PENDING + self._context: SandboxContext | None = None + + # Set after create() + self._sandbox_path: str | None = None + + # -- protocol properties ------------------------------------------------- + + @property + def sandbox_id(self) -> str: + """Unique identifier for this sandbox instance.""" + return self._sandbox_id + + @property + def status(self) -> SandboxStatus: + """Current lifecycle status.""" + return self._status + + @property + def context(self) -> SandboxContext | None: + """Context after creation, ``None`` before ``create``.""" + return self._context + + # -- protocol methods ---------------------------------------------------- + + def create(self, plan_id: str) -> SandboxContext: + """Copy the original directory to a temporary sandbox location. + + Args: + plan_id: The plan that owns this sandbox. + + Returns: + A :class:`SandboxContext` with the sandbox copy path. + + Raises: + ValueError: If *plan_id* is empty. + SandboxStateError: If not in ``PENDING`` status. + SandboxCreationError: If the directory copy fails. + """ + if not plan_id: + raise ValueError("plan_id cannot be empty") + + SandboxStatus.assert_transition(self._status, SandboxStatus.CREATED) + + try: + if not os.path.isdir(self._original_path): + raise SandboxCreationError( + f"Original path is not a directory: {self._original_path}" + ) + + # Create the sandbox copy in a temp directory + parent_dir = tempfile.mkdtemp(prefix="ca-cow-sandbox-") + self._sandbox_path = os.path.join(parent_dir, "sandbox") + + shutil.copytree( + self._original_path, + self._sandbox_path, + symlinks=True, + dirs_exist_ok=False, + ) + + except SandboxCreationError: + self._status = SandboxStatus.ERRORED + raise + except OSError as exc: + self._status = SandboxStatus.ERRORED + raise SandboxCreationError( + f"Failed to copy directory for resource {self._resource_id}: {exc}" + ) from exc + + self._context = SandboxContext( + sandbox_id=self._sandbox_id, + sandbox_path=self._sandbox_path, + original_path=self._original_path, + resource_id=self._resource_id, + plan_id=plan_id, + created_at=datetime.now(), + metadata={ + "strategy": "copy_on_write", + "sandbox_path": self._sandbox_path, + }, + ) + self._status = SandboxStatus.CREATED + + logger.info( + "Created copy-on-write sandbox: plan=%s resource=%s path=%s", + plan_id, + self._resource_id, + self._sandbox_path, + ) + + return self._context + + def get_path(self, resource_path: str) -> str: + """Translate a resource-relative path to a sandbox copy path. + + Args: + resource_path: Path relative to the resource root. + + Returns: + Absolute path inside the sandbox copy. + + Raises: + SandboxStateError: If sandbox is not in a usable status. + ValueError: If *resource_path* attempts directory traversal. + """ + if self._status not in ( + SandboxStatus.CREATED, + SandboxStatus.ACTIVE, + ): + raise SandboxStateError( + f"Cannot resolve path in status {self._status.value}" + ) + + if ".." in resource_path.split("/"): + raise ValueError(f"Path traversal not allowed: {resource_path}") + + if self._status == SandboxStatus.CREATED: + self._status = SandboxStatus.ACTIVE + + if self._sandbox_path is None: + raise SandboxStateError("Sandbox path not set") + + return os.path.join(self._sandbox_path, resource_path) + + def commit(self, message: str | None = None) -> CommitResult: + """Sync changed files from the sandbox copy back to the original. + + Compares the sandbox directory to the original and copies only + files that differ. New files in the sandbox are added; files + deleted from the sandbox are removed from the original. + + Args: + message: Optional log message (stored in result metadata). + + Returns: + A :class:`CommitResult` describing the outcome. + + Raises: + SandboxCommitError: If the sync fails. + SandboxStateError: If sandbox is not in a committable status. + """ + if self._status not in ( + SandboxStatus.CREATED, + SandboxStatus.ACTIVE, + ): + raise SandboxStateError(f"Cannot commit from status {self._status.value}") + + SandboxStatus.assert_transition(self._status, SandboxStatus.COMMITTED) + + if self._sandbox_path is None: + raise SandboxStateError("Sandbox path not set") + + try: + changed_files, added_files, deleted_files = self._compute_diff( + self._sandbox_path, self._original_path + ) + + # Apply changes: copy modified and new files + for rel_path in changed_files + added_files: + src = os.path.join(self._sandbox_path, rel_path) + dst = os.path.join(self._original_path, rel_path) + dst_dir = os.path.dirname(dst) + if dst_dir: + os.makedirs(dst_dir, exist_ok=True) + shutil.copy2(src, dst) + + # Apply deletions + for rel_path in deleted_files: + dst = os.path.join(self._original_path, rel_path) + if os.path.exists(dst): + os.remove(dst) + + except OSError as exc: + self._status = SandboxStatus.ERRORED + raise SandboxCommitError( + f"Failed to sync sandbox {self._sandbox_id} back to original: {exc}" + ) from exc + + self._status = SandboxStatus.COMMITTED + + logger.info( + "Committed copy-on-write sandbox: sandbox_id=%s " + "changed=%d added=%d deleted=%d", + self._sandbox_id, + len(changed_files), + len(added_files), + len(deleted_files), + ) + + return CommitResult( + sandbox_id=self._sandbox_id, + success=True, + commit_ref=None, + changed_files=changed_files, + added_files=added_files, + deleted_files=deleted_files, + error=None, + timestamp=datetime.now(), + ) + + def rollback(self) -> None: + """Discard sandbox changes by resetting the copy to original state. + + Re-copies the original directory over the sandbox. The sandbox + transitions to ``ROLLED_BACK`` and can be re-activated via + ``get_path``. + + Raises: + SandboxRollbackError: If the rollback fails. + SandboxStateError: If called in an invalid status. + """ + if self._status != SandboxStatus.ACTIVE: + raise SandboxStateError(f"Cannot rollback from status {self._status.value}") + + SandboxStatus.assert_transition(self._status, SandboxStatus.ROLLED_BACK) + + if self._sandbox_path is None: + raise SandboxStateError("Sandbox path not set") + + try: + # Remove the current sandbox copy + shutil.rmtree(self._sandbox_path, ignore_errors=True) + + # Re-copy from original + shutil.copytree( + self._original_path, + self._sandbox_path, + symlinks=True, + dirs_exist_ok=False, + ) + + except OSError as exc: + self._status = SandboxStatus.ERRORED + raise SandboxRollbackError( + f"Failed to rollback sandbox {self._sandbox_id}: {exc}" + ) from exc + + self._status = SandboxStatus.ROLLED_BACK + + logger.info( + "Rolled back copy-on-write sandbox: sandbox_id=%s", + self._sandbox_id, + ) + + def cleanup(self) -> None: + """Remove the sandbox copy directory. + + Idempotent -- safe to call multiple times. + + Raises: + SandboxError: On unexpected errors during cleanup. + """ + if self._status == SandboxStatus.CLEANED_UP: + return + + logger.debug( + "Cleaning up copy-on-write sandbox: sandbox_id=%s path=%s", + self._sandbox_id, + self._sandbox_path, + ) + + if self._sandbox_path is not None: + # Remove the sandbox copy and its parent temp dir + parent = os.path.dirname(self._sandbox_path) + if os.path.exists(parent): + shutil.rmtree(parent, ignore_errors=True) + + self._status = SandboxStatus.CLEANED_UP + + logger.info( + "Cleaned up copy-on-write sandbox: sandbox_id=%s", + self._sandbox_id, + ) + + # -- internal helpers ---------------------------------------------------- + + @staticmethod + def _compute_diff( + sandbox_dir: str, original_dir: str + ) -> tuple[list[str], list[str], list[str]]: + """Compare sandbox to original and return changed/added/deleted files. + + Args: + sandbox_dir: Path to the sandbox copy. + original_dir: Path to the original directory. + + Returns: + Tuple of (changed_files, added_files, deleted_files) as + relative paths. + """ + changed: list[str] = [] + added: list[str] = [] + deleted: list[str] = [] + + # Collect all files in both trees + sandbox_files: set[str] = set() + for dirpath, _dirnames, filenames in os.walk(sandbox_dir): + for fname in filenames: + full = os.path.join(dirpath, fname) + rel = os.path.relpath(full, sandbox_dir) + sandbox_files.add(rel) + + original_files: set[str] = set() + for dirpath, _dirnames, filenames in os.walk(original_dir): + for fname in filenames: + full = os.path.join(dirpath, fname) + rel = os.path.relpath(full, original_dir) + original_files.add(rel) + + # Added in sandbox (not in original) + for rel in sorted(sandbox_files - original_files): + added.append(rel) + + # Deleted from sandbox (was in original) + for rel in sorted(original_files - sandbox_files): + deleted.append(rel) + + # Modified (present in both but different) + for rel in sorted(sandbox_files & original_files): + sandbox_file = os.path.join(sandbox_dir, rel) + original_file = os.path.join(original_dir, rel) + if not filecmp.cmp(sandbox_file, original_file, shallow=False): + changed.append(rel) + + return changed, added, deleted diff --git a/src/cleveragents/infrastructure/sandbox/factory.py b/src/cleveragents/infrastructure/sandbox/factory.py index 901135a97..4c6c10cb1 100644 --- a/src/cleveragents/infrastructure/sandbox/factory.py +++ b/src/cleveragents/infrastructure/sandbox/factory.py @@ -5,7 +5,7 @@ implementation class. When the Resource domain model (Stage B1) is available, this factory will accept ``Resource`` objects directly; until then it operates on raw parameters. -Stage B3.6 of the implementation plan. +Stage B3.6 of the implementation plan. Updated in TASK-006 (B4 rework). """ from __future__ import annotations @@ -13,6 +13,8 @@ from __future__ import annotations import logging from typing import Literal +from cleveragents.infrastructure.sandbox.copy_on_write import CopyOnWriteSandbox +from cleveragents.infrastructure.sandbox.git_worktree import GitWorktreeSandbox from cleveragents.infrastructure.sandbox.no_sandbox import NoSandbox from cleveragents.infrastructure.sandbox.protocol import ( Sandbox, @@ -20,31 +22,34 @@ from cleveragents.infrastructure.sandbox.protocol import ( logger = logging.getLogger(__name__) -# Strategy string constants (aligned with future SandboxStrategy enum from B1.4) +# Strategy string constants aligned with spec SandboxStrategy enum (5 values) STRATEGY_GIT_WORKTREE: Literal["git_worktree"] = "git_worktree" STRATEGY_COPY_ON_WRITE: Literal["copy_on_write"] = "copy_on_write" -STRATEGY_OVERLAY: Literal["overlay"] = "overlay" STRATEGY_TRANSACTION_ROLLBACK: Literal["transaction_rollback"] = "transaction_rollback" -STRATEGY_VERSIONING: Literal["versioning"] = "versioning" +STRATEGY_SNAPSHOT: Literal["snapshot"] = "snapshot" STRATEGY_NONE: Literal["none"] = "none" SandboxStrategyStr = Literal[ "git_worktree", "copy_on_write", - "overlay", "transaction_rollback", - "versioning", + "snapshot", "none", ] -# Resource type to supported strategies mapping +# Strategies that have concrete implementations +_IMPLEMENTED_STRATEGIES: frozenset[str] = frozenset( + {"none", "git_worktree", "copy_on_write"} +) + +# Resource type to supported strategies mapping (spec-aligned) _SUPPORTED_STRATEGIES: dict[str, list[SandboxStrategyStr]] = { - "git_repository": ["git_worktree", "copy_on_write", "none"], - "filesystem": ["copy_on_write", "overlay", "none"], - "database": ["transaction_rollback", "none"], + "git-checkout": ["git_worktree", "copy_on_write", "none"], + "git": ["none"], + "fs-mount": ["copy_on_write", "none"], + "fs-directory": ["copy_on_write", "none"], + "fs-file": ["copy_on_write", "none"], "api_endpoint": ["none"], - "document_corpus": ["copy_on_write", "none"], - "cloud_infrastructure": ["none"], } @@ -55,10 +60,10 @@ class SandboxFactory: Currently available: - ``"none"`` -> :class:`NoSandbox` - - ``"git_worktree"`` -> (Hamza's B3.3, not yet implemented) - - ``"copy_on_write"`` -> (Hamza's B3.4, not yet implemented) + - ``"git_worktree"`` -> :class:`GitWorktreeSandbox` + - ``"copy_on_write"`` -> :class:`CopyOnWriteSandbox` - Other strategies raise ``NotImplementedError``. + ``"transaction_rollback"`` and ``"snapshot"`` raise ``NotImplementedError``. """ def create_sandbox( @@ -95,25 +100,15 @@ class SandboxFactory: ) if sandbox_strategy == STRATEGY_GIT_WORKTREE: - raise NotImplementedError( - "GitWorktreeSandbox not yet implemented " - "(see Stage B3.3 -- assigned to Hamza)" + return GitWorktreeSandbox( + resource_id=resource_id, + original_path=original_path, ) if sandbox_strategy == STRATEGY_COPY_ON_WRITE: - raise NotImplementedError( - "FilesystemSandbox (copy-on-write) not yet implemented " - "(see Stage B3.4 -- assigned to Hamza)" - ) - - if sandbox_strategy == STRATEGY_OVERLAY: - logger.warning( - "Overlay sandbox not implemented; would fall back to " - "copy-on-write once available." - ) - raise NotImplementedError( - "Overlay sandbox not yet implemented; copy-on-write " - "fallback also not yet available (see Stage B3.4)" + return CopyOnWriteSandbox( + resource_id=resource_id, + original_path=original_path, ) if sandbox_strategy == STRATEGY_TRANSACTION_ROLLBACK: @@ -121,8 +116,8 @@ class SandboxFactory: "Database transaction sandbox not yet implemented" ) - if sandbox_strategy == STRATEGY_VERSIONING: - raise NotImplementedError("Versioning sandbox not yet implemented") + if sandbox_strategy == STRATEGY_SNAPSHOT: + raise NotImplementedError("Snapshot sandbox not yet implemented") raise ValueError(f"Unknown sandbox strategy: {sandbox_strategy}") @@ -130,9 +125,7 @@ class SandboxFactory: @staticmethod def is_supported(sandbox_strategy: str) -> bool: - """Check whether *sandbox_strategy* has an implementation. - - Currently only ``"none"`` is fully implemented. + """Check whether *sandbox_strategy* has a concrete implementation. Args: sandbox_strategy: The strategy string to check. @@ -140,14 +133,14 @@ class SandboxFactory: Returns: ``True`` if a concrete sandbox class exists for the strategy. """ - return sandbox_strategy == STRATEGY_NONE + return sandbox_strategy in _IMPLEMENTED_STRATEGIES @staticmethod def get_supported_strategies(resource_type: str) -> list[SandboxStrategyStr]: """Return the list of strategies compatible with a resource type. Args: - resource_type: The resource type string (e.g. ``"git_repository"``). + resource_type: The resource type string (e.g. ``"git-checkout"``). Returns: List of compatible strategy strings. Returns ``["none"]`` for diff --git a/src/cleveragents/infrastructure/sandbox/git_worktree.py b/src/cleveragents/infrastructure/sandbox/git_worktree.py new file mode 100644 index 000000000..961eb2866 --- /dev/null +++ b/src/cleveragents/infrastructure/sandbox/git_worktree.py @@ -0,0 +1,569 @@ +"""Git worktree sandbox for git-checkout resources. + +Creates an isolated git worktree where plan modifications are staged and +committed independently of the main working tree. On ``commit``, changes +are merged or cherry-picked back. On ``rollback``, the worktree is +discarded entirely. + +Stage B3.3 / TASK-006 of the implementation plan. +""" + +from __future__ import annotations + +import contextlib +import logging +import os +import re +import shutil +import subprocess +import tempfile +from datetime import datetime + +from ulid import ULID + +from cleveragents.infrastructure.sandbox.protocol import ( + CommitResult, + SandboxCommitError, + SandboxContext, + SandboxCreationError, + SandboxRollbackError, + SandboxStateError, + SandboxStatus, +) + +logger = logging.getLogger(__name__) + +# Default timeout for git commands (seconds) +_GIT_TIMEOUT: int = 30 + +# Regex for sanitising branch names -- keep only alnum, hyphens, underscores, +# slashes, and dots; collapse runs of disallowed chars into a single hyphen. +_BRANCH_SANITISE_RE: re.Pattern[str] = re.compile(r"[^a-zA-Z0-9/_.\-]+") + + +def _sanitise_branch_name(raw: str) -> str: + """Produce a git-safe branch name from an arbitrary string. + + Args: + raw: The raw string to sanitise (typically a plan ID). + + Returns: + A string safe for use as a git branch name. + """ + sanitised = _BRANCH_SANITISE_RE.sub("-", raw).strip("-") + if not sanitised: + sanitised = "sandbox" + return sanitised + + +def _run_git( + args: list[str], + cwd: str, + timeout: int = _GIT_TIMEOUT, +) -> subprocess.CompletedProcess[str]: + """Run a git command with timeout and capture output. + + Args: + args: Git sub-command and arguments (e.g. ``["worktree", "add", ...]``). + cwd: Working directory to run in. + timeout: Maximum seconds to wait. + + Returns: + Completed process result. + + Raises: + subprocess.TimeoutExpired: If the command exceeds *timeout*. + subprocess.CalledProcessError: If the command returns non-zero. + """ + cmd = ["git", *args] + logger.debug("Running: %s (cwd=%s, timeout=%ds)", " ".join(cmd), cwd, timeout) + return subprocess.run( + cmd, + cwd=cwd, + capture_output=True, + text=True, + check=True, + timeout=timeout, + ) + + +class GitWorktreeSandbox: + """Sandbox that isolates changes in a git worktree. + + Creates a detached worktree rooted at a temporary directory with a + dedicated branch. The actor makes changes in the worktree; on commit + those changes are merged back to the original branch. + + Implements the + :class:`~cleveragents.infrastructure.sandbox.protocol.Sandbox` protocol. + + Lifecycle:: + + sandbox = GitWorktreeSandbox(resource_id, original_path) + ctx = sandbox.create(plan_id) # creates worktree + branch + path = sandbox.get_path("src/x.py") # resolve path in worktree + # ... actor writes ... + result = sandbox.commit("msg") # commit in worktree, merge back + sandbox.cleanup() # remove worktree + branch + """ + + def __init__( + self, + resource_id: str, + original_path: str, + git_timeout: int = _GIT_TIMEOUT, + ) -> None: + """Initialise a git worktree sandbox. + + Args: + resource_id: Identifier of the resource being sandboxed. + original_path: Path to the git repository root. + git_timeout: Timeout in seconds for git commands. + + Raises: + ValueError: If *resource_id* or *original_path* is empty. + ValueError: If *git_timeout* is not positive. + """ + if not resource_id: + raise ValueError("resource_id cannot be empty") + if not original_path: + raise ValueError("original_path cannot be empty") + if git_timeout <= 0: + raise ValueError("git_timeout must be positive") + + self._sandbox_id: str = str(ULID()) + self._resource_id: str = resource_id + self._original_path: str = os.path.abspath(original_path) + self._git_timeout: int = git_timeout + self._status: SandboxStatus = SandboxStatus.PENDING + self._context: SandboxContext | None = None + + # Set after create() + self._worktree_path: str | None = None + self._branch_name: str | None = None + self._original_branch: str | None = None + self._base_commit: str | None = None + + # -- protocol properties ------------------------------------------------- + + @property + def sandbox_id(self) -> str: + """Unique identifier for this sandbox instance.""" + return self._sandbox_id + + @property + def status(self) -> SandboxStatus: + """Current lifecycle status.""" + return self._status + + @property + def context(self) -> SandboxContext | None: + """Context after creation, ``None`` before ``create``.""" + return self._context + + # -- protocol methods ---------------------------------------------------- + + def create(self, plan_id: str) -> SandboxContext: + """Create a git worktree for isolated modifications. + + Creates a new branch ``sandbox/`` and a worktree at a + temporary directory. The worktree starts from the current HEAD + of the repository. + + Args: + plan_id: The plan that owns this sandbox. + + Returns: + A :class:`SandboxContext` with the worktree path. + + Raises: + ValueError: If *plan_id* is empty. + SandboxStateError: If not in ``PENDING`` status. + SandboxCreationError: If git worktree creation fails. + """ + if not plan_id: + raise ValueError("plan_id cannot be empty") + + SandboxStatus.assert_transition(self._status, SandboxStatus.CREATED) + + try: + # Verify the original path is a git repo root (not just a + # subdirectory of some unrelated parent repository). + toplevel_result = _run_git( + ["rev-parse", "--show-toplevel"], + cwd=self._original_path, + timeout=self._git_timeout, + ) + toplevel = os.path.realpath(toplevel_result.stdout.strip()) + actual = os.path.realpath(self._original_path) + if toplevel != actual: + raise subprocess.CalledProcessError( + returncode=1, + cmd="git rev-parse --show-toplevel", + stderr=( + f"Path {self._original_path} is not the root of a " + f"git repository (toplevel is {toplevel})" + ), + ) + + # Get the current branch name + result = _run_git( + ["rev-parse", "--abbrev-ref", "HEAD"], + cwd=self._original_path, + timeout=self._git_timeout, + ) + self._original_branch = result.stdout.strip() + + # Get the current HEAD commit + result = _run_git( + ["rev-parse", "HEAD"], + cwd=self._original_path, + timeout=self._git_timeout, + ) + self._base_commit = result.stdout.strip() + + # Create a sanitised branch name + safe_plan_id = _sanitise_branch_name(plan_id) + self._branch_name = f"sandbox/{safe_plan_id}" + + # Create a temporary directory for the worktree + self._worktree_path = tempfile.mkdtemp(prefix=f"ca-sandbox-{safe_plan_id}-") + # mkdtemp creates the dir; git worktree add needs it to not exist + os.rmdir(self._worktree_path) + + # Create the worktree with a new branch + _run_git( + [ + "worktree", + "add", + "-b", + self._branch_name, + self._worktree_path, + "HEAD", + ], + cwd=self._original_path, + timeout=self._git_timeout, + ) + + except subprocess.TimeoutExpired as exc: + self._status = SandboxStatus.ERRORED + raise SandboxCreationError( + f"Git command timed out after {self._git_timeout}s " + f"while creating worktree for resource {self._resource_id}" + ) from exc + except subprocess.CalledProcessError as exc: + self._status = SandboxStatus.ERRORED + raise SandboxCreationError( + f"Failed to create git worktree for resource " + f"{self._resource_id}: {exc.stderr.strip()}" + ) from exc + + self._context = SandboxContext( + sandbox_id=self._sandbox_id, + sandbox_path=self._worktree_path, + original_path=self._original_path, + resource_id=self._resource_id, + plan_id=plan_id, + created_at=datetime.now(), + metadata={ + "strategy": "git_worktree", + "branch": self._branch_name, + "original_branch": self._original_branch, + "base_commit": self._base_commit, + "worktree_path": self._worktree_path, + }, + ) + self._status = SandboxStatus.CREATED + + logger.info( + "Created git worktree sandbox: plan=%s resource=%s branch=%s path=%s", + plan_id, + self._resource_id, + self._branch_name, + self._worktree_path, + ) + + return self._context + + def get_path(self, resource_path: str) -> str: + """Translate a resource-relative path to a worktree path. + + Args: + resource_path: Path relative to the resource root. + + Returns: + Absolute path inside the worktree. + + Raises: + SandboxStateError: If sandbox is not in a usable status. + ValueError: If *resource_path* attempts directory traversal. + """ + if self._status not in ( + SandboxStatus.CREATED, + SandboxStatus.ACTIVE, + ): + raise SandboxStateError( + f"Cannot resolve path in status {self._status.value}" + ) + + if ".." in resource_path.split("/"): + raise ValueError(f"Path traversal not allowed: {resource_path}") + + if self._status == SandboxStatus.CREATED: + self._status = SandboxStatus.ACTIVE + + if self._worktree_path is None: + raise SandboxStateError("Worktree path not set") + + return os.path.join(self._worktree_path, resource_path) + + def commit(self, message: str | None = None) -> CommitResult: + """Commit changes in the worktree and merge back to the original branch. + + Stages all changes in the worktree, commits them, then merges the + sandbox branch into the original branch. + + Args: + message: Commit message. Defaults to a generated message. + + Returns: + A :class:`CommitResult` describing the outcome. + + Raises: + SandboxCommitError: If the commit or merge fails. + SandboxStateError: If sandbox is not in a committable status. + """ + if self._status not in ( + SandboxStatus.CREATED, + SandboxStatus.ACTIVE, + ): + raise SandboxStateError(f"Cannot commit from status {self._status.value}") + + SandboxStatus.assert_transition(self._status, SandboxStatus.COMMITTED) + + if self._worktree_path is None or self._branch_name is None: + raise SandboxStateError("Worktree not initialised") + + commit_message = message or ( + f"sandbox: changes from plan (sandbox_id={self._sandbox_id})" + ) + + try: + # Stage all changes in the worktree + _run_git( + ["add", "-A"], + cwd=self._worktree_path, + timeout=self._git_timeout, + ) + + # Check if there are staged changes + diff_result = subprocess.run( + ["git", "diff", "--cached", "--name-status"], + cwd=self._worktree_path, + capture_output=True, + text=True, + check=False, + timeout=self._git_timeout, + ) + diff_output = diff_result.stdout.strip() + + if not diff_output: + # No changes to commit + self._status = SandboxStatus.COMMITTED + return CommitResult( + sandbox_id=self._sandbox_id, + success=True, + commit_ref=self._base_commit, + changed_files=[], + added_files=[], + deleted_files=[], + error=None, + timestamp=datetime.now(), + ) + + # Parse changed/added/deleted files + changed_files: list[str] = [] + added_files: list[str] = [] + deleted_files: list[str] = [] + + for line in diff_output.splitlines(): + parts = line.split("\t", 1) + if len(parts) == 2: + status_char, file_path = parts + if status_char.startswith("A"): + added_files.append(file_path) + elif status_char.startswith("D"): + deleted_files.append(file_path) + else: + changed_files.append(file_path) + + # Commit in the worktree + _run_git( + ["commit", "-m", commit_message], + cwd=self._worktree_path, + timeout=self._git_timeout, + ) + + # Get the commit hash + result = _run_git( + ["rev-parse", "HEAD"], + cwd=self._worktree_path, + timeout=self._git_timeout, + ) + commit_ref = result.stdout.strip() + + # Merge the sandbox branch into the original branch + _run_git( + ["merge", self._branch_name, "--no-edit"], + cwd=self._original_path, + timeout=self._git_timeout, + ) + + except subprocess.TimeoutExpired as exc: + self._status = SandboxStatus.ERRORED + raise SandboxCommitError( + f"Git command timed out after {self._git_timeout}s " + f"while committing sandbox {self._sandbox_id}" + ) from exc + except subprocess.CalledProcessError as exc: + self._status = SandboxStatus.ERRORED + raise SandboxCommitError( + f"Failed to commit sandbox {self._sandbox_id}: {exc.stderr.strip()}" + ) from exc + + self._status = SandboxStatus.COMMITTED + + logger.info( + "Committed git worktree sandbox: sandbox_id=%s commit=%s " + "changed=%d added=%d deleted=%d", + self._sandbox_id, + commit_ref, + len(changed_files), + len(added_files), + len(deleted_files), + ) + + return CommitResult( + sandbox_id=self._sandbox_id, + success=True, + commit_ref=commit_ref, + changed_files=changed_files, + added_files=added_files, + deleted_files=deleted_files, + error=None, + timestamp=datetime.now(), + ) + + def rollback(self) -> None: + """Discard all worktree changes by resetting to the base commit. + + Resets the worktree branch to the base commit and transitions + back to ``ACTIVE`` for potential re-use. + + Raises: + SandboxRollbackError: If the rollback fails. + SandboxStateError: If called in an invalid status. + """ + if self._status != SandboxStatus.ACTIVE: + raise SandboxStateError(f"Cannot rollback from status {self._status.value}") + + SandboxStatus.assert_transition(self._status, SandboxStatus.ROLLED_BACK) + + if self._worktree_path is None or self._base_commit is None: + raise SandboxStateError("Worktree not initialised") + + try: + _run_git( + ["reset", "--hard", self._base_commit], + cwd=self._worktree_path, + timeout=self._git_timeout, + ) + _run_git( + ["clean", "-fd"], + cwd=self._worktree_path, + timeout=self._git_timeout, + ) + except subprocess.TimeoutExpired as exc: + self._status = SandboxStatus.ERRORED + raise SandboxRollbackError( + f"Git command timed out after {self._git_timeout}s " + f"while rolling back sandbox {self._sandbox_id}" + ) from exc + except subprocess.CalledProcessError as exc: + self._status = SandboxStatus.ERRORED + raise SandboxRollbackError( + f"Failed to rollback sandbox {self._sandbox_id}: {exc.stderr.strip()}" + ) from exc + + self._status = SandboxStatus.ROLLED_BACK + + logger.info( + "Rolled back git worktree sandbox: sandbox_id=%s to commit=%s", + self._sandbox_id, + self._base_commit, + ) + + def cleanup(self) -> None: + """Remove the worktree and sandbox branch. + + Idempotent -- safe to call multiple times. + + Raises: + SandboxError: On unexpected errors during cleanup. + """ + if self._status == SandboxStatus.CLEANED_UP: + return + + logger.debug( + "Cleaning up git worktree sandbox: sandbox_id=%s branch=%s path=%s", + self._sandbox_id, + self._branch_name, + self._worktree_path, + ) + + # Remove the worktree + if self._worktree_path is not None and os.path.exists(self._worktree_path): + try: + _run_git( + ["worktree", "remove", "--force", self._worktree_path], + cwd=self._original_path, + timeout=self._git_timeout, + ) + except (subprocess.CalledProcessError, subprocess.TimeoutExpired): + # Fallback: manual removal + logger.warning( + "git worktree remove failed; removing directory manually: %s", + self._worktree_path, + ) + shutil.rmtree(self._worktree_path, ignore_errors=True) + + # Delete the sandbox branch + if self._branch_name is not None: + try: + _run_git( + ["branch", "-D", self._branch_name], + cwd=self._original_path, + timeout=self._git_timeout, + ) + except (subprocess.CalledProcessError, subprocess.TimeoutExpired): + logger.warning( + "Failed to delete sandbox branch %s; it may need manual cleanup", + self._branch_name, + ) + + # Prune stale worktree entries + with contextlib.suppress( + subprocess.CalledProcessError, subprocess.TimeoutExpired + ): + _run_git( + ["worktree", "prune"], + cwd=self._original_path, + timeout=self._git_timeout, + ) + + self._status = SandboxStatus.CLEANED_UP + + logger.info( + "Cleaned up git worktree sandbox: sandbox_id=%s", + self._sandbox_id, + )