Fix scripts/tests deleting a real checkout at $SHARED_WORKSPACE/repos - #256
Merged
Merged
Conversation
The sv-clone tests hardcoded their fixture path to $SHARED_WORKSPACE/repos/sandvault and rm -rf'd it during setup and teardown. That is exactly where sv-clone puts a contributor's own checkout, and the README documents that invocation, so running the suite from such a checkout deleted it mid-run along with any uncommitted work. The deletion goes through `sv_cmd s -- rm -rf`, which runs as the sandvault user and has write access, so it succeeded. The subsequent getcwd errors made it look like an environment problem. Give the clone tests a unique per-run workspace from mktemp -d and point sv-clone at it via SV_CLONE_WORKSPACE_ROOT, so the fixture path can never collide with a real checkout regardless of repo name. This also covers test_sv_clone_local, whose path derives from the source repo's basename. SV_CLONE_WORKSPACE_ROOT defaults to the original location, so sv-clone behavior is unchanged when it is unset. Fixes #237 Note: the clone tests did not execute during verification. This sandbox cannot create /Users/Shared/sv-avtest*, so the agentsview test fails and aborts the suite before the clone block is reached; that failure reproduces identically on unmodified main. Verified separately: bash -n and shellcheck -S warning clean on both files, the override redirects the clone destination, the default path is unchanged when unset, and a real clone through the override landed in the fixture workspace rather than repos/sandvault. The clone tests themselves still need a run in an environment where the suite gets that far. Known gaps, not addressed here: the fixture workspace is created inside repos/ as a sibling of real checkouts and should live elsewhere, and cleanup runs at the end of the clone block rather than hooking the suite's existing cleanup_guard trap, so it leaks if the run aborts.
webcoyote
force-pushed
the
fix/237-tests-clone-fixture-workspace
branch
from
September 8, 2026 19:14
729e7a4 to
d4630fd
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The sv-clone tests hardcoded their fixture path to $SHARED_WORKSPACE/repos/sandvault and rm -rf'd it during setup and teardown. That is exactly where sv-clone puts a contributor's own checkout, and the README documents that invocation, so running the suite from such a checkout deleted it mid-run along with any uncommitted work. The deletion goes through
sv_cmd s -- rm -rf, which runs as the sandvault user and has write access, so it succeeded. The subsequent getcwd errors made it look like an environment problem.Give the clone tests a unique per-run workspace from mktemp -d and point sv-clone at it via SV_CLONE_WORKSPACE_ROOT, so the fixture path can never collide with a real checkout regardless of repo name. This also covers test_sv_clone_local, whose path derives from the source repo's basename.
SV_CLONE_WORKSPACE_ROOT defaults to the original location, so sv-clone behavior is unchanged when it is unset.
Fixes #237