perf(sync): overlap pre-unlock pull with the password prompt (#103, Tier 1)

cmd/add.go

@@ -114,11 +114,8 @@ func runAdd(cmd *cobra.Command, args []string) error {
 		return fmt.Errorf("failed to create vault service at %s: %w", vaultPath, err)
 	}
 
-	// Smart sync pull before unlock to get latest version
-	syncPullBeforeUnlock(vaultService)
-
-	// Unlock vault
-	if err := unlockVault(vaultService); err != nil {
+	// Pull from remote and unlock, overlapping the pull with the password prompt (#103).
+	if err := unlockVaultWithSync(vaultService); err != nil {
 		return err
 	}
 	defer vaultService.Lock()

cmd/concurrent_unlock_test.go

@@ -0,0 +1,82 @@
+package cmd
+
+import (
+	"io"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/arimxyer/pass-cli/internal/vault"
+)
+
+// feedStdin redirects os.Stdin to a pipe carrying input (one line) for the
+// duration of the test, and enables PASS_CLI_TEST so readPassword reads from it
+// via the shared test scanner instead of a TTY. Only one password read per test
+// process is safe (the scanner initializes once), so keep a single reader here.
+func feedStdin(t *testing.T, input string) {
+	t.Helper()
+	t.Setenv("PASS_CLI_TEST", "1")
+
+	r, w, err := os.Pipe()
+	if err != nil {
+		t.Fatalf("pipe: %v", err)
+	}
+	orig := os.Stdin
+	os.Stdin = r
+	t.Cleanup(func() { os.Stdin = orig; _ = r.Close() })
+
+	go func() {
+		_, _ = io.WriteString(w, input)
+		_ = w.Close()
+	}()
+}
+
+// unlockVaultWithSync's password branch overlaps the network pull (goroutine)
+// with the master-password prompt (main thread), then joins before decrypting.
+// This drives that branch end-to-end against a real, sync-enabled, initialized
+// vault with no keychain — exercising the concurrency so `go test -race` proves
+// the goroutine + join is data-race free and the vault still unlocks correctly.
+func TestUnlockVaultWithSync_PasswordBranchConcurrent(t *testing.T) {
+	const masterPassword = "TestPass!1234"
+
+	tmpDir := t.TempDir()
+	vaultPath := filepath.Join(tmpDir, "vault.enc")
+	cfgPath := filepath.Join(tmpDir, "config.yml")
+
+	// Sync enabled with a bogus remote: SyncPull fails fast / no-ops (offline-
+	// friendly) and returns quickly, so the goroutine joins without real network.
+	cfg := "vault_path: " + vaultPath + "\nsync:\n  enabled: true\n  remote: \"mock-remote:bucket\"\n"
+	if err := os.WriteFile(cfgPath, []byte(cfg), 0600); err != nil {
+		t.Fatalf("write config: %v", err)
+	}
+	t.Setenv("PASS_CLI_CONFIG", cfgPath)
+
+	// Initialize the vault (no keychain) with a first service.
+	initVS, err := vault.New(vaultPath)
+	if err != nil {
+		t.Fatalf("vault.New (init): %v", err)
+	}
+	if err := initVS.Initialize([]byte(masterPassword), false, "", ""); err != nil {
+		t.Fatalf("Initialize: %v", err)
+	}
+
+	// A fresh, locked service is what a command would unlock.
+	vs, err := vault.New(vaultPath)
+	if err != nil {
+		t.Fatalf("vault.New (locked): %v", err)
+	}
+	if !vs.IsSyncEnabled() {
+		t.Fatal("expected sync to be enabled for the concurrent branch")
+	}
+
+	setOffline(t, false)
+	setVerbose(t, false)
+	feedStdin(t, masterPassword+"\n")
+
+	if err := unlockVaultWithSync(vs); err != nil {
+		t.Fatalf("unlockVaultWithSync: %v", err)
+	}
+	if !vs.IsUnlocked() {
+		t.Error("expected vault to be unlocked after the concurrent password branch")
+	}
+}

cmd/delete.go

@@ -61,11 +61,8 @@ func runDelete(cmd *cobra.Command, args []string) error {
 		return fmt.Errorf("failed to create vault service at %s: %w", vaultPath, err)
 	}
 
-	// Smart sync pull before unlock to get latest version
-	syncPullBeforeUnlock(vaultService)
-
-	// Unlock vault
-	if err := unlockVault(vaultService); err != nil {
+	// Pull from remote and unlock, overlapping the pull with the password prompt (#103).
+	if err := unlockVaultWithSync(vaultService); err != nil {
 		return err
 	}
 	defer vaultService.Lock()

cmd/exec.go

@@ -193,11 +193,9 @@ func runExec(cmd *cobra.Command, args []string) error {
 		return fmt.Errorf("failed to create vault service at %s: %w", vaultPath, err)
 	}
 
-	// Smart sync pull before unlock to get latest version (read-only: no push after)
-	syncPullBeforeUnlock(vaultService)
-
-	// Unlock vault
-	if err := unlockVault(vaultService); err != nil {
+	// Pull from remote and unlock, overlapping the pull with the password prompt
+	// (#103; read-only: no push after).
+	if err := unlockVaultWithSync(vaultService); err != nil {
 		return err
 	}
 	defer vaultService.Lock()

cmd/get.go

@@ -101,11 +101,8 @@ func runGet(cmd *cobra.Command, args []string) error {
 		return fmt.Errorf("failed to create vault service at %s: %w", vaultPath, err)
 	}
 
-	// Smart sync pull before unlock to get latest version
-	syncPullBeforeUnlock(vaultService)
-
-	// Unlock vault
-	if err := unlockVault(vaultService); err != nil {
+	// Pull from remote and unlock, overlapping the pull with the password prompt (#103).
+	if err := unlockVaultWithSync(vaultService); err != nil {
 		return err
 	}
 	defer vaultService.Lock()

cmd/helpers.go

@@ -3,6 +3,7 @@ package cmd
 import (
 	"bufio"
 	"fmt"
+	"github.com/arimxyer/pass-cli/internal/crypto"
 	"github.com/arimxyer/pass-cli/internal/recovery"
 	"github.com/arimxyer/pass-cli/internal/vault"
 	"os"
@@ -397,6 +398,82 @@ func unlockVault(vaultService *vault.VaultService) error {
 	return nil
 }
 
+// unlockVaultWithSync pulls from the remote and unlocks, overlapping the network
+// pull with the master-password prompt to hide the pre-unlock probe latency (#103,
+// Tier 1). It replaces the previous sequential `syncPullBeforeUnlock(vs)` +
+// `unlockVault(vs)` pair at command entry points.
+//
+// Ordering constraint: Unlock reads and decrypts vault.enc in one step, and a pull
+// replaces that file — so the decrypt must run strictly AFTER the pull finishes.
+// Only work that does not touch vault.enc may overlap the pull:
+//   - the master-password prompt touches stdin only → safe to overlap (the win);
+//   - keychain unlock decrypts vault.enc, and keychain users have no prompt to hide
+//     behind, so that path stays sequential (Tier 2, overlapping the Argon2 KDF, is
+//     a deferred follow-up).
+func unlockVaultWithSync(vaultService *vault.VaultService) error {
+	// No pull to overlap (sync disabled or --offline): today's exact behavior.
+	if IsOffline() || !vaultService.IsSyncEnabled() {
+		syncPullBeforeUnlock(vaultService) // no-op in these cases
+		return unlockVault(vaultService)
+	}
+
+	// Keychain is resolved BEFORE the pull: retrieving the keychain password reads
+	// metadata + the OS keyring but never decrypts vault.enc. There's no human wait
+	// to hide behind, so pull then decrypt sequentially.
+	if password, err := vaultService.RetrieveKeychainPassword(); err == nil {
+		defer crypto.ClearBytes(password)
+		syncPullBeforeUnlock(vaultService)
+		if err := vaultService.Unlock(password); err != nil {
+			return fmt.Errorf("failed to unlock vault: %w", err)
+		}
+		if IsVerbose() {
+			fmt.Fprintln(os.Stderr, "🔓 Unlocked vault using keychain")
+		}
+		return nil
+	}
+
+	// Password path: run the pull concurrently with the prompt, then join before
+	// decrypting against the now-current file.
+	pullDone := make(chan struct{})
+	var pullErr error
+	go func() {
+		defer close(pullDone)
+		pullErr = vaultService.SyncPull()
+	}()
+
+	if IsVerbose() {
+		// Full line (terminated by newline) so it cannot corrupt the prompt below.
+		fmt.Fprintln(os.Stderr, "🔄 Checking remote for vault changes...")
+	}
+	// No transient "Checking remote..." indicator here: it would garble the prompt
+	// line. The prompt itself signals that work is in flight; the pull runs quietly
+	// in the background and any warnings are re-surfaced cleanly after the join.
+	fmt.Fprint(os.Stderr, "Master password: ")
+	password, readErr := readPassword()
+	defer crypto.ClearBytes(password)
+	fmt.Fprintln(os.Stderr) // newline after password input
+
+	<-pullDone // join on ALL paths before returning, so rclone is never orphaned
+
+	if readErr != nil {
+		return fmt.Errorf("failed to read password: %w", readErr)
+	}
+
+	// Re-surface a sync conflict (or pull error) cleanly below the prompt. SyncPull
+	// may have printed its own warning mid-prompt (garbled), and read commands never
+	// re-echo it (only writes do, via SyncPush) — so a failed signal would be lost.
+	if vaultService.SyncConflictDetected() {
+		fmt.Fprintln(os.Stderr, "Warning: sync conflict — local and remote vault both changed. Use `pass-cli sync resolve` to choose which version to keep.")
+	} else if pullErr != nil {
+		fmt.Fprintf(os.Stderr, "Warning: sync pull failed: %v\n", pullErr)
+	}
+
+	if err := vaultService.Unlock(password); err != nil {
+		return fmt.Errorf("failed to unlock vault: %w", err)
+	}
+	return nil
+}
+
 // syncPullBeforeUnlock performs a smart sync pull before vault unlock.
 // This ensures we have the latest version from remote before reading.
 //

cmd/list.go

@@ -116,11 +116,8 @@ func runList(cmd *cobra.Command, args []string) error {
 		return fmt.Errorf("failed to create vault service at %s: %w", vaultPath, err)
 	}
 
-	// Smart sync pull before unlock to get latest version
-	syncPullBeforeUnlock(vaultService)
-
-	// Unlock vault
-	if err := unlockVault(vaultService); err != nil {
+	// Pull from remote and unlock, overlapping the pull with the password prompt (#103).
+	if err := unlockVaultWithSync(vaultService); err != nil {
 		return err
 	}
 	defer vaultService.Lock()

cmd/update.go

@@ -139,11 +139,8 @@ func runUpdate(cmd *cobra.Command, args []string) error {
 		return fmt.Errorf("failed to create vault service at %s: %w", vaultPath, err)
 	}
 
-	// Smart sync pull before unlock to get latest version
-	syncPullBeforeUnlock(vaultService)
-
-	// Unlock vault
-	if err := unlockVault(vaultService); err != nil {
+	// Pull from remote and unlock, overlapping the pull with the password prompt (#103).
+	if err := unlockVaultWithSync(vaultService); err != nil {
 		return err
 	}
 	defer vaultService.Lock()

internal/vault/concurrent_unlock_test.go

@@ -0,0 +1,44 @@
+package vault
+
+import (
+	"errors"
+	"path/filepath"
+	"testing"
+)
+
+// RetrieveKeychainPassword must report ErrKeychainNotEnabled (and return no
+// bytes) when keychain unlock isn't configured — the signal the concurrent
+// unlock path uses to fall through to the password prompt.
+func TestRetrieveKeychainPassword_NotEnabled(t *testing.T) {
+	tempDir := t.TempDir()
+	vaultPath := filepath.Join(tempDir, "vault.enc")
+
+	v, err := New(vaultPath)
+	if err != nil {
+		t.Fatalf("New: %v", err)
+	}
+	if err := v.Initialize([]byte("TestPass!1234"), false, "", ""); err != nil {
+		t.Fatalf("Initialize: %v", err)
+	}
+
+	pw, err := v.RetrieveKeychainPassword()
+	if !errors.Is(err, ErrKeychainNotEnabled) {
+		t.Errorf("expected ErrKeychainNotEnabled, got err=%v", err)
+	}
+	if pw != nil {
+		t.Errorf("expected nil password on error, got %d bytes", len(pw))
+	}
+}
+
+// The conflict getter defaults to false on a fresh service and is what the
+// concurrent unlock path reads after the join to re-surface a swallowed conflict.
+func TestSyncConflictDetected_DefaultsFalse(t *testing.T) {
+	tempDir := t.TempDir()
+	v, err := New(filepath.Join(tempDir, "vault.enc"))
+	if err != nil {
+		t.Fatalf("New: %v", err)
+	}
+	if v.SyncConflictDetected() {
+		t.Error("expected SyncConflictDetected() == false on a fresh service")
+	}
+}

internal/vault/vault.go

@@ -228,6 +228,14 @@ func (v *VaultService) IsSyncEnabled() bool {
 	return v.syncService != nil && v.syncService.IsEnabled()
 }
 
+// SyncConflictDetected reports whether the most recent SyncPull detected a
+// conflict (both local and remote changed). Used by the concurrent-unlock path
+// to re-surface the conflict cleanly after the password prompt, since SyncPull's
+// own warning may print mid-prompt and read commands never re-echo it (#103).
+func (v *VaultService) SyncConflictDetected() bool {
+	return v.syncConflictDetected
+}
+
 // SyncPush performs a smart sync push if sync is enabled.
 // Should be called once at the end of a command, not per-save.
 // Returns true if a push was actually performed.
@@ -921,14 +929,30 @@ func (v *VaultService) UnlockWithKey(vaultKey []byte) error {
 
 // UnlockWithKeychain attempts to unlock using keychain-stored password
 func (v *VaultService) UnlockWithKeychain() error {
+	password, err := v.RetrieveKeychainPassword()
+	if err != nil {
+		return err
+	}
+	return v.Unlock(password)
+}
+
+// RetrieveKeychainPassword returns the master password from the OS keychain
+// without decrypting the vault. It reads metadata and the keyring (and performs
+// best-effort migration from a legacy global entry) but never touches the
+// vault.enc ciphertext — so callers may run it concurrently with a sync pull
+// that replaces vault.enc, then decrypt with the returned password afterwards
+// (see the concurrent-unlock path in cmd, #103). Returns ErrKeychainNotEnabled
+// when keychain unlock is not configured. The caller owns the returned bytes and
+// must crypto.ClearBytes them.
+func (v *VaultService) RetrieveKeychainPassword() ([]byte, error) {
 	// T018: Check metadata to see if keychain is enabled (FR-007)
 	metadata, err := v.LoadMetadata()
 	if err != nil {
-		return fmt.Errorf("failed to load metadata: %w", err)
+		return nil, fmt.Errorf("failed to load metadata: %w", err)
 	}
 
 	if !metadata.KeychainEnabled {
-		return ErrKeychainNotEnabled
+		return nil, ErrKeychainNotEnabled
 	}
 
 	// Attempt to retrieve password from vault-specific keychain entry
@@ -954,10 +978,10 @@ func (v *VaultService) UnlockWithKeychain() error {
 	}
 
 	if err != nil {
-		return fmt.Errorf("failed to retrieve password from keychain: %w", err)
+		return nil, fmt.Errorf("failed to retrieve password from keychain: %w", err)
 	}
 
-	return v.Unlock([]byte(password))
+	return []byte(password), nil
 }
 
 // Lock clears in-memory credentials and password