tart: make deletion of missing VMs idempotent (#474)

Treat tart delete exit code 2 as successful cleanup so deleting an
already-removed VM does not prevent reconciliation from continuing.
Preserve other command failures and retain the wrapped process error
so callers can inspect its exit status.

Cover repeated deletion, other exit codes, process startup failures,
and wrapped exit-status preservation.

Pin CI actions to their previously used commit SHAs so jobs can start
under the repository action-pinning policy.

Validation: vmmanager package tests and lint for changed lines pass;
CI action pins match the last successful run.

.github/workflows/ci.yml

@@ -18,12 +18,12 @@ jobs:
     runs-on: ubuntu-latest
     timeout-minutes: 15
     steps:
-      - uses: actions/checkout@v6
-      - uses: actions/setup-go@v6
+      - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0
+      - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0
         with:
           go-version-file: go.mod
           cache: true
-      - uses: golangci/golangci-lint-action@v9
+      - uses: golangci/golangci-lint-action@ba0d7d2ec06a0ea1cb5fa41b2e4a3ab91d21278a # v9.3.0
         with:
           version: v2.13.0
           only-new-issues: true
@@ -33,13 +33,13 @@ jobs:
     runs-on: ubuntu-latest
     timeout-minutes: 60
     steps:
-      - uses: actions/checkout@v6
-      - uses: actions/setup-go@v6
+      - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0
+      - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0
         with:
           go-version-file: go.mod
           cache: true
       - name: Check out Vetu
-        uses: actions/checkout@v6
+        uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0
         with:
           repository: openai/vetu
           path: _vetu
@@ -67,8 +67,8 @@ jobs:
     runs-on: macos-26
     timeout-minutes: 45
     steps:
-      - uses: actions/checkout@v6
-      - uses: actions/setup-go@v6
+      - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0
+      - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0
         with:
           go-version-file: go.mod
           cache: true

internal/worker/vmmanager/base/cmd.go

@@ -30,8 +30,8 @@ func Cmd(
 	err := cmd.Run()
 	if err != nil {
 		if errors.Is(err, exec.ErrNotFound) {
-			return "", "", fmt.Errorf("%s command not found in PATH, make sure %s is installed",
-				commandName, strings.ToTitle(commandName))
+			return "", "", fmt.Errorf("%s command not found in PATH, make sure %s is installed: %w",
+				commandName, strings.ToTitle(commandName), err)
 		}
 
 		if exitErr, ok := err.(*exec.ExitError); ok {
@@ -46,9 +46,9 @@ func Cmd(
 				)
 			}
 
-			// Command failed, redefine the error to be the command-specific output
-			err = fmt.Errorf("%s command failed: %q", commandName,
-				firstNonEmptyLine(stderr.String(), stdout.String()))
+			// Preserve the exit status while adding the command-specific output.
+			err = fmt.Errorf("%s command failed: %q: %w", commandName,
+				firstNonEmptyLine(stderr.String(), stdout.String()), exitErr)
 		}
 	}
 

internal/worker/vmmanager/base/cmd_test.go

@@ -0,0 +1,61 @@
+package base_test
+
+import (
+	"context"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strconv"
+	"testing"
+	"time"
+
+	"github.com/cirruslabs/orchard/internal/worker/vmmanager/base"
+	"github.com/stretchr/testify/require"
+	"go.uber.org/zap"
+)
+
+func TestCmdPreservesExitStatus(t *testing.T) {
+	for _, commandName := range []string{"tart", "vetu"} {
+		t.Run(commandName, func(t *testing.T) {
+			commandPath := filepath.Join(t.TempDir(), commandName)
+			script := `#!/bin/sh
+printf 'command output\n'
+printf '\ncommand failed\nmore detail\n' >&2
+exit "$1"
+`
+			require.NoError(t, os.WriteFile(commandPath, []byte(script), 0o600))
+			require.NoError(t, os.Chmod(commandPath, 0o700)) //nolint:gosec // The fake command must be executable.
+
+			for _, exitCode := range []int{1, 2, 64} {
+				t.Run("exit-"+strconv.Itoa(exitCode), func(t *testing.T) {
+					ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second)
+					defer cancel()
+
+					stdout, stderr, err := base.Cmd(ctx, zap.NewNop().Sugar(), commandPath, strconv.Itoa(exitCode))
+
+					require.Equal(t, "command output\n", stdout)
+					require.Equal(t, "\ncommand failed\nmore detail\n", stderr)
+					require.ErrorContains(t, err, "command failed")
+					var exitErr *exec.ExitError
+					require.ErrorAs(t, err, &exitErr)
+					require.Equal(t, exitCode, exitErr.ExitCode())
+				})
+			}
+		})
+	}
+}
+
+func TestCmdPreservesSignalStatus(t *testing.T) {
+	commandPath := filepath.Join(t.TempDir(), "signalled-command")
+	require.NoError(t, os.WriteFile(commandPath, []byte("#!/bin/sh\nkill -TERM $$\n"), 0o600))
+	require.NoError(t, os.Chmod(commandPath, 0o700)) //nolint:gosec // The fake command must be executable.
+	ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second)
+	defer cancel()
+
+	_, _, err := base.Cmd(ctx, zap.NewNop().Sugar(), commandPath)
+
+	require.NoError(t, ctx.Err(), "the command must terminate before the test deadline")
+	var exitErr *exec.ExitError
+	require.ErrorAs(t, err, &exitErr)
+	require.Equal(t, -1, exitErr.ExitCode())
+}

