diff --git a/templates/fix-swarm/README.md b/templates/fix-swarm/README.md index f2da2a8..cc6166e 100644 --- a/templates/fix-swarm/README.md +++ b/templates/fix-swarm/README.md @@ -29,9 +29,9 @@ Use it after the orchestrator has already decided what should be fixed. It is a ## Checks -The manifest invokes `checks/fix-swarm.py`. The validator runs the filled build/test command, validates `fix-summary.md`, stages the worktree, rejects patches that touch files outside the ownership list, and writes a non-empty patch plus summary into the run workdir. +The manifest invokes `checks/fix-swarm.py`. The validator runs the filled build/test command, validates `fix-summary.md`, stages the worktree, rejects patches that touch files outside the ownership list, and writes a non-empty patch plus summary into the run workdir. For compatibility it updates the canonical `.patch` path on each invocation. It also writes a sibling attempt archive, such as `.attempt1.patch` or `.attempt2.patch`, so retries do not destroy earlier patch output. -`expect_files` is intentionally empty because worktrees mode deletes passing task worktrees. The durable deliverables are check-produced files at the filled patch and summary paths in `WORKDIR`; review those before applying anything. +`expect_files` is intentionally empty because worktrees mode deletes passing task worktrees. The durable deliverables are check-produced files at the filled patch and summary paths in `WORKDIR`; review those before applying anything. When a task is retried, inspect the attempt-specific patch archives if you need to compare attempts. ## Mix with @@ -42,6 +42,7 @@ The manifest invokes `checks/fix-swarm.py`. The validator runs the filled build/ ## Gotchas - Worktrees that pass are deleted. Anything not exported by the check is gone. +- The canonical `.patch` is always the latest validator output. Earlier outputs are preserved as `.attemptN.patch` siblings, with the next number chosen from existing attempt archives. - `git add -A` cannot stage ignored build outputs. If a fix must preserve ignored files, extend the validator to copy them outside the worktree. - The ownership list is the safety rail. If a fix needs more files, stop and make a new task boundary. - Workers do not commit. The orchestrator applies patches after review. diff --git a/templates/fix-swarm/checks/fix-swarm.py b/templates/fix-swarm/checks/fix-swarm.py index 9c48215..40e3181 100644 --- a/templates/fix-swarm/checks/fix-swarm.py +++ b/templates/fix-swarm/checks/fix-swarm.py @@ -52,6 +52,38 @@ def run_git(args: list[str]) -> subprocess.CompletedProcess[str]: ) +def attempt_patch_path(canonical_patch: Path) -> Path: + suffix = canonical_patch.suffix + stem = canonical_patch.name[: -len(suffix)] if suffix else canonical_patch.name + pattern = re.compile(rf"^{re.escape(stem)}\.attempt([1-9][0-9]*){re.escape(suffix)}$") + attempts: list[int] = [] + if canonical_patch.parent.exists(): + for sibling in canonical_patch.parent.iterdir(): + match = pattern.match(sibling.name) + if match and sibling.is_file(): + attempts.append(int(match.group(1))) + return canonical_patch.with_name(f"{stem}.attempt{max(attempts, default=0) + 1}{suffix}") + + +def write_patch_archives(canonical_patch: Path, patch_text: str) -> list[str]: + failures: list[str] = [] + attempt_patch = attempt_patch_path(canonical_patch) + try: + canonical_patch.parent.mkdir(parents=True, exist_ok=True) + with attempt_patch.open("x", encoding="utf-8") as fh: + fh.write(patch_text) + except FileExistsError: + failures.append(fail("attempt_patch_exists", f"{attempt_patch} already exists")) + except OSError as exc: + failures.append(fail("attempt_patch_failed", f"could not write {attempt_patch}: {exc}")) + + try: + canonical_patch.write_text(patch_text, encoding="utf-8") + except OSError as exc: + failures.append(fail("patch_write_failed", f"could not write {canonical_patch}: {exc}")) + return failures + + def parse_owned_files(raw: str) -> list[str]: normalized = raw.replace("\\n", "\n").replace(";", "\n").replace(",", "\n") paths: list[str] = [] @@ -153,8 +185,7 @@ def main() -> int: if patch_result.returncode != 0: failures.append(fail("git_diff_failed", output_tail(patch_result.stdout))) elif patch_result.stdout.strip() and not has_placeholder(str(args.patch)): - args.patch.parent.mkdir(parents=True, exist_ok=True) - args.patch.write_text(patch_result.stdout, encoding="utf-8") + failures.extend(write_patch_archives(args.patch, patch_result.stdout)) if not has_placeholder(str(args.patch)) and (not args.patch.is_file() or args.patch.stat().st_size == 0): failures.append(fail("patch_not_written", f"{args.patch} was not written or is empty")) diff --git a/templates/fix-swarm/manifest.json b/templates/fix-swarm/manifest.json index c29621a..f29bd37 100644 --- a/templates/fix-swarm/manifest.json +++ b/templates/fix-swarm/manifest.json @@ -12,7 +12,7 @@ "check": "python3 '{{CHECK_SCRIPT_PATH — absolute path to templates/fix-swarm/checks/fix-swarm.py}}' --verify-command '{{BUILD_OR_TEST_COMMAND — exact command that proves the fix and prints useful errors}}' --patch '{{WORKDIR}}/{{FIX_KEY}}.patch' --summary fix-summary.md --exported-summary '{{WORKDIR}}/{{FIX_KEY}}.summary.md' --owned-files '{{OWNED_FILES — every file or directory this task may modify, comma or newline separated}}'", "expect_files": [], "timeout_s": 1800, - "verified": "the validator ran the requested build or test command, exported a non-empty patch, and confirmed the patch only changes declared owned files" + "verified": "the validator ran the requested build or test command, exported a non-empty canonical patch plus an attempt-specific patch archive, and confirmed the patch only changes declared owned files" } ] } diff --git a/tests/test_fix_swarm_template.py b/tests/test_fix_swarm_template.py new file mode 100644 index 0000000..dd19e03 --- /dev/null +++ b/tests/test_fix_swarm_template.py @@ -0,0 +1,171 @@ +from __future__ import annotations + +import shlex +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[1] +CHECK_SCRIPT = ROOT / "templates" / "fix-swarm" / "checks" / "fix-swarm.py" + + +class FixSwarmTemplateTests(unittest.TestCase): + def setUp(self) -> None: + self.tmp = tempfile.TemporaryDirectory(prefix="fix-swarm-template-") + self.root = Path(self.tmp.name) + self.repo = self.root / "repo" + self.workdir = self.root / "work" + self.workdir.mkdir() + self.repo.mkdir() + self.git("init") + self.git("config", "user.email", "test@example.com") + self.git("config", "user.name", "Test User") + + def tearDown(self) -> None: + self.tmp.cleanup() + + def git(self, *args: str) -> subprocess.CompletedProcess[str]: + return subprocess.run( + ["git", *args], + cwd=self.repo, + text=True, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + check=True, + ) + + def commit_base(self, path: str, content: str | bytes) -> None: + target = self.repo / path + target.parent.mkdir(parents=True, exist_ok=True) + if isinstance(content, bytes): + target.write_bytes(content) + else: + target.write_text(content, encoding="utf-8") + self.git("add", path) + self.git("commit", "-m", "base") + + def write_summary(self) -> None: + (self.repo / "fix-summary.md").write_text( + "\n".join( + [ + "# Fix Summary", + "", + "## Summary", + "Template validator test.", + "", + "## Files Changed", + "Changed the owned file.", + "", + "## Verification", + "Ran the validator command.", + "", + "## Assumptions", + "None.", + "", + ] + ), + encoding="utf-8", + ) + + def run_validator( + self, + *, + key: str = "task", + owned_files: str = "file.txt", + verify_command: str | None = None, + ) -> subprocess.CompletedProcess[str]: + self.write_summary() + command = verify_command or f"{shlex.quote(sys.executable)} -c {shlex.quote('raise SystemExit(0)')}" + return subprocess.run( + [ + sys.executable, + str(CHECK_SCRIPT), + "--verify-command", + command, + "--patch", + str(self.workdir / f"{key}.patch"), + "--summary", + "fix-summary.md", + "--exported-summary", + str(self.workdir / f"{key}.summary.md"), + "--owned-files", + owned_files, + ], + cwd=self.repo, + text=True, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + check=False, + ) + + def test_first_run_writes_attempt1_and_canonical_with_identical_binary_patch(self) -> None: + self.commit_base("payload.bin", b"base\x00old\n") + (self.repo / "payload.bin").write_bytes(b"base\x00new\xff\n") + + result = self.run_validator(key="binary", owned_files="payload.bin") + + self.assertEqual(result.returncode, 0, result.stdout) + canonical = self.workdir / "binary.patch" + attempt1 = self.workdir / "binary.attempt1.patch" + self.assertTrue(canonical.is_file()) + self.assertTrue(attempt1.is_file()) + self.assertEqual(canonical.read_bytes(), attempt1.read_bytes()) + self.assertIn(b"GIT binary patch", canonical.read_bytes()) + + def test_second_changed_run_preserves_attempt1_and_updates_canonical_to_attempt2(self) -> None: + self.commit_base("file.txt", "base\n") + (self.repo / "file.txt").write_text("attempt one\n", encoding="utf-8") + + first = self.run_validator() + self.assertEqual(first.returncode, 0, first.stdout) + attempt1 = self.workdir / "task.attempt1.patch" + attempt1_bytes = attempt1.read_bytes() + + (self.repo / "file.txt").write_text("attempt two\n", encoding="utf-8") + second = self.run_validator() + + self.assertEqual(second.returncode, 0, second.stdout) + canonical = self.workdir / "task.patch" + attempt2 = self.workdir / "task.attempt2.patch" + self.assertTrue(attempt2.is_file()) + self.assertEqual(attempt1.read_bytes(), attempt1_bytes) + self.assertNotEqual(attempt1_bytes, attempt2.read_bytes()) + self.assertEqual(canonical.read_bytes(), attempt2.read_bytes()) + + def test_attempt_numbering_uses_max_existing_attempt_and_ignores_lookalikes(self) -> None: + self.commit_base("file.txt", "base\n") + for name in ( + "gap.attempt1.patch", + "gap.attempt3.patch", + "gap.attempt4.patch.bak", + "gap.attempt5.diff", + "gap.attemptx.patch", + "other.attempt99.patch", + ): + (self.workdir / name).write_text("existing\n", encoding="utf-8") + (self.repo / "file.txt").write_text("changed\n", encoding="utf-8") + + result = self.run_validator(key="gap") + + self.assertEqual(result.returncode, 0, result.stdout) + self.assertTrue((self.workdir / "gap.attempt4.patch").is_file()) + self.assertFalse((self.workdir / "gap.attempt2.patch").exists()) + self.assertEqual((self.workdir / "gap.patch").read_bytes(), (self.workdir / "gap.attempt4.patch").read_bytes()) + + def test_verify_failure_does_not_report_success(self) -> None: + self.commit_base("file.txt", "base\n") + (self.repo / "file.txt").write_text("changed\n", encoding="utf-8") + verify_command = f"{shlex.quote(sys.executable)} -c {shlex.quote('raise SystemExit(7)')}" + + result = self.run_validator(verify_command=verify_command) + + self.assertEqual(result.returncode, 1, result.stdout) + self.assertIn("FAIL [verify_command_failed]", result.stdout) + self.assertNotIn("PASS [fix_contract]", result.stdout) + + +if __name__ == "__main__": + unittest.main()