Fix syncing consistency bug, but with the caveat that there is currently no easy/practical way to write tests for this

client/cmd/saveHistoryEntry.go

@@ -31,6 +31,7 @@ var saveHistoryEntryCmd = &cobra.Command{
 	Run: func(cmd *cobra.Command, args []string) {
 		ctx := hctx.MakeContext()
 		lib.CheckFatalError(maybeUploadSkippedHistoryEntries(ctx))
+		lib.CheckFatalError(maybeSubmitPendingDeletionRequests(ctx))
 		saveHistoryEntry(ctx)
 	},
 }
@@ -46,6 +47,32 @@ var presaveHistoryEntryCmd = &cobra.Command{
 	},
 }
 
+func maybeSubmitPendingDeletionRequests(ctx context.Context) error {
+	config := hctx.GetConf(ctx)
+	if config.IsOffline {
+		return nil
+	}
+	if len(config.PendingDeletionRequests) == 0 {
+		return nil
+	}
+
+	// Upload the missing deletion requests
+	for _, dr := range config.PendingDeletionRequests {
+		err := lib.SendDeletionRequest(dr)
+		if lib.IsOfflineError(err) {
+			// We're still offline, so nothing to do
+			return nil
+		}
+		if err != nil {
+			return err
+		}
+	}
+
+	// Mark down that we sent all of them
+	config.PendingDeletionRequests = make([]shared.DeletionRequest, 0)
+	return hctx.SetConfig(config)
+}
+
 func maybeUploadSkippedHistoryEntries(ctx context.Context) error {
 	config := hctx.GetConf(ctx)
 	if !config.HaveMissedUploads {
@@ -163,19 +190,7 @@ func saveHistoryEntry(ctx context.Context) {
 
 	// Drop any entries from pre-saving since they're no longer needed
 	if config.BetaMode {
-		err := deletePresavedEntries(ctx, entry)
-		if err != nil {
-			if lib.IsOfflineError(err) {
-				// Do Nothing: This means that if an entry is pre-saved, the user goes offline, and the
-				// command finishes running, then there will be a syncing inconsistency where remote devices
-				// will have the pre-saved entry and the main entry.
-				//
-				// TODO: Fix this by having the config store deletion requests to be resent once the device goes
-				// back online.
-			} else {
-				lib.CheckFatalError(err)
-			}
-		}
+		lib.CheckFatalError(deletePresavedEntries(ctx, entry))
 	}
 
 	// Persist it locally
@@ -248,14 +263,23 @@ func deletePresavedEntries(ctx context.Context, entry *data.HistoryEntry) error
 
 	// And delete it remotely
 	config := hctx.GetConf(ctx)
-	var deletionRequest shared.DeletionRequest
-	deletionRequest.SendTime = time.Now()
-	deletionRequest.UserId = data.UserId(config.UserSecret)
-	deletionRequest.Messages.Ids = append(deletionRequest.Messages.Ids,
-		// Note that we aren't specifying an EndTime here since pre-saved entries don't have an EndTime
-		shared.MessageIdentifier{DeviceId: presavedEntry.DeviceId, EntryId: presavedEntry.EntryId},
-	)
-	return lib.SendDeletionRequest(deletionRequest)
+	if !config.IsOffline {
+		var deletionRequest shared.DeletionRequest
+		deletionRequest.SendTime = time.Now()
+		deletionRequest.UserId = data.UserId(config.UserSecret)
+		deletionRequest.Messages.Ids = append(deletionRequest.Messages.Ids,
+			// Note that we aren't specifying an EndTime here since pre-saved entries don't have an EndTime
+			shared.MessageIdentifier{DeviceId: presavedEntry.DeviceId, EntryId: presavedEntry.EntryId},
+		)
+		err = lib.SendDeletionRequest(deletionRequest)
+		if lib.IsOfflineError(err) {
+			// Cache the deletion request to send once the client comes back online
+			config.PendingDeletionRequests = append(config.PendingDeletionRequests, deletionRequest)
+			return hctx.SetConfig(config)
+		}
+		return err
+	}
+	return nil
 }
 
 func init() {

client/cmd/status.go

@@ -33,7 +33,7 @@ func printOnlineStatus(config *hctx.ClientConfig) {
 		fmt.Println("Sync Mode: Disabled")
 	} else {
 		fmt.Println("Sync Mode: Enabled")
-		if config.HaveMissedUploads {
+		if config.HaveMissedUploads || len(config.PendingDeletionRequests) > 0 {
 			fmt.Println("Sync Status: Unsynced (device is offline?)")
 		} else {
 			fmt.Println("Sync Status: Synced")

client/hctx/hctx.go

@@ -11,6 +11,7 @@ import (
 	"time"
 
 	"github.com/ddworken/hishtory/client/data"
+	"github.com/ddworken/hishtory/shared"
 	"github.com/google/uuid"
 	"github.com/sirupsen/logrus"
 	"gopkg.in/natefinch/lumberjack.v2"
@@ -171,6 +172,10 @@ type ClientConfig struct {
 	// Used for uploading history entries that we failed to upload due to a missing network connection
 	HaveMissedUploads     bool  `json:"have_missed_uploads"`
 	MissedUploadTimestamp int64 `json:"missed_upload_timestamp"`
+	// Used for uploading deletion requests that we failed to upload due to a missed network connection
+	// Note that this is only applicable for deleting pre-saved entries. For interactive deletion, we just
+	// show the user an error message if they're offline.
+	PendingDeletionRequests []shared.DeletionRequest `json:"pending_deletion_requests"`
 	// Used for avoiding double imports of .bash_history
 	HaveCompletedInitialImport bool `json:"have_completed_initial_import"`
 	// Whether control-r bindings are enabled