Wait for the interrupted operation to clean up before exit (closes #159)
check / check (pull_request) Successful in 1m22s

On SIGINT/SIGTERM the process could exit before the interrupted command's
cleanup defers ran, leaving decrypted data in the temp directory: the blob
cache (vaultik-blobcache-*) and the decrypted snapshot database
(vaultik-restore-*).

RunApp now mirrors fx's run sequence: start, block on app.Wait(), then
app.Stop(), returning only after Stop completes. fx delivers both an OS
interrupt and the finished operation's Shutdowner.Shutdown() on that one
channel. Stop runs the OnStop hooks; the operation's hook cancels the command
and waits for its goroutine to return (bounded by shutdownTimeout) before
exit. The old code returned as soon as app.Done fired, without Stop, so on a
real interrupt it unwound to os.Exit while cleanup still ran and the wait
never blocked exit.

Restore's loops check the context between chunks and blobs so the wait ends
promptly. A cli test drives RunApp through the OnStop hook and asserts the
scratch file is gone before RunApp returns.

Model: opus-4-8
This commit is contained in:
2026-09-22 11:41:41 +00:00
parent 548a7ae156
commit 141a84fd0d
5 changed files with 350 additions and 57 deletions
+97
View File
@@ -0,0 +1,97 @@
package cli_test
import (
"context"
"os"
"path/filepath"
"testing"
"time"
"github.com/stretchr/testify/require"
"go.uber.org/fx"
"sneak.berlin/go/vaultik/internal/cli"
)
// TestRunAppWaitsForOperationCleanupOnShutdown drives RunApp with an fx app
// wired the way RunOperation wires a command: a single lifecycle hook whose
// OnStart launches the operation in its own goroutine and whose OnStop cancels
// it and blocks until that goroutine returns. The operation stands in for a
// restore blocked mid-download — it holds a decrypted "scratch" file and only
// removes it as it unwinds on cancellation.
//
// The app is asked to stop once the operation is running (standing in for an
// OS interrupt; fx delivers a real signal and Shutdowner.Shutdown() on the
// same app.Wait channel, so both drive the identical shutdown path). RunApp
// must not return until app.Stop has run the OnStop hook, so the scratch file
// must be gone by the time RunApp returns. Before the fix RunApp returned as
// soon as the app.Wait/Done channel fired, without running app.Stop, so the
// cleanup never ran and this file would still be on disk (issue #159).
func TestRunAppWaitsForOperationCleanupOnShutdown(t *testing.T) {
t.Parallel()
scratch := filepath.Join(t.TempDir(), "decrypted-scratch")
require.NoError(t, os.WriteFile(scratch, []byte("secret"), 0o600))
// Cancel and reap the operation even if RunApp returns without doing so
// (the buggy path), so the goroutine cannot leak past the test.
opCtx, opCancel := context.WithCancel(context.Background())
t.Cleanup(opCancel)
var stop func(context.Context) bool
app := fx.New(
fx.NopLogger,
fx.Invoke(func(lc fx.Lifecycle, sh fx.Shutdowner) {
lc.Append(fx.Hook{
OnStart: func(_ context.Context) error {
done := make(chan struct{})
go func() {
defer close(done)
// Blocked mid-operation until cancelled, then run the
// cleanup an interrupted restore would run.
<-opCtx.Done()
_ = os.Remove(scratch)
}()
stop = func(ctx context.Context) bool {
opCancel()
select {
case <-done:
return true
case <-ctx.Done():
return false
}
}
// Ask the app to stop now that the operation is running.
go func() { _ = sh.Shutdown() }()
return nil
},
OnStop: func(ctx context.Context) error {
stop(ctx)
return nil
},
})
}),
)
done := make(chan error, 1)
go func() { done <- cli.RunApp(context.Background(), app) }()
select {
case err := <-done:
require.NoError(t, err)
case <-time.After(30 * time.Second):
t.Fatal("RunApp did not return after shutdown was requested")
}
_, err := os.Stat(scratch)
require.True(t, os.IsNotExist(err),
"RunApp returned before the operation removed its decrypted scratch file")
}