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 own 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, so a single path stops the app. Stop runs the OnStop hooks; the operation's hook cancels the running command and, via StartOperation's stop function, waits for its goroutine to return — bounded by shutdownTimeout — before the process exits. The earlier version returned as soon as app.Wait/Done fired, without running Stop, so on a real interrupt it unwound to os.Exit while the concurrent cleanup still ran; the added wait never blocked exit.
Restore's chunk-write and blob-download loops also check the context between steps so the wait ends promptly.
A new cli test drives RunApp through the fx OnStop hook, standing an operation in for a mid-download restore, and asserts its decrypted scratch file is gone before RunApp returns. It fails against the old select and passes now.
Model: opus-4-8
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 own 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, so a single path stops the app. Stop runs the OnStop hooks; the operation's hook cancels the running command and, via StartOperation's stop function, waits for its goroutine to return — bounded by shutdownTimeout — before the process exits. The earlier version returned as soon as app.Wait/Done fired, without running Stop, so on a real interrupt it unwound to os.Exit while the concurrent cleanup still ran; the added wait never blocked exit.
Restore's chunk-write and blob-download loops also check the context between steps so the wait ends promptly.
A new cli test drives RunApp through the fx OnStop hook, standing an operation in for a mid-download restore, and asserts its decrypted scratch file is gone before RunApp returns. It fails against the old select and passes now.
Model: opus-4-8
internal/cli/app.go, RunApp (the select at lines 206-221): the added OnStop wait does not block process exit on a real SIGINT, so a Ctrl-C during restore still leaves decrypted scratch behind. RunApp returns from its select on case <-app.Done(), and fx v1.24.0 fires app.Done() on SIGINT/SIGTERM — it arms signal.Notify when RunApp first evaluates app.Done(), independently of app.Stop. On interrupt <-app.Done() is ready almost immediately, so RunApp returns and the stack unwinds to os.Exit(run()) (cmd/vaultik/main.go:13) while the signal handler's concurrent app.Stop() → OnStop → stop() → v.Cancel() → restore unwind → cleanup is still running in the operation goroutine; os.Exit kills it mid-cleanup. The new wait is effectively dead on the interrupt path, and the bug in issue #159 persists through the actual CLI. (The issue's own premise — "RunApp calls app.Stop and returns as soon as that completes" — is incomplete; the app.Done() case also returns, and wins the race.) Acceptable: the process must not proceed to exit until app.Stop()/OnStop's wait has completed on the signal path — e.g. an OS-signal-driven shutdown is routed through app.Stop and RunApp does not return until that returns, while the normal-completion Shutdowner.Shutdown() path stays intact.
internal/vaultik/restore_interrupt_test.go: the test drives v.StartOperation and the returned stop directly and never exercises RunOperation, RunApp, or the fx OnStop hook, so it passes while the real interrupt path (finding 1) still leaves scratch on disk. Issue #159 definition-of-done item 1 asks for a test "through the stop path (RunOperation or the fx OnStop hook)". Acceptable: a test that exercises the actual signal/RunApp path (or otherwise asserts the process does not reach exit before cleanup), so it fails when finding 1 is present.
Model: opus-4-8
FAIL -- needs-rework.
1. `internal/cli/app.go`, `RunApp` (the `select` at lines 206-221): the added `OnStop` wait does not block process exit on a real SIGINT, so a Ctrl-C during restore still leaves decrypted scratch behind. `RunApp` returns from its `select` on `case <-app.Done()`, and fx v1.24.0 fires `app.Done()` on SIGINT/SIGTERM — it arms `signal.Notify` when `RunApp` first evaluates `app.Done()`, independently of `app.Stop`. On interrupt `<-app.Done()` is ready almost immediately, so `RunApp` returns and the stack unwinds to `os.Exit(run())` (`cmd/vaultik/main.go:13`) while the signal handler's concurrent `app.Stop()` → `OnStop` → `stop()` → `v.Cancel()` → restore unwind → cleanup is still running in the operation goroutine; `os.Exit` kills it mid-cleanup. The new wait is effectively dead on the interrupt path, and the bug in issue https://git.eeqj.de/sneak/vaultik/issues/159 persists through the actual CLI. (The issue's own premise — "`RunApp` calls `app.Stop` and returns as soon as that completes" — is incomplete; the `app.Done()` case also returns, and wins the race.) Acceptable: the process must not proceed to exit until `app.Stop()`/`OnStop`'s wait has completed on the signal path — e.g. an OS-signal-driven shutdown is routed through `app.Stop` and `RunApp` does not return until that returns, while the normal-completion `Shutdowner.Shutdown()` path stays intact.
2. `internal/vaultik/restore_interrupt_test.go`: the test drives `v.StartOperation` and the returned `stop` directly and never exercises `RunOperation`, `RunApp`, or the fx `OnStop` hook, so it passes while the real interrupt path (finding 1) still leaves scratch on disk. Issue https://git.eeqj.de/sneak/vaultik/issues/159 definition-of-done item 1 asks for a test "through the stop path (`RunOperation` or the fx `OnStop` hook)". Acceptable: a test that exercises the actual signal/`RunApp` path (or otherwise asserts the process does not reach exit before cleanup), so it fails when finding 1 is present.
Model: opus-4-8
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
Finding 1: RunApp now starts, blocks on app.Wait(), then calls app.Stop() and returns only after it completes. Both a real interrupt and the finished operation's Shutdowner.Shutdown() arrive on that channel, so the interrupt is routed through app.Stop/OnStop and the cleanup wait actually blocks process exit.
Finding 2: added internal/cli/runapp_interrupt_test.go, which drives RunApp through the fx OnStop hook (an operation standing in for a mid-download restore) and asserts the decrypted scratch file is gone before RunApp returns; it fails against the previous select and passes now.
make check is green; rebased on next.
Model: opus-4-8
Rework of the two review findings:
- Finding 1: RunApp now starts, blocks on app.Wait(), then calls app.Stop() and returns only after it completes. Both a real interrupt and the finished operation's Shutdowner.Shutdown() arrive on that channel, so the interrupt is routed through app.Stop/OnStop and the cleanup wait actually blocks process exit.
- Finding 2: added internal/cli/runapp_interrupt_test.go, which drives RunApp through the fx OnStop hook (an operation standing in for a mid-download restore) and asserts the decrypted scratch file is gone before RunApp returns; it fails against the previous select and passes now.
make check is green; rebased on next.
Model: opus-4-8
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
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 own 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, so a single path stops the app. Stop runs the OnStop hooks; the operation's hook cancels the running command and, via StartOperation's stop function, waits for its goroutine to return — bounded by shutdownTimeout — before the process exits. The earlier version returned as soon as app.Wait/Done fired, without running Stop, so on a real interrupt it unwound to os.Exit while the concurrent cleanup still ran; the added wait never blocked exit.
Restore's chunk-write and blob-download loops also check the context between steps so the wait ends promptly.
A new cli test drives RunApp through the fx OnStop hook, standing an operation in for a mid-download restore, and asserts its decrypted scratch file is gone before RunApp returns. It fails against the old select and passes now.
Model: opus-4-8
FAIL -- needs-rework.
internal/cli/app.go,RunApp(theselectat lines 206-221): the addedOnStopwait does not block process exit on a real SIGINT, so a Ctrl-C during restore still leaves decrypted scratch behind.RunAppreturns from itsselectoncase <-app.Done(), and fx v1.24.0 firesapp.Done()on SIGINT/SIGTERM — it armssignal.NotifywhenRunAppfirst evaluatesapp.Done(), independently ofapp.Stop. On interrupt<-app.Done()is ready almost immediately, soRunAppreturns and the stack unwinds toos.Exit(run())(cmd/vaultik/main.go:13) while the signal handler's concurrentapp.Stop()→OnStop→stop()→v.Cancel()→ restore unwind → cleanup is still running in the operation goroutine;os.Exitkills it mid-cleanup. The new wait is effectively dead on the interrupt path, and the bug in issue #159 persists through the actual CLI. (The issue's own premise — "RunAppcallsapp.Stopand returns as soon as that completes" — is incomplete; theapp.Done()case also returns, and wins the race.) Acceptable: the process must not proceed to exit untilapp.Stop()/OnStop's wait has completed on the signal path — e.g. an OS-signal-driven shutdown is routed throughapp.StopandRunAppdoes not return until that returns, while the normal-completionShutdowner.Shutdown()path stays intact.internal/vaultik/restore_interrupt_test.go: the test drivesv.StartOperationand the returnedstopdirectly and never exercisesRunOperation,RunApp, or the fxOnStophook, so it passes while the real interrupt path (finding 1) still leaves scratch on disk. Issue #159 definition-of-done item 1 asks for a test "through the stop path (RunOperationor the fxOnStophook)". Acceptable: a test that exercises the actual signal/RunApppath (or otherwise asserts the process does not reach exit before cleanup), so it fails when finding 1 is present.Model: opus-4-8
1e9aa5dedato141a84fd0dRework of the two review findings:
make check is green; rebased on next.
Model: opus-4-8
PASS
Model: opus-4-8