amadan
Install the seed script's trap only once the scratch worktree is its own
The trap used to be installed before the "already exists" guard, so the guard's exit ran cleanup against a worktree another run was using: the two exporter files deleted from under it and a forced remove. The guard now exits first, the worktree is added, and only then is the trap installed. cleanup keeps the run's own exit status, reports a worktree it could not remove instead of hiding it (and makes the run fail), and prunes either way. A signal exits through the same trap. TMPDIR must be set: this runs on a shared machine where /tmp is not scratch.
Summary
hack/seed/run.sh: the existing-path check runs before any trap, so a run that finds another run's worktree exits 1 without touching it; TMPDIR must be set (no /tmp fallback); a failed worktree remove is reported and fails the run, and prune runs either way; INT and TERM exit, and the EXIT trap cleans up.
Agent notes
Task 5 fix round 1, Important 1 and Minor 1. Exercised by hand: TMPDIR unset exits 2; an existing $TMPDIR/rastrillo-seed with a marker file in it exits 1 with the marker still there and no worktree added.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 file changed,
+31
−9
hack/seed/run.sh+31 −9
diff --git a/hack/seed/run.sh b/hack/seed/run.sh| index 84ea925..bbf6bbe 100755 |
| --- a/hack/seed/run.sh |
| +++ b/hack/seed/run.sh |
| @@ -8,8 +8,11 @@ |
| # hack/seed/_rastrillo/ (the leading _ keeps Go's tools out) and are |
| # copied into the worktree for the run. |
| # |
| -# The worktree goes under $TMPDIR, and the trap removes it and the two |
| -# copied files whether the run passed or failed. |
| +# The worktree goes under $TMPDIR, which must be set: this runs on a |
| +# shared machine, and /tmp is not scratch there. A worktree already at |
| +# that path belongs to another run and is left alone. Once this run |
| +# has made its own, the trap removes it and the two copied files |
| +# whether the run passed, failed or was interrupted. |
| # |
| # FIESTA_SEED_DRYRUN=1 hack/seed/run.sh ~/src/rastrillo e14b745a7c08d0efa3bf221a8b6a5e75de15c46f |
| # |
| @@ -20,24 +23,43 @@ if [ $# -ne 2 ]; then |
| echo "usage: $0 <rastrillo checkout> <SEED>" >&2 |
| exit 2 |
| fi |
| +if [ -z "${TMPDIR:-}" ]; then |
| + echo "run.sh: TMPDIR is unset; set it to scratch space on disk, not /tmp" >&2 |
| + exit 2 |
| +fi |
| checkout=$1 |
| seed=$2 |
| here=$(cd "$(dirname "$0")" && pwd) |
| fiesta=$(cd "$here/../.." && pwd) |
| -wt="${TMPDIR:-/tmp}/rastrillo-seed" |
| +wt="$TMPDIR/rastrillo-seed" |
| + |
| +if [ -e "$wt" ]; then |
| + echo "run.sh: $wt already exists; another run may be using it. Remove it by hand if not." >&2 |
| + exit 1 |
| +fi |
| +# cleanup runs from the EXIT trap, installed only once the worktree |
| +# below is ours. It keeps the run's own exit status, and a worktree |
| +# that could not be removed is reported and makes the run fail rather |
| +# than being hidden; prune runs either way. |
| cleanup() { |
| + status=$? |
| rm -f "$wt/internal/designsystem/designsystem_fiestaseed_test.go" "$wt/ui/ui_fiestaseed_test.go" |
| - git -C "$checkout" worktree remove --force "$wt" 2>/dev/null || true |
| + if ! git -C "$checkout" worktree remove --force "$wt"; then |
| + echo "run.sh: could not remove the scratch worktree $wt; remove it by hand" >&2 |
| + if [ "$status" -eq 0 ]; then |
| + status=1 |
| + fi |
| + fi |
| git -C "$checkout" worktree prune |
| + exit "$status" |
| } |
| -trap cleanup EXIT INT TERM |
| -if [ -e "$wt" ]; then |
| - echo "$wt already exists; remove it first" >&2 |
| - exit 1 |
| -fi |
| git -C "$checkout" worktree add --detach "$wt" "$seed" |
| +trap cleanup EXIT |
| +# A signal exits, and the EXIT trap then cleans up. |
| +trap 'exit 130' INT TERM |
| + |
| cp "$here/_rastrillo/designsystem_fiestaseed_test.go" "$wt/internal/designsystem/" |
| cp "$here/_rastrillo/ui_fiestaseed_test.go" "$wt/ui/" |