diff --git a/frontend/docs/e2e.md b/frontend/docs/e2e.md index b6bbd725..e216b057 100644 --- a/frontend/docs/e2e.md +++ b/frontend/docs/e2e.md @@ -183,7 +183,10 @@ including the root itself; `clean --all` sweeps sandboxes untouched for two hour cannot take out a concurrent agent's live one. A bare `clean` is an error, not a sweep. The sandbox is a `git worktree` under `$TMPDIR`, so a deleted file or an edited spec inside -it cannot reach your tree. Two caveats: an absolute path still escapes it, and a tool that +it cannot reach your tree. `--with-deps` needs the root to be on the repo's own filesystem — +hardlinks cannot cross mounts — so where `$TMPDIR` is its own mount (tmpfs `/tmp`, a separate +`/home`) the tool says so and falls back to `.pentagi-review-sandboxes` beside the repo; +`PENTAGI_SANDBOX_ROOT` overrides both. Two caveats: an absolute path still escapes it, and a tool that rewrites a dependency **in place** would reach the shared inode — don't hand `--with-deps` to an agent whose job is patching libraries. Check `git status` in your own tree when a run finishes; that is the only proof nothing leaked. diff --git a/frontend/e2e/tools/review-sandbox.sh b/frontend/e2e/tools/review-sandbox.sh index ec3c92d7..3f4723a0 100755 --- a/frontend/e2e/tools/review-sandbox.sh +++ b/frontend/e2e/tools/review-sandbox.sh @@ -21,10 +21,38 @@ set -euo pipefail REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../../.." && pwd)" -SANDBOX_ROOT="${TMPDIR:-/tmp}/pentagi-review-sandboxes" +SANDBOX_ROOT="${PENTAGI_SANDBOX_ROOT:-${TMPDIR:-/tmp}}/pentagi-review-sandboxes" +# A hardlink cannot cross filesystems, so --with-deps is unusable when $TMPDIR is its own mount +# (tmpfs /tmp, a separate /home, a container volume): `cp -al` fails outright and no sandbox is +# built. This sibling of the repo is on the repo's filesystem by construction. +FALLBACK_ROOT="$(dirname "$REPO_ROOT")/.pentagi-review-sandboxes" + +# Pick a root node_modules can actually be hardlinked into: probe with a real link, fall back to +# the repo's own filesystem when the probe hits EXDEV. +linkable_root() { + local probe="$SANDBOX_ROOT/.linkprobe" + + mkdir -p "$SANDBOX_ROOT" + + if ln "${BASH_SOURCE[0]}" "$probe" 2>/dev/null; then + rm -f "$probe" + echo "$SANDBOX_ROOT" + + return + fi + + if ! mkdir -p "$FALLBACK_ROOT" 2>/dev/null; then + echo "review-sandbox: $SANDBOX_ROOT is on another filesystem than the repo and" \ + "$FALLBACK_ROOT is not writable — set PENTAGI_SANDBOX_ROOT to a same-filesystem path" >&2 + exit 2 + fi + + echo "review-sandbox: hardlinks cannot reach $SANDBOX_ROOT — using $FALLBACK_ROOT" >&2 + echo "$FALLBACK_ROOT" +} create() { - local carry_dirty="" with_deps="" path + local carry_dirty="" with_deps="" path root="$SANDBOX_ROOT" for arg in "$@"; do case "$arg" in @@ -37,8 +65,12 @@ create() { esac done - path="$SANDBOX_ROOT/wt-$$-$(date +%s)" - mkdir -p "$SANDBOX_ROOT" + if [[ -n "$with_deps" ]]; then + root="$(linkable_root)" + fi + + path="$root/wt-$$-$(date +%s)" + mkdir -p "$root" git -C "$REPO_ROOT" worktree add --detach --quiet "$path" HEAD # Anything that fails from here leaves a worktree behind, and the caller reads stdout as the # sandbox path — so take the worktree down and say so on stderr rather than printing a stub. @@ -74,32 +106,48 @@ create() { # every agent on the host, so a bare `clean` used to take out a concurrent agent's live sandbox. STALE_MINUTES=120 +# A strict child of either sandbox root — never a root itself, so a sibling like -old and the +# root's own path are both rejected. +contained() { + local candidate="$1" root + + for root in "$SANDBOX_ROOT" "$FALLBACK_ROOT"; do + mkdir -p "$root" + + case "$candidate" in + "$(cd "$root" && pwd -P)"/?*) return 0 ;; + esac + done + + return 1 +} + clean() { local target="${1:-}" root resolved case "$target" in --all) - if [[ -d "$SANDBOX_ROOT" ]]; then - find "$SANDBOX_ROOT" -maxdepth 1 -name 'wt-*' -mmin "+$STALE_MINUTES" -exec rm -rf {} + - fi + for root in "$SANDBOX_ROOT" "$FALLBACK_ROOT"; do + if [[ -d "$root" ]]; then + find "$root" -maxdepth 1 -name 'wt-*' -mmin "+$STALE_MINUTES" -exec rm -rf {} + + fi + done ;; '') echo "review-sandbox: clean needs a sandbox path, or --all to sweep stale ones" >&2 exit 2 ;; *) - mkdir -p "$SANDBOX_ROOT" - root="$(cd "$SANDBOX_ROOT" && pwd -P)" resolved="$(cd "$(dirname "$target")" 2>/dev/null && pwd -P || true)/$(basename "$target")" case "$resolved" in */. | */..) resolved="" ;; - "$root"/?*) ;; - *) resolved="" ;; + *) contained "$resolved" || resolved="" ;; esac if [[ -z "$resolved" ]]; then - echo "review-sandbox: refusing to clean '$target' — not a sandbox under $root" >&2 + echo "review-sandbox: refusing to clean '$target' — not a sandbox under" \ + "$SANDBOX_ROOT or $FALLBACK_ROOT" >&2 exit 2 fi diff --git a/frontend/e2e/tools/review-sandbox.unit.test.ts b/frontend/e2e/tools/review-sandbox.unit.test.ts index f396daba..9ceb9858 100644 --- a/frontend/e2e/tools/review-sandbox.unit.test.ts +++ b/frontend/e2e/tools/review-sandbox.unit.test.ts @@ -74,6 +74,25 @@ describe('review-sandbox clean — containment', () => { }); }); +describe('review-sandbox create — where the sandbox lands', () => { + it('honours PENTAGI_SANDBOX_ROOT, the escape hatch for a $TMPDIR on another filesystem', () => { + const override = mkdtempSync(join(tmpdir(), 'override-root-')); + const sandbox = execFileSync(SCRIPT, ['create'], { + encoding: 'utf8', + env: { ...process.env, PENTAGI_SANDBOX_ROOT: override, TMPDIR: sandboxRoot }, + }).trim(); + + expect(sandbox.startsWith(`${override}/pentagi-review-sandboxes/`)).toBe(true); + + execFileSync(SCRIPT, ['clean', sandbox], { + env: { ...process.env, PENTAGI_SANDBOX_ROOT: override, TMPDIR: sandboxRoot }, + }); + expect(existsSync(sandbox)).toBe(false); + + rmSync(override, { force: true, recursive: true }); + }); +}); + describe('review-sandbox clean — the sweep', () => { it('refuses a bare clean rather than taking the whole root', () => { const { stdout: sandbox } = run('create');