weichen/Agent-System-Dev
1
0
Fork 0
Code Issues 1 Pull Requests 5 Actions 4 Packages Projects Releases Wiki Activity

Isolate execution agents from shared Git refs

Browse Source 
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
This commit is contained in:
weichen
2026-10-15 06:19:10 +00:00
parent 8c1649b232
commit f93ef567c7
2 changed files with 241 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files
scripts/agent_exec.py
+109 -25
View File
@@ -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/<run>/<attempt>`` 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)
tests/test_agent_exec.py
+132 -6
View File
@@ -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"