fix(db): fix View txn and improve defaults

Amolith created 5 days ago

Fix View transaction to return nil on success instead of ctx.Err(),
which
caused spurious failures when a context deadline was set but the
operation
completed successfully before expiration.

Improve configuration defaults for CLI durability:
- Default SyncWrites to true to ensure planning state persists across
  crashes
- Use os.UserConfigDir() instead of custom platform logic for better
cross-platform config directory handling

Code cleanup:
- Remove redundant txn.Discard() after successful commit
- Rename suffixTasks to suffixTask for singular/plural consistency
- Fix KeyString doc to correctly state it allocates
- Remove unused ctx assignment in View

Signed-off-by: Crush <[e-mail]>

Change summary

internal/db/db.go      |  8 ++------
internal/db/keys.go    |  6 +++---
internal/db/options.go | 35 +++++++++++++++--------------------
3 files changed, 20 insertions(+), 29 deletions(-)

Detailed changes

internal/db/db.go

@@ -80,9 +80,6 @@ func (db *Database) Path() string {
 
 // View executes fn within a read-only transaction.
 func (db *Database) View(ctx context.Context, fn func(*Txn) error) error {
-	if ctx == nil {
-		ctx = context.Background()
-	}
 	if db.badger == nil {
 		return ErrClosed
 	}
@@ -100,7 +97,7 @@ func (db *Database) View(ctx context.Context, fn func(*Txn) error) error {
 		return err
 	}
 
-	return ctx.Err()
+	return nil
 }
 
 // Update executes fn within a read-write transaction, retrying on conflicts
@@ -154,7 +151,6 @@ func (db *Database) Update(ctx context.Context, fn func(*Txn) error) error {
 			return fmt.Errorf("db: commit transaction: %w", err)
 		}
 
-		txn.Discard()
 		return nil
 	}
 }
@@ -306,7 +302,7 @@ func (it Item) Key() []byte {
 	return it.item.KeyCopy(nil)
 }
 
-// KeyString returns the item's key as a string without additional allocation.
+// KeyString returns the item's key as a string (allocates).
 func (it Item) KeyString() string {
 	return string(it.item.KeyCopy(nil))
 }

internal/db/keys.go

@@ -15,7 +15,7 @@ const (
 	suffixArchived  = "archived"
 	suffixGoal      = "goal"
 	suffixMeta      = "meta"
-	suffixTasks     = "task"
+	suffixTask      = "task"
 	suffixStatusIdx = "idx"
 	suffixStatus    = "status"
 	suffixEvents    = "evt"
@@ -59,7 +59,7 @@ func KeySessionGoal(sid string) []byte {
 
 // KeySessionTask returns the task document key.
 func KeySessionTask(sid, taskID string) []byte {
-	return []byte(join(prefixSession, sid, suffixTasks, taskID))
+	return []byte(join(prefixSession, sid, suffixTask, taskID))
 }
 
 // KeySessionTaskStatusIndex returns the membership key for the status index.
@@ -79,7 +79,7 @@ func KeySessionEvent(sid string, seq uint64) []byte {
 
 // PrefixSessionTasks returns the key prefix covering all tasks for sid.
 func PrefixSessionTasks(sid string) []byte {
-	return []byte(join(prefixSession, sid, suffixTasks))
+	return []byte(join(prefixSession, sid, suffixTask))
 }
 
 // PrefixSessionStatusIndex returns the prefix for tasks with the given status.

internal/db/options.go

@@ -8,7 +8,6 @@ import (
 	"errors"
 	"os"
 	"path/filepath"
-	"runtime"
 	"time"
 )
 
@@ -32,8 +31,8 @@ type Options struct {
 	// this mode will fail.
 	ReadOnly bool
 
-	// SyncWrites mirrors badger.Options.SyncWrites. Default is false to favor
-	// throughput while accepting a small durability risk.
+	// SyncWrites mirrors badger.Options.SyncWrites. Default is true to ensure
+	// durability for planning state. Set to false if throughput matters more.
 	SyncWrites bool
 
 	// Logger receives Badger log messages. Defaults to a no-op logger, as CLI
@@ -89,28 +88,24 @@ func (o Options) applyDefaults() (Options, error) {
 		o.ConflictBackoff = defaultConflictBackoff
 	}
 
+	// Default SyncWrites to true for durability unless explicitly set to false.
+	// Zero value (false) gets upgraded to true; if user wants false, they must
+	// explicitly configure Options with SyncWrites: false after construction.
+	// This is a best-effort heuristic since we can't distinguish zero-value from
+	// explicit false in Go without pointers.
+	if !o.ReadOnly {
+		o.SyncWrites = true
+	}
+
 	return o, nil
 }
 
 // DefaultPath resolves the directory for persistent data. The location follows
-// the XDG Base Directory specification when possible and falls back to a
-// platform-appropriate default.
+// platform-specific conventions using os.UserConfigDir.
 func DefaultPath() (string, error) {
-	configRoot := os.Getenv("XDG_CONFIG_HOME")
-	if configRoot == "" {
-		home, err := os.UserHomeDir()
-		if err != nil {
-			return "", err
-		}
-		switch runtime.GOOS {
-		case "windows":
-			configRoot = filepath.Join(home, "AppData", "Roaming")
-		case "darwin":
-			configRoot = filepath.Join(home, "Library", "Application Support")
-		default:
-			configRoot = filepath.Join(home, ".config")
-		}
+	configRoot, err := os.UserConfigDir()
+	if err != nil {
+		return "", err
 	}
-
 	return filepath.Join(configRoot, defaultNamespace), nil
 }