From 6124d0af10a20630b7946ff76f4f2200deae05cc Mon Sep 17 00:00:00 2001 From: Walther Maciel Date: Mon, 27 Jul 2026 15:00:41 -0700 Subject: [PATCH 01/14] feat(dream): add dream-setup.py (JSON-emitting port of dream-setup.sh) Cross-platform Python port of the bash dream-setup.sh. Prints the dream context as JSON (the eval/export env mode is dropped; JSON is the only output). Mirrors the .sh contract: slug/namespace validation, .shadow/ gitignore guard (child-path probe), default-branch detection, namespace resolution (env > TASK_INFO.json > .env > basename), external worktree path, idempotent create-with-retry, and RUN_PREFIX detection. Pins UTF-8 on stdout/stderr and imports the shared _worktree_safety gate (not a subprocess). Auto-GC prefers a dream-gc.py sibling and otherwise falls back to the bash dream-gc.sh; it is best-effort and never breaks setup, cleanly no-opping where no usable shell exists. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 35800a4a-a5d9-4e3a-b0e3-0d093e083b24 --- skills/shadow-frog-dream/dream-setup.py | 390 ++++++++++++++++++++++++ 1 file changed, 390 insertions(+) create mode 100644 skills/shadow-frog-dream/dream-setup.py diff --git a/skills/shadow-frog-dream/dream-setup.py b/skills/shadow-frog-dream/dream-setup.py new file mode 100644 index 0000000..be71bf0 --- /dev/null +++ b/skills/shadow-frog-dream/dream-setup.py @@ -0,0 +1,390 @@ +#!/usr/bin/env python3 +"""Dream experiment setup — create the worktree and print its context as JSON. + +Usage: + python dream-setup.py --slug t01-csv-fuzzer + python dream-setup.py --slug t03-extend --base-branch dream/ns/ + +Prints a JSON object to stdout with keys: + repo_root, default_branch, dream_ns, dream_id, branch_name, parent_branch, + worktree_dir, worktree_base, base_commit, run_prefix, slug + +Exits non-zero (message on stderr) on any failure — always check the exit code. + +This script: + 1. Validates inputs (slug/namespace) against [A-Za-z0-9_-][A-Za-z0-9._-]* + (a non-'.' first char rejects a bare '.'/'..'). + 2. Computes dream_id, branch_name, worktree_dir, base_commit. + 3. Creates the worktree (idempotent — cleans an existing one via a safety gate). + 4. Prints the context as JSON (no shell escaping needed). + +Flags: + --slug NAME Task slug (required, e.g. "t01-csv-fuzzer") + --base-branch REF Branch to base from (default: the repo's default branch) + --namespace NS Override DREAM_NAMESPACE (default: env or repo basename) + --repo-root DIR Override repo root (default: git rev-parse) + --print-json Accepted for compatibility (JSON is the only output mode) + --dry-run Compute values without creating the worktree + --help, -h Show this help message + +Design: + - Idempotent: re-running with the same slug cleans and recreates. + - External path: worktrees always in //dream-. + - Refuses to create a worktree inside the project directory. + - Detects the default branch (main/master) automatically. + - Resolves the namespace from env > TASK_INFO.json > .env > repo basename. +""" +import json +import os +import re +import shutil +import subprocess +import sys +import tempfile +import time +from datetime import datetime, timezone + +# Same regex `_worktree_safety.py` validates ns/slug against — keep in lockstep. +SAFE_RE = re.compile(r"^[A-Za-z0-9_-][A-Za-z0-9._-]*$") +SAFE_INT_RE = re.compile(r"^[0-9]+$") + +SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__)) + + +def _err(msg): + print(msg, file=sys.stderr) + + +def _git(args, cwd=None): + """Run a git command, capturing output as UTF-8. Never raises on non-zero.""" + return subprocess.run( + ["git", *args], cwd=cwd, + capture_output=True, text=True, encoding="utf-8", + ) + + +def _import_safety(): + """Lazily import the shared rm-rf safety gate (sibling module). Raises + ImportError if the module is missing — the caller decides how to react.""" + sys.path.insert(0, SCRIPT_DIR) + try: + from _worktree_safety import safe_worktree_path, UnsafePath + return safe_worktree_path, UnsafePath + finally: + if sys.path and sys.path[0] == SCRIPT_DIR: + sys.path.pop(0) + + +def _read_env_namespace(path): + """Extract DREAM_NAMESPACE from a `.env` file (first match), stripping + surrounding whitespace and one layer of matching quotes.""" + try: + with open(path, encoding="utf-8") as f: + for line in f: + if line.startswith("DREAM_NAMESPACE="): + val = line.split("=", 1)[1].strip() + if len(val) >= 2 and val[0] in "\"'" and val[-1] == val[0]: + val = val[1:-1] + return val.strip() + except OSError: + return "" + return "" + + +def _val(argv, i, flag): + if i + 1 >= len(argv): + _err(f"ERROR: {flag} requires a value") + sys.exit(1) + return argv[i + 1] + + +def _parse_args(argv): + opts = { + "slug": "", "base_branch": "", "namespace": "", + "repo_root": "", "dry_run": False, + } + i = 0 + while i < len(argv): + a = argv[i] + if a == "--slug": + opts["slug"] = _val(argv, i, a); i += 2 + elif a == "--base-branch": + opts["base_branch"] = _val(argv, i, a); i += 2 + elif a == "--namespace": + opts["namespace"] = _val(argv, i, a); i += 2 + elif a == "--repo-root": + opts["repo_root"] = _val(argv, i, a); i += 2 + elif a == "--print-json": + i += 1 # accepted no-op — JSON is the only output mode + elif a == "--dry-run": + opts["dry_run"] = True; i += 1 + elif a in ("--help", "-h"): + print(__doc__) + sys.exit(0) + else: + _err(f"ERROR: Unknown argument: {a}") + sys.exit(1) + return opts + + +def _maybe_auto_gc(repo_root, worktree_base): + """Periodic, throttled, best-effort orphan sweep. NEVER breaks setup. + + Prefers a `dream-gc.py` sibling if present, else falls back to the bash + `dream-gc.sh`. Runs wherever a usable interpreter/shell exists, else a clean + no-op. All GC output is routed to stderr so stdout stays pure JSON. + """ + if os.environ.get("DREAM_GC_AUTO", "1") == "0": + return + + interval_raw = os.environ.get("DREAM_GC_INTERVAL_MIN", "60") + age_raw = os.environ.get("DREAM_GC_AGE_MIN", "60") + if not SAFE_INT_RE.match(interval_raw) or not SAFE_INT_RE.match(age_raw): + _err("WARN: DREAM_GC_INTERVAL_MIN / DREAM_GC_AGE_MIN must be " + "non-negative integers — skipping auto-GC") + return + interval_min = int(interval_raw) + + gc_py = os.path.join(SCRIPT_DIR, "dream-gc.py") + gc_sh = os.path.join(SCRIPT_DIR, "dream-gc.sh") + if os.path.isfile(gc_py): + gc_cmd = [sys.executable, gc_py, "--repo-root", repo_root, + "--quiet", "--min-age-min", age_raw] + elif os.path.isfile(gc_sh): + gc_cmd = ["bash", gc_sh, "--repo-root", repo_root, + "--quiet", "--min-age-min", age_raw] + else: + return + + tombstone = os.path.join(worktree_base, ".last-gc") + should_run = False + if not os.path.isfile(tombstone): + should_run = True + elif interval_min == 0: + # Interval 0 ⇒ "always run" (a fresh tombstone would otherwise + # throttle the very first sweep after touch). + should_run = True + else: + try: + if (time.time() - os.path.getmtime(tombstone)) > interval_min * 60: + should_run = True + except OSError: + should_run = True + if not should_run: + return + + # Touch BEFORE running so a parallel setup for the same ns sees a fresh + # tombstone and skips. Worst case is one extra sweep, never a missed one. + try: + with open(tombstone, "a", encoding="utf-8"): + pass + os.utime(tombstone, None) + except OSError: + pass + + try: + r = subprocess.run( + gc_cmd, cwd=repo_root, + capture_output=True, text=True, encoding="utf-8", timeout=120, + ) + if r.stdout: + sys.stderr.write(r.stdout) + if r.stderr: + sys.stderr.write(r.stderr) + except Exception: # noqa: BLE001 — auto-GC must never break setup + pass + + +def _preclean_worktree(repo_root, worktree_dir, gate_base): + """Idempotent pre-clean of an existing worktree dir. Tries git first, then + a safety-gated rmtree for stale dirs git can't see. Refuses (exit 1) if the + safety module is missing — never an un-gated remove.""" + if not os.path.isdir(worktree_dir): + return + if _git(["worktree", "remove", worktree_dir, "--force"], + cwd=repo_root).returncode != 0: + try: + safe_worktree_path, UnsafePath = _import_safety() + except ImportError: + _err("ERROR: safety module not found, refusing pre-clean rm: " + f"{os.path.join(SCRIPT_DIR, '_worktree_safety.py')}") + sys.exit(1) + try: + resolved = safe_worktree_path(worktree_dir, gate_base) + except UnsafePath: + resolved = None + if resolved is not None: + shutil.rmtree(resolved, ignore_errors=True) + _git(["worktree", "prune"], cwd=repo_root) + + +def main(): + for _stream in (sys.stdout, sys.stderr): + if hasattr(_stream, "reconfigure"): + _stream.reconfigure(encoding="utf-8") + + opts = _parse_args(sys.argv[1:]) + + slug = opts["slug"] + if not slug: + _err("ERROR: --slug is required") + _err("Usage: dream-setup.py --slug t01-name [--base-branch BRANCH]") + sys.exit(1) + if not SAFE_RE.match(slug): + _err(f"ERROR: --slug must match {SAFE_RE.pattern} (got: {slug})") + _err(" Use kebab-case alphanumerics like 't01-csv-fuzzer'.") + sys.exit(1) + if opts["namespace"] and not SAFE_RE.match(opts["namespace"]): + _err(f"ERROR: --namespace must match {SAFE_RE.pattern} " + f"(got: {opts['namespace']})") + sys.exit(1) + + # --- Resolve repo root --- + if opts["repo_root"]: + repo_root = os.path.abspath(opts["repo_root"]) + else: + r = _git(["rev-parse", "--show-toplevel"]) + if r.returncode != 0: + _err("ERROR: Not in a git repository") + sys.exit(1) + repo_root = os.path.abspath(r.stdout.strip()) + os.chdir(repo_root) + + # --- Guard: .shadow/ must be tracked by git (not gitignored) --- + # Probe a NEW child path (not `.shadow` itself): when `.shadow/` is + # gitignored but already tracked, `git check-ignore .shadow` reports + # "not ignored", yet `git add -A` still drops NEW files under it. + probe = ".shadow/_dreams/__shadowfrog_probe__/manifest.json" + if _git(["check-ignore", "-q", probe]).returncode == 0: + _err("ERROR: .shadow/ is gitignored — shadow-frog-dream requires it " + "to be tracked by git.") + _err(" Dream experiments commit .shadow/ artifacts onto a branch, " + "push them, and") + _err(" reconcile reads them back from the remote. A gitignored " + ".shadow/ would be") + _err(" silently dropped at commit time, losing every discovery.") + _err(" Fix: remove the '.shadow/' entry from .gitignore and commit " + ".shadow/,") + _err(" or run shadow-frog-update (which works in local-only mode) " + "instead of dream.") + sys.exit(1) + + # --- Detect default branch --- + default_branch = "" + r = _git(["symbolic-ref", "refs/remotes/origin/HEAD"]) + if r.returncode == 0: + default_branch = r.stdout.strip().replace("refs/remotes/origin/", "") + if not default_branch: + if _git(["show-ref", "--verify", + "refs/remotes/origin/main"]).returncode == 0: + default_branch = "main" + elif _git(["show-ref", "--verify", + "refs/remotes/origin/master"]).returncode == 0: + default_branch = "master" + else: + _err("ERROR: Cannot detect default branch. " + "Fix: git remote set-head origin ") + sys.exit(1) + + # --- Resolve namespace --- + dream_ns = "" + if opts["namespace"]: + dream_ns = opts["namespace"] + elif os.environ.get("DREAM_NAMESPACE"): + dream_ns = os.environ["DREAM_NAMESPACE"] + elif os.path.isfile("TASK_INFO.json"): + try: + with open("TASK_INFO.json", encoding="utf-8") as f: + dream_ns = json.load(f).get("dream_namespace", "") or "" + except (OSError, ValueError): + dream_ns = "" + elif os.path.isfile(".env"): + dream_ns = _read_env_namespace(".env") + if not dream_ns: + dream_ns = os.path.basename(repo_root) + + if not SAFE_RE.match(dream_ns): + _err(f"ERROR: Resolved DREAM_NS contains unsafe characters: {dream_ns}") + _err(f" Allowed: {SAFE_RE.pattern}") + _err(" Override with --namespace or set DREAM_NAMESPACE.") + sys.exit(1) + + # --- Compute identifiers --- + dream_id = datetime.now(timezone.utc).strftime("%Y%m%d-%H%M%SZ") + "-" + slug + branch_name = f"dream/{dream_ns}/{dream_id}" + + # --- Compute worktree path (ALWAYS external, NEVER in project) --- + gate_base = os.environ.get("DREAM_WORKTREE_BASE") or os.path.join( + tempfile.gettempdir(), "shadowfrog-dreams") + worktree_base = os.path.join(gate_base, dream_ns) + worktree_dir = os.path.join(worktree_base, f"dream-{slug}") + + rr = os.path.abspath(repo_root) + wd = os.path.abspath(worktree_dir) + if wd == rr or wd.startswith(rr + os.sep): + _err(f"ERROR: Worktree would be inside project: {worktree_dir}") + _err("MUST use external path (default: /shadowfrog-dreams/)") + sys.exit(1) + + # --- Resolve base reference --- + if not opts["base_branch"]: + base_ref = f"origin/{default_branch}" + parent_branch = default_branch + else: + base_ref = f"origin/{opts['base_branch']}" + parent_branch = opts["base_branch"] + if _git(["show-ref", "--verify", + f"refs/remotes/{base_ref}"]).returncode != 0: + _err(f"ERROR: Base branch not found: {base_ref}") + sys.exit(1) + + # --- Create worktree (unless dry-run) --- + base_commit = "" + if not opts["dry_run"]: + os.makedirs(worktree_base, exist_ok=True) + _maybe_auto_gc(repo_root, worktree_base) + _preclean_worktree(repo_root, worktree_dir, gate_base) + + def _add(): + return _git(["worktree", "add", worktree_dir, "-b", branch_name, + base_ref], cwd=repo_root).returncode + + if _add() != 0: + _git(["branch", "-D", branch_name], cwd=repo_root) + _git(["worktree", "prune"], cwd=repo_root) + if _add() != 0: + _err(f"ERROR: git worktree add failed for {worktree_dir} " + f"(branch {branch_name}, base {base_ref})") + sys.exit(1) + + base_commit = _git(["rev-parse", "HEAD"], cwd=worktree_dir).stdout.strip() + else: + r = _git(["rev-parse", base_ref]) + base_commit = r.stdout.strip() if r.returncode == 0 else "DRY_RUN" + + # --- Detect RUN_PREFIX --- + run_prefix = "" + if os.path.isfile("uv.lock"): + run_prefix = "uv run" + elif os.path.isfile("package-lock.json"): + run_prefix = "npx" + elif os.path.isfile("yarn.lock"): + run_prefix = "npx" + + print(json.dumps({ + "repo_root": repo_root, + "default_branch": default_branch, + "dream_ns": dream_ns, + "dream_id": dream_id, + "branch_name": branch_name, + "parent_branch": parent_branch, + "worktree_dir": worktree_dir, + "worktree_base": worktree_base, + "base_commit": base_commit, + "run_prefix": run_prefix, + "slug": slug, + }, indent=2)) + + +if __name__ == "__main__": + main() From 58d6b0f8d0ef6724f613511fc53fbb34496f08f7 Mon Sep 17 00:00:00 2001 From: Walther Maciel Date: Mon, 27 Jul 2026 15:00:42 -0700 Subject: [PATCH 02/14] test(dream): port dream-setup tests to the Python entry point Port test_dream_setup_sh.py to invoke dream-setup.py via sys.executable (no bash), so the suite runs on both OSes. The base env starts from the real environment (Python + git need SystemRoot/PATH/TEMP on Windows), pins git config to devnull, and clears inherited DREAM_* vars. Worktree registration is compared via resolved Paths since `git worktree list` prints forward-slash paths on Windows. The auto-GC throttle/opt-out tests (dream-setup's own logic) run on every OS. The two tests that assert an orphan is actually swept still need the bash dream-gc.sh, so they sit in a requires_bash class skipped on Windows. The eval-stdout-purity test becomes a JSON-stdout-purity assertion. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 35800a4a-a5d9-4e3a-b0e3-0d093e083b24 --- .../shadow_frog_dream/test_dream_setup.py | 561 ++++++++++++++++++ 1 file changed, 561 insertions(+) create mode 100644 tests/skills/shadow_frog_dream/test_dream_setup.py diff --git a/tests/skills/shadow_frog_dream/test_dream_setup.py b/tests/skills/shadow_frog_dream/test_dream_setup.py new file mode 100644 index 0000000..8d589b6 --- /dev/null +++ b/tests/skills/shadow_frog_dream/test_dream_setup.py @@ -0,0 +1,561 @@ +"""Tests for skills/shadow-frog-dream/dream-setup.py — Dream worktree setup. + +Exercises: --help, happy-path worktree+branch creation, RUN_PREFIX detection, +namespace override, slug validation, the .shadow/ gitignore guard, dry-run, +and the throttled best-effort auto-GC. + +Cross-platform: invokes the Python entry point directly (no bash). The two +auto-GC tests that assert an orphan is actually *swept* still need the bash +`dream-gc.sh`, so they live in a `requires_bash` class skipped on Windows; +every other test runs on all OSes. +""" +import json +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parent.parent.parent.parent +DREAM_SETUP = REPO_ROOT / "skills" / "shadow-frog-dream" / "dream-setup.py" + +# Cleared from the base env so a developer's shell can't leak into namespace / +# worktree resolution; each test sets exactly what it needs via `extras`. +_DREAM_ENV_KEYS = ( + "DREAM_NAMESPACE", "DREAM_WORKTREE_BASE", "DREAM_GC_AUTO", + "DREAM_GC_INTERVAL_MIN", "DREAM_GC_AGE_MIN", +) + + +def _base_env(cwd: Path, extras: dict | None = None) -> dict: + """Isolated env for the setup subprocess. Starts from the real environment + (Python + git need SystemRoot/PATH/TEMP on Windows), then pins git config to + devnull and drops any inherited DREAM_* vars.""" + env = os.environ.copy() + for k in _DREAM_ENV_KEYS: + env.pop(k, None) + env["HOME"] = str(cwd) + env["GIT_CONFIG_GLOBAL"] = os.devnull + env["GIT_CONFIG_SYSTEM"] = os.devnull + env["LANG"] = "en_US.UTF-8" + if extras: + env.update(extras) + return env + + +def _make_git_repo(path: Path, branch: str = "main") -> None: + """Create a git repo with an initial commit and an origin/main ref.""" + env = _base_env(path) + subprocess.run(["git", "init", "-q", "-b", branch], cwd=path, check=True, env=env) + subprocess.run(["git", "config", "user.email", "test@test.invalid"], cwd=path, check=True, env=env) + subprocess.run(["git", "config", "user.name", "Test"], cwd=path, check=True, env=env) + subprocess.run(["git", "config", "commit.gpgsign", "false"], cwd=path, check=True, env=env) + (path / "README.md").write_text("# test\n", encoding="utf-8") + subprocess.run(["git", "add", "-A"], cwd=path, check=True, env=env) + subprocess.run(["git", "commit", "-q", "-m", "init"], cwd=path, check=True, env=env) + # Fake origin remote pointing at self, for the origin/main ref. + subprocess.run(["git", "remote", "add", "origin", str(path)], cwd=path, check=True, env=env) + subprocess.run(["git", "fetch", "-q", "origin"], cwd=path, check=True, env=env) + + +def run_dream_setup( + args: list[str], cwd: Path, env_extra: dict | None = None +) -> subprocess.CompletedProcess: + """Run dream-setup.py with the given args via the current interpreter.""" + env = _base_env(cwd, env_extra) + return subprocess.run( + [sys.executable, str(DREAM_SETUP), *args], + capture_output=True, text=True, cwd=cwd, env=env, encoding="utf-8", + ) + + +def _plant_orphan(base: Path, ns: str, name: str = "dream-orphan") -> Path: + """Plant an orphan worktree (broken .git pointer, ancient mtime).""" + d = base / ns / name + d.mkdir(parents=True) + (d / ".git").write_text("gitdir: /nonexistent/path\n", encoding="utf-8") + (d / "leftover.txt").write_text("orphaned\n", encoding="utf-8") + ancient = 946684800 + os.utime(d, (ancient, ancient)) + return d + + +@pytest.mark.slow +@pytest.mark.integration +class TestDreamSetupHelp: + def test_help_exits_zero(self, tmp_path): + # --help should work even outside a git repo (it just prints and exits). + result = run_dream_setup(["--help"], cwd=tmp_path) + assert result.returncode == 0 + assert "slug" in result.stdout.lower() or "slug" in result.stderr.lower() or "Usage" in result.stdout + + +@pytest.mark.slow +@pytest.mark.integration +class TestDreamSetupHappyPath: + """Creates worktree and branch correctly.""" + + def test_creates_worktree_and_branch(self, tmp_path): + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + worktree_base = tmp_path / "worktrees" + + result = run_dream_setup( + ["--slug", "t01-test", "--repo-root", str(repo), "--print-json"], + cwd=repo, + env_extra={"DREAM_WORKTREE_BASE": str(worktree_base)}, + ) + assert result.returncode == 0, f"stderr: {result.stderr}" + data = json.loads(result.stdout) + + assert "dream_ns" in data + assert "branch_name" in data + assert "worktree_dir" in data + assert data["slug"] == "t01-test" + assert "dream/" in data["branch_name"] + assert "t01-test" in data["dream_id"] + + # Worktree exists on disk. + wt_dir = Path(data["worktree_dir"]) + assert wt_dir.is_dir() + + # And is registered with git. `git worktree list` prints '/'-paths on + # Windows while the JSON path is OS-native; compare resolved Paths. + env = _base_env(repo) + wt_list = subprocess.run( + ["git", "worktree", "list", "--porcelain"], cwd=repo, + capture_output=True, text=True, env=env, encoding="utf-8", + ) + registered = { + Path(line[len("worktree "):]).resolve() + for line in wt_list.stdout.splitlines() + if line.startswith("worktree ") + } + assert wt_dir.resolve() in registered + + def test_worktree_has_same_head_as_base(self, tmp_path): + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + worktree_base = tmp_path / "worktrees" + env = _base_env(repo) + + main_head = subprocess.run( + ["git", "rev-parse", "HEAD"], cwd=repo, + capture_output=True, text=True, check=True, env=env, encoding="utf-8", + ).stdout.strip() + + result = run_dream_setup( + ["--slug", "t02-head", "--repo-root", str(repo), "--print-json"], + cwd=repo, + env_extra={"DREAM_WORKTREE_BASE": str(worktree_base)}, + ) + assert result.returncode == 0, f"stderr: {result.stderr}" + data = json.loads(result.stdout) + assert data["base_commit"] == main_head + + +@pytest.mark.slow +@pytest.mark.integration +class TestDreamSetupRunPrefix: + """RUN_PREFIX detection based on lock files.""" + + def test_no_lock_files_empty_prefix(self, tmp_path): + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + worktree_base = tmp_path / "wt" + + result = run_dream_setup( + ["--slug", "t03-nolock", "--repo-root", str(repo), "--print-json"], + cwd=repo, + env_extra={"DREAM_WORKTREE_BASE": str(worktree_base)}, + ) + assert result.returncode == 0, f"stderr: {result.stderr}" + data = json.loads(result.stdout) + assert data["run_prefix"] == "" + + def test_uv_lock_gives_uv_run_prefix(self, tmp_path): + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + (repo / "uv.lock").write_text("", encoding="utf-8") + worktree_base = tmp_path / "wt" + + result = run_dream_setup( + ["--slug", "t04-uvlock", "--repo-root", str(repo), "--print-json"], + cwd=repo, + env_extra={"DREAM_WORKTREE_BASE": str(worktree_base)}, + ) + assert result.returncode == 0, f"stderr: {result.stderr}" + data = json.loads(result.stdout) + assert data["run_prefix"] == "uv run" + + def test_package_lock_gives_npx_prefix(self, tmp_path): + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + (repo / "package-lock.json").write_text("{}", encoding="utf-8") + worktree_base = tmp_path / "wt" + + result = run_dream_setup( + ["--slug", "t05-npm", "--repo-root", str(repo), "--print-json"], + cwd=repo, + env_extra={"DREAM_WORKTREE_BASE": str(worktree_base)}, + ) + assert result.returncode == 0, f"stderr: {result.stderr}" + data = json.loads(result.stdout) + assert data["run_prefix"] == "npx" + + +@pytest.mark.slow +@pytest.mark.integration +class TestDreamSetupIdempotent: + """Re-running with same slug cleans and recreates (idempotent).""" + + def test_same_slug_twice_succeeds(self, tmp_path): + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + worktree_base = tmp_path / "wt" + + args = ["--slug", "t06-idem", "--repo-root", str(repo), "--print-json"] + extras = {"DREAM_WORKTREE_BASE": str(worktree_base)} + + r1 = run_dream_setup(args, cwd=repo, env_extra=extras) + assert r1.returncode == 0, f"stderr: {r1.stderr}" + + r2 = run_dream_setup(args, cwd=repo, env_extra=extras) + assert r2.returncode == 0, f"stderr: {r2.stderr}" + d1 = json.loads(r1.stdout) + d2 = json.loads(r2.stdout) + assert d1["worktree_dir"] == d2["worktree_dir"] + + +@pytest.mark.slow +@pytest.mark.integration +class TestDreamSetupNamespace: + """DREAM_NAMESPACE / --namespace honored in branch name.""" + + def test_namespace_override_in_branch(self, tmp_path): + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + worktree_base = tmp_path / "wt" + + result = run_dream_setup( + ["--slug", "t07-ns", "--namespace", "my-custom-ns", + "--repo-root", str(repo), "--print-json"], + cwd=repo, + env_extra={"DREAM_WORKTREE_BASE": str(worktree_base)}, + ) + assert result.returncode == 0, f"stderr: {result.stderr}" + data = json.loads(result.stdout) + assert data["dream_ns"] == "my-custom-ns" + assert "dream/my-custom-ns/" in data["branch_name"] + + def test_env_namespace_used(self, tmp_path): + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + worktree_base = tmp_path / "wt" + + result = run_dream_setup( + ["--slug", "t08-envns", "--repo-root", str(repo), "--print-json"], + cwd=repo, + env_extra={ + "DREAM_WORKTREE_BASE": str(worktree_base), + "DREAM_NAMESPACE": "env-ns-test", + }, + ) + assert result.returncode == 0, f"stderr: {result.stderr}" + data = json.loads(result.stdout) + assert data["dream_ns"] == "env-ns-test" + + +@pytest.mark.slow +@pytest.mark.integration +class TestDreamSetupValidation: + """Input validation prevents bad slugs.""" + + def test_missing_slug_fails(self, tmp_path): + result = run_dream_setup([], cwd=tmp_path) + assert result.returncode != 0 + assert "slug" in result.stderr.lower() + + def test_invalid_slug_rejected(self, tmp_path): + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + result = run_dream_setup( + ["--slug", "bad slug!!", "--repo-root", str(repo)], + cwd=repo, + ) + assert result.returncode != 0 + assert "must match" in result.stderr + + +@pytest.mark.slow +@pytest.mark.integration +class TestDreamSetupGitignoreGuard: + """dream requires .shadow/ to be git-tracked, not gitignored.""" + + def test_refuses_when_shadow_gitignored(self, tmp_path): + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + (repo / ".gitignore").write_text(".shadow/\n", encoding="utf-8") + (repo / ".shadow").mkdir() + result = run_dream_setup( + ["--slug", "t10-ignored", "--repo-root", str(repo), + "--dry-run", "--print-json"], + cwd=repo, + ) + assert result.returncode != 0 + assert ".shadow/ is gitignored" in result.stderr + + def test_proceeds_when_shadow_tracked(self, tmp_path): + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + # .gitignore present but does NOT ignore .shadow/ + (repo / ".gitignore").write_text("build/\n__pycache__/\n", encoding="utf-8") + (repo / ".shadow").mkdir() + worktree_base = tmp_path / "wt" + result = run_dream_setup( + ["--slug", "t10-tracked", "--repo-root", str(repo), + "--dry-run", "--print-json"], + cwd=repo, + env_extra={"DREAM_WORKTREE_BASE": str(worktree_base)}, + ) + assert result.returncode == 0, f"stderr: {result.stderr}" + assert "gitignored" not in result.stderr + + def test_refuses_when_shadow_gitignored_but_already_tracked(self, tmp_path): + """Edge case: .shadow/ is gitignored AND has previously-committed + content. `git check-ignore .shadow` reports not-ignored (tracked wins), + but `git add -A` still drops NEW children — so the guard must probe a + child path and still refuse.""" + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + shadow_meta = repo / ".shadow" / "_meta" + shadow_meta.mkdir(parents=True) + (shadow_meta / "state.json").write_text("{}\n", encoding="utf-8") + env = _base_env(repo) + subprocess.run(["git", "add", "-A"], cwd=repo, check=True, env=env) + subprocess.run(["git", "commit", "-qm", "track shadow"], + cwd=repo, check=True, env=env) + (repo / ".gitignore").write_text(".shadow/\n", encoding="utf-8") + subprocess.run(["git", "add", ".gitignore"], cwd=repo, check=True, env=env) + subprocess.run(["git", "commit", "-qm", "ignore shadow"], + cwd=repo, check=True, env=env) + + result = run_dream_setup( + ["--slug", "t10-tracked-ignored", "--repo-root", str(repo), + "--dry-run", "--print-json"], + cwd=repo, + ) + assert result.returncode != 0 + assert ".shadow/ is gitignored" in result.stderr + + +@pytest.mark.slow +@pytest.mark.integration +class TestDreamSetupDryRun: + """--dry-run computes values without creating worktree.""" + + def test_dry_run_no_worktree_created(self, tmp_path): + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + worktree_base = tmp_path / "wt" + + result = run_dream_setup( + ["--slug", "t09-dry", "--repo-root", str(repo), + "--dry-run", "--print-json"], + cwd=repo, + env_extra={"DREAM_WORKTREE_BASE": str(worktree_base)}, + ) + assert result.returncode == 0, f"stderr: {result.stderr}" + data = json.loads(result.stdout) + assert not Path(data["worktree_dir"]).exists() + assert data["slug"] == "t09-dry" + + +# =========================================================================== +# Auto-GC throttle (Bug A fix from bug-cleanup-gaps.md) +# +# These four exercise dream-setup's OWN throttle/opt-out logic, which never +# invokes the GC sweeper — so they run on every OS. The two tests that assert +# an orphan is actually swept need bash `dream-gc.sh` and live in the +# `requires_bash` class below (skipped on Windows). +# =========================================================================== + +@pytest.mark.slow +@pytest.mark.integration +class TestDreamSetupAutoGCThrottle: + """`dream-setup.py` decides *whether* to trigger the sweeper, throttled by + a per-namespace `.last-gc` tombstone.""" + + def test_auto_gc_throttled_by_recent_tombstone(self, tmp_path): + """Fresh tombstone (< DREAM_GC_INTERVAL_MIN) suppresses the trigger.""" + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + worktree_base = tmp_path / "worktrees" + ns = repo.name + + (worktree_base / ns).mkdir(parents=True) + tombstone = worktree_base / ns / ".last-gc" + tombstone.touch() + + orphan = _plant_orphan(worktree_base, ns) + + result = run_dream_setup( + ["--slug", "t02-throttle", "--repo-root", str(repo), "--print-json"], + cwd=repo, + env_extra={ + "DREAM_WORKTREE_BASE": str(worktree_base), + "DREAM_GC_INTERVAL_MIN": "60", # tombstone is fresh, won't trigger + "DREAM_GC_AGE_MIN": "0", + }, + ) + assert result.returncode == 0, f"stderr: {result.stderr}" + # Orphan must STILL exist — auto-GC was throttled (no sweeper invoked). + assert orphan.exists(), ( + f"Fresh tombstone should suppress auto-GC\nstderr: {result.stderr}" + ) + + def test_auto_gc_disabled_via_env(self, tmp_path): + """`DREAM_GC_AUTO=0` opts out of the auto-trigger entirely.""" + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + worktree_base = tmp_path / "worktrees" + ns = repo.name + + orphan = _plant_orphan(worktree_base, ns) + + result = run_dream_setup( + ["--slug", "t03-disabled", "--repo-root", str(repo), "--print-json"], + cwd=repo, + env_extra={ + "DREAM_WORKTREE_BASE": str(worktree_base), + "DREAM_GC_AUTO": "0", + "DREAM_GC_AGE_MIN": "0", + }, + ) + assert result.returncode == 0, f"stderr: {result.stderr}" + # Orphan must STILL exist — GC was opted out. + assert orphan.exists(), ( + f"DREAM_GC_AUTO=0 should disable auto-GC\nstderr: {result.stderr}" + ) + # Tombstone NOT created. + assert not (worktree_base / ns / ".last-gc").exists() + + def test_auto_gc_invalid_env_warns_and_continues(self, tmp_path): + """Non-integer interval/age must not break dream setup.""" + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + worktree_base = tmp_path / "worktrees" + + result = run_dream_setup( + ["--slug", "t04-badenv", "--repo-root", str(repo), "--print-json"], + cwd=repo, + env_extra={ + "DREAM_WORKTREE_BASE": str(worktree_base), + "DREAM_GC_INTERVAL_MIN": "not-a-number", + }, + ) + # Dream setup must still succeed — auto-GC is best-effort. + assert result.returncode == 0, f"stderr: {result.stderr}" + data = json.loads(result.stdout) + assert Path(data["worktree_dir"]).is_dir() + + def test_auto_gc_does_not_pollute_json_stdout(self, tmp_path): + """Auto-GC output MUST go to stderr so stdout stays pure JSON.""" + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + worktree_base = tmp_path / "worktrees" + ns = repo.name + + # Plant an orphan so the GC actually has work to log about. + _plant_orphan(worktree_base, ns) + + result = run_dream_setup( + ["--slug", "t05-stdout", "--repo-root", str(repo), "--print-json"], + cwd=repo, + env_extra={ + "DREAM_WORKTREE_BASE": str(worktree_base), + "DREAM_GC_AGE_MIN": "0", + }, + ) + assert result.returncode == 0, f"stderr: {result.stderr}" + # stdout must parse as a single clean JSON object — any GC noise leaking + # onto stdout would break json.loads. + data = json.loads(result.stdout) + assert data["slug"] == "t05-stdout" + + +@pytest.mark.slow +@pytest.mark.integration +@pytest.mark.requires_bash +class TestDreamSetupAutoGCSweep: + """These assert the orphan is actually *removed*, which needs the bash + `dream-gc.sh` sweeper. Skipped on Windows by the requires_bash hook; the + mark is dropped once a cross-platform `dream-gc.py` exists.""" + + def test_auto_gc_runs_when_no_tombstone(self, tmp_path): + """First invocation sweeps orphans (no tombstone yet).""" + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + worktree_base = tmp_path / "worktrees" + ns = repo.name + + orphan = _plant_orphan(worktree_base, ns) + assert orphan.exists() + + result = run_dream_setup( + ["--slug", "t01-gc", "--repo-root", str(repo), "--print-json"], + cwd=repo, + env_extra={ + "DREAM_WORKTREE_BASE": str(worktree_base), + # Force min-age-min=0 so the ancient orphan is in the sweep window. + "DREAM_GC_AGE_MIN": "0", + }, + ) + assert result.returncode == 0, f"stderr: {result.stderr}" + assert not orphan.exists(), ( + f"Auto-GC should have swept the orphan\nstderr: {result.stderr}" + ) + tombstone = worktree_base / ns / ".last-gc" + assert tombstone.exists() + + def test_auto_gc_sweeps_other_namespace_orphans_too(self, tmp_path): + """The auto-trigger sweeps the whole base, not just its own ns.""" + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + worktree_base = tmp_path / "worktrees" + + other_orphan = _plant_orphan(worktree_base, ns="other-repo") + assert other_orphan.exists() + + result = run_dream_setup( + ["--slug", "t06-cross", "--repo-root", str(repo), "--print-json"], + cwd=repo, + env_extra={ + "DREAM_WORKTREE_BASE": str(worktree_base), + "DREAM_GC_AGE_MIN": "0", + }, + ) + assert result.returncode == 0, f"stderr: {result.stderr}" + assert not other_orphan.exists(), ( + f"Auto-GC sweeps the whole base\nstderr: {result.stderr}" + ) From fe1c056314d68d9d74b0b15944ad2a78b6bcead9 Mon Sep 17 00:00:00 2001 From: Walther Maciel Date: Mon, 27 Jul 2026 15:00:42 -0700 Subject: [PATCH 03/14] docs(dream): switch SKILL.md setup flow to dream-setup.py JSON Point the dream skill at the ported dream-setup.py and its JSON contract: frontmatter scripts list, the helper table, the .shadow/ gitignore-guard note, the Script-Failure-Recovery list, the setup-script finder, and the two setup examples now run `dream-setup.py` and parse the JSON object it prints instead of eval-ing shell exports. References to dream-gc.sh being auto-triggered now name dream-setup.py. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 35800a4a-a5d9-4e3a-b0e3-0d093e083b24 --- skills/shadow-frog-dream/SKILL.md | 56 +++++++++++++------------------ 1 file changed, 24 insertions(+), 32 deletions(-) diff --git a/skills/shadow-frog-dream/SKILL.md b/skills/shadow-frog-dream/SKILL.md index 94ad0cc..da991c5 100644 --- a/skills/shadow-frog-dream/SKILL.md +++ b/skills/shadow-frog-dream/SKILL.md @@ -13,7 +13,7 @@ scripts: - dream-coverage.py - dream-validate.py - dream-reconcile.py - - dream-setup.sh + - dream-setup.py - dream-cleanup.sh - dream-gc.sh --- @@ -37,7 +37,7 @@ the dream branch, pushed to the remote, then read back by the reconciler via `git show origin/ .shadow/...`. If `.shadow/` is gitignored (the "local only" option in `shadow-frog-init`), `git add -A` silently skips those files, nothing reaches the remote, and the reconciler finds no manifest — -**every discovery is lost without warning**. `dream-setup.sh` runs +**every discovery is lost without warning**. `dream-setup.py` runs `git check-ignore .shadow` up front and refuses to start if it's ignored. Use `shadow-frog-update` instead for local-only shadows. @@ -54,7 +54,7 @@ WORKTREE_DIR = $WORKTREE_BASE/dream- - `DREAM_NS` (namespace) isolates branches per task/instance. Resolved from: `DREAM_NAMESPACE` env → `TASK_INFO.json` → `.env` → repo basename. - Override only with `DREAM_WORKTREE_BASE` env var if `/tmp` is too small. -- `dream-setup.sh` computes and enforces all paths. Use it. +- `dream-setup.py` computes and enforces all paths. Use it. ### Branch Naming @@ -101,7 +101,7 @@ each batch or each individual dream. ### Script Failure Recovery -All helper scripts (`dream-setup.sh`, `dream-reconcile.py`, +All helper scripts (`dream-setup.py`, `dream-reconcile.py`, `dream-validate.py`) are **self-documenting**. If a script fails or is unavailable: **read the script source**, understand what it does, and adapt its logic manually for your situation. Never skip steps just because a @@ -143,27 +143,22 @@ done | Script | Purpose | When to use | |--------|---------|-------------| -| `dream-setup.sh` | Creates worktree + branch with namespace isolation | **Phase 3** — start of every experiment | +| `dream-setup.py` | Creates worktree + branch with namespace isolation | **Phase 3** — start of every experiment | | `dream-validate.py` | Validates artifacts before push (hard gate) | **Phase 5** — before `git push` | | `dream-reconcile.py` | Merges dream branches into main's `.shadow/` | **Phase 6** — after all experiments done | | `dream-coverage.py` | Computes exploration coverage map | **Phase 2** — task planning for diversity | | `dream-cleanup.sh` | Safely removes ONE dream worktree (with safety gate) | **After push** — replaces the old inline cleanup snippet | -| `dream-gc.sh` | Sweeps orphan dream worktrees from `$DREAM_WORKTREE_BASE` | **Auto** — triggered by `dream-setup.sh` (per-namespace throttle, default 1× / hour) in orphan-only mode; also `--task-complete --namespace "$DREAM_NS" --min-age-min 0` for end-of-session sweep of registered-but-stale dirs | +| `dream-gc.sh` | Sweeps orphan dream worktrees from `$DREAM_WORKTREE_BASE` | **Auto** — triggered by `dream-setup.py` (per-namespace throttle, default 1× / hour) in orphan-only mode; also `--task-complete --namespace "$DREAM_NS" --min-age-min 0` for end-of-session sweep of registered-but-stale dirs | **Usage patterns:** ```bash -# Setup: creates worktree, prints export vars. -# IMPORTANT: capture the output FIRST, then eval it. Writing -# `eval "$(dream-setup.sh ...)" || exit 1` does NOT catch failures: if the -# command substitution exits non-zero and prints nothing, `eval ""` still -# succeeds (exit 0) and the agent silently proceeds with empty env vars. -# Assigning to a variable makes `|| exit 1` fire on the script's real exit code. -SETUP_OUT="$("$SKILL_DIR/dream-setup.sh" --slug t01-my-experiment)" || exit 1 -eval "$SETUP_OUT" -# → exports (keep in sync with dream-setup.sh emit_export block): -# REPO_ROOT, DEFAULT_BRANCH, DREAM_NS, DREAM_ID, BRANCH_NAME, PARENT_BRANCH, -# WORKTREE_DIR, WORKTREE_BASE, BASE_COMMIT, RUN_PREFIX, SLUG +# Setup: creates the worktree and prints its context as JSON on stdout. +# Run it, check the exit code, then parse the JSON object it prints. +SETUP_JSON="$("$SKILL_DIR/dream-setup.py" --slug t01-my-experiment)" || exit 1 +# SETUP_JSON keys: repo_root, default_branch, dream_ns, dream_id, branch_name, +# parent_branch, worktree_dir, worktree_base, base_commit, run_prefix, slug. +# The steps below use values parsed from that JSON (e.g. $DREAM_ID = .dream_id). # Validate: hard gate before push python3 "$SKILL_DIR/dream-validate.py" "$DREAM_ID" "$WORKTREE_DIR" @@ -498,34 +493,31 @@ written, code run, results recorded. ### Experiment Setup -Use `dream-setup.sh` to create worktrees (handles all path computation, +Use `dream-setup.py` to create worktrees (handles all path computation, namespace resolution, worktree creation, and validation): ```bash # Find the setup script SETUP_SCRIPT="" for DIR in .github/skills/shadow-frog-dream .claude/skills/shadow-frog-dream; do - [ -f "$DIR/dream-setup.sh" ] && SETUP_SCRIPT="$DIR/dream-setup.sh" && break + [ -f "$DIR/dream-setup.py" ] && SETUP_SCRIPT="$DIR/dream-setup.py" && break done -# Fresh experiment from main: -SETUP_OUT="$("$SETUP_SCRIPT" --slug t01-csv-fuzzer)" || exit 1 -eval "$SETUP_OUT" +# Fresh experiment from main (prints JSON — parse it, don't eval): +SETUP_JSON="$(python "$SETUP_SCRIPT" --slug t01-csv-fuzzer)" || exit 1 # Compounding from prior dream: -SETUP_OUT="$("$SETUP_SCRIPT" --slug t03-extend --base-branch dream//)" || exit 1 -eval "$SETUP_OUT" +SETUP_JSON="$(python "$SETUP_SCRIPT" --slug t03-extend --base-branch dream//)" || exit 1 ``` -Capture into `SETUP_OUT` first, then `eval` it — see Helper Scripts § -usage patterns (above) for why bare `eval "$(…)" || exit 1` silently -swallows the script's exit code. +Capture the JSON into `SETUP_JSON` and check the exit code (`|| exit 1`), +then parse it for the values below. -This exports (keep in sync with `dream-setup.sh`): `REPO_ROOT`, -`DEFAULT_BRANCH`, `DREAM_NS`, `DREAM_ID`, `BRANCH_NAME`, `PARENT_BRANCH`, -`WORKTREE_DIR`, `WORKTREE_BASE`, `BASE_COMMIT`, `RUN_PREFIX`, `SLUG`. +This prints a JSON object with keys: `repo_root`, `default_branch`, +`dream_ns`, `dream_id`, `branch_name`, `parent_branch`, `worktree_dir`, +`worktree_base`, `base_commit`, `run_prefix`, `slug`. -**If `dream-setup.sh` fails or is not found:** Apply the Script Failure +**If `dream-setup.py` fails or is not found:** Apply the Script Failure Recovery rule (read the script source, adapt its logic). Common causes: missing git remote, branch already exists, `/tmp` permissions. @@ -1078,7 +1070,7 @@ are four places they get cleaned up: "Worktree Cleanup" earlier in this skill). Removes ONE worktree. 2. **`dream-reconcile.py --cleanup-branches`** — after deleting a merged branch, also `rm -rf`s its worktree directory. No extra command needed. -3. **`dream-gc.sh` (auto-triggered)** — `dream-setup.sh` invokes this +3. **`dream-gc.sh` (auto-triggered)** — `dream-setup.py` invokes this sweeper at the start of each new dream, throttled by a per-namespace `.last-gc` tombstone to run at most once per `DREAM_GC_INTERVAL_MIN` minutes (default 60). Catches orphans from crashed dreams, machine From 93ffabcca6116d34d2120d2091181287b9579665 Mon Sep 17 00:00:00 2001 From: Walther Maciel Date: Mon, 27 Jul 2026 15:00:43 -0700 Subject: [PATCH 04/14] refactor(dream): remove dream-setup.sh + its bash test The Python dream-setup.py fully replaces the bash entry point, so drop dream-setup.sh and its bash-only test_dream_setup_sh.py. This also retires that module's requires_bash Windows skip. dream-cleanup.sh and dream-gc.sh remain until they are ported to Python. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 35800a4a-a5d9-4e3a-b0e3-0d093e083b24 --- skills/shadow-frog-dream/dream-setup.sh | 351 ----------- .../shadow_frog_dream/test_dream_setup.py | 15 +- .../shadow_frog_dream/test_dream_setup_sh.py | 582 ------------------ 3 files changed, 9 insertions(+), 939 deletions(-) delete mode 100755 skills/shadow-frog-dream/dream-setup.sh delete mode 100644 tests/skills/shadow_frog_dream/test_dream_setup_sh.py diff --git a/skills/shadow-frog-dream/dream-setup.sh b/skills/shadow-frog-dream/dream-setup.sh deleted file mode 100755 index 9e1062b..0000000 --- a/skills/shadow-frog-dream/dream-setup.sh +++ /dev/null @@ -1,351 +0,0 @@ -#!/usr/bin/env bash -# Dream experiment setup — creates worktree and exports environment. -# -# Usage: -# eval "$(dream-setup.sh --slug t01-csv-fuzzer)" || { echo "setup failed" >&2; exit 1; } -# eval "$(dream-setup.sh --slug t03-extend --base-branch dream/ns/20250612-143012Z-csv-fuzzer)" || exit 1 -# -# IMPORTANT: ALWAYS check the exit status of `eval` — on failure this script -# prints to stderr and exits non-zero, which `eval "$(...)"` cannot detect on -# its own. Without `|| exit 1` the agent silently proceeds with empty env vars. -# -# Why `eval` is safe HERE (do not cargo-cult it elsewhere): this script never -# echoes untrusted input back. All inputs (slug/namespace) are validated -# against [A-Za-z0-9_-][A-Za-z0-9._-]* before use, and every exported value is -# shell-escaped with `printf %q`, so the emitted text is a fixed set of safe -# `export` lines. Only `eval` output you produced under these guarantees. -# -# This script: -# 1. Validates inputs (slug/namespace) against [A-Za-z0-9_-][A-Za-z0-9._-]* -# (non-`.` first char rejects bare `.`/`..`) to prevent shell-metachar -# injection through the eval contract -# 2. Computes DREAM_ID, BRANCH_NAME, WORKTREE_DIR, BASE_COMMIT -# 3. Creates the worktree (idempotent — cleans existing if found) -# 4. Prints export statements (values shell-escaped via printf %q) -# -# Flags: -# --slug NAME Task slug (required, e.g., "t01-csv-fuzzer") -# --base-branch REF Branch to base from (default: default branch = fresh) -# --namespace NS Override DREAM_NAMESPACE (default: env or repo basename) -# --repo-root DIR Override repo root (default: git rev-parse) -# --print-env Print export statements (default behavior) -# --print-json Print JSON instead of shell exports -# --dry-run Compute values without creating worktree -# --help, -h Show this help message -# -# Design decisions: -# - Idempotent: re-running with same slug cleans and recreates -# - External path: worktrees always in /tmp/shadowfrog-dreams// -# - Validates worktree is NOT inside project directory -# - Detects default branch (main/master) automatically -# - Resolves DREAM_NAMESPACE from env > TASK_INFO.json > .env > repo name - -set -euo pipefail - -# --- Argument parsing --- -SLUG="" -BASE_BRANCH="" -NAMESPACE_OVERRIDE="" -REPO_ROOT_OVERRIDE="" -OUTPUT_MODE="env" -DRY_RUN=false - -show_help() { - sed -n '2,/^$/p' "$0" | sed 's/^# \{0,1\}//' - exit 0 -} - -while [[ $# -gt 0 ]]; do - case "$1" in - --slug) SLUG="$2"; shift 2 ;; - --base-branch) BASE_BRANCH="$2"; shift 2 ;; - --namespace) NAMESPACE_OVERRIDE="$2"; shift 2 ;; - --repo-root) REPO_ROOT_OVERRIDE="$2"; shift 2 ;; - --print-json) OUTPUT_MODE="json"; shift ;; - --print-env) OUTPUT_MODE="env"; shift ;; - --dry-run) DRY_RUN=true; shift ;; - --help|-h) show_help ;; - *) echo "ERROR: Unknown argument: $1" >&2; exit 1 ;; - esac -done - -if [[ -z "$SLUG" ]]; then - echo "ERROR: --slug is required" >&2 - echo "Usage: dream-setup.sh --slug t01-name [--base-branch BRANCH]" >&2 - exit 1 -fi - -# --- Input validation: prevent shell injection via the `eval` contract --- -# The script's output is fed to `eval`, so any unescaped shell metachars in -# slug/namespace would execute as code. Restrict to filesystem-safe chars, -# AND require a non-`.` first char to reject bare `.`/`..` as a slug or ns. -SAFE_RE='^[A-Za-z0-9_-][A-Za-z0-9._-]*$' -if ! [[ "$SLUG" =~ $SAFE_RE ]]; then - echo "ERROR: --slug must match $SAFE_RE (got: $SLUG)" >&2 - echo " Use kebab-case alphanumerics like 't01-csv-fuzzer'." >&2 - exit 1 -fi -if [[ -n "$NAMESPACE_OVERRIDE" ]] && ! [[ "$NAMESPACE_OVERRIDE" =~ $SAFE_RE ]]; then - echo "ERROR: --namespace must match $SAFE_RE (got: $NAMESPACE_OVERRIDE)" >&2 - exit 1 -fi - -# --- Resolve repo root --- -if [[ -n "$REPO_ROOT_OVERRIDE" ]]; then - REPO_ROOT="$REPO_ROOT_OVERRIDE" -else - REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null) || { - echo "ERROR: Not in a git repository" >&2; exit 1 - } -fi -cd "$REPO_ROOT" - -# --- Guard: .shadow/ must be tracked by git (not gitignored) --- -# The dream workflow moves .shadow/ content through git: artifacts are -# committed onto the dream branch, pushed, then read back by the reconciler -# via `git show origin/ .shadow/...`. If .shadow/ is gitignored, -# `git add -A` silently skips those files, nothing is pushed, and the -# reconciler finds no manifest — every discovery is lost without warning. -# Fail fast with a clear message instead. (shadow-frog-init asks the user -# whether to commit or gitignore .shadow/; the dream skill requires committed.) -# Probe a NEW child path rather than `.shadow` itself: when `.shadow/` is -# gitignored but already tracked, `git check-ignore .shadow` reports "not -# ignored" (tracked content wins), yet `git add -A` still silently drops any -# NEW files created under it (e.g. _dreams//manifest.json) — the exact -# data-loss case this guard exists to prevent. A child path under .shadow/ -# reflects the gitignore rule regardless of tracking state. -if git check-ignore -q .shadow/_dreams/__shadowfrog_probe__/manifest.json 2>/dev/null; then - echo "ERROR: .shadow/ is gitignored — shadow-frog-dream requires it to be tracked by git." >&2 - echo " Dream experiments commit .shadow/ artifacts onto a branch, push them, and" >&2 - echo " reconcile reads them back from the remote. A gitignored .shadow/ would be" >&2 - echo " silently dropped at commit time, losing every discovery." >&2 - echo " Fix: remove the '.shadow/' entry from .gitignore and commit .shadow/," >&2 - echo " or run shadow-frog-update (which works in local-only mode) instead of dream." >&2 - exit 1 -fi - -# --- Detect default branch --- -# `git symbolic-ref` exits non-zero when origin/HEAD is unset (git < 2.48 does -# not auto-create it on fetch). Guard with `|| true` so `set -euo pipefail` -# does not abort here — an empty result is expected and handled by the -# origin/main / origin/master fallback below. -DEFAULT_BRANCH=$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null \ - | sed 's|refs/remotes/origin/||') || true -if [[ -z "$DEFAULT_BRANCH" ]]; then - if git show-ref --verify refs/remotes/origin/main >/dev/null 2>&1; then - DEFAULT_BRANCH="main" - elif git show-ref --verify refs/remotes/origin/master >/dev/null 2>&1; then - DEFAULT_BRANCH="master" - else - echo "ERROR: Cannot detect default branch. Fix: git remote set-head origin " >&2 - exit 1 - fi -fi - -# --- Resolve namespace --- -if [[ -n "$NAMESPACE_OVERRIDE" ]]; then - DREAM_NS="$NAMESPACE_OVERRIDE" -elif [[ -n "${DREAM_NAMESPACE:-}" ]]; then - DREAM_NS="$DREAM_NAMESPACE" -elif [[ -f TASK_INFO.json ]]; then - DREAM_NS=$(python3 -c "import json; print(json.load(open('TASK_INFO.json')).get('dream_namespace',''))" 2>/dev/null || echo "") -elif [[ -f .env ]]; then - DREAM_NS=$(grep '^DREAM_NAMESPACE=' .env 2>/dev/null | head -1 | cut -d'=' -f2- | sed -E 's/^[[:space:]]*["'\'']?//; s/["'\'']?[[:space:]]*$//' || echo "") -fi -DREAM_NS="${DREAM_NS:-$(basename "$REPO_ROOT")}" - -# Validate resolved namespace too (could come from TASK_INFO/.env/basename) -if ! [[ "$DREAM_NS" =~ $SAFE_RE ]]; then - echo "ERROR: Resolved DREAM_NS contains unsafe characters: $DREAM_NS" >&2 - echo " Allowed: $SAFE_RE" >&2 - echo " Override with --namespace or set DREAM_NAMESPACE." >&2 - exit 1 -fi - -# --- Compute identifiers --- -DREAM_ID="$(date -u +%Y%m%d-%H%M%SZ)-${SLUG}" -BRANCH_NAME="dream/${DREAM_NS}/${DREAM_ID}" - -# --- Compute worktree path (ALWAYS in /tmp, NEVER in project) --- -WORKTREE_BASE="${DREAM_WORKTREE_BASE:-/tmp/shadowfrog-dreams}/${DREAM_NS}" -WORKTREE_DIR="${WORKTREE_BASE}/dream-${SLUG}" - -# Validate worktree is external to project -case "$WORKTREE_DIR" in - "$REPO_ROOT"|"$REPO_ROOT"/*) - echo "ERROR: Worktree would be inside project: $WORKTREE_DIR" >&2 - echo "MUST use external path (default: /tmp/shadowfrog-dreams/)" >&2 - exit 1 - ;; -esac - -# --- Resolve base reference --- -if [[ -z "$BASE_BRANCH" ]]; then - BASE_REF="origin/$DEFAULT_BRANCH" - PARENT_BRANCH="$DEFAULT_BRANCH" -else - BASE_REF="origin/$BASE_BRANCH" - PARENT_BRANCH="$BASE_BRANCH" - # Verify the base branch exists on remote - if ! git show-ref --verify "refs/remotes/$BASE_REF" >/dev/null 2>&1; then - echo "ERROR: Base branch not found: $BASE_REF" >&2 - exit 1 - fi -fi - -# --- Create worktree (unless dry-run) --- -BASE_COMMIT="" -if [[ "$DRY_RUN" == "false" ]]; then - mkdir -p "$WORKTREE_BASE" - - # --- Periodic auto-GC (Bug A fix from bug-cleanup-gaps.md) --- - # `dream-gc.sh` is documented as "run periodically" but had no caller in - # the skill flow, so long-running fleets accumulated orphans forever - # (machine reboots, OOM-killed agents, `$REPO_ROOT` unset, races, …). - # Trigger it from here, throttled by a per-namespace tombstone file so - # the cost amortizes across many dreams. ALL output is redirected to - # stderr or /dev/null to preserve the `eval "$(...)"` contract. - if [[ "${DREAM_GC_AUTO:-1}" != "0" ]]; then - SCRIPT_DIR_SH="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" - GC_SCRIPT="$SCRIPT_DIR_SH/dream-gc.sh" - TOMBSTONE="$WORKTREE_BASE/.last-gc" - GC_INTERVAL_MIN="${DREAM_GC_INTERVAL_MIN:-60}" - GC_AGE_MIN="${DREAM_GC_AGE_MIN:-60}" - - # Validate the env-supplied integers so a hostile value can't reach - # `find -mmin` or `dream-gc.sh --min-age-min` as injected args. - # (dream-gc.sh itself also re-validates, but defense in depth.) - SAFE_INT='^[0-9]+$' - if ! [[ "$GC_INTERVAL_MIN" =~ $SAFE_INT ]] || ! [[ "$GC_AGE_MIN" =~ $SAFE_INT ]]; then - echo "WARN: DREAM_GC_INTERVAL_MIN / DREAM_GC_AGE_MIN must be non-negative integers — skipping auto-GC" >&2 - elif [[ -x "$GC_SCRIPT" ]] || [[ -f "$GC_SCRIPT" ]]; then - should_run=false - if [[ ! -f "$TOMBSTONE" ]]; then - should_run=true - elif [[ "$GC_INTERVAL_MIN" -eq 0 ]]; then - # Interval 0 ⇒ "always run". `-mmin +0` would NOT match a - # tombstone touched in the last ~1 min, so we'd silently - # skip-throttle the very first invocation after touch. Bypass - # the find entirely. - should_run=true - elif [[ -n "$(find "$TOMBSTONE" -mmin "+$GC_INTERVAL_MIN" 2>/dev/null)" ]]; then - should_run=true - fi - if [[ "$should_run" == true ]]; then - # Touch BEFORE running, so a parallel `dream-setup.sh` for - # the same ns sees a fresh tombstone and skips. Worst case - # of a tombstone-vs-gc race is one extra GC pass; never a - # missed cleanup. - touch "$TOMBSTONE" 2>/dev/null || true - bash "$GC_SCRIPT" \ - --repo-root "$REPO_ROOT" \ - --quiet \ - --min-age-min "$GC_AGE_MIN" \ - >&2 || true - fi - fi - fi - - # Clean existing worktree (idempotent). Try git first, then fall back - # to a safety-gated rm -rf for stale worktrees git can't see. Matches - # the dream-cleanup.sh path so a leak healed during a retry instead of - # cascading into "branch already exists" errors at git worktree add. - if [[ -d "$WORKTREE_DIR" ]]; then - SAFETY_SH="$(dirname "${BASH_SOURCE[0]}")/_worktree_safety.py" - if ! git worktree remove "$WORKTREE_DIR" --force 2>/dev/null; then - # Safety gate: only rm if path matches `//dream-`. - # If the safety module is missing, REFUSE — never fall through - # to an un-gated rm just because the gate is unloadable. - if [[ ! -f "$SAFETY_SH" ]]; then - echo "ERROR: safety module not found, refusing pre-clean rm: $SAFETY_SH" >&2 - exit 1 - fi - if python3 "$SAFETY_SH" "$WORKTREE_DIR" "${DREAM_WORKTREE_BASE:-/tmp/shadowfrog-dreams}" >/dev/null 2>&1; then - rm -rf -- "$WORKTREE_DIR" - fi - fi - git worktree prune - fi - - # Create worktree with new branch. - # IMPORTANT: redirect BOTH stdout and stderr — modern git prints - # "branch '...' set up to track..." and "HEAD is now at..." to stdout, - # which would corrupt the `eval "$(...)"` contract used by callers. - git worktree add "$WORKTREE_DIR" -b "$BRANCH_NAME" "$BASE_REF" >/dev/null 2>&1 || { - git branch -D "$BRANCH_NAME" >/dev/null 2>&1 || true - git worktree prune >/dev/null 2>&1 - git worktree add "$WORKTREE_DIR" -b "$BRANCH_NAME" "$BASE_REF" >/dev/null 2>&1 || { - echo "ERROR: git worktree add failed for $WORKTREE_DIR (branch $BRANCH_NAME, base $BASE_REF)" >&2 - exit 1 - } - } - - BASE_COMMIT=$(git -C "$WORKTREE_DIR" rev-parse HEAD) -else - BASE_COMMIT=$(git rev-parse "$BASE_REF" 2>/dev/null || echo "DRY_RUN") -fi - -# --- Detect RUN_PREFIX --- -RUN_PREFIX="" -if [[ -f uv.lock ]]; then - RUN_PREFIX="uv run" -elif [[ -f package-lock.json ]]; then - RUN_PREFIX="npx" -elif [[ -f yarn.lock ]]; then - RUN_PREFIX="npx" -fi - -# --- Output (values shell-escaped via printf %q for safe `eval`) --- -emit_export() { - # printf %q produces a string that bash/zsh/sh can parse back losslessly. - printf 'export %s=%q\n' "$1" "$2" -} - -if [[ "$OUTPUT_MODE" == "json" ]]; then - # JSON output — use python for safe escaping (handles quotes, backslashes, - # control chars). Values are passed via the environment (NOT interpolated - # into the Python source) so a repo path containing ", \, or $ can't break - # the script. Falls back with a clear error if python is missing. - SF_REPO_ROOT="$REPO_ROOT" \ - SF_DEFAULT_BRANCH="$DEFAULT_BRANCH" \ - SF_DREAM_NS="$DREAM_NS" \ - SF_DREAM_ID="$DREAM_ID" \ - SF_BRANCH_NAME="$BRANCH_NAME" \ - SF_PARENT_BRANCH="$PARENT_BRANCH" \ - SF_WORKTREE_DIR="$WORKTREE_DIR" \ - SF_WORKTREE_BASE="$WORKTREE_BASE" \ - SF_BASE_COMMIT="$BASE_COMMIT" \ - SF_RUN_PREFIX="$RUN_PREFIX" \ - SF_SLUG="$SLUG" \ - python3 - <<'PYEOF' || { -import json, os -print(json.dumps({ - "repo_root": os.environ["SF_REPO_ROOT"], - "default_branch": os.environ["SF_DEFAULT_BRANCH"], - "dream_ns": os.environ["SF_DREAM_NS"], - "dream_id": os.environ["SF_DREAM_ID"], - "branch_name": os.environ["SF_BRANCH_NAME"], - "parent_branch": os.environ["SF_PARENT_BRANCH"], - "worktree_dir": os.environ["SF_WORKTREE_DIR"], - "worktree_base": os.environ["SF_WORKTREE_BASE"], - "base_commit": os.environ["SF_BASE_COMMIT"], - "run_prefix": os.environ["SF_RUN_PREFIX"], - "slug": os.environ["SF_SLUG"], -}, indent=2)) -PYEOF - echo "ERROR: python3 required for --print-json" >&2 - exit 1 - } -else - emit_export REPO_ROOT "$REPO_ROOT" - emit_export DEFAULT_BRANCH "$DEFAULT_BRANCH" - emit_export DREAM_NS "$DREAM_NS" - emit_export DREAM_ID "$DREAM_ID" - emit_export BRANCH_NAME "$BRANCH_NAME" - emit_export PARENT_BRANCH "$PARENT_BRANCH" - emit_export WORKTREE_DIR "$WORKTREE_DIR" - emit_export WORKTREE_BASE "$WORKTREE_BASE" - emit_export BASE_COMMIT "$BASE_COMMIT" - emit_export RUN_PREFIX "$RUN_PREFIX" - emit_export SLUG "$SLUG" -fi diff --git a/tests/skills/shadow_frog_dream/test_dream_setup.py b/tests/skills/shadow_frog_dream/test_dream_setup.py index 8d589b6..3422c59 100644 --- a/tests/skills/shadow_frog_dream/test_dream_setup.py +++ b/tests/skills/shadow_frog_dream/test_dream_setup.py @@ -6,7 +6,7 @@ Cross-platform: invokes the Python entry point directly (no bash). The two auto-GC tests that assert an orphan is actually *swept* still need the bash -`dream-gc.sh`, so they live in a `requires_bash` class skipped on Windows; +`dream-gc.sh`, so they are skipped on Windows until `dream-gc.py` exists; every other test runs on all OSes. """ import json @@ -390,8 +390,8 @@ def test_dry_run_no_worktree_created(self, tmp_path): # # These four exercise dream-setup's OWN throttle/opt-out logic, which never # invokes the GC sweeper — so they run on every OS. The two tests that assert -# an orphan is actually swept need bash `dream-gc.sh` and live in the -# `requires_bash` class below (skipped on Windows). +# an orphan is actually swept need bash `dream-gc.sh` and are skipped on +# Windows until a cross-platform `dream-gc.py` exists. # =========================================================================== @pytest.mark.slow @@ -504,11 +504,14 @@ def test_auto_gc_does_not_pollute_json_stdout(self, tmp_path): @pytest.mark.slow @pytest.mark.integration -@pytest.mark.requires_bash +@pytest.mark.skipif( + os.name == "nt", + reason="dream-gc.sh sweep uses POSIX path/realpath semantics", +) class TestDreamSetupAutoGCSweep: """These assert the orphan is actually *removed*, which needs the bash - `dream-gc.sh` sweeper. Skipped on Windows by the requires_bash hook; the - mark is dropped once a cross-platform `dream-gc.py` exists.""" + `dream-gc.sh` sweeper. Skipped on Windows until a cross-platform + `dream-gc.py` exists.""" def test_auto_gc_runs_when_no_tombstone(self, tmp_path): """First invocation sweeps orphans (no tombstone yet).""" diff --git a/tests/skills/shadow_frog_dream/test_dream_setup_sh.py b/tests/skills/shadow_frog_dream/test_dream_setup_sh.py deleted file mode 100644 index c7cd4db..0000000 --- a/tests/skills/shadow_frog_dream/test_dream_setup_sh.py +++ /dev/null @@ -1,582 +0,0 @@ -"""Tests for skills/shadow-frog-dream/dream-setup.sh — Dream worktree setup. - -Exercises: --help, happy path worktree+branch creation, RUN_PREFIX detection, -namespace override, slug validation, and dry-run mode. -""" -import json -import os -import subprocess -from pathlib import Path - -import pytest - -from tests._shell import BASH, HAVE_BASH, shell_path - -# POSIX-shell integration tests: these shell out to a POSIX `bash`. On Windows -# that is Git Bash (resolved via BASH — never the System32 WSL launcher stub). -# Skip only when no POSIX shell is available at all. -pytestmark = pytest.mark.skipif( - not HAVE_BASH, - reason="no POSIX bash (Git Bash) available for shell integration tests", -) - -REPO_ROOT = Path(__file__).resolve().parent.parent.parent.parent -DREAM_SETUP = REPO_ROOT / "skills" / "shadow-frog-dream" / "dream-setup.sh" - -# dream-gc.sh sweeps orphan worktrees using POSIX absolute-path/realpath -# semantics (it detects an orphan gitdir via the `/*` glob and resolves paths -# with `realpath`). On the Windows CI runner the repo and the temp worktree -# base live on different drives (D: vs C:), so the cross-drive sweep is a -# no-op and the orphan survives. Dream-mode Windows support is out of scope; -# skip only the two tests that assert an actual sweep occurred. -_skip_win_gc_sweep = pytest.mark.skipif( - os.name == "nt", - reason="dream-gc worktree sweep relies on POSIX path/realpath semantics", -) - - -def _base_env(cwd: Path, extras: dict | None = None) -> dict: - env = { - "PATH": shell_path(), - "HOME": str(cwd), - "GIT_CONFIG_GLOBAL": "/dev/null", - "GIT_CONFIG_SYSTEM": "/dev/null", - "LANG": "en_US.UTF-8", - } - if extras: - env.update(extras) - return env - - -def _make_git_repo(path: Path, branch: str = "main") -> None: - """Create a git repo with an initial commit and origin/main ref.""" - env = _base_env(path) - subprocess.run(["git", "init", "-q", "-b", branch], cwd=path, check=True, env=env) - subprocess.run(["git", "config", "user.email", "test@test.invalid"], cwd=path, check=True, env=env) - subprocess.run(["git", "config", "user.name", "Test"], cwd=path, check=True, env=env) - subprocess.run(["git", "config", "commit.gpgsign", "false"], cwd=path, check=True, env=env) - (path / "README.md").write_text("# test\n") - subprocess.run(["git", "add", "-A"], cwd=path, check=True, env=env) - subprocess.run(["git", "commit", "-q", "-m", "init"], cwd=path, check=True, env=env) - # Create a fake origin remote pointing to self for origin/main ref - subprocess.run(["git", "remote", "add", "origin", str(path)], cwd=path, check=True, env=env) - subprocess.run(["git", "fetch", "-q", "origin"], cwd=path, check=True, env=env) - - -def run_dream_setup( - args: list[str], cwd: Path, env_extra: dict | None = None -) -> subprocess.CompletedProcess: - """Run dream-setup.sh with given args.""" - env = _base_env(cwd, env_extra) - return subprocess.run( - [BASH, str(DREAM_SETUP), *args], - capture_output=True, - text=True, - cwd=cwd, - env=env, - ) - - -@pytest.mark.slow -@pytest.mark.integration -class TestDreamSetupHelp: - def test_help_exits_zero(self, tmp_path): - # --help should work even outside a git repo (it just prints and exits) - result = run_dream_setup(["--help"], cwd=tmp_path) - assert result.returncode == 0 - assert "slug" in result.stdout.lower() or "slug" in result.stderr.lower() or "Usage" in result.stdout - - -@pytest.mark.slow -@pytest.mark.integration -class TestDreamSetupHappyPath: - """Creates worktree and branch correctly.""" - - def test_creates_worktree_and_branch(self, tmp_path): - repo = tmp_path / "repo" - repo.mkdir() - _make_git_repo(repo) - worktree_base = tmp_path / "worktrees" - - result = run_dream_setup( - ["--slug", "t01-test", "--repo-root", str(repo), "--print-json"], - cwd=repo, - env_extra={"DREAM_WORKTREE_BASE": str(worktree_base)}, - ) - assert result.returncode == 0, f"stderr: {result.stderr}" - data = json.loads(result.stdout) - - assert "dream_ns" in data - assert "branch_name" in data - assert "worktree_dir" in data - assert data["slug"] == "t01-test" - assert "dream/" in data["branch_name"] - assert "t01-test" in data["dream_id"] - - # Verify worktree exists - wt_dir = Path(data["worktree_dir"]) - assert wt_dir.is_dir() - - # Verify branch exists in repo - env = _base_env(repo) - branches = subprocess.run( - ["git", "branch", "--list", data["branch_name"]], - cwd=repo, capture_output=True, text=True, env=env, - ) - # Branch may be in worktree, check via worktree list - wt_list = subprocess.run( - ["git", "worktree", "list"], cwd=repo, - capture_output=True, text=True, env=env, - ) - # `git worktree list` always prints POSIX-style separators; normalize - # so the comparison holds on Windows too. - assert wt_dir.as_posix() in wt_list.stdout.replace("\\", "/") - - def test_worktree_has_same_head_as_base(self, tmp_path): - repo = tmp_path / "repo" - repo.mkdir() - _make_git_repo(repo) - worktree_base = tmp_path / "worktrees" - env = _base_env(repo) - - # Get main HEAD - main_head = subprocess.run( - ["git", "rev-parse", "HEAD"], cwd=repo, - capture_output=True, text=True, check=True, env=env, - ).stdout.strip() - - result = run_dream_setup( - ["--slug", "t02-head", "--repo-root", str(repo), "--print-json"], - cwd=repo, - env_extra={"DREAM_WORKTREE_BASE": str(worktree_base)}, - ) - assert result.returncode == 0, f"stderr: {result.stderr}" - data = json.loads(result.stdout) - assert data["base_commit"] == main_head - - -@pytest.mark.slow -@pytest.mark.integration -class TestDreamSetupRunPrefix: - """RUN_PREFIX detection based on lock files.""" - - def test_no_lock_files_empty_prefix(self, tmp_path): - repo = tmp_path / "repo" - repo.mkdir() - _make_git_repo(repo) - worktree_base = tmp_path / "wt" - - result = run_dream_setup( - ["--slug", "t03-nolock", "--repo-root", str(repo), "--print-json"], - cwd=repo, - env_extra={"DREAM_WORKTREE_BASE": str(worktree_base)}, - ) - assert result.returncode == 0, f"stderr: {result.stderr}" - data = json.loads(result.stdout) - assert data["run_prefix"] == "" - - def test_uv_lock_gives_uv_run_prefix(self, tmp_path): - repo = tmp_path / "repo" - repo.mkdir() - _make_git_repo(repo) - # Add uv.lock - (repo / "uv.lock").write_text("") - worktree_base = tmp_path / "wt" - - result = run_dream_setup( - ["--slug", "t04-uvlock", "--repo-root", str(repo), "--print-json"], - cwd=repo, - env_extra={"DREAM_WORKTREE_BASE": str(worktree_base)}, - ) - assert result.returncode == 0, f"stderr: {result.stderr}" - data = json.loads(result.stdout) - assert data["run_prefix"] == "uv run" - - def test_package_lock_gives_npx_prefix(self, tmp_path): - repo = tmp_path / "repo" - repo.mkdir() - _make_git_repo(repo) - (repo / "package-lock.json").write_text("{}") - worktree_base = tmp_path / "wt" - - result = run_dream_setup( - ["--slug", "t05-npm", "--repo-root", str(repo), "--print-json"], - cwd=repo, - env_extra={"DREAM_WORKTREE_BASE": str(worktree_base)}, - ) - assert result.returncode == 0, f"stderr: {result.stderr}" - data = json.loads(result.stdout) - assert data["run_prefix"] == "npx" - - -@pytest.mark.slow -@pytest.mark.integration -class TestDreamSetupIdempotent: - """Re-running with same slug cleans and recreates (idempotent).""" - - def test_same_slug_twice_succeeds(self, tmp_path): - repo = tmp_path / "repo" - repo.mkdir() - _make_git_repo(repo) - worktree_base = tmp_path / "wt" - - args = ["--slug", "t06-idem", "--repo-root", str(repo), "--print-json"] - extras = {"DREAM_WORKTREE_BASE": str(worktree_base)} - - r1 = run_dream_setup(args, cwd=repo, env_extra=extras) - assert r1.returncode == 0, f"stderr: {r1.stderr}" - - r2 = run_dream_setup(args, cwd=repo, env_extra=extras) - assert r2.returncode == 0, f"stderr: {r2.stderr}" - # Both should produce valid JSON with same worktree dir - d1 = json.loads(r1.stdout) - d2 = json.loads(r2.stdout) - assert d1["worktree_dir"] == d2["worktree_dir"] - - -@pytest.mark.slow -@pytest.mark.integration -class TestDreamSetupNamespace: - """DREAM_NAMESPACE / --namespace honored in branch name.""" - - def test_namespace_override_in_branch(self, tmp_path): - repo = tmp_path / "repo" - repo.mkdir() - _make_git_repo(repo) - worktree_base = tmp_path / "wt" - - result = run_dream_setup( - ["--slug", "t07-ns", "--namespace", "my-custom-ns", - "--repo-root", str(repo), "--print-json"], - cwd=repo, - env_extra={"DREAM_WORKTREE_BASE": str(worktree_base)}, - ) - assert result.returncode == 0, f"stderr: {result.stderr}" - data = json.loads(result.stdout) - assert data["dream_ns"] == "my-custom-ns" - assert "dream/my-custom-ns/" in data["branch_name"] - - def test_env_namespace_used(self, tmp_path): - repo = tmp_path / "repo" - repo.mkdir() - _make_git_repo(repo) - worktree_base = tmp_path / "wt" - - result = run_dream_setup( - ["--slug", "t08-envns", "--repo-root", str(repo), "--print-json"], - cwd=repo, - env_extra={ - "DREAM_WORKTREE_BASE": str(worktree_base), - "DREAM_NAMESPACE": "env-ns-test", - }, - ) - assert result.returncode == 0, f"stderr: {result.stderr}" - data = json.loads(result.stdout) - assert data["dream_ns"] == "env-ns-test" - - -@pytest.mark.slow -@pytest.mark.integration -class TestDreamSetupValidation: - """Input validation prevents bad slugs.""" - - def test_missing_slug_fails(self, tmp_path): - result = run_dream_setup([], cwd=tmp_path) - assert result.returncode != 0 - assert "slug" in result.stderr.lower() - - def test_invalid_slug_rejected(self, tmp_path): - repo = tmp_path / "repo" - repo.mkdir() - _make_git_repo(repo) - result = run_dream_setup( - ["--slug", "bad slug!!", "--repo-root", str(repo)], - cwd=repo, - ) - assert result.returncode != 0 - assert "must match" in result.stderr - - -@pytest.mark.slow -@pytest.mark.integration -class TestDreamSetupGitignoreGuard: - """dream requires .shadow/ to be git-tracked, not gitignored.""" - - def test_refuses_when_shadow_gitignored(self, tmp_path): - repo = tmp_path / "repo" - repo.mkdir() - _make_git_repo(repo) - (repo / ".gitignore").write_text(".shadow/\n") - (repo / ".shadow").mkdir() - result = run_dream_setup( - ["--slug", "t10-ignored", "--repo-root", str(repo), - "--dry-run", "--print-json"], - cwd=repo, - ) - assert result.returncode != 0 - assert ".shadow/ is gitignored" in result.stderr - - def test_proceeds_when_shadow_tracked(self, tmp_path): - repo = tmp_path / "repo" - repo.mkdir() - _make_git_repo(repo) - # .gitignore present but does NOT ignore .shadow/ - (repo / ".gitignore").write_text("build/\n__pycache__/\n") - (repo / ".shadow").mkdir() - worktree_base = tmp_path / "wt" - result = run_dream_setup( - ["--slug", "t10-tracked", "--repo-root", str(repo), - "--dry-run", "--print-json"], - cwd=repo, - env_extra={"DREAM_WORKTREE_BASE": str(worktree_base)}, - ) - assert result.returncode == 0, f"stderr: {result.stderr}" - assert "gitignored" not in result.stderr - - def test_refuses_when_shadow_gitignored_but_already_tracked(self, tmp_path): - """Edge case: .shadow/ is gitignored AND has previously-committed - content. `git check-ignore .shadow` reports not-ignored (tracked wins), - but `git add -A` still drops NEW children — so the guard must probe a - child path and still refuse.""" - repo = tmp_path / "repo" - repo.mkdir() - _make_git_repo(repo) - # Commit some .shadow content FIRST, then add the gitignore rule. - shadow_meta = repo / ".shadow" / "_meta" - shadow_meta.mkdir(parents=True) - (shadow_meta / "state.json").write_text("{}\n") - env = _base_env(repo) - subprocess.run(["git", "add", "-A"], cwd=repo, check=True, env=env) - subprocess.run(["git", "commit", "-qm", "track shadow"], - cwd=repo, check=True, env=env) - (repo / ".gitignore").write_text(".shadow/\n") - subprocess.run(["git", "add", ".gitignore"], cwd=repo, check=True, env=env) - subprocess.run(["git", "commit", "-qm", "ignore shadow"], - cwd=repo, check=True, env=env) - - result = run_dream_setup( - ["--slug", "t10-tracked-ignored", "--repo-root", str(repo), - "--dry-run", "--print-json"], - cwd=repo, - ) - assert result.returncode != 0 - assert ".shadow/ is gitignored" in result.stderr - - -@pytest.mark.slow -@pytest.mark.integration -class TestDreamSetupDryRun: - """--dry-run computes values without creating worktree.""" - - def test_dry_run_no_worktree_created(self, tmp_path): - repo = tmp_path / "repo" - repo.mkdir() - _make_git_repo(repo) - worktree_base = tmp_path / "wt" - - result = run_dream_setup( - ["--slug", "t09-dry", "--repo-root", str(repo), - "--dry-run", "--print-json"], - cwd=repo, - env_extra={"DREAM_WORKTREE_BASE": str(worktree_base)}, - ) - assert result.returncode == 0, f"stderr: {result.stderr}" - data = json.loads(result.stdout) - # Worktree dir should NOT exist - assert not Path(data["worktree_dir"]).exists() - assert data["slug"] == "t09-dry" - - -# =========================================================================== -# Auto-GC throttle (Bug A fix from bug-cleanup-gaps.md) -# =========================================================================== - -@pytest.mark.slow -@pytest.mark.integration -class TestDreamSetupAutoGC: - """`dream-setup.sh` triggers `dream-gc.sh` periodically. - - Bug A from bug-cleanup-gaps.md: dream-gc.sh existed but had no caller - in the skill flow, so long-running fleets accumulated orphans - indefinitely. dream-setup.sh now invokes it at the start of each new - dream, throttled to once per DREAM_GC_INTERVAL_MIN (default 60). - """ - - def _orphan(self, base: Path, ns: str, name: str = "dream-orphan") -> Path: - """Plant an orphan worktree under the namespace dir.""" - d = base / ns / name - d.mkdir(parents=True) - (d / ".git").write_text("gitdir: /nonexistent/path\n") - (d / "leftover.txt").write_text("orphaned\n") - ancient = 946684800 - os.utime(d, (ancient, ancient)) - return d - - @_skip_win_gc_sweep - def test_auto_gc_runs_when_no_tombstone(self, tmp_path): - """First invocation sweeps orphans (no tombstone yet).""" - repo = tmp_path / "repo" - repo.mkdir() - _make_git_repo(repo) - worktree_base = tmp_path / "worktrees" - ns = repo.name - - # Plant an orphan that the auto-GC should clean up. - orphan = self._orphan(worktree_base, ns) - assert orphan.exists() - - result = run_dream_setup( - ["--slug", "t01-gc", "--repo-root", str(repo), "--print-json"], - cwd=repo, - env_extra={ - "DREAM_WORKTREE_BASE": str(worktree_base), - # Force min-age-min=0 so the ancient orphan is in the find window - "DREAM_GC_AGE_MIN": "0", - }, - ) - assert result.returncode == 0, f"stderr: {result.stderr}" - # Orphan must be gone — auto-GC ran. - assert not orphan.exists(), ( - f"Auto-GC should have swept the orphan\nstderr: {result.stderr}" - ) - # Tombstone created. - tombstone = worktree_base / ns / ".last-gc" - assert tombstone.exists() - - def test_auto_gc_throttled_by_recent_tombstone(self, tmp_path): - """Fresh tombstone (< DREAM_GC_INTERVAL_MIN) suppresses the trigger.""" - repo = tmp_path / "repo" - repo.mkdir() - _make_git_repo(repo) - worktree_base = tmp_path / "worktrees" - ns = repo.name - - # Pre-create the tombstone with current mtime (fresh). - (worktree_base / ns).mkdir(parents=True) - tombstone = worktree_base / ns / ".last-gc" - tombstone.touch() - - orphan = self._orphan(worktree_base, ns) - - result = run_dream_setup( - ["--slug", "t02-throttle", "--repo-root", str(repo), "--print-json"], - cwd=repo, - env_extra={ - "DREAM_WORKTREE_BASE": str(worktree_base), - "DREAM_GC_INTERVAL_MIN": "60", # tombstone is fresh, won't trigger - "DREAM_GC_AGE_MIN": "0", - }, - ) - assert result.returncode == 0, f"stderr: {result.stderr}" - # Orphan must STILL exist — auto-GC was throttled. - assert orphan.exists(), ( - f"Fresh tombstone should suppress auto-GC\nstderr: {result.stderr}" - ) - - def test_auto_gc_disabled_via_env(self, tmp_path): - """`DREAM_GC_AUTO=0` opts out of the auto-trigger entirely.""" - repo = tmp_path / "repo" - repo.mkdir() - _make_git_repo(repo) - worktree_base = tmp_path / "worktrees" - ns = repo.name - - orphan = self._orphan(worktree_base, ns) - - result = run_dream_setup( - ["--slug", "t03-disabled", "--repo-root", str(repo), "--print-json"], - cwd=repo, - env_extra={ - "DREAM_WORKTREE_BASE": str(worktree_base), - "DREAM_GC_AUTO": "0", - "DREAM_GC_AGE_MIN": "0", - }, - ) - assert result.returncode == 0, f"stderr: {result.stderr}" - # Orphan must STILL exist — GC was opt'd out. - assert orphan.exists(), ( - f"DREAM_GC_AUTO=0 should disable auto-GC\nstderr: {result.stderr}" - ) - # Tombstone NOT created. - assert not (worktree_base / ns / ".last-gc").exists() - - def test_auto_gc_invalid_env_warns_and_continues(self, tmp_path): - """Non-integer interval/age must not break dream setup.""" - repo = tmp_path / "repo" - repo.mkdir() - _make_git_repo(repo) - worktree_base = tmp_path / "worktrees" - - result = run_dream_setup( - ["--slug", "t04-badenv", "--repo-root", str(repo), "--print-json"], - cwd=repo, - env_extra={ - "DREAM_WORKTREE_BASE": str(worktree_base), - "DREAM_GC_INTERVAL_MIN": "not-a-number", - }, - ) - # Dream setup must still succeed — auto-GC is best-effort. - assert result.returncode == 0, f"stderr: {result.stderr}" - data = json.loads(result.stdout) - # Worktree was still created. - assert Path(data["worktree_dir"]).is_dir() - - def test_auto_gc_does_not_pollute_eval_stdout(self, tmp_path): - """Auto-GC output MUST go to stderr to preserve the `eval` contract.""" - repo = tmp_path / "repo" - repo.mkdir() - _make_git_repo(repo) - worktree_base = tmp_path / "worktrees" - ns = repo.name - - # Plant an orphan so the GC actually has work to log about. - self._orphan(worktree_base, ns) - - # Use --print-env (NOT --print-json) — this is the eval-consumed path. - result = run_dream_setup( - ["--slug", "t05-stdout", "--repo-root", str(repo)], # default = --print-env - cwd=repo, - env_extra={ - "DREAM_WORKTREE_BASE": str(worktree_base), - "DREAM_GC_AGE_MIN": "0", - }, - ) - assert result.returncode == 0, f"stderr: {result.stderr}" - # Stdout must contain ONLY export lines — nothing from the GC. - for line in result.stdout.splitlines(): - stripped = line.strip() - if not stripped: - continue - assert stripped.startswith("export "), ( - f"Non-export line on stdout would break `eval`: {line!r}\n" - f"Full stdout:\n{result.stdout}" - ) - - @_skip_win_gc_sweep - def test_auto_gc_sweeps_other_namespace_orphans_too(self, tmp_path): - """The auto-trigger sweeps the whole base, not just its own ns. - - That's deliberate — leaks in any ns count, and the find walk is cheap. - """ - repo = tmp_path / "repo" - repo.mkdir() - _make_git_repo(repo) - worktree_base = tmp_path / "worktrees" - - # Orphan under a DIFFERENT namespace - other_orphan = self._orphan(worktree_base, ns="other-repo") - assert other_orphan.exists() - - result = run_dream_setup( - ["--slug", "t06-cross", "--repo-root", str(repo), "--print-json"], - cwd=repo, - env_extra={ - "DREAM_WORKTREE_BASE": str(worktree_base), - "DREAM_GC_AGE_MIN": "0", - }, - ) - assert result.returncode == 0, f"stderr: {result.stderr}" - # The cross-namespace orphan was swept too. - assert not other_orphan.exists(), ( - f"Auto-GC sweeps the whole base\nstderr: {result.stderr}" - ) From 7eb1ee183ad691ee4543dd15c1345a26f4791bd0 Mon Sep 17 00:00:00 2001 From: Walther Maciel Date: Tue, 15 Sep 2026 12:44:30 -0700 Subject: [PATCH 05/14] fix(dream): harden setup Python port contract Route documented setup through a Python interpreter, document the JSON-to-variable mapping, canonicalize repo roots before isolation checks, and avoid throttling auto-GC when the sweeper cannot launch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 2 +- claude.md | 4 +- skills/shadow-frog-dream/SKILL.md | 102 ++++++++++++++---- skills/shadow-frog-dream/_worktree_safety.py | 8 +- skills/shadow-frog-dream/dream-gc.sh | 2 +- skills/shadow-frog-dream/dream-reconcile.py | 4 +- skills/shadow-frog-dream/dream-setup.py | 55 ++++++---- skills/shadow-frog-dream/dream-validate.py | 2 +- skills/shadow-frog-init/SKILL.md | 2 +- .../shadow_frog_dream/test_dream_gc_sh.py | 2 +- .../shadow_frog_dream/test_dream_reconcile.py | 4 +- .../shadow_frog_dream/test_dream_setup.py | 63 +++++++++++ 12 files changed, 192 insertions(+), 58 deletions(-) diff --git a/README.md b/README.md index fa26f59..e26976e 100644 --- a/README.md +++ b/README.md @@ -320,7 +320,7 @@ the repo. > git: experiment branches carry `.shadow/_dreams/` reports, manifests, and > diffs, then reconciliation commits the accumulated `.shadow/` updates back > to the default branch. If you chose "local only" (gitignored `.shadow/`) -> during init, dream is disabled. `dream-setup.sh` will tell you. The other +> during init, dream is disabled. `dream-setup.py` will tell you. The other > skills (update, meditate, viewer) work either way. #### Remote requirements diff --git a/claude.md b/claude.md index 0823e98..816c754 100644 --- a/claude.md +++ b/claude.md @@ -16,7 +16,7 @@ ShadowFrog/ shadow-frog-update/SKILL.md Incremental update (after changes) shadow-frog-dream/ Autonomous exploration + experimentation (AFK mode) SKILL.md Dream instructions + pipeline phases - dream-setup.sh Worktree + branch creation + dream-setup.py Worktree + branch creation dream-validate.py Pre-push artifact validation dream-reconcile.py Merge dream branches into main's shadow dream-coverage.py Exploration coverage map @@ -239,7 +239,7 @@ Two agent platforms, two hook-config shapes, **one set of shared scripts**: - **Fail-open — the hooks are advisory and MUST always exit 0.** Copilot CLI ≥ 1.0.57 denies the tool call when a `preToolUse` command hook exits non-zero. The scripts therefore use a **multi-layer defense** (interactive - scripts like `install.sh` and `dream-setup.sh` are the opposite — they + scripts like `install.sh` and `dream-setup.py` are the opposite — they fail-fast): 1. **No `set -e`/`-u`/`pipefail`** — failing sub-steps don't abort the script. diff --git a/skills/shadow-frog-dream/SKILL.md b/skills/shadow-frog-dream/SKILL.md index da991c5..ffc88eb 100644 --- a/skills/shadow-frog-dream/SKILL.md +++ b/skills/shadow-frog-dream/SKILL.md @@ -38,22 +38,24 @@ the dream branch, pushed to the remote, then read back by the reconciler via "local only" option in `shadow-frog-init`), `git add -A` silently skips those files, nothing reaches the remote, and the reconciler finds no manifest — **every discovery is lost without warning**. `dream-setup.py` runs -`git check-ignore .shadow` up front and refuses to start if it's ignored. +`git check-ignore` on a new `.shadow/_dreams/` child path up front and refuses +to start if it's ignored. Use `shadow-frog-update` instead for local-only shadows. ### Path Isolation ``` -WORKTREE_BASE = /tmp/shadowfrog-dreams// +WORKTREE_BASE = /shadowfrog-dreams// WORKTREE_DIR = $WORKTREE_BASE/dream- ``` -- Worktrees are ALWAYS in `/tmp/shadowfrog-dreams//`, NEVER in - the project directory. This prevents conflicts between parallel agents - and keeps the main repo clean. +- Worktrees are ALWAYS in the system temp directory under + `shadowfrog-dreams//`, NEVER in the project directory. This + prevents conflicts between parallel agents and keeps the main repo clean. - `DREAM_NS` (namespace) isolates branches per task/instance. Resolved from: `DREAM_NAMESPACE` env → `TASK_INFO.json` → `.env` → repo basename. -- Override only with `DREAM_WORKTREE_BASE` env var if `/tmp` is too small. +- Override only with `DREAM_WORKTREE_BASE` env var if the system temp volume is + too small. - `dream-setup.py` computes and enforces all paths. Use it. ### Branch Naming @@ -153,26 +155,52 @@ done **Usage patterns:** ```bash +PYTHON_BIN="${PYTHON_BIN:-$(command -v python3 || command -v python)}" +[ -n "$PYTHON_BIN" ] || { echo "ERROR: python3/python not found"; exit 1; } + # Setup: creates the worktree and prints its context as JSON on stdout. -# Run it, check the exit code, then parse the JSON object it prints. -SETUP_JSON="$("$SKILL_DIR/dream-setup.py" --slug t01-my-experiment)" || exit 1 -# SETUP_JSON keys: repo_root, default_branch, dream_ns, dream_id, branch_name, -# parent_branch, worktree_dir, worktree_base, base_commit, run_prefix, slug. -# The steps below use values parsed from that JSON (e.g. $DREAM_ID = .dream_id). +SETUP_JSON="$("$PYTHON_BIN" "$SKILL_DIR/dream-setup.py" --slug t01-my-experiment)" || exit 1 + +# Parse setup JSON into the shell variables used by the steps below. +while IFS=$'\t' read -r key value; do + case "$key" in + REPO_ROOT|DEFAULT_BRANCH|DREAM_NS|DREAM_ID|BRANCH_NAME|PARENT_BRANCH|WORKTREE_DIR|WORKTREE_BASE|BASE_COMMIT|RUN_PREFIX|SLUG) + printf -v "$key" '%s' "$value" + export "$key" + ;; + esac +done < <(printf '%s' "$SETUP_JSON" | "$PYTHON_BIN" -c ' +import json, sys +data = json.load(sys.stdin) +for env_key, json_key in ( + ("REPO_ROOT", "repo_root"), + ("DEFAULT_BRANCH", "default_branch"), + ("DREAM_NS", "dream_ns"), + ("DREAM_ID", "dream_id"), + ("BRANCH_NAME", "branch_name"), + ("PARENT_BRANCH", "parent_branch"), + ("WORKTREE_DIR", "worktree_dir"), + ("WORKTREE_BASE", "worktree_base"), + ("BASE_COMMIT", "base_commit"), + ("RUN_PREFIX", "run_prefix"), + ("SLUG", "slug"), +): + print(f"{env_key}\t{data[json_key]}") +') # Validate: hard gate before push -python3 "$SKILL_DIR/dream-validate.py" "$DREAM_ID" "$WORKTREE_DIR" +"$PYTHON_BIN" "$SKILL_DIR/dream-validate.py" "$DREAM_ID" "$WORKTREE_DIR" # Reconcile: merge all dream branches into main -python3 "$SKILL_DIR/dream-reconcile.py" "$REPO_ROOT" +"$PYTHON_BIN" "$SKILL_DIR/dream-reconcile.py" "$REPO_ROOT" # After `git push` succeeds, optionally clean up reconciled branches. # Cleanup REFUSES to run if `.shadow/` has uncommitted changes, or unless # HEAD is already on origin/ — so the canonical flow is: # reconcile → git add .shadow/ && git commit && git push → re-run --cleanup-branches -python3 "$SKILL_DIR/dream-reconcile.py" "$REPO_ROOT" --cleanup-branches +"$PYTHON_BIN" "$SKILL_DIR/dream-reconcile.py" "$REPO_ROOT" --cleanup-branches # Coverage: show which files still need exploration -python3 "$SKILL_DIR/dream-coverage.py" "$REPO_ROOT" +"$PYTHON_BIN" "$SKILL_DIR/dream-coverage.py" "$REPO_ROOT" # All scripts support --help. ``` @@ -497,21 +525,51 @@ Use `dream-setup.py` to create worktrees (handles all path computation, namespace resolution, worktree creation, and validation): ```bash +PYTHON_BIN="${PYTHON_BIN:-$(command -v python3 || command -v python)}" +[ -n "$PYTHON_BIN" ] || { echo "ERROR: python3/python not found"; exit 1; } + # Find the setup script SETUP_SCRIPT="" for DIR in .github/skills/shadow-frog-dream .claude/skills/shadow-frog-dream; do [ -f "$DIR/dream-setup.py" ] && SETUP_SCRIPT="$DIR/dream-setup.py" && break done -# Fresh experiment from main (prints JSON — parse it, don't eval): -SETUP_JSON="$(python "$SETUP_SCRIPT" --slug t01-csv-fuzzer)" || exit 1 +# Fresh experiment from main: +SETUP_JSON="$("$PYTHON_BIN" "$SETUP_SCRIPT" --slug t01-csv-fuzzer)" || exit 1 # Compounding from prior dream: -SETUP_JSON="$(python "$SETUP_SCRIPT" --slug t03-extend --base-branch dream//)" || exit 1 +SETUP_JSON="$("$PYTHON_BIN" "$SETUP_SCRIPT" --slug t03-extend --base-branch dream//)" || exit 1 + +# Parse setup JSON into shell variables used below. +while IFS=$'\t' read -r key value; do + case "$key" in + REPO_ROOT|DEFAULT_BRANCH|DREAM_NS|DREAM_ID|BRANCH_NAME|PARENT_BRANCH|WORKTREE_DIR|WORKTREE_BASE|BASE_COMMIT|RUN_PREFIX|SLUG) + printf -v "$key" '%s' "$value" + export "$key" + ;; + esac +done < <(printf '%s' "$SETUP_JSON" | "$PYTHON_BIN" -c ' +import json, sys +data = json.load(sys.stdin) +for env_key, json_key in ( + ("REPO_ROOT", "repo_root"), + ("DEFAULT_BRANCH", "default_branch"), + ("DREAM_NS", "dream_ns"), + ("DREAM_ID", "dream_id"), + ("BRANCH_NAME", "branch_name"), + ("PARENT_BRANCH", "parent_branch"), + ("WORKTREE_DIR", "worktree_dir"), + ("WORKTREE_BASE", "worktree_base"), + ("BASE_COMMIT", "base_commit"), + ("RUN_PREFIX", "run_prefix"), + ("SLUG", "slug"), +): + print(f"{env_key}\t{data[json_key]}") +') ``` Capture the JSON into `SETUP_JSON` and check the exit code (`|| exit 1`), -then parse it for the values below. +then parse it into the exported variables above before using later steps. This prints a JSON object with keys: `repo_root`, `default_branch`, `dream_ns`, `dream_id`, `branch_name`, `parent_branch`, `worktree_dir`, @@ -519,7 +577,7 @@ This prints a JSON object with keys: `repo_root`, `default_branch`, **If `dream-setup.py` fails or is not found:** Apply the Script Failure Recovery rule (read the script source, adapt its logic). Common causes: -missing git remote, branch already exists, `/tmp` permissions. +missing git remote, branch already exists, or temp-directory permissions. **Note:** Shell variables don't persist across tool calls. Either run multi-step setup in a single shell, or re-derive values. From inside a @@ -842,7 +900,7 @@ followed by `git worktree prune`, but ALSO falls back to a safety-gated `rm -rf` if `git worktree remove` silently fails — the failure mode that leaked tens of dream worktrees per AFK session under the previous inline snippet (see bug-worktree-leak.md). The rm fallback ONLY fires for paths -that match `${DREAM_WORKTREE_BASE:-/tmp/shadowfrog-dreams}//dream-` +that match `${DREAM_WORKTREE_BASE:-/shadowfrog-dreams}//dream-` exactly; any other path is refused. Remove as you go. If push failed, keep the worktree. @@ -1063,7 +1121,7 @@ set), apply the rules in Phase 6 → Post-Reconciliation Branch Cleanup ### Worktree Pruning Dream worktrees live OUTSIDE the repo at -`${DREAM_WORKTREE_BASE:-/tmp/shadowfrog-dreams}//dream-/`. There +`${DREAM_WORKTREE_BASE:-/shadowfrog-dreams}//dream-/`. There are four places they get cleaned up: 1. **`dream-cleanup.sh`** — called by the agent after each `git push` (see diff --git a/skills/shadow-frog-dream/_worktree_safety.py b/skills/shadow-frog-dream/_worktree_safety.py index 4fc70fa..8a09a91 100644 --- a/skills/shadow-frog-dream/_worktree_safety.py +++ b/skills/shadow-frog-dream/_worktree_safety.py @@ -16,11 +16,10 @@ path is STRICTLY INSIDE the resolved base — not equal to it, and not above it. 6. The path matches the exact dream-worktree shape `//dream-` - where `` and `` are each `[A-Za-z0-9._-]+` (the same `SAFE_RE` - that `dream-setup.sh` already enforces on the inputs). + where `` and `` are each `[A-Za-z0-9._-]+`. These rules are deliberately strict: they reject anything that doesn't look -like a dream worktree created by `dream-setup.sh`. That means we will NEVER +like a dream worktree created by `dream-setup.py`. That means we will NEVER `rm -rf` a path the user happens to point us at — only paths that match the namespace's own creation contract. @@ -38,8 +37,7 @@ import sys from pathlib import Path, PurePath -# Same regex `dream-setup.sh` validates --slug and --namespace against. -# Keep these in lockstep — if one widens, the other must follow. +# Cleanup accepts the broader shape that legacy dream worktrees may have used. _SAFE_RE = re.compile(r"^[A-Za-z0-9._-]+$") # Defense-in-depth: even if rule 5 (strictly under base) holds, refuse diff --git a/skills/shadow-frog-dream/dream-gc.sh b/skills/shadow-frog-dream/dream-gc.sh index 884db0c..344db46 100755 --- a/skills/shadow-frog-dream/dream-gc.sh +++ b/skills/shadow-frog-dream/dream-gc.sh @@ -185,7 +185,7 @@ fi # Shape: $BASE//dream-. We use `find` to keep this fast on large # bases. `-mindepth 2 -maxdepth 2` matches exactly that level. # `-mmin +N` requires modification time older than N minutes (avoids racing -# with a fresh `dream-setup.sh` mid-creation). +# with a fresh `dream-setup.py` mid-creation). removed=0 kept=0 refused=0 diff --git a/skills/shadow-frog-dream/dream-reconcile.py b/skills/shadow-frog-dream/dream-reconcile.py index ded8f1b..b14e666 100755 --- a/skills/shadow-frog-dream/dream-reconcile.py +++ b/skills/shadow-frog-dream/dream-reconcile.py @@ -1464,7 +1464,7 @@ def cleanup_branches(repo_root, manifests, dream_ns, dry_run=False): return deleted, kept -# Compiled here so the error message is consistent with `dream-setup.sh`. +# Compiled here so the error message is consistent with `dream-setup.py`. # DREAM_ID format: YYYYMMDD-HHMMSSZ-. The leading timestamp is # fixed-width (8 digits + '-' + 6 digits + 'Z' + '-' = 17 chars), but we # anchor on the regex to be robust against drift. @@ -1545,7 +1545,7 @@ def _gc_worktree_after_merge(repo_root, dream_ns, dream_id, deleted_branch=None) NEVER `rm -rf` a path outside `$DREAM_WORKTREE_BASE//dream-`. Cross-deletion guard: worktree paths are keyed on slug only (see - `dream-setup.sh`: `WORKTREE_DIR=//dream-`), but + `dream-setup.py`: `WORKTREE_DIR=//dream-`), but `dream_id` includes a timestamp. So two dreams that re-use the same slug at different times share a worktree path. If the path we're about to GC is currently registered to a DIFFERENT branch — i.e. a diff --git a/skills/shadow-frog-dream/dream-setup.py b/skills/shadow-frog-dream/dream-setup.py index be71bf0..d9c0fa8 100644 --- a/skills/shadow-frog-dream/dream-setup.py +++ b/skills/shadow-frog-dream/dream-setup.py @@ -44,7 +44,7 @@ import time from datetime import datetime, timezone -# Same regex `_worktree_safety.py` validates ns/slug against — keep in lockstep. +# Setup is stricter than the cleanup safety gate: it rejects leading ".". SAFE_RE = re.compile(r"^[A-Za-z0-9_-][A-Za-z0-9._-]*$") SAFE_INT_RE = re.compile(r"^[0-9]+$") @@ -63,6 +63,24 @@ def _git(args, cwd=None): ) +def _touch(path): + try: + with open(path, "a", encoding="utf-8"): + pass + os.utime(path, None) + except OSError: + pass + + +def _resolve_repo_root(path): + candidate = os.path.abspath(path) if path else None + r = _git(["rev-parse", "--show-toplevel"], cwd=candidate) + if r.returncode != 0: + _err("ERROR: Not in a git repository") + sys.exit(1) + return os.path.abspath(r.stdout.strip()) + + def _import_safety(): """Lazily import the shared rm-rf safety gate (sibling module). Raises ImportError if the module is missing — the caller decides how to react.""" @@ -173,20 +191,12 @@ def _maybe_auto_gc(repo_root, worktree_base): if not should_run: return - # Touch BEFORE running so a parallel setup for the same ns sees a fresh - # tombstone and skips. Worst case is one extra sweep, never a missed one. - try: - with open(tombstone, "a", encoding="utf-8"): - pass - os.utime(tombstone, None) - except OSError: - pass - try: r = subprocess.run( gc_cmd, cwd=repo_root, capture_output=True, text=True, encoding="utf-8", timeout=120, ) + _touch(tombstone) if r.stdout: sys.stderr.write(r.stdout) if r.stderr: @@ -241,13 +251,9 @@ def main(): # --- Resolve repo root --- if opts["repo_root"]: - repo_root = os.path.abspath(opts["repo_root"]) + repo_root = _resolve_repo_root(opts["repo_root"]) else: - r = _git(["rev-parse", "--show-toplevel"]) - if r.returncode != 0: - _err("ERROR: Not in a git repository") - sys.exit(1) - repo_root = os.path.abspath(r.stdout.strip()) + repo_root = _resolve_repo_root("") os.chdir(repo_root) # --- Guard: .shadow/ must be tracked by git (not gitignored) --- @@ -295,7 +301,12 @@ def main(): elif os.path.isfile("TASK_INFO.json"): try: with open("TASK_INFO.json", encoding="utf-8") as f: - dream_ns = json.load(f).get("dream_namespace", "") or "" + task_ns = json.load(f).get("dream_namespace", "") + if task_ns: + if not isinstance(task_ns, str): + _err("ERROR: TASK_INFO.json dream_namespace must be a string") + sys.exit(1) + dream_ns = task_ns except (OSError, ValueError): dream_ns = "" elif os.path.isfile(".env"): @@ -319,9 +330,13 @@ def main(): worktree_base = os.path.join(gate_base, dream_ns) worktree_dir = os.path.join(worktree_base, f"dream-{slug}") - rr = os.path.abspath(repo_root) - wd = os.path.abspath(worktree_dir) - if wd == rr or wd.startswith(rr + os.sep): + rr = os.path.normcase(os.path.realpath(repo_root)) + wd = os.path.normcase(os.path.realpath(worktree_dir)) + try: + inside_repo = os.path.commonpath([rr, wd]) == rr + except ValueError: + inside_repo = False + if inside_repo: _err(f"ERROR: Worktree would be inside project: {worktree_dir}") _err("MUST use external path (default: /shadowfrog-dreams/)") sys.exit(1) diff --git a/skills/shadow-frog-dream/dream-validate.py b/skills/shadow-frog-dream/dream-validate.py index d4f3b86..76a2c86 100644 --- a/skills/shadow-frog-dream/dream-validate.py +++ b/skills/shadow-frog-dream/dream-validate.py @@ -200,7 +200,7 @@ def main(): errors.append( "manifest declares discoveries but report.md frontmatter " "is missing `base_commit` — cannot verify shadow files were " - "updated. Add base_commit (full SHA emitted by dream-setup.sh)." + "updated. Add base_commit (full SHA emitted by dream-setup.py)." ) else: base = bm_match.group(1) diff --git a/skills/shadow-frog-init/SKILL.md b/skills/shadow-frog-init/SKILL.md index 6298466..aa2affc 100644 --- a/skills/shadow-frog-init/SKILL.md +++ b/skills/shadow-frog-init/SKILL.md @@ -256,7 +256,7 @@ Before they decide, make the trade-off explicit: - **`shadow-frog-dream` will NOT work.** Dreams move `.shadow/` through git (commit → push → reconcile from the remote); a gitignored `.shadow/` is silently skipped by `git add`, so discoveries never reach the remote and - are lost. `dream-setup.sh` detects this and refuses to start with a clear + are lost. `dream-setup.py` detects this and refuses to start with a clear error rather than failing silently. - If gitignored: add `.shadow/` to `.gitignore`. diff --git a/tests/skills/shadow_frog_dream/test_dream_gc_sh.py b/tests/skills/shadow_frog_dream/test_dream_gc_sh.py index 8b6159a..4cabe8c 100644 --- a/tests/skills/shadow_frog_dream/test_dream_gc_sh.py +++ b/tests/skills/shadow_frog_dream/test_dream_gc_sh.py @@ -182,7 +182,7 @@ def test_keeps_live_worktree(self, tmp_path): def test_min_age_skips_fresh_orphan(self, tmp_path): """A fresh orphan (mtime = now) gets skipped — protects races - with `dream-setup.sh` that just created the dir but hasn't + with `dream-setup.py` that just created the dir but hasn't finished registering the worktree yet.""" repo = _make_repo(tmp_path / "repo") base = tmp_path / "wt-base" diff --git a/tests/skills/shadow_frog_dream/test_dream_reconcile.py b/tests/skills/shadow_frog_dream/test_dream_reconcile.py index c13b9fa..bb87246 100644 --- a/tests/skills/shadow_frog_dream/test_dream_reconcile.py +++ b/tests/skills/shadow_frog_dream/test_dream_reconcile.py @@ -3293,7 +3293,7 @@ def test_cleanup_branches_worktree_gc_skips_unparseable_dream_id( # =========================================================================== # Cross-deletion guard for slug-collision (S2 from 5-model review panel) # =========================================================================== -# Worktree paths are slug-only (see dream-setup.sh:165), but dream_ids +# Worktree paths are slug-only (see dream-setup.py), but dream_ids # include a timestamp. So two dreams with the same slug at different # times share a worktree path. If dream A is reconciled AFTER dream B has # reclaimed the shared path, A's GC must NOT delete B's live worktree. @@ -3384,7 +3384,7 @@ def test_cleanup_branches_does_not_clobber_concurrent_slug_collision( _git("commit", "-q", "-m", "reconcile A", cwd=tmp_git_repo, env=env) _git("push", "-q", "origin", "main", cwd=tmp_git_repo, env=env) - # Dream B claims the shared path. (In production, dream-setup.sh's + # Dream B claims the shared path. (In production, dream-setup.py's # idempotent pre-clean would have already wiped any stale A worktree.) shared_path = base / "proj" / "dream-same" shared_path.parent.mkdir(parents=True) diff --git a/tests/skills/shadow_frog_dream/test_dream_setup.py b/tests/skills/shadow_frog_dream/test_dream_setup.py index 3422c59..e30f57c 100644 --- a/tests/skills/shadow_frog_dream/test_dream_setup.py +++ b/tests/skills/shadow_frog_dream/test_dream_setup.py @@ -10,6 +10,7 @@ every other test runs on all OSes. """ import json +import importlib.util import os import subprocess import sys @@ -70,6 +71,14 @@ def run_dream_setup( ) +def _load_dream_setup_module(): + spec = importlib.util.spec_from_file_location("dream_setup", DREAM_SETUP) + module = importlib.util.module_from_spec(spec) + assert spec.loader is not None + spec.loader.exec_module(module) + return module + + def _plant_orphan(base: Path, ns: str, name: str = "dream-orphan") -> Path: """Plant an orphan worktree (broken .git pointer, ancient mtime).""" d = base / ns / name @@ -156,6 +165,24 @@ def test_worktree_has_same_head_as_base(self, tmp_path): data = json.loads(result.stdout) assert data["base_commit"] == main_head + def test_repo_root_subdir_is_canonicalized_for_isolation(self, tmp_path): + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + subdir = repo / "subdir" + subdir.mkdir() + worktree_base = repo / ".dream-worktrees" + + result = run_dream_setup( + ["--slug", "t02-subdir", "--repo-root", str(subdir), + "--dry-run", "--print-json"], + cwd=subdir, + env_extra={"DREAM_WORKTREE_BASE": str(worktree_base)}, + ) + + assert result.returncode != 0 + assert "Worktree would be inside project" in result.stderr + @pytest.mark.slow @pytest.mark.integration @@ -274,6 +301,23 @@ def test_env_namespace_used(self, tmp_path): data = json.loads(result.stdout) assert data["dream_ns"] == "env-ns-test" + def test_task_info_namespace_must_be_string(self, tmp_path): + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + (repo / "TASK_INFO.json").write_text( + '{"dream_namespace": 123}\n', encoding="utf-8", + ) + + result = run_dream_setup( + ["--slug", "t08-taskns", "--repo-root", str(repo), + "--dry-run", "--print-json"], + cwd=repo, + ) + + assert result.returncode != 0 + assert "dream_namespace must be a string" in result.stderr + @pytest.mark.slow @pytest.mark.integration @@ -501,6 +545,25 @@ def test_auto_gc_does_not_pollute_json_stdout(self, tmp_path): data = json.loads(result.stdout) assert data["slug"] == "t05-stdout" + def test_auto_gc_launch_failure_does_not_touch_tombstone(self, tmp_path, monkeypatch): + dream_setup = _load_dream_setup_module() + script_dir = tmp_path / "skill" + script_dir.mkdir() + (script_dir / "dream-gc.sh").write_text("#!/bin/sh\n", encoding="utf-8") + worktree_base = tmp_path / "worktrees" / "repo" + worktree_base.mkdir(parents=True) + tombstone = worktree_base / ".last-gc" + + def fail_to_launch(*args, **kwargs): + raise FileNotFoundError("bash") + + monkeypatch.setattr(dream_setup, "SCRIPT_DIR", str(script_dir)) + monkeypatch.setattr(dream_setup.subprocess, "run", fail_to_launch) + + dream_setup._maybe_auto_gc(str(tmp_path), str(worktree_base)) + + assert not tombstone.exists() + @pytest.mark.slow @pytest.mark.integration From ab3aba21b4563507847309ea2c146d3b2919ac05 Mon Sep 17 00:00:00 2001 From: Walther Maciel Date: Wed, 16 Sep 2026 11:03:30 -0700 Subject: [PATCH 06/14] fix(dream): share the worktree root across the lifecycle Use one platform-aware root resolver for setup and reconciliation, propagate it to auto-GC, and preserve failed cleanup worktrees for recovery. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- skills/shadow-frog-dream/_worktree_paths.py | 10 ++++ skills/shadow-frog-dream/dream-reconcile.py | 54 +++++++++++++++------ skills/shadow-frog-dream/dream-setup.py | 52 ++++++++++++++------ 3 files changed, 86 insertions(+), 30 deletions(-) create mode 100644 skills/shadow-frog-dream/_worktree_paths.py diff --git a/skills/shadow-frog-dream/_worktree_paths.py b/skills/shadow-frog-dream/_worktree_paths.py new file mode 100644 index 0000000..0247f47 --- /dev/null +++ b/skills/shadow-frog-dream/_worktree_paths.py @@ -0,0 +1,10 @@ +"""Shared worktree-root resolution for the dream lifecycle.""" +import os +import tempfile + + +def resolve_worktree_root(override=None): + """Return the configured root or the platform's default dream root.""" + return override or os.environ.get("DREAM_WORKTREE_BASE") or os.path.join( + tempfile.gettempdir(), "shadowfrog-dreams" + ) diff --git a/skills/shadow-frog-dream/dream-reconcile.py b/skills/shadow-frog-dream/dream-reconcile.py index b14e666..302620f 100755 --- a/skills/shadow-frog-dream/dream-reconcile.py +++ b/skills/shadow-frog-dream/dream-reconcile.py @@ -10,6 +10,7 @@ --verify-only Only run post-reconciliation verification (checks all dreams already in _index.md, not just unreconciled) --namespace NS Override DREAM_NAMESPACE + --worktree-base DIR Override the dream worktree root used for cleanup --cleanup-branches Delete reconciled branches after verification. REFUSES to run unless the reconciliation commit is already on origin/. Run AFTER push. @@ -30,6 +31,7 @@ """ import json +import importlib.util import os import re import shutil @@ -1285,7 +1287,8 @@ def verify_reconciliation(repo_root, manifests): # --- Step 9: Cleanup branches --- -def cleanup_branches(repo_root, manifests, dream_ns, dry_run=False): +def cleanup_branches(repo_root, manifests, dream_ns, dry_run=False, + worktree_root=None): """Delete reconciled dream branches (local and remote). Only deletes a branch if: @@ -1424,6 +1427,12 @@ def cleanup_branches(repo_root, manifests, dream_ns, dry_run=False): deleted += 1 continue + # Remove the registered worktree before deleting its branch. Git + # refuses to delete a branch that remains checked out in a worktree. + _gc_worktree_after_merge( + repo_root, dream_ns, dream_id, branch, worktree_root, + ) + # Delete remote first (network op that can fail) result = subprocess.run( ['git', 'push', 'origin', '--delete', branch], @@ -1443,22 +1452,16 @@ def cleanup_branches(repo_root, manifests, dream_ns, dry_run=False): ) if result.returncode == 0: print(f" 🗑 Deleted local: {branch}") + else: + print(f" ⚠️ Failed to delete local {branch}: {result.stderr.strip()}") + kept += 1 + continue # Also delete the remote-tracking ref subprocess.run( ['git', 'branch', '-dr', f'origin/{branch}'], capture_output=True, text=True, cwd=repo_root, encoding="utf-8" ) - # Best-effort worktree GC — the directory at - # `${DREAM_WORKTREE_BASE:-/tmp/shadowfrog-dreams}//dream-` - # is now orphaned (its branch is gone). Removing it here closes the - # leak documented in bug-worktree-leak.md. - # - # `branch` is the branch we just deleted; passing it lets the GC - # refuse to remove a path that another dream (sharing the same - # slug) has reclaimed for its own live worktree. - _gc_worktree_after_merge(repo_root, dream_ns, dream_id, branch) - deleted += 1 return deleted, kept @@ -1539,7 +1542,8 @@ def _matches(p): return None -def _gc_worktree_after_merge(repo_root, dream_ns, dream_id, deleted_branch=None): +def _gc_worktree_after_merge(repo_root, dream_ns, dream_id, deleted_branch=None, + worktree_root=None): """Remove the dream worktree directory after its branch has been deleted. Safety-gated by `_worktree_safety.safe_worktree_path` — will NEVER `rm -rf` a path outside `$DREAM_WORKTREE_BASE//dream-`. @@ -1560,7 +1564,18 @@ def _gc_worktree_after_merge(repo_root, dream_ns, dream_id, deleted_branch=None) slug = _slug_from_dream_id(dream_id) if not slug: return # Can't derive worktree path — bail silently. - base = os.environ.get('DREAM_WORKTREE_BASE', '/tmp/shadowfrog-dreams') + paths_path = os.path.join( + os.path.dirname(os.path.abspath(__file__)), "_worktree_paths.py" + ) + paths_spec = importlib.util.spec_from_file_location( + "_worktree_paths", paths_path + ) + if paths_spec is None or paths_spec.loader is None: + raise ImportError(f"cannot load worktree path helper: {paths_path}") + paths_module = importlib.util.module_from_spec(paths_spec) + paths_spec.loader.exec_module(paths_module) + resolve_worktree_root = paths_module.resolve_worktree_root + base = resolve_worktree_root(worktree_root) candidate = os.path.join(base, dream_ns, f'dream-{slug}') # Import lazily so this module remains importable for tests that @@ -1636,6 +1651,7 @@ def main(): verify_only = False cleanup = False namespace_override = None + worktree_base_override = None args = sys.argv[1:] i = 0 @@ -1655,6 +1671,12 @@ def main(): print("ERROR: --namespace requires a value", file=sys.stderr) sys.exit(1) namespace_override = args[i] + elif args[i] == '--worktree-base': + i += 1 + if i >= len(args): + print("ERROR: --worktree-base requires a value", file=sys.stderr) + sys.exit(1) + worktree_base_override = args[i] elif not args[i].startswith('-'): repo_root = args[i] else: @@ -1765,7 +1787,8 @@ def main(): print() print("Step 9: Cleaning up reconciled branches...") deleted, kept = cleanup_branches( - repo_root, cleanup_manifests, dream_ns, dry_run=dry_run + repo_root, cleanup_manifests, dream_ns, dry_run=dry_run, + worktree_root=worktree_base_override, ) print(f" Deleted: {deleted}, Kept: {kept}") sys.exit(0) @@ -1849,7 +1872,8 @@ def main(): print(" ⚠️ Run this AFTER 'git push' succeeds on main.") print(" Checking artifacts on main...") deleted, kept = cleanup_branches( - repo_root, manifests, dream_ns, dry_run=dry_run + repo_root, manifests, dream_ns, dry_run=dry_run, + worktree_root=worktree_base_override, ) print(f" Deleted: {deleted}, Kept: {kept}") diff --git a/skills/shadow-frog-dream/dream-setup.py b/skills/shadow-frog-dream/dream-setup.py index d9c0fa8..cb69655 100644 --- a/skills/shadow-frog-dream/dream-setup.py +++ b/skills/shadow-frog-dream/dream-setup.py @@ -7,7 +7,7 @@ Prints a JSON object to stdout with keys: repo_root, default_branch, dream_ns, dream_id, branch_name, parent_branch, - worktree_dir, worktree_base, base_commit, run_prefix, slug + worktree_dir, worktree_root, worktree_base, base_commit, run_prefix, slug Exits non-zero (message on stderr) on any failure — always check the exit code. @@ -40,7 +40,6 @@ import shutil import subprocess import sys -import tempfile import time from datetime import datetime, timezone @@ -93,6 +92,17 @@ def _import_safety(): sys.path.pop(0) +def _import_paths(): + """Lazily import the shared worktree-root resolver.""" + sys.path.insert(0, SCRIPT_DIR) + try: + from _worktree_paths import resolve_worktree_root + return resolve_worktree_root + finally: + if sys.path and sys.path[0] == SCRIPT_DIR: + sys.path.pop(0) + + def _read_env_namespace(path): """Extract DREAM_NAMESPACE from a `.env` file (first match), stripping surrounding whitespace and one layer of matching quotes.""" @@ -145,7 +155,7 @@ def _parse_args(argv): return opts -def _maybe_auto_gc(repo_root, worktree_base): +def _maybe_auto_gc(repo_root, worktree_base, worktree_root=None): """Periodic, throttled, best-effort orphan sweep. NEVER breaks setup. Prefers a `dream-gc.py` sibling if present, else falls back to the bash @@ -192,15 +202,22 @@ def _maybe_auto_gc(repo_root, worktree_base): return try: + env = os.environ.copy() + if worktree_root: + env["DREAM_WORKTREE_BASE"] = worktree_root r = subprocess.run( gc_cmd, cwd=repo_root, capture_output=True, text=True, encoding="utf-8", timeout=120, + env=env, ) - _touch(tombstone) if r.stdout: sys.stderr.write(r.stdout) if r.stderr: sys.stderr.write(r.stderr) + if r.returncode == 0: + _touch(tombstone) + else: + _err(f"WARN: auto-GC exited with code {r.returncode}; will retry later") except Exception: # noqa: BLE001 — auto-GC must never break setup pass @@ -301,12 +318,16 @@ def main(): elif os.path.isfile("TASK_INFO.json"): try: with open("TASK_INFO.json", encoding="utf-8") as f: - task_ns = json.load(f).get("dream_namespace", "") - if task_ns: - if not isinstance(task_ns, str): - _err("ERROR: TASK_INFO.json dream_namespace must be a string") - sys.exit(1) - dream_ns = task_ns + task_info = json.load(f) + if not isinstance(task_info, dict): + _err("ERROR: TASK_INFO.json must contain a JSON object") + sys.exit(1) + task_ns = task_info.get("dream_namespace", "") + if task_ns: + if not isinstance(task_ns, str): + _err("ERROR: TASK_INFO.json dream_namespace must be a string") + sys.exit(1) + dream_ns = task_ns except (OSError, ValueError): dream_ns = "" elif os.path.isfile(".env"): @@ -325,9 +346,9 @@ def main(): branch_name = f"dream/{dream_ns}/{dream_id}" # --- Compute worktree path (ALWAYS external, NEVER in project) --- - gate_base = os.environ.get("DREAM_WORKTREE_BASE") or os.path.join( - tempfile.gettempdir(), "shadowfrog-dreams") - worktree_base = os.path.join(gate_base, dream_ns) + resolve_worktree_root = _import_paths() + worktree_root = resolve_worktree_root() + worktree_base = os.path.join(worktree_root, dream_ns) worktree_dir = os.path.join(worktree_base, f"dream-{slug}") rr = os.path.normcase(os.path.realpath(repo_root)) @@ -357,8 +378,8 @@ def main(): base_commit = "" if not opts["dry_run"]: os.makedirs(worktree_base, exist_ok=True) - _maybe_auto_gc(repo_root, worktree_base) - _preclean_worktree(repo_root, worktree_dir, gate_base) + _maybe_auto_gc(repo_root, worktree_base, worktree_root) + _preclean_worktree(repo_root, worktree_dir, worktree_root) def _add(): return _git(["worktree", "add", worktree_dir, "-b", branch_name, @@ -394,6 +415,7 @@ def _add(): "branch_name": branch_name, "parent_branch": parent_branch, "worktree_dir": worktree_dir, + "worktree_root": worktree_root, "worktree_base": worktree_base, "base_commit": base_commit, "run_prefix": run_prefix, From 55e0a5fd1d8fda7e5800867a7b79ce656e9994b1 Mon Sep 17 00:00:00 2001 From: Walther Maciel Date: Wed, 16 Sep 2026 11:03:37 -0700 Subject: [PATCH 07/14] test(dream): cover worktree root lifecycle regressions Protect root propagation, failed auto-GC retry behavior, controlled TASK_INFO validation, safe bridge parsing, and cleanup reporting. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../shadow_frog_dream/test_dream_reconcile.py | 41 ++++++ .../shadow_frog_dream/test_dream_setup.py | 117 ++++++++++++++++++ 2 files changed, 158 insertions(+) diff --git a/tests/skills/shadow_frog_dream/test_dream_reconcile.py b/tests/skills/shadow_frog_dream/test_dream_reconcile.py index bb87246..b4ed9c7 100644 --- a/tests/skills/shadow_frog_dream/test_dream_reconcile.py +++ b/tests/skills/shadow_frog_dream/test_dream_reconcile.py @@ -2232,6 +2232,47 @@ def test_cleanup_branches_actually_deletes_when_all_checks_pass( assert branch not in post +@pytest.mark.slow +def test_cleanup_branches_keeps_branch_when_local_delete_fails( + dream_reconcile, tmp_git_repo, monkeypatch, capsys +): + env = _seed_repo(tmp_git_repo) + _add_bare_remote(tmp_git_repo, env) + dream_id = "20260420-050050Z-local-failure" + branch = make_dream_branch(tmp_git_repo, env, "proj", dream_id, + _default_manifest(dream_id)) + _seed_dream_artifacts(tmp_git_repo, dream_id) + _write_index( + tmp_git_repo, + "# Dream Experiments\n\n" + "| dream_id | category | verdict | title | branch | parent | tip_commit |\n" + "|----------|----------|---------|-------|--------|--------|------------|\n" + f"| {dream_id} | bug hunting | useful | T | {branch} | main | abc1234 |\n", + ) + _git("add", "-A", cwd=tmp_git_repo, env=env) + _git("commit", "-q", "-m", "reconcile", cwd=tmp_git_repo, env=env) + _git("push", "-q", "origin", "main", cwd=tmp_git_repo, env=env) + + original_run = dream_reconcile.subprocess.run + + def reject_local_delete(args, **kwargs): + if args == ["git", "branch", "-D", branch]: + return subprocess.CompletedProcess(args, 1, "", "branch is in use") + return original_run(args, **kwargs) + + monkeypatch.setattr(dream_reconcile.subprocess, "run", reject_local_delete) + deleted, kept = dream_reconcile.cleanup_branches( + str(tmp_git_repo), + [(branch, dream_id, _default_manifest(dream_id))], + "proj", + dry_run=False, + ) + + assert deleted == 0 + assert kept == 1 + assert f"Failed to delete local {branch}: branch is in use" in capsys.readouterr().out + + @pytest.mark.slow def test_cleanup_branches_refuses_when_head_not_pushed( dream_reconcile, tmp_git_repo diff --git a/tests/skills/shadow_frog_dream/test_dream_setup.py b/tests/skills/shadow_frog_dream/test_dream_setup.py index e30f57c..2c519e7 100644 --- a/tests/skills/shadow_frog_dream/test_dream_setup.py +++ b/tests/skills/shadow_frog_dream/test_dream_setup.py @@ -17,6 +17,7 @@ from pathlib import Path import pytest +from tests._shell import BASH, HAVE_BASH, shell_path REPO_ROOT = Path(__file__).resolve().parent.parent.parent.parent DREAM_SETUP = REPO_ROOT / "skills" / "shadow-frog-dream" / "dream-setup.py" @@ -90,6 +91,58 @@ def _plant_orphan(base: Path, ns: str, name: str = "dream-orphan") -> Path: return d +@pytest.mark.skipif(not HAVE_BASH, reason="requires a POSIX shell") +def test_documented_json_bridge_preserves_tabs_and_newlines(tmp_path): + """The NUL-delimited bridge must not corrupt valid POSIX path characters.""" + worktree_dir = (tmp_path / "tab\tand\nnewline").as_posix() + setup_json = json.dumps({ + "repo_root": "/repo", + "default_branch": "main", + "dream_ns": "namespace", + "dream_id": "20260915-000000Z-bridge", + "branch_name": "dream/namespace/20260915-000000Z-bridge", + "parent_branch": "main", + "worktree_dir": worktree_dir, + "worktree_root": "/tmp/shadowfrog-dreams", + "worktree_base": "/tmp/shadowfrog-dreams/namespace", + "base_commit": "abc123", + "run_prefix": "", + "slug": "bridge", + }) + bridge = r''' +while IFS= read -r -d '' key && IFS= read -r -d '' value; do + case "$key" in + REPO_ROOT|DEFAULT_BRANCH|DREAM_NS|DREAM_ID|BRANCH_NAME|PARENT_BRANCH|WORKTREE_DIR|WORKTREE_ROOT|WORKTREE_BASE|BASE_COMMIT|RUN_PREFIX|SLUG) + printf -v "$key" '%s' "$value" + export "$key" + ;; + esac +done < <(python3 -c ' +import json, sys +data = json.loads(sys.argv[1]) +out = sys.stdout.buffer +for env_key, json_key in ( + ("REPO_ROOT", "repo_root"), ("DEFAULT_BRANCH", "default_branch"), + ("DREAM_NS", "dream_ns"), ("DREAM_ID", "dream_id"), + ("BRANCH_NAME", "branch_name"), ("PARENT_BRANCH", "parent_branch"), + ("WORKTREE_DIR", "worktree_dir"), ("WORKTREE_ROOT", "worktree_root"), + ("WORKTREE_BASE", "worktree_base"), ("BASE_COMMIT", "base_commit"), + ("RUN_PREFIX", "run_prefix"), ("SLUG", "slug"), +): + out.write(env_key.encode() + b"\0" + data[json_key].encode() + b"\0") +' "$1") +printf '%s' "$WORKTREE_DIR" +''' + result = subprocess.run( + [BASH, "-c", bridge, "--", setup_json], + capture_output=True, text=True, encoding="utf-8", + env={**_base_env(tmp_path), "PATH": shell_path()}, + ) + + assert result.returncode == 0, result.stderr + assert result.stdout == worktree_dir + + @pytest.mark.slow @pytest.mark.integration class TestDreamSetupHelp: @@ -165,6 +218,30 @@ def test_worktree_has_same_head_as_base(self, tmp_path): data = json.loads(result.stdout) assert data["base_commit"] == main_head + def test_uses_system_temp_root_without_override(self, tmp_path): + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + system_temp = tmp_path / "system-temp" + system_temp.mkdir() + + result = run_dream_setup( + ["--slug", "t02-default-root", "--repo-root", str(repo), "--print-json"], + cwd=repo, + env_extra={ + "DREAM_GC_AUTO": "0", + "TEMP": str(system_temp), + "TMP": str(system_temp), + }, + ) + + assert result.returncode == 0, f"stderr: {result.stderr}" + data = json.loads(result.stdout) + expected_root = system_temp / "shadowfrog-dreams" + assert Path(data["worktree_root"]) == expected_root + assert Path(data["worktree_base"]) == expected_root / repo.name + assert Path(data["worktree_dir"]) == expected_root / repo.name / "dream-t02-default-root" + def test_repo_root_subdir_is_canonicalized_for_isolation(self, tmp_path): repo = tmp_path / "repo" repo.mkdir() @@ -318,6 +395,22 @@ def test_task_info_namespace_must_be_string(self, tmp_path): assert result.returncode != 0 assert "dream_namespace must be a string" in result.stderr + @pytest.mark.parametrize("task_info", ["[]", '"not an object"']) + def test_task_info_must_be_object(self, tmp_path, task_info): + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + (repo / "TASK_INFO.json").write_text(task_info, encoding="utf-8") + + result = run_dream_setup( + ["--slug", "t08-taskinfo-object", "--repo-root", str(repo), + "--dry-run", "--print-json"], + cwd=repo, + ) + + assert result.returncode != 0 + assert "TASK_INFO.json must contain a JSON object" in result.stderr + @pytest.mark.slow @pytest.mark.integration @@ -564,6 +657,30 @@ def fail_to_launch(*args, **kwargs): assert not tombstone.exists() + def test_auto_gc_nonzero_result_does_not_touch_tombstone( + self, tmp_path, monkeypatch, capsys + ): + dream_setup = _load_dream_setup_module() + script_dir = tmp_path / "skill" + script_dir.mkdir() + (script_dir / "dream-gc.sh").write_text("#!/bin/sh\n", encoding="utf-8") + worktree_base = tmp_path / "worktrees" / "repo" + worktree_base.mkdir(parents=True) + tombstone = worktree_base / ".last-gc" + + monkeypatch.setattr(dream_setup, "SCRIPT_DIR", str(script_dir)) + monkeypatch.setattr( + dream_setup.subprocess, + "run", + lambda *args, **kwargs: subprocess.CompletedProcess(args[0], 1), + ) + + dream_setup._maybe_auto_gc( + str(tmp_path), str(worktree_base), str(tmp_path / "worktrees") + ) + + assert not tombstone.exists() + assert "auto-GC exited with code 1" in capsys.readouterr().err @pytest.mark.slow @pytest.mark.integration From 6ffb44d2b59d35e39cc882bb27b50b4ee5cbbf01 Mon Sep 17 00:00:00 2001 From: Walther Maciel Date: Wed, 16 Sep 2026 11:03:43 -0700 Subject: [PATCH 08/14] docs(dream): propagate the resolved worktree root Document the NUL-safe JSON bridge and pass the setup-selected root to reconciliation and Bash cleanup commands. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- skills/shadow-frog-dream/SKILL.md | 60 ++++++++++++------------------- 1 file changed, 23 insertions(+), 37 deletions(-) diff --git a/skills/shadow-frog-dream/SKILL.md b/skills/shadow-frog-dream/SKILL.md index ffc88eb..dd071de 100644 --- a/skills/shadow-frog-dream/SKILL.md +++ b/skills/shadow-frog-dream/SKILL.md @@ -161,10 +161,12 @@ PYTHON_BIN="${PYTHON_BIN:-$(command -v python3 || command -v python)}" # Setup: creates the worktree and prints its context as JSON on stdout. SETUP_JSON="$("$PYTHON_BIN" "$SKILL_DIR/dream-setup.py" --slug t01-my-experiment)" || exit 1 -# Parse setup JSON into the shell variables used by the steps below. -while IFS=$'\t' read -r key value; do +# Consume setup JSON into the shell variables used by the steps below. +# The NUL-delimited stream preserves every valid filesystem path byte without +# evaluating JSON as shell code. +while IFS= read -r -d '' key && IFS= read -r -d '' value; do case "$key" in - REPO_ROOT|DEFAULT_BRANCH|DREAM_NS|DREAM_ID|BRANCH_NAME|PARENT_BRANCH|WORKTREE_DIR|WORKTREE_BASE|BASE_COMMIT|RUN_PREFIX|SLUG) + REPO_ROOT|DEFAULT_BRANCH|DREAM_NS|DREAM_ID|BRANCH_NAME|PARENT_BRANCH|WORKTREE_DIR|WORKTREE_ROOT|WORKTREE_BASE|BASE_COMMIT|RUN_PREFIX|SLUG) printf -v "$key" '%s' "$value" export "$key" ;; @@ -172,6 +174,7 @@ while IFS=$'\t' read -r key value; do done < <(printf '%s' "$SETUP_JSON" | "$PYTHON_BIN" -c ' import json, sys data = json.load(sys.stdin) +out = sys.stdout.buffer for env_key, json_key in ( ("REPO_ROOT", "repo_root"), ("DEFAULT_BRANCH", "default_branch"), @@ -180,24 +183,27 @@ for env_key, json_key in ( ("BRANCH_NAME", "branch_name"), ("PARENT_BRANCH", "parent_branch"), ("WORKTREE_DIR", "worktree_dir"), + ("WORKTREE_ROOT", "worktree_root"), ("WORKTREE_BASE", "worktree_base"), ("BASE_COMMIT", "base_commit"), ("RUN_PREFIX", "run_prefix"), ("SLUG", "slug"), ): - print(f"{env_key}\t{data[json_key]}") + out.write(env_key.encode() + b"\0" + data[json_key].encode() + b"\0") ') # Validate: hard gate before push "$PYTHON_BIN" "$SKILL_DIR/dream-validate.py" "$DREAM_ID" "$WORKTREE_DIR" # Reconcile: merge all dream branches into main -"$PYTHON_BIN" "$SKILL_DIR/dream-reconcile.py" "$REPO_ROOT" +"$PYTHON_BIN" "$SKILL_DIR/dream-reconcile.py" "$REPO_ROOT" \ + --worktree-base "$WORKTREE_ROOT" # After `git push` succeeds, optionally clean up reconciled branches. # Cleanup REFUSES to run if `.shadow/` has uncommitted changes, or unless # HEAD is already on origin/ — so the canonical flow is: # reconcile → git add .shadow/ && git commit && git push → re-run --cleanup-branches -"$PYTHON_BIN" "$SKILL_DIR/dream-reconcile.py" "$REPO_ROOT" --cleanup-branches +"$PYTHON_BIN" "$SKILL_DIR/dream-reconcile.py" "$REPO_ROOT" \ + --worktree-base "$WORKTREE_ROOT" --cleanup-branches # Coverage: show which files still need exploration "$PYTHON_BIN" "$SKILL_DIR/dream-coverage.py" "$REPO_ROOT" @@ -540,32 +546,10 @@ SETUP_JSON="$("$PYTHON_BIN" "$SETUP_SCRIPT" --slug t01-csv-fuzzer)" || exit 1 # Compounding from prior dream: SETUP_JSON="$("$PYTHON_BIN" "$SETUP_SCRIPT" --slug t03-extend --base-branch dream//)" || exit 1 -# Parse setup JSON into shell variables used below. -while IFS=$'\t' read -r key value; do - case "$key" in - REPO_ROOT|DEFAULT_BRANCH|DREAM_NS|DREAM_ID|BRANCH_NAME|PARENT_BRANCH|WORKTREE_DIR|WORKTREE_BASE|BASE_COMMIT|RUN_PREFIX|SLUG) - printf -v "$key" '%s' "$value" - export "$key" - ;; - esac -done < <(printf '%s' "$SETUP_JSON" | "$PYTHON_BIN" -c ' -import json, sys -data = json.load(sys.stdin) -for env_key, json_key in ( - ("REPO_ROOT", "repo_root"), - ("DEFAULT_BRANCH", "default_branch"), - ("DREAM_NS", "dream_ns"), - ("DREAM_ID", "dream_id"), - ("BRANCH_NAME", "branch_name"), - ("PARENT_BRANCH", "parent_branch"), - ("WORKTREE_DIR", "worktree_dir"), - ("WORKTREE_BASE", "worktree_base"), - ("BASE_COMMIT", "base_commit"), - ("RUN_PREFIX", "run_prefix"), - ("SLUG", "slug"), -): - print(f"{env_key}\t{data[json_key]}") -') +# Use the canonical NUL-safe JSON bridge in "Helper Scripts" above. +# It exports REPO_ROOT, DEFAULT_BRANCH, DREAM_NS, DREAM_ID, BRANCH_NAME, +# PARENT_BRANCH, WORKTREE_DIR, WORKTREE_ROOT, WORKTREE_BASE, BASE_COMMIT, +# RUN_PREFIX, and SLUG. ``` Capture the JSON into `SETUP_JSON` and check the exit code (`|| exit 1`), @@ -573,7 +557,7 @@ then parse it into the exported variables above before using later steps. This prints a JSON object with keys: `repo_root`, `default_branch`, `dream_ns`, `dream_id`, `branch_name`, `parent_branch`, `worktree_dir`, -`worktree_base`, `base_commit`, `run_prefix`, `slug`. +`worktree_root`, `worktree_base`, `base_commit`, `run_prefix`, `slug`. **If `dream-setup.py` fails or is not found:** Apply the Script Failure Recovery rule (read the script source, adapt its logic). Common causes: @@ -892,7 +876,8 @@ the orchestrator discovers pushed branches from the remote. ### Worktree Cleanup ```bash -bash "$SKILL_DIR/dream-cleanup.sh" "$WORKTREE_DIR" --repo-root "$REPO_ROOT" +DREAM_WORKTREE_BASE="$WORKTREE_ROOT" \ + bash "$SKILL_DIR/dream-cleanup.sh" "$WORKTREE_DIR" --repo-root "$REPO_ROOT" ``` `dream-cleanup.sh` does the equivalent of `git worktree remove --force` @@ -969,7 +954,8 @@ for DIR in .github/skills/shadow-frog-dream .claude/skills/shadow-frog-dream; do done if [ -n "$RECONCILE_SCRIPT" ]; then - python3 "$RECONCILE_SCRIPT" "$REPO_ROOT" + python3 "$RECONCILE_SCRIPT" "$REPO_ROOT" \ + --worktree-base "$WORKTREE_ROOT" else echo "WARNING: dream-reconcile.py not found. Apply Script Failure Recovery: read dream-reconcile.py source, adapt its 9 steps manually." fi @@ -1078,7 +1064,7 @@ exited). Only run this once the agent has asserted no more dreams are starting **in this namespace**: ```bash -bash "$SKILL_DIR/dream-gc.sh" \ +DREAM_WORKTREE_BASE="$WORKTREE_ROOT" bash "$SKILL_DIR/dream-gc.sh" \ --task-complete --namespace "$DREAM_NS" \ --repo-root "$REPO_ROOT" --min-age-min 0 ``` @@ -1162,7 +1148,7 @@ are four places they get cleaned up: ```bash # At the end of the dream loop, before the final summary: - bash "$SKILL_DIR/dream-gc.sh" \ + DREAM_WORKTREE_BASE="$WORKTREE_ROOT" bash "$SKILL_DIR/dream-gc.sh" \ --task-complete \ --namespace "$DREAM_NS" \ --repo-root "$REPO_ROOT" \ From b881f7939541cc71643e3c68ecb6374e1cce6e77 Mon Sep 17 00:00:00 2001 From: Walther Maciel Date: Wed, 16 Sep 2026 15:10:51 -0700 Subject: [PATCH 09/14] fix(dream): guard Windows worktree lifecycle Canonicalize shared worktree roots and prevent the POSIX GC fallback from running on Windows, avoiding cleanup paths that cannot safely interpret Windows worktree metadata. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- skills/shadow-frog-dream/SKILL.md | 5 +- skills/shadow-frog-dream/_worktree_paths.py | 10 +++- skills/shadow-frog-dream/dream-setup.py | 3 + .../shadow_frog_dream/test_dream_setup.py | 59 +++++++++++++++++++ 4 files changed, 73 insertions(+), 4 deletions(-) diff --git a/skills/shadow-frog-dream/SKILL.md b/skills/shadow-frog-dream/SKILL.md index dd071de..6eba6d6 100644 --- a/skills/shadow-frog-dream/SKILL.md +++ b/skills/shadow-frog-dream/SKILL.md @@ -55,7 +55,8 @@ WORKTREE_DIR = $WORKTREE_BASE/dream- - `DREAM_NS` (namespace) isolates branches per task/instance. Resolved from: `DREAM_NAMESPACE` env → `TASK_INFO.json` → `.env` → repo basename. - Override only with `DREAM_WORKTREE_BASE` env var if the system temp volume is - too small. + too small. Relative overrides are resolved to an absolute path before setup + emits the lifecycle context. - `dream-setup.py` computes and enforces all paths. Use it. ### Branch Naming @@ -150,7 +151,7 @@ done | `dream-reconcile.py` | Merges dream branches into main's `.shadow/` | **Phase 6** — after all experiments done | | `dream-coverage.py` | Computes exploration coverage map | **Phase 2** — task planning for diversity | | `dream-cleanup.sh` | Safely removes ONE dream worktree (with safety gate) | **After push** — replaces the old inline cleanup snippet | -| `dream-gc.sh` | Sweeps orphan dream worktrees from `$DREAM_WORKTREE_BASE` | **Auto** — triggered by `dream-setup.py` (per-namespace throttle, default 1× / hour) in orphan-only mode; also `--task-complete --namespace "$DREAM_NS" --min-age-min 0` for end-of-session sweep of registered-but-stale dirs | +| `dream-gc.sh` | Sweeps orphan dream worktrees from `$DREAM_WORKTREE_BASE` | **Auto** — triggered by `dream-setup.py` (per-namespace throttle, default 1× / hour) in orphan-only mode; also `--task-complete --namespace "$DREAM_NS" --min-age-min 0` for end-of-session sweep of registered-but-stale dirs. On Windows, automatic sweeping remains disabled until `dream-gc.py` replaces the POSIX-only helper. | **Usage patterns:** diff --git a/skills/shadow-frog-dream/_worktree_paths.py b/skills/shadow-frog-dream/_worktree_paths.py index 0247f47..5673b57 100644 --- a/skills/shadow-frog-dream/_worktree_paths.py +++ b/skills/shadow-frog-dream/_worktree_paths.py @@ -4,7 +4,13 @@ def resolve_worktree_root(override=None): - """Return the configured root or the platform's default dream root.""" - return override or os.environ.get("DREAM_WORKTREE_BASE") or os.path.join( + """Return the absolute, symlink-resolved configured dream worktree root.""" + root = override or os.environ.get("DREAM_WORKTREE_BASE") or os.path.join( tempfile.gettempdir(), "shadowfrog-dreams" ) + return os.path.realpath(os.path.abspath(root)) + + +def canonical_worktree_path(path): + """Return a path suitable for identity comparison on every supported OS.""" + return os.path.normcase(os.path.realpath(os.path.abspath(path))) diff --git a/skills/shadow-frog-dream/dream-setup.py b/skills/shadow-frog-dream/dream-setup.py index cb69655..faf2381 100644 --- a/skills/shadow-frog-dream/dream-setup.py +++ b/skills/shadow-frog-dream/dream-setup.py @@ -179,6 +179,9 @@ def _maybe_auto_gc(repo_root, worktree_base, worktree_root=None): gc_cmd = [sys.executable, gc_py, "--repo-root", repo_root, "--quiet", "--min-age-min", age_raw] elif os.path.isfile(gc_sh): + if os.name == "nt": + _err("WARN: auto-GC skipped on Windows until dream-gc.py is available") + return gc_cmd = ["bash", gc_sh, "--repo-root", repo_root, "--quiet", "--min-age-min", age_raw] else: diff --git a/tests/skills/shadow_frog_dream/test_dream_setup.py b/tests/skills/shadow_frog_dream/test_dream_setup.py index 2c519e7..a1a44bd 100644 --- a/tests/skills/shadow_frog_dream/test_dream_setup.py +++ b/tests/skills/shadow_frog_dream/test_dream_setup.py @@ -21,6 +21,7 @@ REPO_ROOT = Path(__file__).resolve().parent.parent.parent.parent DREAM_SETUP = REPO_ROOT / "skills" / "shadow-frog-dream" / "dream-setup.py" +DREAM_RECONCILE = REPO_ROOT / "skills" / "shadow-frog-dream" / "dream-reconcile.py" # Cleared from the base env so a developer's shell can't leak into namespace / # worktree resolution; each test sets exactly what it needs via `extras`. @@ -80,6 +81,14 @@ def _load_dream_setup_module(): return module +def _load_dream_reconcile_module(): + spec = importlib.util.spec_from_file_location("dream_reconcile", DREAM_RECONCILE) + module = importlib.util.module_from_spec(spec) + assert spec.loader is not None + spec.loader.exec_module(module) + return module + + def _plant_orphan(base: Path, ns: str, name: str = "dream-orphan") -> Path: """Plant an orphan worktree (broken .git pointer, ancient mtime).""" d = base / ns / name @@ -378,6 +387,55 @@ def test_env_namespace_used(self, tmp_path): data = json.loads(result.stdout) assert data["dream_ns"] == "env-ns-test" + def test_quoted_dotenv_namespace_used(self, tmp_path): + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + (repo / ".env").write_text( + 'DREAM_NAMESPACE="quoted-ns"\n', encoding="utf-8", + ) + + result = run_dream_setup( + ["--slug", "t08-quoted-dotenv", "--repo-root", str(repo), + "--dry-run", "--print-json"], + cwd=repo, + ) + + assert result.returncode == 0, f"stderr: {result.stderr}" + assert json.loads(result.stdout)["dream_ns"] == "quoted-ns" + + def test_setup_namespace_is_explicitly_usable_by_reconciliation(self, tmp_path): + """The namespace emitted by setup must select its remote dream branch.""" + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + setup = run_dream_setup( + ["--slug", "t08-reconcile", "--namespace", "setup-ns", + "--repo-root", str(repo), "--print-json"], + cwd=repo, + env_extra={"DREAM_WORKTREE_BASE": str(tmp_path / "worktrees")}, + ) + assert setup.returncode == 0, f"stderr: {setup.stderr}" + context = json.loads(setup.stdout) + + env = _base_env(repo) + subprocess.run( + ["git", "push", "-q", "origin", context["branch_name"]], + cwd=repo, check=True, env=env, + ) + reconcile = subprocess.run( + [ + sys.executable, str(DREAM_RECONCILE), str(repo), + "--namespace", context["dream_ns"], "--dry-run", + ], + capture_output=True, text=True, encoding="utf-8", + cwd=repo, + env=_base_env(repo, {"DREAM_NAMESPACE": "wrong-ns"}), + ) + + assert reconcile.returncode == 0, reconcile.stdout + reconcile.stderr + assert context["branch_name"] in reconcile.stdout + def test_task_info_namespace_must_be_string(self, tmp_path): repo = tmp_path / "repo" repo.mkdir() @@ -669,6 +727,7 @@ def test_auto_gc_nonzero_result_does_not_touch_tombstone( tombstone = worktree_base / ".last-gc" monkeypatch.setattr(dream_setup, "SCRIPT_DIR", str(script_dir)) + monkeypatch.setattr(dream_setup.os, "name", "posix") monkeypatch.setattr( dream_setup.subprocess, "run", From 8edba43c00ab3a145a8758d633029701e83db9b1 Mon Sep 17 00:00:00 2001 From: Walther Maciel Date: Wed, 16 Sep 2026 15:11:36 -0700 Subject: [PATCH 10/14] fix(dream): scope reconciliation cleanup by namespace Resolve namespaces consistently across setup and reconciliation, propagate the setup namespace explicitly, and prevent cleanup from touching indexed branches outside the selected namespace. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- claude.md | 2 + skills/shadow-frog-dream/SKILL.md | 26 ++-- skills/shadow-frog-dream/_dream_namespace.py | 64 +++++++++ skills/shadow-frog-dream/dream-reconcile.py | 25 +++- skills/shadow-frog-dream/dream-setup.py | 54 ++------ .../shadow_frog_dream/test_dream_reconcile.py | 129 +++++++++++++++++- 6 files changed, 237 insertions(+), 63 deletions(-) create mode 100644 skills/shadow-frog-dream/_dream_namespace.py diff --git a/claude.md b/claude.md index 816c754..d217e75 100644 --- a/claude.md +++ b/claude.md @@ -22,6 +22,8 @@ ShadowFrog/ dream-coverage.py Exploration coverage map dream-cleanup.sh Safe per-worktree cleanup (replaces inline snippet) dream-gc.sh Orphan-worktree sweep (defense-in-depth) + _dream_namespace.py Shared setup/reconcile namespace resolution + _worktree_paths.py Shared worktree-root and path identity helpers _worktree_safety.py Shared safety gate for rm-rf paths shadow-frog-meditate/SKILL.md Dedup, merge, and resolve conflicting discoveries shadow-frog-viewer/ Browse and query the shadow knowledge base diff --git a/skills/shadow-frog-dream/SKILL.md b/skills/shadow-frog-dream/SKILL.md index 6eba6d6..c07bf88 100644 --- a/skills/shadow-frog-dream/SKILL.md +++ b/skills/shadow-frog-dream/SKILL.md @@ -198,13 +198,13 @@ for env_key, json_key in ( # Reconcile: merge all dream branches into main "$PYTHON_BIN" "$SKILL_DIR/dream-reconcile.py" "$REPO_ROOT" \ - --worktree-base "$WORKTREE_ROOT" + --namespace "$DREAM_NS" --worktree-base "$WORKTREE_ROOT" # After `git push` succeeds, optionally clean up reconciled branches. # Cleanup REFUSES to run if `.shadow/` has uncommitted changes, or unless # HEAD is already on origin/ — so the canonical flow is: # reconcile → git add .shadow/ && git commit && git push → re-run --cleanup-branches "$PYTHON_BIN" "$SKILL_DIR/dream-reconcile.py" "$REPO_ROOT" \ - --worktree-base "$WORKTREE_ROOT" --cleanup-branches + --namespace "$DREAM_NS" --worktree-base "$WORKTREE_ROOT" --cleanup-branches # Coverage: show which files still need exploration "$PYTHON_BIN" "$SKILL_DIR/dream-coverage.py" "$REPO_ROOT" @@ -224,17 +224,15 @@ inline fallback instructions). REPO_ROOT=$(git rev-parse --show-toplevel) cd "$REPO_ROOT" -# 0. Auto-detect DREAM_NAMESPACE -if [ -z "${DREAM_NAMESPACE:-}" ]; then - if [ -f TASK_INFO.json ]; then - DREAM_NAMESPACE=$(python3 -c "import json; print(json.load(open('TASK_INFO.json')).get('dream_namespace',''))" 2>/dev/null) - export DREAM_NAMESPACE - elif [ -f .env ]; then - DREAM_NAMESPACE=$(grep '^DREAM_NAMESPACE=' .env | head -1 | cut -d'=' -f2-) - export DREAM_NAMESPACE - fi -fi -[ -n "${DREAM_NAMESPACE:-}" ] && echo "Dream namespace: $DREAM_NAMESPACE" +# 0. Resolve DREAM_NAMESPACE with the same parser setup/reconciliation use. +DREAM_NAMESPACE="$("$PYTHON_BIN" -c ' +import sys +sys.path.insert(0, sys.argv[1]) +from _dream_namespace import resolve_dream_namespace +print(resolve_dream_namespace(sys.argv[2])) +' "$SKILL_DIR" "$REPO_ROOT")" || exit 1 +export DREAM_NAMESPACE +echo "Dream namespace: $DREAM_NAMESPACE" # 1. Detect default branch DEFAULT_BRANCH=$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's|refs/remotes/origin/||') @@ -956,7 +954,7 @@ done if [ -n "$RECONCILE_SCRIPT" ]; then python3 "$RECONCILE_SCRIPT" "$REPO_ROOT" \ - --worktree-base "$WORKTREE_ROOT" + --namespace "$DREAM_NS" --worktree-base "$WORKTREE_ROOT" else echo "WARNING: dream-reconcile.py not found. Apply Script Failure Recovery: read dream-reconcile.py source, adapt its 9 steps manually." fi diff --git a/skills/shadow-frog-dream/_dream_namespace.py b/skills/shadow-frog-dream/_dream_namespace.py new file mode 100644 index 0000000..a5ee8cd --- /dev/null +++ b/skills/shadow-frog-dream/_dream_namespace.py @@ -0,0 +1,64 @@ +"""Shared dream namespace resolution for setup and reconciliation.""" +import json +import os + + +class NamespaceConfigurationError(ValueError): + """Raised when a namespace source exists but cannot be used safely.""" + + +def _read_env_namespace(path): + """Read the first DREAM_NAMESPACE value, stripping matching quotes.""" + try: + with open(path, encoding="utf-8") as f: + for line in f: + if line.startswith("DREAM_NAMESPACE="): + value = line.split("=", 1)[1].strip() + if len(value) >= 2 and value[0] in "\"'" and value[-1] == value[0]: + value = value[1:-1] + return value.strip() + except OSError as exc: + raise NamespaceConfigurationError( + f"could not read {os.path.basename(path)}: {exc}" + ) from exc + return "" + + +def resolve_dream_namespace(repo_root, override=None, environ=None): + """Resolve namespace with one precedence/order contract for all callers.""" + if override: + return override + + environ = os.environ if environ is None else environ + if environ.get("DREAM_NAMESPACE"): + return environ["DREAM_NAMESPACE"] + + task_info_path = os.path.join(repo_root, "TASK_INFO.json") + if os.path.isfile(task_info_path): + try: + with open(task_info_path, encoding="utf-8") as f: + task_info = json.load(f) + except (OSError, json.JSONDecodeError) as exc: + raise NamespaceConfigurationError( + f"could not parse TASK_INFO.json: {exc}" + ) from exc + if not isinstance(task_info, dict): + raise NamespaceConfigurationError( + "TASK_INFO.json must contain a JSON object" + ) + if "dream_namespace" in task_info: + task_namespace = task_info["dream_namespace"] + if not isinstance(task_namespace, str): + raise NamespaceConfigurationError( + "TASK_INFO.json dream_namespace must be a string" + ) + if task_namespace: + return task_namespace + + env_file = os.path.join(repo_root, ".env") + if os.path.isfile(env_file): + namespace = _read_env_namespace(env_file) + if namespace: + return namespace + + return os.path.basename(repo_root) diff --git a/skills/shadow-frog-dream/dream-reconcile.py b/skills/shadow-frog-dream/dream-reconcile.py index 302620f..3ce1c9d 100755 --- a/skills/shadow-frog-dream/dream-reconcile.py +++ b/skills/shadow-frog-dream/dream-reconcile.py @@ -218,8 +218,8 @@ def _read_indexed_dream_ids(repo_root): return existing -def _read_indexed_branches(repo_root): - """Return list of (branch, dream_id) tuples from _index.md (column 5).""" +def _read_indexed_branches(repo_root, dream_ns=None): + """Return indexed (branch, dream_id) tuples, optionally namespace-filtered.""" index_path = os.path.join(repo_root, '.shadow', '_dreams', '_index.md') rows = [] if not os.path.isfile(index_path): @@ -232,7 +232,9 @@ def _read_indexed_branches(repo_root): if len(parts) >= 6 and parts[1]: dream_id = parts[1] branch = parts[5] - if branch: + if branch and ( + dream_ns is None or branch.startswith(f"dream/{dream_ns}/") + ): rows.append((branch, dream_id)) return rows @@ -1368,8 +1370,13 @@ def cleanup_branches(repo_root, manifests, dream_ns, dry_run=False, deleted = 0 kept = 0 + namespace_prefix = f"dream/{dream_ns}/" for branch, dream_id, manifest in manifests: + if not branch.startswith(namespace_prefix): + print(f" ⚠️ KEEPING {branch} — outside namespace {dream_ns}") + kept += 1 + continue dream_dir = os.path.join(repo_root, '.shadow', '_dreams', dream_id) # Safety check 1: all artifacts on main @@ -1486,10 +1493,16 @@ def _slug_from_dream_id(dream_id): return m.group(2) if m else None +@dataclass(frozen=True) +class WorktreeRegistration: + """The registration state for one candidate worktree path.""" + + state: str + branch: str | None = None + + def _registered_worktree_branch(repo_root, candidate_path): - """Return the branch name (without `refs/heads/` prefix) that git has - registered at `candidate_path`, or `None` if no worktree is registered - at that path (or git can't tell). Detached-HEAD worktrees return `None`. + """Return a registration result for `candidate_path`. Parses `git worktree list --porcelain` output: worktree /abs/path diff --git a/skills/shadow-frog-dream/dream-setup.py b/skills/shadow-frog-dream/dream-setup.py index faf2381..089fd9c 100644 --- a/skills/shadow-frog-dream/dream-setup.py +++ b/skills/shadow-frog-dream/dream-setup.py @@ -103,22 +103,6 @@ def _import_paths(): sys.path.pop(0) -def _read_env_namespace(path): - """Extract DREAM_NAMESPACE from a `.env` file (first match), stripping - surrounding whitespace and one layer of matching quotes.""" - try: - with open(path, encoding="utf-8") as f: - for line in f: - if line.startswith("DREAM_NAMESPACE="): - val = line.split("=", 1)[1].strip() - if len(val) >= 2 and val[0] in "\"'" and val[-1] == val[0]: - val = val[1:-1] - return val.strip() - except OSError: - return "" - return "" - - def _val(argv, i, flag): if i + 1 >= len(argv): _err(f"ERROR: {flag} requires a value") @@ -313,30 +297,20 @@ def main(): sys.exit(1) # --- Resolve namespace --- - dream_ns = "" - if opts["namespace"]: - dream_ns = opts["namespace"] - elif os.environ.get("DREAM_NAMESPACE"): - dream_ns = os.environ["DREAM_NAMESPACE"] - elif os.path.isfile("TASK_INFO.json"): - try: - with open("TASK_INFO.json", encoding="utf-8") as f: - task_info = json.load(f) - if not isinstance(task_info, dict): - _err("ERROR: TASK_INFO.json must contain a JSON object") - sys.exit(1) - task_ns = task_info.get("dream_namespace", "") - if task_ns: - if not isinstance(task_ns, str): - _err("ERROR: TASK_INFO.json dream_namespace must be a string") - sys.exit(1) - dream_ns = task_ns - except (OSError, ValueError): - dream_ns = "" - elif os.path.isfile(".env"): - dream_ns = _read_env_namespace(".env") - if not dream_ns: - dream_ns = os.path.basename(repo_root) + sys.path.insert(0, SCRIPT_DIR) + try: + from _dream_namespace import ( + NamespaceConfigurationError, + resolve_dream_namespace, + ) + finally: + if sys.path and sys.path[0] == SCRIPT_DIR: + sys.path.pop(0) + try: + dream_ns = resolve_dream_namespace(repo_root, opts["namespace"]) + except NamespaceConfigurationError as exc: + _err(f"ERROR: {exc}") + sys.exit(1) if not SAFE_RE.match(dream_ns): _err(f"ERROR: Resolved DREAM_NS contains unsafe characters: {dream_ns}") diff --git a/tests/skills/shadow_frog_dream/test_dream_reconcile.py b/tests/skills/shadow_frog_dream/test_dream_reconcile.py index b4ed9c7..746842e 100644 --- a/tests/skills/shadow_frog_dream/test_dream_reconcile.py +++ b/tests/skills/shadow_frog_dream/test_dream_reconcile.py @@ -642,6 +642,21 @@ def test_read_indexed_branches_parses_table(dream_reconcile, tmp_path): assert len(rows) == 3 +def test_read_indexed_branches_filters_namespace(dream_reconcile, tmp_path): + _write_index( + tmp_path, + INDEX_FIXTURE.replace( + "dream/p/20260103-000000Z-gamma", + "dream/other/20260103-000000Z-gamma", + ), + ) + + rows = dream_reconcile._read_indexed_branches(str(tmp_path), "p") + + assert len(rows) == 2 + assert all(branch.startswith("dream/p/") for branch, _ in rows) + + # =========================================================================== # verify_reconciliation — PREFIX FALSE-PASS regression # =========================================================================== @@ -2273,6 +2288,27 @@ def reject_local_delete(args, **kwargs): assert f"Failed to delete local {branch}: branch is in use" in capsys.readouterr().out +@pytest.mark.slow +def test_cleanup_branches_keeps_manifest_outside_namespace( + dream_reconcile, tmp_git_repo, capsys +): + env = _seed_repo(tmp_git_repo) + _add_bare_remote(tmp_git_repo, env) + dream_id = "20260420-050075Z-outside" + branch = f"dream/other/{dream_id}" + + deleted, kept = dream_reconcile.cleanup_branches( + str(tmp_git_repo), + [(branch, dream_id, _default_manifest(dream_id))], + "proj", + dry_run=True, + ) + + assert deleted == 0 + assert kept == 1 + assert f"KEEPING {branch} — outside namespace proj" in capsys.readouterr().out + + @pytest.mark.slow def test_cleanup_branches_refuses_when_head_not_pushed( dream_reconcile, tmp_git_repo @@ -2727,6 +2763,47 @@ def test_cli_namespace_from_dotenv_file(tmp_git_repo): assert "Namespace: fromdotenv" in result.stdout +@pytest.mark.slow +@pytest.mark.integration +def test_cli_namespace_strips_dotenv_quotes(tmp_git_repo): + """Setup and reconciliation must resolve quoted .env values identically.""" + env = _seed_repo(tmp_git_repo) + _add_bare_remote(tmp_git_repo, env) + (tmp_git_repo / ".env").write_text( + 'DREAM_NAMESPACE="fromdotenv"\n', encoding="utf-8" + ) + full_env = _cli_env() + full_env.pop("DREAM_NAMESPACE", None) + + result = subprocess.run( + [sys.executable, str(SCRIPT), str(tmp_git_repo), "--dry-run"], + capture_output=True, text=True, env=full_env, encoding="utf-8", + ) + + assert result.returncode == 0, result.stdout + result.stderr + assert "Namespace: fromdotenv" in result.stdout + + +@pytest.mark.slow +@pytest.mark.integration +@pytest.mark.parametrize("task_info", ["[]", '"not an object"', '{"dream_namespace": 1}']) +def test_cli_rejects_invalid_task_info_namespace(tmp_git_repo, task_info): + """Setup and reconciliation must reject invalid task configuration alike.""" + env = _seed_repo(tmp_git_repo) + _add_bare_remote(tmp_git_repo, env) + (tmp_git_repo / "TASK_INFO.json").write_text(task_info, encoding="utf-8") + full_env = _cli_env() + full_env.pop("DREAM_NAMESPACE", None) + + result = subprocess.run( + [sys.executable, str(SCRIPT), str(tmp_git_repo), "--dry-run"], + capture_output=True, text=True, env=full_env, encoding="utf-8", + ) + + assert result.returncode == 1 + assert "ERROR:" in result.stderr + + @pytest.mark.slow @pytest.mark.integration def test_cli_namespace_from_task_info_json(tmp_git_repo): @@ -2867,6 +2944,54 @@ def test_cli_cleanup_branches_after_reconcile(tmp_git_repo): assert branch not in post +@pytest.mark.slow +@pytest.mark.integration +def test_cli_cleanup_branches_never_deletes_other_namespace(tmp_git_repo): + """Review 02: post-push cleanup is scoped to the requested namespace.""" + env = _seed_repo(tmp_git_repo) + _add_bare_remote(tmp_git_repo, env) + inside_id = "20260420-065100Z-inside" + outside_id = "20260420-065200Z-outside" + inside_branch = make_dream_branch( + tmp_git_repo, env, "inside", inside_id, _default_manifest(inside_id) + ) + outside_branch = make_dream_branch( + tmp_git_repo, env, "outside", outside_id, _default_manifest(outside_id) + ) + _seed_dream_artifacts(tmp_git_repo, inside_id) + _seed_dream_artifacts(tmp_git_repo, outside_id) + _write_index( + tmp_git_repo, + "# Dream Experiments\n\n" + "| dream_id | category | verdict | title | branch | parent | tip_commit |\n" + "|----------|----------|---------|-------|--------|--------|------------|\n" + f"| {inside_id} | test | useful | Inside | {inside_branch} | main | deadbeef |\n" + f"| {outside_id} | test | useful | Outside | {outside_branch} | main | deadbeef |\n", + ) + _git("add", "-A", cwd=tmp_git_repo, env=env) + _git("commit", "-q", "-m", "reconcile", cwd=tmp_git_repo, env=env) + _git("push", "-q", "origin", "main", cwd=tmp_git_repo, env=env) + + result = subprocess.run( + [ + sys.executable, str(SCRIPT), str(tmp_git_repo), + "--namespace", "inside", "--cleanup-branches", + ], + capture_output=True, text=True, env=_cli_env(), encoding="utf-8", + ) + + assert result.returncode == 0, result.stdout + result.stderr + assert "Deleted: 1, Kept: 0" in result.stdout + assert not _git( + "ls-remote", "--heads", "origin", inside_branch, + cwd=tmp_git_repo, env=env, + ).stdout.strip() + assert outside_branch in _git( + "ls-remote", "--heads", "origin", outside_branch, + cwd=tmp_git_repo, env=env, + ).stdout + + @pytest.mark.slow @pytest.mark.integration def test_cli_cleanup_no_new_no_indexed_exits_zero(tmp_git_repo): @@ -3341,9 +3466,7 @@ def test_cleanup_branches_worktree_gc_skips_unparseable_dream_id( class TestRegisteredWorktreeBranch: - """Unit-level coverage of _registered_worktree_branch — the helper - that lets the GC distinguish 'our worktree' from 'someone else's - worktree at the same shared path'.""" + """Coverage of registration states used by fail-closed worktree cleanup.""" @pytest.mark.slow def test_returns_branch_for_registered_path( From 4abbfe2411ce16bf246bbe6b8d3785c290dec699 Mon Sep 17 00:00:00 2001 From: Walther Maciel Date: Wed, 16 Sep 2026 15:11:45 -0700 Subject: [PATCH 11/14] fix(dream): fail closed on uncertain worktree ownership Preserve slug-colliding worktrees whenever Git reports a detached or indeterminate registration, and normalize worktree identity for Windows path casing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- skills/shadow-frog-dream/dream-reconcile.py | 83 ++++++++------ .../shadow_frog_dream/test_dream_reconcile.py | 104 +++++++++++++++++- 2 files changed, 149 insertions(+), 38 deletions(-) diff --git a/skills/shadow-frog-dream/dream-reconcile.py b/skills/shadow-frog-dream/dream-reconcile.py index 3ce1c9d..9328e23 100755 --- a/skills/shadow-frog-dream/dream-reconcile.py +++ b/skills/shadow-frog-dream/dream-reconcile.py @@ -37,6 +37,7 @@ import shutil import subprocess import sys +from dataclasses import dataclass from datetime import datetime, timezone from pathlib import Path @@ -1513,28 +1514,37 @@ def _registered_worktree_branch(repo_root, candidate_path): HEAD detached - Paths are compared after `realpath` so macOS `/tmp` ↔ `/private/tmp` and - other symlinked-base setups don't break the match. + Paths are compared after normalized realpath resolution so macOS + `/tmp` ↔ `/private/tmp` and Windows case-only differences do not break + the match. Detached and indeterminate results are intentionally distinct + from an unregistered path so deletion can fail closed. """ + script_dir = os.path.dirname(os.path.abspath(__file__)) try: + sys.path.insert(0, script_dir) + try: + from _worktree_paths import canonical_worktree_path + finally: + if sys.path and sys.path[0] == script_dir: + sys.path.pop(0) result = subprocess.run( ['git', 'worktree', 'list', '--porcelain'], capture_output=True, text=True, cwd=repo_root, timeout=10, encoding="utf-8", ) except (OSError, subprocess.SubprocessError): - return None + return WorktreeRegistration("indeterminate") if result.returncode != 0: - return None + return WorktreeRegistration("indeterminate") try: - target = os.path.realpath(candidate_path) + target = canonical_worktree_path(candidate_path) except (OSError, ValueError): - return None + return WorktreeRegistration("indeterminate") def _matches(p): try: - return os.path.realpath(p) == target + return canonical_worktree_path(p) == target except (OSError, ValueError): return False @@ -1544,15 +1554,19 @@ def _matches(p): if line.startswith('worktree '): # Flush previous entry if it matched. if cur_path is not None and _matches(cur_path): - return cur_branch + return WorktreeRegistration( + "attached" if cur_branch else "detached", cur_branch + ) cur_path = line[len('worktree '):] cur_branch = None elif line.startswith('branch '): ref = line[len('branch '):] cur_branch = ref[len('refs/heads/'):] if ref.startswith('refs/heads/') else ref if cur_path is not None and _matches(cur_path): - return cur_branch - return None + return WorktreeRegistration( + "attached" if cur_branch else "detached", cur_branch + ) + return WorktreeRegistration("unregistered") def _gc_worktree_after_merge(repo_root, dream_ns, dream_id, deleted_branch=None, @@ -1614,10 +1628,18 @@ def _gc_worktree_after_merge(repo_root, dream_ns, dream_id, deleted_branch=None, # the original behavior — still gated by the safety check above. if deleted_branch: registered = _registered_worktree_branch(repo_root, str(resolved)) - if registered is not None and registered != deleted_branch: + if registered.state == "attached" and registered.branch == deleted_branch: + pass + elif registered.state == "unregistered": + pass + else: + if registered.state == "attached": + detail = f"now belongs to {registered.branch}" + else: + detail = f"has {registered.state} ownership" print( f" ↳ Skipping worktree GC for {dream_id}: " - f"path {resolved} now belongs to {registered}" + f"path {resolved} {detail}" ) return @@ -1708,25 +1730,22 @@ def main(): print("ERROR: Not in a git repository", file=sys.stderr) sys.exit(1) - # Resolve namespace - dream_ns = namespace_override or os.environ.get('DREAM_NAMESPACE', '') - if not dream_ns: - task_info = os.path.join(repo_root, 'TASK_INFO.json') - if os.path.isfile(task_info): - try: - dream_ns = json.load(open(task_info, encoding="utf-8")).get('dream_namespace', '') - except (json.JSONDecodeError, OSError): - pass - if not dream_ns: - env_file = os.path.join(repo_root, '.env') - if os.path.isfile(env_file): - with open(env_file, encoding="utf-8") as f: - for line in f: - if line.startswith('DREAM_NAMESPACE='): - dream_ns = line.split('=', 1)[1].strip() - break - if not dream_ns: - dream_ns = os.path.basename(repo_root) + # Resolve namespace using the same precedence and parsing as setup. + script_dir = os.path.dirname(os.path.abspath(__file__)) + try: + sys.path.insert(0, script_dir) + try: + from _dream_namespace import ( + NamespaceConfigurationError, + resolve_dream_namespace, + ) + finally: + if sys.path and sys.path[0] == script_dir: + sys.path.pop(0) + dream_ns = resolve_dream_namespace(repo_root, namespace_override) + except (ImportError, NamespaceConfigurationError) as exc: + print(f"ERROR: {exc}", file=sys.stderr) + sys.exit(1) print(f"=== Dream Reconciliation ===") print(f"Repo: {repo_root}") @@ -1778,7 +1797,7 @@ def main(): # the branches already in the index. This is the canonical post-push # flow: reconcile → push → re-run with --cleanup-branches. if cleanup: - indexed = _read_indexed_branches(repo_root) + indexed = _read_indexed_branches(repo_root, dream_ns) if not indexed: print(" Nothing in _index.md to clean up either.") sys.exit(0) diff --git a/tests/skills/shadow_frog_dream/test_dream_reconcile.py b/tests/skills/shadow_frog_dream/test_dream_reconcile.py index 746842e..334b34f 100644 --- a/tests/skills/shadow_frog_dream/test_dream_reconcile.py +++ b/tests/skills/shadow_frog_dream/test_dream_reconcile.py @@ -3476,10 +3476,12 @@ def test_returns_branch_for_registered_path( wt = tmp_path / "wt" _git("worktree", "add", "-q", "-b", "feature-x", str(wt), cwd=tmp_git_repo, env=env) - branch = dream_reconcile._registered_worktree_branch( + registration = dream_reconcile._registered_worktree_branch( str(tmp_git_repo), str(wt) ) - assert branch == "feature-x" + assert registration == dream_reconcile.WorktreeRegistration( + "attached", "feature-x" + ) @pytest.mark.slow def test_returns_none_for_unregistered_path( @@ -3487,10 +3489,10 @@ def test_returns_none_for_unregistered_path( ): _seed_repo(tmp_git_repo) # A path that's not registered as a worktree at all. - branch = dream_reconcile._registered_worktree_branch( + registration = dream_reconcile._registered_worktree_branch( str(tmp_git_repo), str(tmp_path / "nope") ) - assert branch is None + assert registration == dream_reconcile.WorktreeRegistration("unregistered") @pytest.mark.slow def test_matches_through_symlink( @@ -3504,10 +3506,100 @@ def test_matches_through_symlink( _git("worktree", "add", "-q", "-b", "feature-y", str(real_wt), cwd=tmp_git_repo, env=env) make_symlink(link_wt, real_wt) - branch_via_link = dream_reconcile._registered_worktree_branch( + registration = dream_reconcile._registered_worktree_branch( str(tmp_git_repo), str(link_wt) ) - assert branch_via_link == "feature-y" + assert registration == dream_reconcile.WorktreeRegistration( + "attached", "feature-y" + ) + + @pytest.mark.slow + def test_returns_detached_for_detached_worktree( + self, dream_reconcile, tmp_git_repo, tmp_path + ): + env = _seed_repo(tmp_git_repo) + wt = tmp_path / "detached-wt" + _git("worktree", "add", "-q", "-b", "feature-detached", str(wt), + cwd=tmp_git_repo, env=env) + _git("checkout", "-q", "--detach", cwd=wt, env=env) + + registration = dream_reconcile._registered_worktree_branch( + str(tmp_git_repo), str(wt) + ) + + assert registration == dream_reconcile.WorktreeRegistration("detached") + + @pytest.mark.slow + def test_gc_preserves_detached_worktree(self, dream_reconcile, tmp_git_repo, + tmp_path, monkeypatch, capsys): + """Review 02: detached ownership must fail closed, not force-remove.""" + env = _seed_repo(tmp_git_repo) + base = tmp_path / "wt-base" + worktree = base / "proj" / "dream-same" + worktree.parent.mkdir(parents=True) + branch = "dream/proj/20260420-110000Z-same" + _git("worktree", "add", "-q", "-b", branch, str(worktree), + cwd=tmp_git_repo, env=env) + _git("checkout", "-q", "--detach", cwd=worktree, env=env) + marker = worktree / "uncommitted-work.txt" + marker.write_text("preserve me\n", encoding="utf-8") + monkeypatch.setenv("DREAM_WORKTREE_BASE", str(base)) + + dream_reconcile._gc_worktree_after_merge( + str(tmp_git_repo), + "proj", + "20260420-100000Z-same", + "dream/proj/20260420-100000Z-same", + ) + + assert marker.read_text(encoding="utf-8") == "preserve me\n" + assert "detached ownership" in capsys.readouterr().out + + def test_returns_indeterminate_when_git_query_fails( + self, dream_reconcile, tmp_git_repo, tmp_path, monkeypatch + ): + _seed_repo(tmp_git_repo) + original_run = dream_reconcile.subprocess.run + + def failed_query(args, **kwargs): + if args == ["git", "worktree", "list", "--porcelain"]: + return subprocess.CompletedProcess(args, 1, "", "git failed") + return original_run(args, **kwargs) + + monkeypatch.setattr(dream_reconcile.subprocess, "run", failed_query) + registration = dream_reconcile._registered_worktree_branch( + str(tmp_git_repo), str(tmp_path / "candidate") + ) + + assert registration == dream_reconcile.WorktreeRegistration("indeterminate") + + def test_matches_case_insensitively_when_platform_requires_it( + self, dream_reconcile, monkeypatch + ): + result = subprocess.CompletedProcess( + ["git", "worktree", "list", "--porcelain"], + 0, + "worktree C:\\Dreams\\Dream-Same\n" + "HEAD deadbeef\n" + "branch refs/heads/dream/proj/20260420-110000Z-same\n", + "", + ) + monkeypatch.setattr( + dream_reconcile.subprocess, "run", lambda *args, **kwargs: result + ) + monkeypatch.setattr(dream_reconcile.os.path, "abspath", lambda path: path) + monkeypatch.setattr(dream_reconcile.os.path, "realpath", lambda path: path) + monkeypatch.setattr( + dream_reconcile.os.path, "normcase", lambda path: path.lower() + ) + + registration = dream_reconcile._registered_worktree_branch( + "repo", "c:\\dreams\\dream-same" + ) + + assert registration == dream_reconcile.WorktreeRegistration( + "attached", "dream/proj/20260420-110000Z-same" + ) @pytest.mark.slow From 56ad7707b6c19e6fc330fb203f2977f3d0df76ef Mon Sep 17 00:00:00 2001 From: Walther Maciel Date: Wed, 16 Sep 2026 15:11:52 -0700 Subject: [PATCH 12/14] test(dream): cover setup root cleanup lifecycle Exercise setup JSON root propagation through reconciler cleanup with a real worktree and bare remote. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../shadow_frog_dream/test_dream_setup.py | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/tests/skills/shadow_frog_dream/test_dream_setup.py b/tests/skills/shadow_frog_dream/test_dream_setup.py index a1a44bd..c89db88 100644 --- a/tests/skills/shadow_frog_dream/test_dream_setup.py +++ b/tests/skills/shadow_frog_dream/test_dream_setup.py @@ -251,6 +251,89 @@ def test_uses_system_temp_root_without_override(self, tmp_path): assert Path(data["worktree_base"]) == expected_root / repo.name assert Path(data["worktree_dir"]) == expected_root / repo.name / "dream-t02-default-root" + def test_relative_worktree_root_is_emitted_as_absolute(self, tmp_path): + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + + result = run_dream_setup( + ["--slug", "t02-relative-root", "--repo-root", str(repo), "--print-json"], + cwd=repo, + env_extra={ + "DREAM_GC_AUTO": "0", + "DREAM_WORKTREE_BASE": "../dream-worktrees", + }, + ) + + assert result.returncode == 0, f"stderr: {result.stderr}" + data = json.loads(result.stdout) + expected_root = (repo / ".." / "dream-worktrees").resolve() + assert Path(data["worktree_root"]) == expected_root + assert Path(data["worktree_dir"]) == expected_root / repo.name / "dream-t02-relative-root" + + def test_setup_context_drives_reconciler_cleanup_at_same_root(self, tmp_path): + """Review 01: setup JSON must identify the exact worktree cleanup removes.""" + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + setup = run_dream_setup( + ["--slug", "t02-lifecycle", "--repo-root", str(repo), "--print-json"], + cwd=repo, + env_extra={ + "DREAM_GC_AUTO": "0", + "DREAM_WORKTREE_BASE": "../dream-worktrees", + }, + ) + assert setup.returncode == 0, f"stderr: {setup.stderr}" + context = json.loads(setup.stdout) + worktree_dir = Path(context["worktree_dir"]) + assert worktree_dir.is_dir() + + dream_dir = repo / ".shadow" / "_dreams" / context["dream_id"] + dream_dir.mkdir(parents=True) + for name in ("report.md", "manifest.json", "patch.diff"): + (dream_dir / name).write_text("{}\n", encoding="utf-8") + index = repo / ".shadow" / "_dreams" / "_index.md" + index.write_text( + "# Dream Experiments\n\n" + "| dream_id | category | verdict | title | branch | parent | tip_commit |\n" + "|----------|----------|---------|-------|--------|--------|------------|\n" + f"| {context['dream_id']} | test | useful | Test | " + f"{context['branch_name']} | main | deadbeef |\n", + encoding="utf-8", + ) + env = _base_env(repo) + subprocess.run(["git", "add", ".shadow"], cwd=repo, check=True, env=env) + subprocess.run( + ["git", "commit", "-qm", "reconcile"], cwd=repo, check=True, env=env, + ) + bare_remote = repo.parent / "remote.git" + subprocess.run( + ["git", "init", "--bare", "-q", str(bare_remote)], + cwd=repo.parent, check=True, env=env, + ) + subprocess.run( + ["git", "remote", "set-url", "origin", f"file://{bare_remote}"], + cwd=repo, check=True, env=env, + ) + subprocess.run(["git", "push", "-qu", "origin", "main"], + cwd=repo, check=True, env=env) + subprocess.run( + ["git", "push", "-q", "origin", context["branch_name"]], + cwd=repo, check=True, env=env, + ) + + reconcile = _load_dream_reconcile_module() + deleted, kept = reconcile.cleanup_branches( + str(repo), + [(context["branch_name"], context["dream_id"], {})], + context["dream_ns"], + worktree_root=context["worktree_root"], + ) + + assert (deleted, kept) == (1, 0) + assert not worktree_dir.exists() + def test_repo_root_subdir_is_canonicalized_for_isolation(self, tmp_path): repo = tmp_path / "repo" repo.mkdir() @@ -741,6 +824,29 @@ def test_auto_gc_nonzero_result_does_not_touch_tombstone( assert not tombstone.exists() assert "auto-GC exited with code 1" in capsys.readouterr().err + def test_auto_gc_does_not_launch_bash_fallback_on_windows( + self, tmp_path, monkeypatch, capsys + ): + dream_setup = _load_dream_setup_module() + script_dir = tmp_path / "skill" + script_dir.mkdir() + (script_dir / "dream-gc.sh").write_text("#!/bin/sh\n", encoding="utf-8") + worktree_base = tmp_path / "worktrees" / "repo" + worktree_base.mkdir(parents=True) + + monkeypatch.setattr(dream_setup, "SCRIPT_DIR", str(script_dir)) + monkeypatch.setattr(dream_setup.os, "name", "nt") + monkeypatch.setattr( + dream_setup.subprocess, + "run", + lambda *args, **kwargs: pytest.fail("Bash GC must not run on Windows"), + ) + + dream_setup._maybe_auto_gc(str(tmp_path), str(worktree_base)) + + assert not (worktree_base / ".last-gc").exists() + assert "auto-GC skipped on Windows" in capsys.readouterr().err + @pytest.mark.slow @pytest.mark.integration @pytest.mark.skipif( From 9ef4e112efabb4d7340702087fe050ef679671f5 Mon Sep 17 00:00:00 2001 From: "Xingdi (Eric) Yuan" <4028684+xingdi-eric-yuan@users.noreply.github.com> Date: Mon, 28 Sep 2026 10:24:20 -0400 Subject: [PATCH 13/14] Preserve unarchived Dream work and repository path whitespace Check branch tips against the archived experiment before cleanup, retain registered worktrees that cannot be safely removed, and isolate TMPDIR in the native-temp test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 3 + skills/shadow-frog-dream/SKILL.md | 5 + skills/shadow-frog-dream/dream-reconcile.py | 88 +++++++++++---- skills/shadow-frog-dream/dream-setup.py | 2 +- .../shadow_frog_dream/test_dream_reconcile.py | 102 ++++++++++++++++-- .../shadow_frog_dream/test_dream_setup.py | 37 ++++++- 6 files changed, 203 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 26f4aae..866ed87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,9 @@ shadow knowledge bases for any codebase. helpers, and cleanup receives the resolved root explicitly. ### Fixed +- **Dream setup and cleanup safeguards** — preserve repository-path whitespace + and retain unarchived branch commits or dirty registered worktrees during + reconciliation cleanup. - **Reconciliation path containment** — validate untrusted manifest destinations and filesystem aliases before writing discoveries, cross-references, or archives. Unsafe paths fail explicitly, including during dry runs, rather diff --git a/skills/shadow-frog-dream/SKILL.md b/skills/shadow-frog-dream/SKILL.md index 079a334..b645564 100644 --- a/skills/shadow-frog-dream/SKILL.md +++ b/skills/shadow-frog-dream/SKILL.md @@ -1019,6 +1019,11 @@ artifacts, remote state, descendants, and coherent retention. A broad run can reconcile pending coherent branches from an earlier session; the current run's mode is never permission to delete those branches. +Cleanup also retains branches whose local or remote-tracking tips are not covered +by the indexed `tip_commit`, and never force-removes a registered worktree. +Preserve follow-up work and commit/push its updated reconciliation before retrying; +repair missing or invalid tips only after verifying the archived experiment. + Never use an inline deletion loop or manually duplicate these checks. A retained branch is not failed cleanup: coherent branches and their canonical index ancestors remain available until explicit curation. `SHADOWFROG_KEEP_BRANCHES` diff --git a/skills/shadow-frog-dream/dream-reconcile.py b/skills/shadow-frog-dream/dream-reconcile.py index 1d86d6e..f17eed1 100755 --- a/skills/shadow-frog-dream/dream-reconcile.py +++ b/skills/shadow-frog-dream/dream-reconcile.py @@ -1541,11 +1541,21 @@ def cleanup_branches(repo_root, manifests, dream_ns, dry_run=False, - The dream_id appears in _index.md - No un-reconciled branches list this branch as parent - The branch is not part of a coherent lineage retained for task baselines + - Local and remote-tracking tips are covered by the indexed tip_commit + - Its registered worktree can be removed without forcing away changes Returns (deleted, kept) counts. """ index_path = os.path.join(repo_root, '.shadow', '_dreams', '_index.md') indexed_ids = _read_indexed_dream_ids(repo_root) if os.path.isfile(index_path) else set() + indexed_tips = {} + if os.path.isfile(index_path): + with open(index_path, encoding="utf-8") as f: + for line in f: + if line.startswith('|'): + parts = [p.strip() for p in line.split('|')] + if len(parts) >= 8: + indexed_tips.setdefault((parts[5], parts[1]), set()).add(parts[7]) # Check if SHADOWFROG_KEEP_BRANCHES is set if os.environ.get('SHADOWFROG_KEEP_BRANCHES', '').strip() in ('1', 'true', 'yes'): @@ -1684,6 +1694,36 @@ def cleanup_branches(repo_root, manifests, dream_ns, dry_run=False, kept += 1 continue + # Published artifacts cover the indexed tip, not later branch commits. + tips = indexed_tips.get((branch, dream_id), set()) + archived_tip = next(iter(tips)) if len(tips) == 1 else '' + if not re.fullmatch(r'[0-9a-fA-F]{7,40}', archived_tip): + print( + f" KEEPING {branch} - missing, invalid, or ambiguous indexed tip_commit; " + "verify the archive and repair its index entry before retrying cleanup." + ) + kept += 1 + continue + refs = subprocess.run( + ['git', 'for-each-ref', '--format=%(objectname)', + f'refs/heads/{branch}', f'refs/remotes/origin/{branch}'], + capture_output=True, text=True, cwd=repo_root, encoding="utf-8", + ) + if refs.returncode != 0 or not refs.stdout.strip() or any( + subprocess.run( + ['git', 'merge-base', '--is-ancestor', tip, archived_tip], + capture_output=True, text=True, cwd=repo_root, encoding="utf-8", + ).returncode != 0 + for tip in refs.stdout.splitlines() + ): + print( + f" KEEPING {branch} - cannot confirm its tips are covered by indexed " + f"tip_commit {archived_tip}; preserve follow-up work and commit/push " + "the updated reconciliation before retrying cleanup." + ) + kept += 1 + continue + # All checks passed — delete if dry_run: print(f" Would delete: {branch}") @@ -1692,9 +1732,15 @@ def cleanup_branches(repo_root, manifests, dream_ns, dry_run=False, # Remove the registered worktree before deleting its branch. Git # refuses to delete a branch that remains checked out in a worktree. - _gc_worktree_after_merge( + if not _gc_worktree_after_merge( repo_root, dream_ns, dream_id, branch, worktree_root, - ) + ): + print( + f" KEEPING {branch} - worktree cleanup was unsafe or failed; " + "preserve its changes or repair its metadata before retrying cleanup." + ) + kept += 1 + continue # Delete remote first (network op that can fail) result = subprocess.run( @@ -1826,8 +1872,8 @@ def _matches(p): def _gc_worktree_after_merge(repo_root, dream_ns, dream_id, deleted_branch=None, worktree_root=None): - """Remove the dream worktree directory after its branch has been - deleted. Safety-gated by `_worktree_safety.safe_worktree_path` — will + """Remove the dream worktree before deleting its archived branch. + Safety-gated by `_worktree_safety.safe_worktree_path` — will NEVER `rm -rf` a path outside `$DREAM_WORKTREE_BASE//dream-`. Cross-deletion guard: worktree paths are keyed on slug only (see @@ -1838,14 +1884,14 @@ def _gc_worktree_after_merge(repo_root, dream_ns, dream_id, deleted_branch=None, later dream has reclaimed it — we must NOT touch it. Pass `deleted_branch` to enable this check. - All failures are swallowed: the branch delete already succeeded, so a - leaked worktree (the pre-fix steady state) is strictly less bad than - an aborted cleanup_branches() loop. + Return False on uncertain ownership or removal failure, so the caller + retains the branch. Unrelated worktrees and unsafe GC paths are skipped. + Registered worktrees are never force-removed or deleted by the fallback. """ try: slug = _slug_from_dream_id(dream_id) if not slug: - return # Can't derive worktree path — bail silently. + return True # No matching worktree path to remove. paths_path = os.path.join( os.path.dirname(os.path.abspath(__file__)), "_worktree_paths.py" ) @@ -1874,15 +1920,14 @@ def _gc_worktree_after_merge(repo_root, dream_ns, dream_id, deleted_branch=None, resolved = safe_worktree_path(candidate, base) except UnsafePath as exc: print(f" ⚠️ Skipping worktree GC for {dream_id}: {exc}") - return + return True # Cross-deletion guard: refuse to touch a path another dream owns. - # Only consult git when we know which branch we expected (i.e., - # `deleted_branch` was passed by `cleanup_branches`). Callers that - # pre-date this guard (tests, future ad-hoc invocations) default to - # the original behavior — still gated by the safety check above. + registered = _registered_worktree_branch(repo_root, str(resolved)) + if registered.state == "indeterminate": + print(f" Skipping worktree GC for {dream_id}: indeterminate ownership") + return False if deleted_branch: - registered = _registered_worktree_branch(repo_root, str(resolved)) if registered.state == "attached" and registered.branch == deleted_branch: pass elif registered.state == "unregistered": @@ -1896,18 +1941,23 @@ def _gc_worktree_after_merge(repo_root, dream_ns, dream_id, deleted_branch=None, f" ↳ Skipping worktree GC for {dream_id}: " f"path {resolved} {detail}" ) - return + return True # Polite path first: let git update its own bookkeeping. worktree_removed = False result = subprocess.run( - ['git', 'worktree', 'remove', str(resolved), '--force'], + ['git', 'worktree', 'remove', str(resolved)], capture_output=True, text=True, cwd=repo_root, encoding="utf-8", ) if result.returncode == 0: worktree_removed = True print(f" 🗑 Removed worktree: {resolved}") + elif registered.state != "unregistered": + print( + f" Skipping worktree GC for {dream_id}: {result.stderr.strip()}" + ) + return False # Fallback: directory may still be on disk (git failed, dead # gitdir pointer, etc.). The safety gate already proved the path # is `//dream-` so the rm is bounded. @@ -1918,15 +1968,17 @@ def _gc_worktree_after_merge(repo_root, dream_ns, dream_id, deleted_branch=None, except OSError as exc: # Worst case: leak the directory but don't break cleanup. print(f" ⚠️ Worktree rm failed for {resolved}: {exc}") - return + return False # Clean up git's stale-worktree bookkeeping. subprocess.run( ['git', 'worktree', 'prune'], capture_output=True, text=True, cwd=repo_root, encoding="utf-8", ) - except Exception as exc: # noqa: BLE001 — GC must never crash cleanup. + return True + except (ImportError, OSError, ValueError, subprocess.SubprocessError) as exc: print(f" ⚠️ Worktree GC raised for {dream_id}: {exc}") + return False # --- Main orchestration --- diff --git a/skills/shadow-frog-dream/dream-setup.py b/skills/shadow-frog-dream/dream-setup.py index 089fd9c..b23a679 100644 --- a/skills/shadow-frog-dream/dream-setup.py +++ b/skills/shadow-frog-dream/dream-setup.py @@ -77,7 +77,7 @@ def _resolve_repo_root(path): if r.returncode != 0: _err("ERROR: Not in a git repository") sys.exit(1) - return os.path.abspath(r.stdout.strip()) + return os.path.abspath(r.stdout.removesuffix("\n")) def _import_safety(): diff --git a/tests/skills/shadow_frog_dream/test_dream_reconcile.py b/tests/skills/shadow_frog_dream/test_dream_reconcile.py index 8cb450d..03e78ff 100644 --- a/tests/skills/shadow_frog_dream/test_dream_reconcile.py +++ b/tests/skills/shadow_frog_dream/test_dream_reconcile.py @@ -860,6 +860,7 @@ def test_cleanup_branches_proceeds_when_shadow_committed_and_pushed( dream_id = "20260420-150500Z-clean" branch = make_dream_branch(tmp_git_repo, env, "proj", dream_id, _default_manifest(dream_id)) + tip = _git("rev-parse", branch, cwd=tmp_git_repo, env=env).stdout.strip() # Mirror artifacts + index onto main, then commit and push. _seed_dream_artifacts(tmp_git_repo, dream_id) @@ -868,7 +869,7 @@ def test_cleanup_branches_proceeds_when_shadow_committed_and_pushed( "# Dream Experiment Archive\n\n" "| dream_id | category | verdict | title | branch | parent | tip_commit |\n" "|----------|----------|---------|-------|--------|--------|------------|\n" - f"| {dream_id} | bug hunting | useful | T | {branch} | main | abc1234 |\n", + f"| {dream_id} | bug hunting | useful | T | {branch} | main | {tip} |\n", ) _git("add", "-A", cwd=tmp_git_repo, env=env) _git("commit", "-q", "-m", "reconcile", cwd=tmp_git_repo, env=env) @@ -2240,6 +2241,7 @@ def test_cleanup_branches_actually_deletes_when_all_checks_pass( dream_id = "20260420-050000Z-realdel" branch = make_dream_branch(tmp_git_repo, env, "proj", dream_id, _default_manifest(dream_id)) + tip = _git("rev-parse", branch, cwd=tmp_git_repo, env=env).stdout.strip() # All safety conditions satisfied: # 1. HEAD == origin/main → ancestor check OK. @@ -2251,7 +2253,7 @@ def test_cleanup_branches_actually_deletes_when_all_checks_pass( "# Dream Experiments\n\n" "| dream_id | category | verdict | title | branch | parent | tip_commit |\n" "|----------|----------|---------|-------|--------|--------|------------|\n" - f"| {dream_id} | bug hunting | useful | T | {branch} | main | abc1234 |\n", + f"| {dream_id} | bug hunting | useful | T | {branch} | main | {tip} |\n", ) # Commit + push the reconciliation so .shadow/ is clean and HEAD is on @@ -2288,13 +2290,14 @@ def test_cleanup_branches_keeps_branch_when_local_delete_fails( dream_id = "20260420-050050Z-local-failure" branch = make_dream_branch(tmp_git_repo, env, "proj", dream_id, _default_manifest(dream_id)) + tip = _git("rev-parse", branch, cwd=tmp_git_repo, env=env).stdout.strip() _seed_dream_artifacts(tmp_git_repo, dream_id) _write_index( tmp_git_repo, "# Dream Experiments\n\n" "| dream_id | category | verdict | title | branch | parent | tip_commit |\n" "|----------|----------|---------|-------|--------|--------|------------|\n" - f"| {dream_id} | bug hunting | useful | T | {branch} | main | abc1234 |\n", + f"| {dream_id} | bug hunting | useful | T | {branch} | main | {tip} |\n", ) _git("add", "-A", cwd=tmp_git_repo, env=env) _git("commit", "-q", "-m", "reconcile", cwd=tmp_git_repo, env=env) @@ -2730,13 +2733,14 @@ def test_cleanup_branches_keep_branches_env_falsy_does_not_block( dream_id = "20260420-050500Z-zero" branch = make_dream_branch(tmp_git_repo, env, "proj", dream_id, _default_manifest(dream_id)) + tip = _git("rev-parse", branch, cwd=tmp_git_repo, env=env).stdout.strip() _seed_dream_artifacts(tmp_git_repo, dream_id) _write_index( tmp_git_repo, "# Dream Experiments\n\n" "| dream_id | category | verdict | title | branch | parent | tip_commit |\n" "|----------|----------|---------|-------|--------|--------|------------|\n" - f"| {dream_id} | bug hunting | useful | T | {branch} | main | abc1234 |\n", + f"| {dream_id} | bug hunting | useful | T | {branch} | main | {tip} |\n", ) monkeypatch.setenv("SHADOWFROG_KEEP_BRANCHES", "0") # Commit + push so .shadow/ is clean (dirty-tree guard would otherwise fire). @@ -3236,6 +3240,8 @@ def test_cli_cleanup_branches_never_deletes_other_namespace(tmp_git_repo): outside_branch = make_dream_branch( tmp_git_repo, env, "outside", outside_id, _default_manifest(outside_id) ) + inside_tip = _git("rev-parse", inside_branch, cwd=tmp_git_repo, env=env).stdout.strip() + outside_tip = _git("rev-parse", outside_branch, cwd=tmp_git_repo, env=env).stdout.strip() _seed_dream_artifacts(tmp_git_repo, inside_id) _seed_dream_artifacts(tmp_git_repo, outside_id) _write_index( @@ -3243,8 +3249,8 @@ def test_cli_cleanup_branches_never_deletes_other_namespace(tmp_git_repo): "# Dream Experiments\n\n" "| dream_id | category | verdict | title | branch | parent | tip_commit |\n" "|----------|----------|---------|-------|--------|--------|------------|\n" - f"| {inside_id} | test | useful | Inside | {inside_branch} | main | deadbeef |\n" - f"| {outside_id} | test | useful | Outside | {outside_branch} | main | deadbeef |\n", + f"| {inside_id} | test | useful | Inside | {inside_branch} | main | {inside_tip} |\n" + f"| {outside_id} | test | useful | Outside | {outside_branch} | main | {outside_tip} |\n", ) _git("add", "-A", cwd=tmp_git_repo, env=env) _git("commit", "-q", "-m", "reconcile", cwd=tmp_git_repo, env=env) @@ -3563,13 +3569,14 @@ def test_cleanup_branches_also_removes_worktree( dream_id = "20260420-070000Z-gctest" branch = make_dream_branch(tmp_git_repo, env, "proj", dream_id, _default_manifest(dream_id)) + tip = _git("rev-parse", branch, cwd=tmp_git_repo, env=env).stdout.strip() _seed_dream_artifacts(tmp_git_repo, dream_id) _write_index( tmp_git_repo, "# Dream Experiments\n\n" "| dream_id | category | verdict | title | branch | parent | tip_commit |\n" "|----------|----------|---------|-------|--------|--------|------------|\n" - f"| {dream_id} | bug hunting | useful | T | {branch} | main | abc1234 |\n", + f"| {dream_id} | bug hunting | useful | T | {branch} | main | {tip} |\n", ) _git("add", "-A", cwd=tmp_git_repo, env=env) _git("commit", "-q", "-m", "reconcile", cwd=tmp_git_repo, env=env) @@ -3597,6 +3604,75 @@ def test_cleanup_branches_also_removes_worktree( ) +@pytest.mark.slow +@pytest.mark.parametrize("change", [ + "unpushed", "diverged", "pushed", "tracked-edit", "untracked-file", + "unknown-tip", "unresolvable-tip", +]) +def test_cleanup_branches_preserves_unarchived_work( + dream_reconcile, tmp_git_repo, tmp_path, capsys, change, +): + env = _seed_repo(tmp_git_repo) + _add_bare_remote(tmp_git_repo, env) + dream_id = "20260928-100000Z-preserve" + manifest = _default_manifest(dream_id) + branch = make_dream_branch(tmp_git_repo, env, "proj", dream_id, manifest) + published_tip = _git("rev-parse", branch, cwd=tmp_git_repo, env=env).stdout.strip() + indexed_tip = published_tip + if change == "unknown-tip": + indexed_tip = "unknown" + elif change == "unresolvable-tip": + indexed_tip = "0" * 40 + _seed_dream_artifacts(tmp_git_repo, dream_id) + _write_index( + tmp_git_repo, + "| dream_id | category | verdict | title | branch | parent | tip_commit |\n" + "|----------|----------|---------|-------|--------|--------|------------|\n" + f"| {dream_id} | bug hunting | useful | T | {branch} | main | {indexed_tip} |\n", + ) + _git("add", "-A", cwd=tmp_git_repo, env=env) + _git("commit", "-q", "-m", "reconcile", cwd=tmp_git_repo, env=env) + _git("push", "-q", "origin", "main", cwd=tmp_git_repo, env=env) + + if change == "diverged": + _git("update-ref", f"refs/heads/{branch}", f"{published_tip}^", published_tip, + cwd=tmp_git_repo, env=env) + base = tmp_path / "worktrees" + worktree = base / "proj" / "dream-preserve" + worktree.parent.mkdir(parents=True) + _git("worktree", "add", "-q", str(worktree), branch, cwd=tmp_git_repo, env=env) + marker = worktree / ("new.txt" if change == "untracked-file" else "README.md") + if change in {"unpushed", "diverged", "pushed", "tracked-edit", "untracked-file"}: + marker.write_text("Keep this work.\n", encoding="utf-8") + if change in {"unpushed", "diverged", "pushed"}: + _git("add", "-A", cwd=worktree, env=env) + _git("commit", "-q", "-m", "follow-up work", cwd=worktree, env=env) + assert not _git("status", "--porcelain", cwd=worktree, env=env).stdout + if change == "pushed": + _git("push", "-q", "origin", branch, cwd=worktree, env=env) + contents = marker.read_text(encoding="utf-8") + local_tip = _git("rev-parse", branch, cwd=tmp_git_repo, env=env).stdout.strip() + remote_before = _git("ls-remote", "--heads", "origin", branch, + cwd=tmp_git_repo, env=env).stdout + + assert dream_reconcile.cleanup_branches( + str(tmp_git_repo), [(branch, dream_id, manifest)], "proj", + worktree_root=str(base), + ) == (0, 1) + + assert marker.read_text(encoding="utf-8") == contents + assert _git("rev-parse", branch, cwd=tmp_git_repo, env=env).stdout.strip() == local_tip + assert _git("ls-remote", "--heads", "origin", branch, + cwd=tmp_git_repo, env=env).stdout == remote_before + assert f"refs/heads/{branch}" in _git( + "for-each-ref", "--format=%(refname)", "--contains", local_tip, + cwd=tmp_git_repo, env=env, + ).stdout + output = capsys.readouterr().out + assert "KEEPING" in output + assert "retry" in output + + @pytest.mark.slow def test_cleanup_branches_worktree_gc_falls_back_on_dead_gitdir( dream_reconcile, tmp_git_repo, tmp_path, monkeypatch @@ -3609,13 +3685,14 @@ def test_cleanup_branches_worktree_gc_falls_back_on_dead_gitdir( dream_id = "20260420-080000Z-dead" branch = make_dream_branch(tmp_git_repo, env, "proj", dream_id, _default_manifest(dream_id)) + tip = _git("rev-parse", branch, cwd=tmp_git_repo, env=env).stdout.strip() _seed_dream_artifacts(tmp_git_repo, dream_id) _write_index( tmp_git_repo, "# Dream Experiments\n\n" "| dream_id | category | verdict | title | branch | parent | tip_commit |\n" "|----------|----------|---------|-------|--------|--------|------------|\n" - f"| {dream_id} | bug hunting | useful | T | {branch} | main | abc1234 |\n", + f"| {dream_id} | bug hunting | useful | T | {branch} | main | {tip} |\n", ) _git("add", "-A", cwd=tmp_git_repo, env=env) _git("commit", "-q", "-m", "reconcile", cwd=tmp_git_repo, env=env) @@ -3656,13 +3733,14 @@ def test_cleanup_branches_worktree_gc_refuses_unsafe_base( dream_id = "20260420-090000Z-unsafe" branch = make_dream_branch(tmp_git_repo, env, "proj", dream_id, _default_manifest(dream_id)) + tip = _git("rev-parse", branch, cwd=tmp_git_repo, env=env).stdout.strip() _seed_dream_artifacts(tmp_git_repo, dream_id) _write_index( tmp_git_repo, "# Dream Experiments\n\n" "| dream_id | category | verdict | title | branch | parent | tip_commit |\n" "|----------|----------|---------|-------|--------|--------|------------|\n" - f"| {dream_id} | bug hunting | useful | T | {branch} | main | abc1234 |\n", + f"| {dream_id} | bug hunting | useful | T | {branch} | main | {tip} |\n", ) _git("add", "-A", cwd=tmp_git_repo, env=env) _git("commit", "-q", "-m", "reconcile", cwd=tmp_git_repo, env=env) @@ -3711,13 +3789,14 @@ def test_cleanup_branches_worktree_gc_skips_unparseable_dream_id( dream_id = "not-a-real-dream-id" branch = make_dream_branch(tmp_git_repo, env, "proj", dream_id, _default_manifest(dream_id)) + tip = _git("rev-parse", branch, cwd=tmp_git_repo, env=env).stdout.strip() _seed_dream_artifacts(tmp_git_repo, dream_id) _write_index( tmp_git_repo, "# Dream Experiments\n\n" "| dream_id | category | verdict | title | branch | parent | tip_commit |\n" "|----------|----------|---------|-------|--------|--------|------------|\n" - f"| {dream_id} | bug hunting | useful | T | {branch} | main | abc1234 |\n", + f"| {dream_id} | bug hunting | useful | T | {branch} | main | {tip} |\n", ) _git("add", "-A", cwd=tmp_git_repo, env=env) _git("commit", "-q", "-m", "reconcile", cwd=tmp_git_repo, env=env) @@ -3906,13 +3985,14 @@ def test_cleanup_branches_does_not_clobber_concurrent_slug_collision( _default_manifest(dream_a_id)) branch_b = make_dream_branch(tmp_git_repo, env, "proj", dream_b_id, _default_manifest(dream_b_id)) + tip_a = _git("rev-parse", branch_a, cwd=tmp_git_repo, env=env).stdout.strip() _seed_dream_artifacts(tmp_git_repo, dream_a_id) _write_index( tmp_git_repo, "# Dream Experiments\n\n" "| dream_id | category | verdict | title | branch | parent | tip_commit |\n" "|----------|----------|---------|-------|--------|--------|------------|\n" - f"| {dream_a_id} | bug hunting | useful | A | {branch_a} | main | abc1234 |\n", + f"| {dream_a_id} | bug hunting | useful | A | {branch_a} | main | {tip_a} |\n", ) _git("add", "-A", cwd=tmp_git_repo, env=env) _git("commit", "-q", "-m", "reconcile A", cwd=tmp_git_repo, env=env) diff --git a/tests/skills/shadow_frog_dream/test_dream_setup.py b/tests/skills/shadow_frog_dream/test_dream_setup.py index c89db88..4b5dfae 100644 --- a/tests/skills/shadow_frog_dream/test_dream_setup.py +++ b/tests/skills/shadow_frog_dream/test_dream_setup.py @@ -6,8 +6,8 @@ Cross-platform: invokes the Python entry point directly (no bash). The two auto-GC tests that assert an orphan is actually *swept* still need the bash -`dream-gc.sh`, so they are skipped on Windows until `dream-gc.py` exists; -every other test runs on all OSes. +`dream-gc.sh`, so they are skipped on Windows until `dream-gc.py` exists. +POSIX filename regressions also skip Windows; other tests run on all OSes. """ import json import importlib.util @@ -227,10 +227,37 @@ def test_worktree_has_same_head_as_base(self, tmp_path): data = json.loads(result.stdout) assert data["base_commit"] == main_head - def test_uses_system_temp_root_without_override(self, tmp_path): + @pytest.mark.skipif(os.name == "nt", reason="requires POSIX path characters") + @pytest.mark.parametrize("suffix", [" ", "\t", "\n"]) + @pytest.mark.parametrize("explicit_root", [False, True]) + def test_preserves_repository_path_whitespace(self, tmp_path, suffix, explicit_root): + repo = tmp_path / f"source repo{suffix}" + repo.mkdir() + _make_git_repo(repo) + args = ["--slug", "path-whitespace", "--namespace", "test"] + if explicit_root: + args += ["--repo-root", str(repo)] + + result = run_dream_setup( + args, cwd=repo, + env_extra={ + "DREAM_GC_AUTO": "0", + "DREAM_WORKTREE_BASE": str(tmp_path / "worktrees"), + }, + ) + + assert result.returncode == 0, result.stderr + data = json.loads(result.stdout) + assert Path(data["repo_root"]) == repo.resolve() + assert Path(data["worktree_dir"]).is_dir() + + def test_uses_system_temp_root_without_override(self, tmp_path, monkeypatch): repo = tmp_path / "repo" repo.mkdir() _make_git_repo(repo) + inherited_temp = tmp_path / "inherited-temp" + inherited_temp.mkdir() + monkeypatch.setenv("TMPDIR", str(inherited_temp)) system_temp = tmp_path / "system-temp" system_temp.mkdir() @@ -239,6 +266,7 @@ def test_uses_system_temp_root_without_override(self, tmp_path): cwd=repo, env_extra={ "DREAM_GC_AUTO": "0", + "TMPDIR": str(system_temp), "TEMP": str(system_temp), "TMP": str(system_temp), }, @@ -250,6 +278,7 @@ def test_uses_system_temp_root_without_override(self, tmp_path): assert Path(data["worktree_root"]) == expected_root assert Path(data["worktree_base"]) == expected_root / repo.name assert Path(data["worktree_dir"]) == expected_root / repo.name / "dream-t02-default-root" + assert not (inherited_temp / "shadowfrog-dreams").exists() def test_relative_worktree_root_is_emitted_as_absolute(self, tmp_path): repo = tmp_path / "repo" @@ -299,7 +328,7 @@ def test_setup_context_drives_reconciler_cleanup_at_same_root(self, tmp_path): "| dream_id | category | verdict | title | branch | parent | tip_commit |\n" "|----------|----------|---------|-------|--------|--------|------------|\n" f"| {context['dream_id']} | test | useful | Test | " - f"{context['branch_name']} | main | deadbeef |\n", + f"{context['branch_name']} | main | {context['base_commit']} |\n", encoding="utf-8", ) env = _base_env(repo) From f39bb318902eec56ffdec9a744ff60a391e028a3 Mon Sep 17 00:00:00 2001 From: "Xingdi (Eric) Yuan" <4028684+xingdi-eric-yuan@users.noreply.github.com> Date: Mon, 28 Sep 2026 12:22:05 -0400 Subject: [PATCH 14/14] Fix Dream lifecycle edge cases found in integration review Preserve Python 3.9 execution and lossless NUL-delimited worktree paths, correctly account for remote-only cleanup, and reject newline-terminated identifiers. Extend real-Git regression cases and the existing Python 3.9 smoke job. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/tests.yml | 6 +++ CHANGELOG.md | 3 ++ skills/shadow-frog-dream/dream-reconcile.py | 47 ++++++++++-------- skills/shadow-frog-dream/dream-setup.py | 6 +-- .../shadow_frog_dream/test_dream_reconcile.py | 49 ++++++++++++++----- .../shadow_frog_dream/test_dream_setup.py | 26 +++++++++- 6 files changed, 98 insertions(+), 39 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 0d089c3..2f4a2bc 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -22,6 +22,12 @@ jobs: run: | python skills/shadow-frog/shadow-cite.py --help python skills/shadow-frog-viewer/shadow-viewer.py --shadow-dir examples/coupon-demo/.shadow --check-invariants + - name: Check Dream entry points + run: | + python skills/shadow-frog-dream/dream-setup.py --help + python skills/shadow-frog-dream/dream-tools.py --help + python skills/shadow-frog-dream/dream-validate.py --help + python skills/shadow-frog-dream/dream-reconcile.py --help pytest: name: pytest (${{ matrix.os }}, Python 3.12) diff --git a/CHANGELOG.md b/CHANGELOG.md index 866ed87..5c8b866 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,9 @@ shadow knowledge bases for any codebase. - **Dream setup and cleanup safeguards** — preserve repository-path whitespace and retain unarchived branch commits or dirty registered worktrees during reconciliation cleanup. +- **Dream lifecycle compatibility** — preserve Python 3.9 execution and + worktree identity for unusual paths, correctly report remote-only branch + cleanup, and reject newline-terminated slugs and namespaces. - **Reconciliation path containment** — validate untrusted manifest destinations and filesystem aliases before writing discoveries, cross-references, or archives. Unsafe paths fail explicitly, including during dry runs, rather diff --git a/skills/shadow-frog-dream/dream-reconcile.py b/skills/shadow-frog-dream/dream-reconcile.py index f17eed1..74256aa 100755 --- a/skills/shadow-frog-dream/dream-reconcile.py +++ b/skills/shadow-frog-dream/dream-reconcile.py @@ -45,6 +45,7 @@ from dataclasses import dataclass from datetime import datetime, timezone from pathlib import Path, PureWindowsPath +from typing import Optional _bytecode = sys.dont_write_bytecode sys.dont_write_bytecode = True @@ -1705,16 +1706,17 @@ def cleanup_branches(repo_root, manifests, dream_ns, dry_run=False, kept += 1 continue refs = subprocess.run( - ['git', 'for-each-ref', '--format=%(objectname)', + ['git', 'for-each-ref', '--format=%(refname) %(objectname)', f'refs/heads/{branch}', f'refs/remotes/origin/{branch}'], capture_output=True, text=True, cwd=repo_root, encoding="utf-8", ) - if refs.returncode != 0 or not refs.stdout.strip() or any( + branch_tips = dict(line.split(' ', 1) for line in refs.stdout.splitlines()) + if refs.returncode != 0 or not branch_tips or any( subprocess.run( ['git', 'merge-base', '--is-ancestor', tip, archived_tip], capture_output=True, text=True, cwd=repo_root, encoding="utf-8", ).returncode != 0 - for tip in refs.stdout.splitlines() + for tip in branch_tips.values() ): print( f" KEEPING {branch} - cannot confirm its tips are covered by indexed " @@ -1754,17 +1756,18 @@ def cleanup_branches(repo_root, manifests, dream_ns, dry_run=False, if 'remote ref does not exist' not in result.stderr: print(f" ⚠️ Failed to delete remote {branch}: {result.stderr.strip()}") - # Delete local - result = subprocess.run( - ['git', 'branch', '-D', branch], - capture_output=True, text=True, cwd=repo_root, encoding="utf-8" - ) - if result.returncode == 0: - print(f" 🗑 Deleted local: {branch}") - else: - print(f" ⚠️ Failed to delete local {branch}: {result.stderr.strip()}") - kept += 1 - continue + # Experiments reconciled on another clone may have no local branch. + if f'refs/heads/{branch}' in branch_tips: + result = subprocess.run( + ['git', 'branch', '-D', branch], + capture_output=True, text=True, cwd=repo_root, encoding="utf-8" + ) + if result.returncode == 0: + print(f" 🗑 Deleted local: {branch}") + else: + print(f" ⚠️ Failed to delete local {branch}: {result.stderr.strip()}") + kept += 1 + continue # Also delete the remote-tracking ref subprocess.run( ['git', 'branch', '-dr', f'origin/{branch}'], @@ -1800,13 +1803,14 @@ class WorktreeRegistration: """The registration state for one candidate worktree path.""" state: str - branch: str | None = None + branch: Optional[str] = None def _registered_worktree_branch(repo_root, candidate_path): """Return a registration result for `candidate_path`. - Parses `git worktree list --porcelain` output: + Parses NUL-delimited `git worktree list --porcelain -z` output + (shown as lines here): worktree /abs/path HEAD branch refs/heads/ @@ -1829,11 +1833,12 @@ def _registered_worktree_branch(repo_root, candidate_path): if sys.path and sys.path[0] == script_dir: sys.path.pop(0) result = subprocess.run( - ['git', 'worktree', 'list', '--porcelain'], - capture_output=True, text=True, cwd=repo_root, timeout=10, - encoding="utf-8", + ['git', 'worktree', 'list', '--porcelain', '-z'], + capture_output=True, cwd=repo_root, timeout=10, ) - except (OSError, subprocess.SubprocessError): + # Text mode would rewrite carriage returns inside otherwise valid paths. + output = result.stdout.decode("utf-8") + except (OSError, subprocess.SubprocessError, UnicodeError): return WorktreeRegistration("indeterminate") if result.returncode != 0: return WorktreeRegistration("indeterminate") @@ -1851,7 +1856,7 @@ def _matches(p): cur_path = None cur_branch = None - for line in result.stdout.splitlines(): + for line in output.split('\0'): if line.startswith('worktree '): # Flush previous entry if it matched. if cur_path is not None and _matches(cur_path): diff --git a/skills/shadow-frog-dream/dream-setup.py b/skills/shadow-frog-dream/dream-setup.py index b23a679..3fd92dc 100644 --- a/skills/shadow-frog-dream/dream-setup.py +++ b/skills/shadow-frog-dream/dream-setup.py @@ -244,11 +244,11 @@ def main(): _err("ERROR: --slug is required") _err("Usage: dream-setup.py --slug t01-name [--base-branch BRANCH]") sys.exit(1) - if not SAFE_RE.match(slug): + if not SAFE_RE.fullmatch(slug): _err(f"ERROR: --slug must match {SAFE_RE.pattern} (got: {slug})") _err(" Use kebab-case alphanumerics like 't01-csv-fuzzer'.") sys.exit(1) - if opts["namespace"] and not SAFE_RE.match(opts["namespace"]): + if opts["namespace"] and not SAFE_RE.fullmatch(opts["namespace"]): _err(f"ERROR: --namespace must match {SAFE_RE.pattern} " f"(got: {opts['namespace']})") sys.exit(1) @@ -312,7 +312,7 @@ def main(): _err(f"ERROR: {exc}") sys.exit(1) - if not SAFE_RE.match(dream_ns): + if not SAFE_RE.fullmatch(dream_ns): _err(f"ERROR: Resolved DREAM_NS contains unsafe characters: {dream_ns}") _err(f" Allowed: {SAFE_RE.pattern}") _err(" Override with --namespace or set DREAM_NAMESPACE.") diff --git a/tests/skills/shadow_frog_dream/test_dream_reconcile.py b/tests/skills/shadow_frog_dream/test_dream_reconcile.py index 03e78ff..705d237 100644 --- a/tests/skills/shadow_frog_dream/test_dream_reconcile.py +++ b/tests/skills/shadow_frog_dream/test_dream_reconcile.py @@ -2233,8 +2233,9 @@ def test_update_state_records_last_commit_sha(dream_reconcile, tmp_git_repo): # =========================================================================== @pytest.mark.slow +@pytest.mark.parametrize("local_branch", [True, False]) def test_cleanup_branches_actually_deletes_when_all_checks_pass( - dream_reconcile, tmp_git_repo + dream_reconcile, tmp_git_repo, local_branch ): env = _seed_repo(tmp_git_repo) _add_bare_remote(tmp_git_repo, env) @@ -2262,6 +2263,9 @@ def test_cleanup_branches_actually_deletes_when_all_checks_pass( _git("commit", "-q", "-m", "reconcile", cwd=tmp_git_repo, env=env) _git("push", "-q", "origin", "main", cwd=tmp_git_repo, env=env) + if not local_branch: + _git("branch", "-D", branch, cwd=tmp_git_repo, env=env) + # Pre-check: branch exists on origin. pre = _git("ls-remote", "--heads", "origin", branch, cwd=tmp_git_repo, env=env).stdout @@ -3609,8 +3613,17 @@ def test_cleanup_branches_also_removes_worktree( "unpushed", "diverged", "pushed", "tracked-edit", "untracked-file", "unknown-tip", "unresolvable-tip", ]) +@pytest.mark.parametrize("base_name", [ + "worktrees", "worktrees-caf\u00e9", + pytest.param("worktrees-line\t\n", marks=pytest.mark.skipif( + os.name == "nt", reason="requires POSIX path characters", + )), + pytest.param("worktrees-carriage\r", marks=pytest.mark.skipif( + os.name == "nt", reason="requires POSIX path characters", + )), +]) def test_cleanup_branches_preserves_unarchived_work( - dream_reconcile, tmp_git_repo, tmp_path, capsys, change, + dream_reconcile, tmp_git_repo, tmp_path, capsys, change, base_name, ): env = _seed_repo(tmp_git_repo) _add_bare_remote(tmp_git_repo, env) @@ -3637,7 +3650,7 @@ def test_cleanup_branches_preserves_unarchived_work( if change == "diverged": _git("update-ref", f"refs/heads/{branch}", f"{published_tip}^", published_tip, cwd=tmp_git_repo, env=env) - base = tmp_path / "worktrees" + base = tmp_path / base_name worktree = base / "proj" / "dream-preserve" worktree.parent.mkdir(parents=True) _git("worktree", "add", "-q", str(worktree), branch, cwd=tmp_git_repo, env=env) @@ -3826,11 +3839,20 @@ class TestRegisteredWorktreeBranch: """Coverage of registration states used by fail-closed worktree cleanup.""" @pytest.mark.slow + @pytest.mark.parametrize("name", [ + "wt", "wt-caf\u00e9", + pytest.param("wt\tline\n", marks=pytest.mark.skipif( + os.name == "nt", reason="requires POSIX path characters", + )), + pytest.param("wt-carriage\r", marks=pytest.mark.skipif( + os.name == "nt", reason="requires POSIX path characters", + )), + ]) def test_returns_branch_for_registered_path( - self, dream_reconcile, tmp_git_repo, tmp_path + self, dream_reconcile, tmp_git_repo, tmp_path, name ): env = _seed_repo(tmp_git_repo) - wt = tmp_path / "wt" + wt = tmp_path / name _git("worktree", "add", "-q", "-b", "feature-x", str(wt), cwd=tmp_git_repo, env=env) registration = dream_reconcile._registered_worktree_branch( @@ -3912,15 +3934,16 @@ def test_gc_preserves_detached_worktree(self, dream_reconcile, tmp_git_repo, assert marker.read_text(encoding="utf-8") == "preserve me\n" assert "detached ownership" in capsys.readouterr().out + @pytest.mark.parametrize("returncode, stdout", [(1, b""), (0, b"\xff")]) def test_returns_indeterminate_when_git_query_fails( - self, dream_reconcile, tmp_git_repo, tmp_path, monkeypatch + self, dream_reconcile, tmp_git_repo, tmp_path, monkeypatch, returncode, stdout ): _seed_repo(tmp_git_repo) original_run = dream_reconcile.subprocess.run def failed_query(args, **kwargs): - if args == ["git", "worktree", "list", "--porcelain"]: - return subprocess.CompletedProcess(args, 1, "", "git failed") + if args == ["git", "worktree", "list", "--porcelain", "-z"]: + return subprocess.CompletedProcess(args, returncode, stdout, b"git failed") return original_run(args, **kwargs) monkeypatch.setattr(dream_reconcile.subprocess, "run", failed_query) @@ -3934,12 +3957,12 @@ def test_matches_case_insensitively_when_platform_requires_it( self, dream_reconcile, monkeypatch ): result = subprocess.CompletedProcess( - ["git", "worktree", "list", "--porcelain"], + ["git", "worktree", "list", "--porcelain", "-z"], 0, - "worktree C:\\Dreams\\Dream-Same\n" - "HEAD deadbeef\n" - "branch refs/heads/dream/proj/20260420-110000Z-same\n", - "", + b"worktree C:\\Dreams\\Dream-Same\0" + b"HEAD deadbeef\0" + b"branch refs/heads/dream/proj/20260420-110000Z-same\0\0", + b"", ) monkeypatch.setattr( dream_reconcile.subprocess, "run", lambda *args, **kwargs: result diff --git a/tests/skills/shadow_frog_dream/test_dream_setup.py b/tests/skills/shadow_frog_dream/test_dream_setup.py index 4b5dfae..7fc595a 100644 --- a/tests/skills/shadow_frog_dream/test_dream_setup.py +++ b/tests/skills/shadow_frog_dream/test_dream_setup.py @@ -592,17 +592,39 @@ def test_missing_slug_fails(self, tmp_path): assert result.returncode != 0 assert "slug" in result.stderr.lower() - def test_invalid_slug_rejected(self, tmp_path): + @pytest.mark.parametrize("slug", ["bad slug!!", "valid\n"]) + def test_invalid_slug_rejected(self, tmp_path, slug): repo = tmp_path / "repo" repo.mkdir() _make_git_repo(repo) result = run_dream_setup( - ["--slug", "bad slug!!", "--repo-root", str(repo)], + ["--slug", slug, "--repo-root", str(repo), "--dry-run"], cwd=repo, ) assert result.returncode != 0 assert "must match" in result.stderr + @pytest.mark.parametrize("source", ["argument", "environment", "task_info"]) + def test_rejects_namespace_with_trailing_newline(self, tmp_path, source): + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + args = ["--slug", "valid", "--repo-root", str(repo), "--dry-run"] + env = {} + if source == "argument": + args += ["--namespace", "valid\n"] + elif source == "environment": + env["DREAM_NAMESPACE"] = "valid\n" + else: + (repo / "TASK_INFO.json").write_text( + json.dumps({"dream_namespace": "valid\n"}), encoding="utf-8", + ) + + result = run_dream_setup(args, cwd=repo, env_extra=env) + + assert result.returncode != 0 + assert "must match" in result.stderr or "unsafe characters" in result.stderr + @pytest.mark.slow @pytest.mark.integration