internal/worker/vmmanager/tart/delete_test.go

@@ -0,0 +1,106 @@
+package tart //nolint:testpackage // VM fixtures require private state without starting clone or run goroutines.
+
+import (
+	"context"
+	"fmt"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"testing"
+
+	"github.com/cirruslabs/orchard/internal/worker/ondiskname"
+	"github.com/cirruslabs/orchard/internal/worker/vmmanager/base"
+	v1 "github.com/cirruslabs/orchard/pkg/resource/v1"
+	"github.com/stretchr/testify/require"
+	"go.uber.org/zap"
+)
+
+func TestDeleteIsIdempotent(t *testing.T) {
+	vm, vmPath := newVMForDelete(t, `#!/bin/sh
+[ "$#" -eq 2 ] && [ "$1" = delete ] || exit 64
+vm_dir="$TART_HOME/vms/$2"
+if [ ! -e "$vm_dir" ]; then
+  printf 'the specified VM "%s" does not exist\n' "$2" >&2
+  exit 2
+fi
+/bin/rm -r "$vm_dir"
+`)
+	require.NoError(t, os.MkdirAll(vmPath, 0o700))
+
+	require.NoError(t, vm.Delete())
+	require.NoDirExists(t, vmPath)
+	require.NoError(t, vm.Delete(), "deleting the same VM again must succeed")
+
+	select {
+	case <-vm.ctx.Done():
+	default:
+		t.Fatal("Delete did not cancel the VM context")
+	}
+}
+
+func TestDeletePreservesCommandFailures(t *testing.T) {
+	for _, exitCode := range []int{1, 64, 126} {
+		t.Run(fmt.Sprintf("exit-%d", exitCode), func(t *testing.T) {
+			vm, _ := newVMForDelete(t, fmt.Sprintf(
+				"#!/bin/sh\nprintf 'the specified VM does not exist: permission denied\\n' >&2\nexit %d\n",
+				exitCode,
+			))
+
+			err := vm.Delete()
+
+			require.ErrorIs(t, err, base.ErrVMFailed)
+			require.ErrorContains(t, err, "permission denied")
+			var exitErr *exec.ExitError
+			require.ErrorAs(t, err, &exitErr)
+			require.Equal(t, exitCode, exitErr.ExitCode())
+		})
+	}
+}
+
+func TestDeletePreservesProcessStartFailure(t *testing.T) {
+	vm, _ := newVMForDelete(t, "#!/nonexistent/tart-interpreter\n")
+
+	err := vm.Delete()
+
+	require.ErrorIs(t, err, base.ErrVMFailed)
+	require.ErrorIs(t, err, os.ErrNotExist)
+	var pathErr *os.PathError
+	require.ErrorAs(t, err, &pathErr)
+}
+
+func TestDeletePreservesMissingExecutable(t *testing.T) {
+	vm, _ := newVMForDelete(t, "#!/bin/sh\nexit 0\n")
+	t.Setenv("PATH", t.TempDir())
+
+	err := vm.Delete()
+
+	require.ErrorIs(t, err, base.ErrVMFailed)
+	require.ErrorIs(t, err, exec.ErrNotFound)
+	require.ErrorContains(t, err, "tart command not found in PATH")
+}
+
+func newVMForDelete(t *testing.T, script string) (*VM, string) {
+	t.Helper()
+
+	binDir := t.TempDir()
+	commandPath := filepath.Join(binDir, "tart")
+	require.NoError(t, os.WriteFile(commandPath, []byte(script), 0o600))
+	require.NoError(t, os.Chmod(commandPath, 0o700)) //nolint:gosec // Fake Tart must be executable.
+	t.Setenv("PATH", binDir)
+	tartHome := t.TempDir()
+	t.Setenv("TART_HOME", tartHome)
+
+	logger := zap.NewNop().Sugar()
+	ctx, cancel := context.WithCancel(t.Context())
+	t.Cleanup(cancel)
+	vm := &VM{ //nolint:exhaustruct_v5 // The fixture initializes only the state used by Delete.
+		onDiskName: ondiskname.New("delete-test", "11111111-2222-4333-8444-555555555555", 0),
+		logger:     logger,
+		ctx:        ctx,
+		cancel:     cancel,
+		VM:         base.NewVM(logger),
+	}
+	vm.ConditionsSet().Remove(v1.ConditionTypeCloning)
+
+	return vm, filepath.Join(tartHome, "vms", vm.id())
+}

internal/worker/vmmanager/tart/tart.go

@@ -2,7 +2,9 @@ package tart
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"os/exec"
 	"strconv"
 	"strings"
 	"sync"
@@ -19,6 +21,8 @@ import (
 	"go.uber.org/zap"
 )
 
+const tartDeleteExitCodeNotFound = 2
+
 type VM struct {
 	onDiskName ondiskname.OnDiskName
 	resource   v1.VM
@@ -480,7 +484,12 @@ func (vm *VM) Delete() error {
 
 	_, _, err := Tart(context.Background(), vm.logger, "delete", vm.id())
 	if err != nil {
-		return fmt.Errorf("%w: failed to delete VM: %v", base.ErrVMFailed, err)
+		var exitErr *exec.ExitError
+		if errors.As(err, &exitErr) && exitErr.ExitCode() == tartDeleteExitCodeNotFound {
+			return nil
+		}
+
+		return fmt.Errorf("%w: failed to delete VM: %w", base.ErrVMFailed, err)
 	}
 
 	return nil