Write rclone uploads under a temporary name and move them into place (closes #266)

The rclone backend wrote each object straight to its key, so killing an
upload to a local or sftp remote left a truncated object there that the
next backup trusted.

On every remote with a server-side move, an object is now written under
a name ending in `.partial` and moved onto its key with rclone's
operations.Move, which removes an object already at the key first;
drive, dropbox and others will not move onto one. Listings skip
`.partial` names. Rclone's own copy also requires the PartialUploads
flag; this does not, because hdfs shows a file while it is written
without setting it. Remotes without a move are written in place.

Model: opus-5-5

TODO.md

@@ -33,15 +33,6 @@ the tag exists and is exercised; what is left is merging `next` to
   `.partial` and moved into place, and listings skip such names. Remotes
   without one are still written in place.
 
-- 2026-10-07: Made an interrupted command exit 130 and say so
-  ([issue #267](https://git.eeqj.de/sneak/vaultik/issues/267)). Ctrl-C
-  or SIGTERM during `snapshot create`, `snapshot restore` or `snapshot
-  verify` exited 0 with no error line (`snapshot verify --json` exited
-  1, also without one), so a `--cron` run that never finished looked
-  like a success. A command stopped by either signal now exits 130 and
-  prints `interrupted before the command finished` on stderr, under
-  `--cron` and `--json` too.
-
 - 2026-10-07: Corrected documentation, help text and comments that were
   false about the code
   ([issue #233](https://git.eeqj.de/sneak/vaultik/issues/233)). A blob

internal/cli/app.go

@@ -206,10 +206,6 @@ func RunApp(ctx context.Context, app *fx.App) error {
 // RunOperation through cobra to Entry.
 var errReported = errors.New("operation failed")
 
-// errInterrupted marks an operation that SIGINT or SIGTERM stopped
-// before it finished. Entry shows it and returns exitCodeInterrupted.
-var errInterrupted = errors.New("interrupted before the command finished")
-
 // RunOperation runs op against the Vaultik instance inside the fx app
 // and turns a failure into a returned error rather than an os.Exit from
 // within the goroutine. An os.Exit there skipped main's deferred
@@ -224,21 +220,17 @@ var errInterrupted = errors.New("interrupted before the command finished")
 // interrupt OnStop cancels op and waits for the goroutine to return, so
 // op's cleanup (removing decrypted scratch files) runs before the
 // process exits; the wait is bounded by shutdownTimeout. report is
-// called with a failure so the caller can show it to the user before
+// called with a non-canceled failure so the caller can show it to the
-// it becomes errReported.
+// user before it becomes errReported. A context cancellation is the
-//
+// interrupt path, not a failure: it is neither reported nor counted as
-// The run counts as interrupted unless op returned, without an
+// one.
-// interrupt having cancelled it, before RunWithApp returned. An
-// interrupted op is not reported, whatever it returned; RunOperation
-// returns errInterrupted instead.
 func RunOperation(
 	ctx context.Context, opts AppOptions,
 	op func(v *vaultik.Vaultik) error, report func(err error),
 ) error {
 	var (
-		mu       sync.Mutex
+		mu     sync.Mutex
-		finished bool // op returned before any interrupt cancelled it
+		failed bool
-		failed   bool // op finished with an error
 	)
 
 	opts.Invokes = append(opts.Invokes,
@@ -249,21 +241,11 @@ func RunOperation(
 				OnStart: func(_ context.Context) error {
 					stop = v.StartOperation(func() {
 						err := op(v)
-
+						if err != nil && !errors.Is(err, context.Canceled) {
-						// Only stop, called from OnStop below, cancels the
+							report(err)
-						// Vaultik context, so a live context means no
-						// interrupt cancelled op. Check the context, not
-						// err: an interrupted op need not return
-						// context.Canceled (`snapshot verify --json`
-						// returns a verification failure).
-						if v.Context().Err() == nil {
-							if err != nil {
-								report(err)
-							}
 
 							mu.Lock()
-							finished = true
+							failed = true
-							failed = err != nil
 							mu.Unlock()
 						}
 
@@ -296,23 +278,16 @@ func RunOperation(
 		return err
 	}
 
-	// RunWithApp returns only after the app was asked to stop, either by
+	// The goroutine sets failed before triggering the shutdown that lets
-	// an interrupt or by the goroutine's Shutdown call. When op finished
+	// RunWithApp return, so the write is in place by the time we read it.
-	// without being cancelled, the goroutine set finished before that
-	// call. So if finished is unset here, an interrupt stopped the app,
-	// and op either returned after it was cancelled or is still running
-	// because the shutdown timed out.
 	mu.Lock()
 	defer mu.Unlock()
 
-	switch {
+	if failed {
-	case !finished:
-		return errInterrupted
-	case failed:
 		return errReported
-	default:
-		return nil
 	}
+
+	return nil
 }
 
 // runVaultikApp runs the standard single-operation command lifecycle

internal/cli/entry.go

@@ -15,11 +15,6 @@ import (
 // the startup banner.
 const shortCommitLen = 12
 
-// exitCodeInterrupted is the exit status of a command that SIGINT or
-// SIGTERM stopped. It is 128 plus SIGINT's number, 2, which is what a
-// shell reports for a command stopped by Ctrl-C.
-const exitCodeInterrupted = 130
-
 // Entry is the main entry point for the CLI application.
 // It prints the startup banner to stderr (unless a banner-suppressing
 // flag is present in os.Args — see bannerSuppressedInArgs), executes the
@@ -28,10 +23,9 @@ const exitCodeInterrupted = 130
 // The banner goes to stderr because stdout carries only the output the
 // user asked for, such as a completion script or a `config get` value.
 //
-// It returns the process exit code (0 on success, 130 when interrupted,
+// It returns the process exit code (0 on success, 1 on error) rather
-// 1 on any other error) rather than calling os.Exit, so that main's
+// than calling os.Exit, so that main's deferred profile writers run
-// deferred profile writers run before the process ends. See run in
+// before the process ends. See run in cmd/vaultik/main.go.
-// cmd/vaultik/main.go.
 func Entry() int {
 	emitStartupBanner(os.Args[1:], os.Stderr)
 
@@ -45,16 +39,11 @@ func Entry() int {
 		// document instead); errReported says so. Printing it again
 		// here would double the error line.
 		// Every other error — bad arguments, a config that would not
-		// load, an interrupt — reaches Entry unreported, so it is shown
+		// load — reaches Entry unreported, so it is shown here.
-		// here.
 		if !errors.Is(err, errReported) {
 			ReportErrorf("%s", err.Error())
 		}
 
-		if errors.Is(err, errInterrupted) {
-			return exitCodeInterrupted
-		}
-
 		return 1
 	}
 

internal/cli/entry_interrupt_test.go

@@ -1,203 +0,0 @@
-package cli //nolint:testpackage // shares runEntry and the argument constants
-
-import (
-	"fmt"
-	"net/http"
-	"net/http/httptest"
-	"os"
-	"os/signal"
-	"path/filepath"
-	"strings"
-	"testing"
-	"time"
-
-	"github.com/adrg/xdg"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-)
-
-// stalledStoreConfig is hermeticConfig with an s3:// destination store
-// in place of the file:// one. The server behind it accepts any
-// credentials.
-const stalledStoreConfig = `age_recipients:
-  - age1278m9q7dp3chsh2dcy82qk27v047zywyvtxwnj4cvt0z65jw6a7q5dqhfj
-snapshots:
-  test:
-    paths:
-      - %s
-storage_url: s3://bucket?endpoint=%s&ssl=false
-s3:
-  access_key_id: key
-  secret_access_key: secret
-index_path: %s
-hostname: test-host
-`
-
-// interruptRepeat is how often interruptOnFirstRequest sends SIGINT.
-const interruptRepeat = 50 * time.Millisecond
-
-// TestEntryInterruptedRun sends SIGINT to the test process while a
-// command waits on the destination store, and checks that Entry returns
-// 130 and prints one line on stderr saying the run was interrupted. The
-// store is a local HTTP server that holds every request open, so the
-// command is always mid-operation when the signal arrives. The two
-// cases cover --cron and --json, which silence other output.
-//
-// Not parallel: it signals the process and replaces os.Args, os.Stdout,
-// os.Stderr and the xdg globals.
-//
-//nolint:paralleltest // signals the process and replaces process globals
-func TestEntryInterruptedRun(t *testing.T) {
-	for _, testCase := range []struct {
-		name string
-		args []string
-	}{
-		{
-			name: "snapshot create --cron",
-			args: []string{cmdSnapshot, cmdCreate, "--cron"},
-		},
-		{
-			name: "snapshot verify --json",
-			args: []string{cmdSnapshot, cmdVerify, someSnapshotID, flagJSON},
-		},
-	} {
-		t.Run(testCase.name, func(t *testing.T) {
-			endpoint, requestArrived := startStalledStore(t)
-			configPath := writeStalledStoreConfig(t, endpoint)
-			interruptOnFirstRequest(t, requestArrived)
-
-			code, _, stderr := runEntry(t,
-				append([]string{flagConfig, configPath}, testCase.args...)...)
-
-			assert.Equal(t, 130, code)
-			assert.Equal(t, 1,
-				strings.Count(stderr, errInterrupted.Error()), stderr)
-		})
-	}
-}
-
-// interruptOnFirstRequest sends SIGINT to the test process every
-// interruptRepeat, from the first request to the destination store until
-// the test ends. One signal is not enough: the command can reach the
-// store before fx has started catching signals. The test catches SIGINT
-// too, so that a signal fx is not catching does not kill the test
-// binary.
-func interruptOnFirstRequest(t *testing.T, requestArrived <-chan struct{}) {
-	t.Helper()
-
-	self, err := os.FindProcess(os.Getpid())
-	require.NoError(t, err)
-
-	caught := make(chan os.Signal, 1)
-	signal.Notify(caught, os.Interrupt)
-
-	testEnded := make(chan struct{})
-	senderDone := make(chan struct{})
-
-	// Stop catching SIGINT only after the sender has returned. The sender
-	// waits for each SIGINT it sends to arrive on caught; one still on
-	// its way after signal.Stop would kill the test binary.
-	t.Cleanup(func() {
-		close(testEnded)
-		<-senderDone
-		signal.Stop(caught)
-	})
-
-	go func() {
-		defer close(senderDone)
-
-		select {
-		case <-requestArrived:
-		case <-testEnded:
-			return
-		}
-
-		ticker := time.NewTicker(interruptRepeat)
-		defer ticker.Stop()
-
-		for {
-			// Empty caught, so that the receive below waits for this
-			// SIGINT rather than an earlier one.
-			select {
-			case <-caught:
-			default:
-			}
-
-			sendErr := self.Signal(os.Interrupt)
-			if sendErr != nil {
-				t.Errorf("sending SIGINT: %v", sendErr)
-
-				return
-			}
-
-			<-caught
-
-			select {
-			case <-testEnded:
-				return
-			case <-ticker.C:
-			}
-		}
-	}()
-}
-
-// startStalledStore starts an HTTP server that never answers: each
-// request is held until the client gives up on it or the test ends.
-// It returns the server's host:port and a channel that receives a value
-// when the first request arrives.
-func startStalledStore(t *testing.T) (string, <-chan struct{}) {
-	t.Helper()
-
-	requestArrived := make(chan struct{}, 1)
-	release := make(chan struct{})
-
-	server := httptest.NewServer(http.HandlerFunc(
-		func(_ http.ResponseWriter, r *http.Request) {
-			select {
-			case requestArrived <- struct{}{}:
-			default:
-			}
-
-			select {
-			case <-r.Context().Done():
-			case <-release:
-			}
-		}))
-
-	// Cleanups run last-registered first, so release lets any held
-	// request return before Close waits for it.
-	t.Cleanup(server.Close)
-	t.Cleanup(func() { close(release) })
-
-	return server.Listener.Addr().String(), requestArrived
-}
-
-// writeStalledStoreConfig writes a config whose destination store is the
-// server at endpoint and whose snapshot source holds one small file, so
-// that `snapshot create` has a blob to upload. Returns the config path.
-func writeStalledStoreConfig(t *testing.T, endpoint string) string {
-	t.Helper()
-
-	dir := t.TempDir()
-	configPath := filepath.Join(dir, "config.yml")
-	sourceDir := filepath.Join(dir, "source")
-
-	require.NoError(t, os.Mkdir(sourceDir, 0o750))
-	require.NoError(t, os.WriteFile(filepath.Join(sourceDir, "file.txt"),
-		[]byte("contents"), 0o600))
-
-	contents := fmt.Sprintf(stalledStoreConfig,
-		sourceDir, endpoint, filepath.Join(dir, "index.sqlite"))
-
-	require.NoError(t,
-		os.WriteFile(configPath, []byte(contents), configFileMode))
-
-	// The PID lock lives under xdg.DataHome, which xdg resolves at
-	// package init; point it at the temp dir so the test neither
-	// touches nor collides with the real one.
-	t.Setenv("XDG_DATA_HOME", filepath.Join(dir, "data"))
-	xdg.Reload()
-	t.Cleanup(xdg.Reload)
-
-	return configPath
-}

internal/storage/rclone_test.go

@@ -63,33 +63,34 @@ func TestNewRcloneStorerConstruction(t *testing.T) {
 	}
 }
 
-// unbufferedUploadContext returns a context in which rclone neither reads
+// TestRcloneStorerObjectAppearsOnlyWhenComplete checks that on a remote
-// ahead of the write nor holds a small upload in memory. Without it, a
+// with a server-side move, such as local, nothing is at the key until the
-// progress callback runs before anything is written to the remote.
+// upload has finished, so a killed upload cannot leave a truncated object
-func unbufferedUploadContext() context.Context {
+// there.
+//
+//nolint:paralleltest // NewRcloneStorer installs the process-global rclone config
+func TestRcloneStorerObjectAppearsOnlyWhenComplete(t *testing.T) {
+	// Without this, rclone reads ahead of the write and holds a small
+	// upload in memory, so the progress callback would run before anything
+	// is written to the remote.
 	ctx, ci := fs.AddConfig(context.Background())
 	ci.BufferSize = 0
 	ci.StreamingUploadCutoff = 0
 
-	return ctx
+	s, err := storage.NewRcloneStorer(ctx, ":local", t.TempDir())
-}
+	if err != nil {
-
+		t.Fatalf("NewRcloneStorer: %v", err)
-// objectAtKeyDuringUpload uploads data to testBlobKey and reports whether
+	}
-// an object was at the key before the upload finished. It fails the test
-// unless the key then reads back as the uploaded data.
-func objectAtKeyDuringUpload(
-	ctx context.Context, t *testing.T, s *storage.RcloneStorer,
-) bool {
-	t.Helper()
 
+	key := testBlobKey
 	data := bytes.Repeat([]byte("blob-bytes"), 1000)
-	seen := false
+	seenEarly := false
 
-	err := s.PutWithProgress(ctx, testBlobKey, bytes.NewReader(data),
+	err = s.PutWithProgress(ctx, key, bytes.NewReader(data), int64(len(data)),
-		int64(len(data)), func(int64) error {
+		func(int64) error {
-			_, statErr := s.Stat(ctx, testBlobKey)
+			_, statErr := s.Stat(ctx, key)
 			if statErr == nil {
-				seen = true
+				seenEarly = true
 			}
 
 			return nil
@@ -98,53 +99,18 @@ func objectAtKeyDuringUpload(
 		t.Fatalf("PutWithProgress: %v", err)
 	}
 
-	got := readObject(ctx, t, s, testBlobKey)
+	if seenEarly {
-	if !bytes.Equal(got, data) {
-		t.Errorf("object read back as %d bytes, want the %d uploaded",
-			len(got), len(data))
-	}
-
-	return seen
-}
-
-// readObject returns the contents of the object at key.
-func readObject(
-	ctx context.Context, t *testing.T, s *storage.RcloneStorer, key string,
-) []byte {
-	t.Helper()
-
-	rc, err := s.Get(ctx, key)
-	if err != nil {
-		t.Fatalf("Get: %v", err)
-	}
-
-	defer func() { _ = rc.Close() }()
-
-	got, err := io.ReadAll(rc)
-	if err != nil {
-		t.Fatalf("reading object: %v", err)
-	}
-
-	return got
-}
-
-// TestRcloneStorerObjectAppearsOnlyWhenComplete checks that on a remote
-// with a server-side move, such as local, nothing is at the key until the
-// upload has finished, so a killed upload cannot leave a truncated object
-// there.
-//
-//nolint:paralleltest // NewRcloneStorer installs the process-global rclone config
-func TestRcloneStorerObjectAppearsOnlyWhenComplete(t *testing.T) {
-	ctx := unbufferedUploadContext()
-
-	s, err := storage.NewRcloneStorer(ctx, ":local", t.TempDir())
-	if err != nil {
-		t.Fatalf("NewRcloneStorer: %v", err)
-	}
-
-	if objectAtKeyDuringUpload(ctx, t, s) {
 		t.Error("object was at its key before the upload finished")
 	}
+
+	info, err := s.Stat(ctx, key)
+	if err != nil {
+		t.Fatalf("Stat after upload: %v", err)
+	}
+
+	if info.Size != int64(len(data)) {
+		t.Errorf("stored size = %d, want %d", info.Size, len(data))
+	}
 }
 
 // TestRcloneStorerListSkipsPartialFiles checks that a temporary file left
@@ -198,94 +164,6 @@ func TestRcloneStorerListSkipsPartialFiles(t *testing.T) {
 	}
 }
 
-// newRcloneStorerOnWrappedLocal registers name as rclone's local backend
-// wrapped by wrap, and builds an rclone backend on it rooted at a fresh
-// temp directory. wrap changes the features the local backend reports, so
-// that it behaves like a remote a unit test cannot reach.
-func newRcloneStorerOnWrappedLocal(
-	ctx context.Context, t *testing.T, name string, wrap func(fs.Fs) fs.Fs,
-) *storage.RcloneStorer {
-	t.Helper()
-
-	fs.Register(&fs.RegInfo{
-		Name: name,
-		NewFs: func(
-			ctx context.Context, _, root string, _ configmap.Mapper,
-		) (fs.Fs, error) {
-			local, err := fs.NewFs(ctx, ":local:"+root)
-			if err != nil {
-				return nil, err
-			}
-
-			return wrap(local), nil
-		},
-	})
-
-	s, err := storage.NewRcloneStorer(ctx, ":"+name, t.TempDir())
-	if err != nil {
-		t.Fatalf("NewRcloneStorer: %v", err)
-	}
-
-	return s
-}
-
-// withoutPartialUploads is rclone's local backend with the PartialUploads
-// flag cleared. Like hdfs, it then has a server-side move and shows a file
-// while it is written, without setting that flag.
-type withoutPartialUploads struct {
-	fs.Fs
-}
-
-func (f *withoutPartialUploads) Features() *fs.Features {
-	features := *f.Fs.Features()
-	features.PartialUploads = false
-
-	return &features
-}
-
-// TestRcloneStorerMovesIntoPlaceWithoutPartialUploads checks that on a
-// remote with a server-side move nothing is at the key until the upload has
-// finished, even when rclone does not mark the remote as showing partial
-// uploads.
-//
-//nolint:paralleltest // NewRcloneStorer installs the process-global rclone config
-func TestRcloneStorerMovesIntoPlaceWithoutPartialUploads(t *testing.T) {
-	ctx := unbufferedUploadContext()
-	s := newRcloneStorerOnWrappedLocal(ctx, t, "withoutpartialuploads",
-		func(local fs.Fs) fs.Fs { return &withoutPartialUploads{Fs: local} })
-
-	if objectAtKeyDuringUpload(ctx, t, s) {
-		t.Error("object was at its key before the upload finished")
-	}
-}
-
-// withoutMove is rclone's local backend reporting no server-side move.
-type withoutMove struct {
-	fs.Fs
-}
-
-func (f *withoutMove) Features() *fs.Features {
-	features := *f.Fs.Features()
-	features.Move = nil
-
-	return &features
-}
-
-// TestRcloneStorerWritesInPlaceWithoutMove checks that on a remote with no
-// server-side move an object is written straight to its key and reads back
-// from there.
-//
-//nolint:paralleltest // NewRcloneStorer installs the process-global rclone config
-func TestRcloneStorerWritesInPlaceWithoutMove(t *testing.T) {
-	ctx := unbufferedUploadContext()
-	s := newRcloneStorerOnWrappedLocal(ctx, t, "withoutmove",
-		func(local fs.Fs) fs.Fs { return &withoutMove{Fs: local} })
-
-	if !objectAtKeyDuringUpload(ctx, t, s) {
-		t.Error("object was not at its key while it was uploaded")
-	}
-}
-
 var errNameConflict = errors.New("an object with this name already exists")
 
 // moveRefusesExisting is rclone's local backend with a server-side move
@@ -320,18 +198,46 @@ func (f *moveRefusesExisting) move(
 //
 //nolint:paralleltest // NewRcloneStorer installs the process-global rclone config
 func TestRcloneStorerOverwritesWhereMoveRefusesExisting(t *testing.T) {
+	fs.Register(&fs.RegInfo{
+		Name: "moverefusesexisting",
+		NewFs: func(
+			ctx context.Context, _, root string, _ configmap.Mapper,
+		) (fs.Fs, error) {
+			local, err := fs.NewFs(ctx, ":local:"+root)
+			if err != nil {
+				return nil, err
+			}
+
+			return &moveRefusesExisting{Fs: local}, nil
+		},
+	})
+
 	ctx := context.Background()
-	s := newRcloneStorerOnWrappedLocal(ctx, t, "moverefusesexisting",
+
-		func(local fs.Fs) fs.Fs { return &moveRefusesExisting{Fs: local} })
+	s, err := storage.NewRcloneStorer(ctx, ":moverefusesexisting", t.TempDir())
+	if err != nil {
+		t.Fatalf("NewRcloneStorer: %v", err)
+	}
 
 	for _, content := range []string{"first", "second"} {
-		err := s.Put(ctx, testBlobKey, strings.NewReader(content))
+		err = s.Put(ctx, testBlobKey, strings.NewReader(content))
 		if err != nil {
 			t.Fatalf("Put %q: %v", content, err)
 		}
 	}
 
-	got := readObject(ctx, t, s, testBlobKey)
+	rc, err := s.Get(ctx, testBlobKey)
+	if err != nil {
+		t.Fatalf("Get: %v", err)
+	}
+
+	defer func() { _ = rc.Close() }()
+
+	got, err := io.ReadAll(rc)
+	if err != nil {
+		t.Fatalf("reading object: %v", err)
+	}
+
 	if string(got) != "second" {
 		t.Errorf("object = %q, want %q", got, "second")
 	}