Came across this issue while working on #236. I'll let Opus 5 take it from here…
Summary
Running ./scripts/tests from a sandvault checkout located at
/Users/Shared/sv-$USER/repos/sandvault deletes that checkout mid-run,
discarding any uncommitted work.
This is the exact path sv-clone produces for this repository, and the README
documents that invocation:
sv-clone https://github.com/webcoyote/sandvault.git -- codex
So the documented way to get a sandboxed sandvault checkout puts it precisely
where the test suite will delete it.
Cause
Six sv-clone tests hardcode the clone target and rm -rf it during setup and
teardown, without checking whether something is already there:
scripts/tests:1305: clone_path="$SHARED_WORKSPACE/repos/sandvault"
scripts/tests:1308: sv_cmd s -- rm -rf "$clone_path"
Same pattern at lines 1392, 1431, 1515, 1558 and 1627. Line 1230 has a related
form, clone_path="$SHARED_WORKSPACE/repos/$clone_name", where clone_name is
basename "$src_repository".
Because the deletion runs through sv_cmd s -- rm -rf, it executes as the
sandvault user, which has write access to the shared workspace — so it succeeds.
Reproduction
sv-clone https://github.com/webcoyote/sandvault.git
cd /Users/Shared/sv-$USER/repos/sandvault
echo "wip" > NEWFILE.md
./scripts/tests
ls # the directory no longer exists
Observed symptoms once the deletion happens: the rest of the run fails with
shell-init: error retrieving current directory: getcwd: cannot access parent directories: No such file or directory
/bin/bash: .../sv: No such file or directory
and roughly 22 tests fail for that reason alone, which obscures the real cause.
Impact
Uncommitted work in the checkout is lost with no warning and no prompt. A
contributor's first run of the test suite is a plausible way to hit this — and
the misleading getcwd errors make it easy to misdiagnose as an environment
problem rather than deletion.
Suggested fix
Refuse to delete a path the suite did not create. For example, before each
rm -rf "$clone_path":
if [[ -e "$clone_path" && ! -e "$clone_path/.sv-test-clone" ]]; then
fail "clone fixture path is occupied" \
"no pre-existing checkout at $clone_path" \
"refusing to delete $clone_path"
return
fi
writing the .sv-test-clone marker after each fixture clone. Alternatives:
- Derive the fixture path from
mktemp -d -p "$SHARED_WORKSPACE/repos" so it is
unique per run and can never collide with a real checkout.
- Abort the whole suite early if
$SHARED_WORKSPACE/repos/sandvault exists and
is a git work tree whose origin matches this repository — that is almost
certainly the user's own checkout.
The unique-temp-path option is the smallest change and removes the class of bug
rather than one instance of it.
Environment
Came across this issue while working on #236. I'll let Opus 5 take it from here…
Summary
Running
./scripts/testsfrom a sandvault checkout located at/Users/Shared/sv-$USER/repos/sandvaultdeletes that checkout mid-run,discarding any uncommitted work.
This is the exact path
sv-cloneproduces for this repository, and the READMEdocuments that invocation:
So the documented way to get a sandboxed sandvault checkout puts it precisely
where the test suite will delete it.
Cause
Six sv-clone tests hardcode the clone target and
rm -rfit during setup andteardown, without checking whether something is already there:
Same pattern at lines 1392, 1431, 1515, 1558 and 1627. Line 1230 has a related
form,
clone_path="$SHARED_WORKSPACE/repos/$clone_name", whereclone_nameisbasename "$src_repository".Because the deletion runs through
sv_cmd s -- rm -rf, it executes as thesandvault user, which has write access to the shared workspace — so it succeeds.
Reproduction
Observed symptoms once the deletion happens: the rest of the run fails with
and roughly 22 tests fail for that reason alone, which obscures the real cause.
Impact
Uncommitted work in the checkout is lost with no warning and no prompt. A
contributor's first run of the test suite is a plausible way to hit this — and
the misleading
getcwderrors make it easy to misdiagnose as an environmentproblem rather than deletion.
Suggested fix
Refuse to delete a path the suite did not create. For example, before each
rm -rf "$clone_path":writing the
.sv-test-clonemarker after each fixture clone. Alternatives:mktemp -d -p "$SHARED_WORKSPACE/repos"so it isunique per run and can never collide with a real checkout.
$SHARED_WORKSPACE/repos/sandvaultexists andis a git work tree whose
originmatches this repository — that is almostcertainly the user's own checkout.
The unique-temp-path option is the smallest change and removes the class of bug
rather than one instance of it.
Environment
42460b7(Merge pull request Block sandbox account from mounting disks #233)