From f93ef567c7e138f529847622f5e0020f6a809e62 Mon Sep 17 00:00:00 2001 From: weichen Date: Thu, 15 Oct 2026 06:19:10 +0000 Subject: [PATCH] Isolate execution agents from shared Git refs Why: prevent unrelated sibling worktree delivery from invalidating otherwise safe agent attempts Evidence: 58 focused tests, configuration validation, governance regression, and diff check Impact: uses private shared clones and copies only host-owned transport refs back to canonical Git Refs: U5 runtime acceptance; docs/superpowers/plans/2026-10-15-project-agent-execution.md --- scripts/agent_exec.py | 134 ++++++++++++++++++++++++++++++------- tests/test_agent_exec.py | 138 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 241 insertions(+), 31 deletions(-) diff --git a/scripts/agent_exec.py b/scripts/agent_exec.py index de6b0043..f6c735e5 100644 --- a/scripts/agent_exec.py +++ b/scripts/agent_exec.py @@ -523,6 +523,38 @@ def _create_private_run_dir(run_id: str) -> Path: return run_dir +def _create_private_attempt_clone(repo: Path, dest: Path, base_head: str) -> None: + """Create a private local shared clone with isolated Git administration. + + Objects are reused read-only via ``git clone --shared`` alternates. The clone + owns its own ``.git`` config, refs, index, HEAD, and worktree. Never contacts + a remote. + """ + if dest.exists(): + shutil.rmtree(dest) + proc = subprocess.run( + [ + "git", + "clone", + "--shared", + "--no-checkout", + "--", + str(repo.resolve()), + str(dest), + ], + check=False, + capture_output=True, + text=True, + ) + if proc.returncode != 0: + raise subprocess.CalledProcessError( + proc.returncode, proc.args, proc.stdout, proc.stderr + ) + # Drop the auto-configured origin so the attempt cannot reach a remote URL. + _git(dest, "remote", "remove", "origin", check=False) + _git(dest, "checkout", "--detach", base_head) + + def _run_attempt( *, repo: Path, @@ -550,21 +582,25 @@ def _run_attempt( worktree = run_dir / f"wt-{attempt_id}" try: - _git( - repo, - "worktree", - "add", - "--detach", - str(worktree), - base["head"], - ) + _create_private_attempt_clone(repo, worktree, base["head"]) except subprocess.CalledProcessError as exc: + err = (exc.stderr or str(exc)).strip() return AttemptRecord( attempt_id=attempt_id, profile=profile.name, adapter=profile.adapter, terminal_status="unavailable", - reason=f"worktree create failed: {exc.stderr}", + reason=f"private clone create failed: {err}", + cleanup={"exact_restore": True, "skipped": True}, + observed_version=probe.observed_version, + ) + except OSError as exc: + return AttemptRecord( + attempt_id=attempt_id, + profile=profile.name, + adapter=profile.adapter, + terminal_status="unavailable", + reason=f"private clone create failed: {exc}", cleanup={"exact_restore": True, "skipped": True}, observed_version=probe.observed_version, ) @@ -1376,7 +1412,12 @@ def _path_allowed(path: str, allowed: tuple[str, ...]) -> bool: def _git_admin_snapshot(worktree: Path, repo: Path) -> dict[str, str]: - """Snapshot Git administration state before/after worker launch.""" + """Snapshot the private attempt clone's Git administration state. + + Canonical checkout identity is snapshotted separately. ``repo`` is unused now + that each attempt owns private config/refs; kept for call-site stability. + """ + del repo head = _git(worktree, "rev-parse", "HEAD").stdout.strip() index_tree = _git(worktree, "write-tree").stdout.strip() staged_proc = _git( @@ -1390,7 +1431,7 @@ def _git_admin_snapshot(worktree: Path, repo: Path) -> dict[str, str]: if staged_proc.returncode != 0: raise RuntimeError(f"admin staged snapshot failed: {staged_proc.stderr.strip()}") staged = staged_proc.stdout - local_config_proc = _git(repo, "config", "--local", "--list", check=False) + local_config_proc = _git(worktree, "config", "--local", "--list", check=False) if local_config_proc.returncode != 0: raise RuntimeError( f"admin local-config snapshot failed: {local_config_proc.stderr.strip()}" @@ -1399,7 +1440,7 @@ def _git_admin_snapshot(worktree: Path, repo: Path) -> dict[str, str]: sorted(line for line in local_config_proc.stdout.splitlines() if line) ) refs_proc = _git( - repo, + worktree, "for-each-ref", "--format=%(refname) %(objectname)", check=False, @@ -1407,12 +1448,18 @@ def _git_admin_snapshot(worktree: Path, repo: Path) -> dict[str, str]: if refs_proc.returncode != 0: raise RuntimeError(f"admin refs snapshot failed: {refs_proc.stderr.strip()}") refs = "\n".join(sorted(line for line in refs_proc.stdout.splitlines() if line)) + abs_git = _git(worktree, "rev-parse", "--absolute-git-dir").stdout.strip() + common_dir = _git( + worktree, "rev-parse", "--path-format=absolute", "--git-common-dir" + ).stdout.strip() return { "head": head, "index_tree": index_tree, "staged": staged, "local_config": local_config, "refs": refs, + "abs_git": abs_git, + "common_dir": common_dir, } @@ -1429,7 +1476,14 @@ def _snapshot_delta(before: dict[str, str], after: dict[str, str]) -> list[str]: def _canonical_checkout_snapshot(repo: Path) -> dict[str, str]: - """Snapshot the canonical checkout identity (not the attempt worktree).""" + """Snapshot the canonical checkout identity (not the attempt workspace). + + Protects HEAD, current branch, index, tracked/untracked status, and + git/common-dir identity. Unrelated sibling branch refs, branch config, and + worktree-list registration are intentionally excluded — worker Git is + isolated in a private clone, so those host-side delivery signals are not + worker mutation. + """ repo = repo.resolve() head = _git(repo, "rev-parse", "HEAD").stdout.strip() branch = _git(repo, "rev-parse", "--abbrev-ref", "HEAD").stdout.strip() @@ -1439,7 +1493,6 @@ def _canonical_checkout_snapshot(repo: Path) -> dict[str, str]: common = _git( repo, "rev-parse", "--path-format=absolute", "--git-common-dir" ).stdout.strip() - worktrees = _git(repo, "worktree", "list", "--porcelain").stdout return { "head": head, "branch": branch, @@ -1447,14 +1500,13 @@ def _canonical_checkout_snapshot(repo: Path) -> dict[str, str]: "status": status, "git_dir": git_dir, "common_dir": common, - "worktree_list": worktrees, } def _worktree_pointer_snapshot( worktree: Path, *, require_git: bool = True ) -> dict[str, str]: - """Snapshot the detached worktree `.git` pointer / admin identity.""" + """Snapshot the private attempt `.git` directory / admin identity.""" git_path = worktree / ".git" if git_path.is_symlink(): kind = "symlink" @@ -1547,8 +1599,14 @@ def _create_transport_commit( attempt_id: str, inventory: dict[str, Any], ) -> str: - """Stage inside the isolated worktree only; create base-parented transport commit.""" - # Stage all tracked/untracked changes in the worktree. + """Create a base-parented transport commit in the private clone and copy it back. + + Staging and ``commit-tree`` run only inside the attempt clone. Exact + commit/tree/blob objects are fetched into the canonical repository, which + receives solely the host-owned ``refs/agent-exec//`` ref. + """ + del inventory # Inventory already validated; staging uses the full private tree. + # Stage all tracked/untracked changes in the private clone only. _git(worktree, "add", "-A", "--", ".") # Include deletions. _git(worktree, "add", "-u", check=False) @@ -1564,13 +1622,41 @@ def _create_transport_commit( msg, ).stdout.strip() ref = f"refs/agent-exec/{run_id}/{attempt_id}" - # Write private ref into the shared object store via the canonical repo. - # commit-tree leaves HEAD/index of both checkouts untouched; no reset needed. - _git(repo, "update-ref", ref, commit) + # Park the commit on a private ref so a local fetch can name it. + _git(worktree, "update-ref", ref, commit) + # Copy objects into the canonical store and create only the host-owned ref. + fetch = _git( + repo, + "fetch", + "--no-tags", + "--no-write-fetch-head", + str(worktree.resolve()), + f"+{ref}:{ref}", + check=False, + ) + if fetch.returncode != 0: + raise RuntimeError( + f"transport object transfer failed: {fetch.stderr.strip() or fetch.stdout.strip()}" + ) + resolved = _git(repo, "rev-parse", ref).stdout.strip() + if resolved != commit: + raise RuntimeError( + f"transport ref resolved to {resolved}, expected {commit}" + ) + parents = ( + _git(repo, "rev-list", "--parents", "-n", "1", commit) + .stdout.strip() + .split() + ) + if len(parents) != 2 or parents[0] != commit or parents[1] != base_head: + raise RuntimeError( + f"transport commit parent check failed: {' '.join(parents)}" + ) return ref def _restore_worktree(repo: Path, worktree: Path, base_head: str) -> bool: + """Exact-restore the private clone then remove it after verification.""" try: _git(worktree, "reset", "--hard", base_head) _git(worktree, "clean", "-fdx") @@ -1584,10 +1670,8 @@ def _restore_worktree(repo: Path, worktree: Path, base_head: str) -> bool: def _safe_remove_worktree(repo: Path, worktree: Path) -> None: - try: - _git(repo, "worktree", "remove", "--force", str(worktree), check=False) - except Exception: - pass + """Remove a private attempt clone directory (not a linked worktree).""" + del repo # Private clones are not registered linked worktrees. if worktree.exists(): shutil.rmtree(worktree, ignore_errors=True) diff --git a/tests/test_agent_exec.py b/tests/test_agent_exec.py index 457ac1da..351a05bb 100644 --- a/tests/test_agent_exec.py +++ b/tests/test_agent_exec.py @@ -14,6 +14,7 @@ import stat import subprocess import tempfile import textwrap +import threading import time import unittest import uuid @@ -1071,21 +1072,22 @@ class AgentExecControllerTests(unittest.TestCase): self.assertTrue(cleanup.get("transport_created")) def test_canonical_checkout_mutation_is_failed_unknown(self) -> None: - """Worker mutating canonical files/index while keeping worktree clean is blocked.""" + """Worker mutating canonical files/index while keeping private clone clean is blocked.""" launched = self.root / "canon-next.log" launched.write_text("", encoding="utf-8") - # Resolve the main checkout from the linked worktree .git pointer. + # Resolve the canonical checkout via the shared-clone object alternates. _write_fake_worker( self.bindir, "canonmut", body=textwrap.dedent( """\ workspace="$1" - common=$(git -C "$workspace" rev-parse --path-format=absolute --git-common-dir) - canonical=$(dirname "$common") + alternates="$workspace/.git/objects/info/alternates" + objdir=$(head -n1 "$alternates") + canonical=$(dirname "$(dirname "$objdir")") printf 'canonical-mutated\\n' >> "$canonical/README.md" git -C "$canonical" add README.md - # Detached worktree left clean on purpose. + # Private clone left clean on purpose. exit 0 """ ), @@ -1135,13 +1137,14 @@ class AgentExecControllerTests(unittest.TestCase): self.assertNotEqual(after["status"], before["status"]) def test_corrupt_worktree_git_pointer_yields_failed_unknown_receipt(self) -> None: - """Corrupted worktree .git pointer must produce a stable blocked receipt.""" + """Corrupted private-clone .git directory must produce a stable blocked receipt.""" _write_fake_worker( self.bindir, "corruptgit", body=textwrap.dedent( """\ workspace="$1" + rm -rf "$workspace/.git" printf 'not-a-valid-gitdir\\n' > "$workspace/.git" exit 0 """ @@ -1300,6 +1303,129 @@ class AgentExecControllerTests(unittest.TestCase): self.assertEqual(data.get("host_fallback_authorizations", 0), 0) self._assert_canonical_untouched() + def test_sibling_canonical_ref_config_mutation_during_private_clone_succeeds( + self, + ) -> None: + """Host sibling branch/ref/config churn must not fail-stop a private clone.""" + ready = self.root / "sibling-ready" + proceed = self.root / "sibling-proceed" + for path in (ready, proceed): + if path.exists(): + path.unlink() + + _write_fake_worker( + self.bindir, + "sibworker", + body=textwrap.dedent( + f"""\ + workspace="$1" + result="$3" + mkdir -p "$workspace/src" + printf 'ok\\n' > "$workspace/src/marker.txt" + printf 'ready\\n' > "{ready}" + # Wait for the host-side sibling mutation before finishing. + i=0 + while [ ! -f "{proceed}" ]; do + i=$((i + 1)) + if [ "$i" -gt 400 ]; then + echo "timed out waiting for host sibling mutation" >&2 + exit 99 + fi + sleep 0.025 + done + printf '%s\\n' '{{"terminal_status":"completed","summary":"done","changed_files":["src/marker.txt"],"evidence":["wrote marker"],"scope_expansion":null}}' > "$result" + exit 0 + """ + ), + ) + self._configure( + profiles={"sibworker": {"adapter": "custom", "executable": "sibworker"}}, + role_order=["sibworker"], + ) + unit = _write_unit(self.root / "unit-sibling.json") + receipt = self.receipts / "sibling.json" + before = _canonical_snapshot(self.repo) + sibling_ref = "refs/heads/feature/host-sibling-delivery" + sibling_head = before["head"] + + def _host_sibling_mutation() -> None: + deadline = time.time() + 10 + while not ready.exists(): + if time.time() > deadline: + return + time.sleep(0.025) + _git(self.repo, "update-ref", sibling_ref, sibling_head) + _git( + self.repo, + "config", + "--local", + "branch.feature/host-sibling-delivery.remote", + "origin", + ) + _git( + self.repo, + "config", + "--local", + "branch.feature/host-sibling-delivery.merge", + sibling_ref, + ) + proceed.write_text("go\n", encoding="utf-8") + + host_thread = threading.Thread(target=_host_sibling_mutation, daemon=True) + host_thread.start() + try: + proc = self._run_cli( + "run", + "--repo", + str(self.repo), + "--role", + "implementation", + "--unit", + str(unit), + "--receipt", + str(receipt), + ) + finally: + host_thread.join(timeout=15) + + self.assertEqual(proc.returncode, agent_exec.EXIT_SUCCESS, proc.stderr + proc.stdout) + data = json.loads(receipt.read_text(encoding="utf-8")) + self.assertEqual(data["terminal_status"], "success") + attempt = data["attempts"][0] + worktree = Path(attempt["worktree"]) + self.assertTrue(worktree.is_dir()) + self.assertTrue((worktree / ".git").is_dir()) + self.assertTrue(ready.exists()) + self.assertTrue(proceed.exists()) + self.assertEqual( + _git(self.repo, "rev-parse", sibling_ref).stdout.strip(), sibling_head + ) + + porcelain = _git(self.repo, "worktree", "list", "--porcelain").stdout + self.assertNotIn(str(worktree.resolve()), porcelain) + canon_git = _git(self.repo, "rev-parse", "--absolute-git-dir").stdout.strip() + worker_git = _git(worktree, "rev-parse", "--absolute-git-dir").stdout.strip() + self.assertNotEqual(Path(canon_git).resolve(), Path(worker_git).resolve()) + canon_common = _git( + self.repo, "rev-parse", "--path-format=absolute", "--git-common-dir" + ).stdout.strip() + worker_common = _git( + worktree, "rev-parse", "--path-format=absolute", "--git-common-dir" + ).stdout.strip() + self.assertNotEqual(Path(canon_common).resolve(), Path(worker_common).resolve()) + + transport = data["transport"] + ref = transport["ref"] + self.assertRegex(ref, r"^refs/agent-exec/[^/]+/[^/]+$") + parent = _git(self.repo, "rev-parse", f"{ref}^").stdout.strip() + self.assertEqual(parent, before["head"]) + # Canonical HEAD/index identity preserved; sibling delivery side effects are allowed. + after = _canonical_snapshot(self.repo) + self.assertEqual(after["head"], before["head"]) + self.assertEqual(after["index"], before["index"]) + branch_head = _git(self.repo, "rev-parse", "HEAD").stdout.strip() + self.assertEqual(branch_head, before["head"]) + def test_symlink_allowed_path_escape_rejects_without_external_write(self) -> None: """Allowed path through symlink escaping the worktree cannot touch outside.""" outside = self.root / "outside-target"