diff --git a/cmd/backups/restore.go b/cmd/backups/restore.go index b6117b1..8a100b6 100644 --- a/cmd/backups/restore.go +++ b/cmd/backups/restore.go @@ -23,6 +23,33 @@ func RestoreCmd() *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { ui.NewlineAbove() + db, err := storage.Initialize() + if err != nil { + ui.PrintError(ui.EmojiError, fmt.Sprintf("%v", err)) + ui.NewlineBelow() + return ui.ErrHandled + } + + running, err := db.GetRunningEntry() + if err != nil { + db.Close() + ui.PrintError(ui.EmojiError, fmt.Sprintf("checking for active timer: %v", err)) + ui.NewlineBelow() + return ui.ErrHandled + } + if running != nil { + db.Close() + ui.PrintError(ui.EmojiError, fmt.Sprintf(`timer is running for %s — stop it before restoring a backup`, ui.Bold(running.ProjectName))) + ui.NewlineBelow() + return ui.ErrHandled + } + + if err := db.Close(); err != nil { + ui.PrintError(ui.EmojiError, fmt.Sprintf("closing database: %v", err)) + ui.NewlineBelow() + return ui.ErrHandled + } + backups, err := storage.ListBackups() if err != nil { ui.PrintError(ui.EmojiError, fmt.Sprintf("listing backups: %v", err)) diff --git a/cmd/backups/restore_test.go b/cmd/backups/restore_test.go new file mode 100644 index 0000000..87d0e97 --- /dev/null +++ b/cmd/backups/restore_test.go @@ -0,0 +1,89 @@ +package backups + +import ( + "errors" + "testing" + + "github.com/DylanDevelops/tmpo/internal/storage" + "github.com/DylanDevelops/tmpo/internal/ui" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// setupBackupTestHome points the storage layer at an isolated temp home so +// tests never touch the real ~/.tmpo directory. +func setupBackupTestHome(t *testing.T) { + t.Helper() + tmpHome := t.TempDir() + t.Setenv("HOME", tmpHome) + t.Setenv("USERPROFILE", tmpHome) + t.Setenv("TMPO_DEV", "1") +} + +// TestRestoreCmd_BlocksWhenTimerRunning verifies the guard added for issue #134: +// a restore must refuse while a timer is running instead of silently +// obliterating the in-flight entry. +func TestRestoreCmd_BlocksWhenTimerRunning(t *testing.T) { + setupBackupTestHome(t) + + // Start a running timer, then take a backup so a restore target exists. + db, err := storage.Initialize() + require.NoError(t, err) + _, err = db.CreateEntry("proj", "in flight", nil, nil) + require.NoError(t, err) + _, err = db.CreateBackup() + require.NoError(t, err) + require.NoError(t, db.Close()) + + cmd := RestoreCmd() + err = cmd.RunE(cmd, []string{}) + + // The command must report a handled failure and stop before any restore. + assert.True(t, errors.Is(err, ui.ErrHandled), "expected ui.ErrHandled, got %v", err) + + // The running entry must survive untouched. + db2, err := storage.Initialize() + require.NoError(t, err) + defer db2.Close() + running, err := db2.GetRunningEntry() + require.NoError(t, err) + require.NotNil(t, running, "running timer should not have been destroyed") + assert.Equal(t, "in flight", running.Description) +} + +// TestRestoreCmd_NoTimerNoBackups confirms the guard passes cleanly when no +// timer is running: with no backups present the command exits without error. +func TestRestoreCmd_NoTimerNoBackups(t *testing.T) { + setupBackupTestHome(t) + + // Initialize the DB (no running timer, no backups created). + db, err := storage.Initialize() + require.NoError(t, err) + require.NoError(t, db.Close()) + + cmd := RestoreCmd() + err = cmd.RunE(cmd, []string{}) + + // No timer and no backups is a clean no-op, not a failure. + assert.NoError(t, err) +} + +// TestRestoreCmd_NoTimerInvalidID confirms that once the guard passes, the +// --id lookup still runs and reports a handled error for an unknown ID. +func TestRestoreCmd_NoTimerInvalidID(t *testing.T) { + setupBackupTestHome(t) + + // A backup must exist so the command reaches the ID lookup rather than the + // "no backups found" early return. + db, err := storage.Initialize() + require.NoError(t, err) + _, err = db.CreateBackup() + require.NoError(t, err) + require.NoError(t, db.Close()) + + cmd := RestoreCmd() + require.NoError(t, cmd.Flags().Set("id", "9999")) + err = cmd.RunE(cmd, []string{}) + + assert.True(t, errors.Is(err, ui.ErrHandled), "expected ui.ErrHandled, got %v", err) +} diff --git a/internal/storage/backup.go b/internal/storage/backup.go index aa02112..0e81db1 100644 --- a/internal/storage/backup.go +++ b/internal/storage/backup.go @@ -146,6 +146,12 @@ func RestoreBackup(backupPath string) error { } defer src.Close() + for _, suffix := range []string{"-journal", "-wal", "-shm"} { + if err := os.Remove(dbPath + suffix); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("failed to remove stale %s file: %w", suffix, err) + } + } + dst, err := os.Create(dbPath) if err != nil { return fmt.Errorf("failed to open database for writing: %w", err) diff --git a/internal/storage/backup_test.go b/internal/storage/backup_test.go index bf4b221..a49f9dc 100644 --- a/internal/storage/backup_test.go +++ b/internal/storage/backup_test.go @@ -260,6 +260,74 @@ func TestRestoreBackup(t *testing.T) { assert.Equal(t, "before backup", entries[0].Description) } +func TestRestoreBackup_RemovesStaleWALSidecars(t *testing.T) { + tmpHome := t.TempDir() + t.Setenv("HOME", tmpHome) + t.Setenv("USERPROFILE", tmpHome) + t.Setenv("TMPO_DEV", "") + + // Create and populate the live DB, then snapshot it + db, err := Initialize() + assert.NoError(t, err) + _, err = db.CreateEntry("test-project", "before backup", nil, nil) + assert.NoError(t, err) + + backup, err := db.CreateBackup() + assert.NoError(t, err) + db.Close() + + // Simulate leftover sidecar files next to the live DB. tmpo runs in the + // default (rollback) journal mode, so -journal is the sidecar that can + // actually occur; -wal/-shm are covered defensively. + dbPath, err := GetDBPath() + assert.NoError(t, err) + journalPath := dbPath + "-journal" + walPath := dbPath + "-wal" + shmPath := dbPath + "-shm" + assert.NoError(t, os.WriteFile(journalPath, []byte("stale journal"), 0600)) + assert.NoError(t, os.WriteFile(walPath, []byte("stale wal"), 0600)) + assert.NoError(t, os.WriteFile(shmPath, []byte("stale shm"), 0600)) + + // Restore should clear the stale sidecars so they cannot shadow the DB + assert.NoError(t, RestoreBackup(backup.Path)) + + _, err = os.Stat(journalPath) + assert.True(t, os.IsNotExist(err), "expected stale -journal file to be removed") + _, err = os.Stat(walPath) + assert.True(t, os.IsNotExist(err), "expected stale -wal file to be removed") + _, err = os.Stat(shmPath) + assert.True(t, os.IsNotExist(err), "expected stale -shm file to be removed") + + // Restored data must still be intact + db2, err := Initialize() + assert.NoError(t, err) + defer db2.Close() + + entries, err := db2.GetEntries(0) + assert.NoError(t, err) + assert.Len(t, entries, 1) + assert.Equal(t, "before backup", entries[0].Description) +} + +func TestRestoreBackup_SucceedsWithoutSidecars(t *testing.T) { + tmpHome := t.TempDir() + t.Setenv("HOME", tmpHome) + t.Setenv("USERPROFILE", tmpHome) + t.Setenv("TMPO_DEV", "") + + db, err := Initialize() + assert.NoError(t, err) + _, err = db.CreateEntry("test-project", "before backup", nil, nil) + assert.NoError(t, err) + + backup, err := db.CreateBackup() + assert.NoError(t, err) + db.Close() + + // No sidecar files exist; restore must still succeed cleanly + assert.NoError(t, RestoreBackup(backup.Path)) +} + func TestCreateBackup_SetsPrivatePermissions(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("POSIX permissions are not enforced on Windows") diff --git a/internal/storage/db.go b/internal/storage/db.go index 2e61def..ffb105b 100644 --- a/internal/storage/db.go +++ b/internal/storage/db.go @@ -108,7 +108,7 @@ func Initialize() (*Database, error) { if err := fsperm.SecureFile(dbPath); err != nil { return nil, err } - for _, suffix := range []string{"-wal", "-shm"} { + for _, suffix := range []string{"-journal", "-wal", "-shm"} { if err := fsperm.SecureFile(dbPath + suffix); err != nil { return nil, err }