perf(sync): overlap pre-unlock pull with the password prompt (#103, Tier 1) - #109
Merged
Conversation
…ier 1) Every read/write that has sync enabled does a synchronous pre-unlock pull (CheckRemoteMetadata → rclone lsjson, a network round-trip) before the vault unlocks. Tier 1 hides that latency for password-prompt users by running the pull concurrently with the master-password prompt — work where the human is already waiting — with no staleness tradeoff. Ordering constraint: Unlock reads + decrypts vault.enc in one step and a pull replaces that file, so the decrypt must run strictly after the pull. Only work that doesn't touch vault.enc may overlap: - New cmd/unlockVaultWithSync replaces the sequential `syncPullBeforeUnlock(vs)` + `unlockVault(vs)` pair at the 6 pull-path commands (add/delete/list/get/ exec/update). usage/migrate, which never pulled, are unchanged. - Password path: kick off SyncPull in a goroutine, prompt + read the password (stdin only — no vault.enc access), JOIN on the channel (on every return path, so rclone is never orphaned), then decrypt against the now-current file. - Keychain path stays sequential: retrieving the keychain password never touches vault.enc but there's no prompt to hide behind. Tier 2 (overlapping the Argon2 KDF for keychain users — the common case) is a deferred follow-up; salt, iterations and wrappedDEK are all invariant across normal saves, so it's viable but carries the real concurrency/crypto risk and is split out. Supporting changes in internal/vault: - RetrieveKeychainPassword: retrieve the master password from the keychain WITHOUT decrypting (the seam that lets the pull overlap). UnlockWithKeychain now calls it then Unlock — behavior unchanged. - SyncConflictDetected getter: SyncPull's conflict warning can print mid-prompt (garbled) and read commands never re-echo it (only writes do, via SyncPush) — so the orchestrator re-surfaces the conflict cleanly below the prompt after the join, ensuring the "sync resolve" signal is never lost. Feedback: no transient indicator during the overlap (it would corrupt the prompt line); the prompt itself signals work in flight, and the verbose pull line is a full line printed above the prompt. TUI stays on the sequential path (low- frequency, interactive) — a deliberate scope call. Tests: RetrieveKeychainPassword (not-enabled → ErrKeychainNotEnabled); SyncConflictDetected default; and a -race test driving the concurrent password branch end-to-end against a real sync-enabled, initialized vault. Full unit + integration suites green; vault+cmd pass `go test -race`. Refs #103 (Tier 1; Tier 2 tracked separately). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019sxsM218vNzDbuMZ2nhMzx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Every sync-enabled read/write does a synchronous pre-unlock pull (
CheckRemoteMetadata→rclone lsjson, a network round-trip) before the vault unlocks. Tier 1 hides that latency for password-prompt users by running the pull concurrently with the master-password prompt — where the human is already waiting — with no staleness tradeoff (unlike a TTL).The ordering constraint
Unlockreads + decryptsvault.encin one step, and a pull replaces that file, so the decrypt must run strictly after the pull. Only work that doesn't touchvault.encmay overlap it.cmd/unlockVaultWithSyncreplaces the sequentialsyncPullBeforeUnlock(vs)+unlockVault(vs)pair at the 6 pull-path commands (add/delete/list/get/exec/update).usage/migrate, which never pulled, are unchanged.SyncPullin a goroutine → prompt + read the password (stdin only) → join the channel on every return path (so rclone is never orphaned on a prompt error/Ctrl-C) → decrypt against the now-current file.vault.enc, but there's no prompt to hide behind.Scope: Tier 1 only — Tier 2 (the common case) is deferred to #103
Keychain is the default unlock and gets ~zero benefit here (no prompt to overlap). Tier 2 — overlapping the Argon2 KDF for keychain users — is the headline win and is intentionally split out (it's where the concurrency/crypto risk lives and needs its own
-racetreatment). Prerequisite verified:SaveVault/SaveVaultWithDEKonly touchUpdatedAt, so salt, iterations, and wrappedDEK are invariant across normal saves → Tier 2 is viable with a post-join re-derive fallback. #103 stays open, scoped to Tier 2.Tier 1 is still worth shipping alone: it's the goroutine+join skeleton Tier 2 builds on, and a real win for headless/CI/scripted runs where keychain is absent.
Correctness details
SyncPull's conflict warning can print mid-prompt (garbled) and read commands never re-echo it (only writes do, viaSyncPush). Newvault.SyncConflictDetected()lets the orchestrator re-print the conflict cleanly below the prompt after the join, so the "sync resolve" signal is never lost.RetrieveKeychainPassword: retrieves the master password from the keychain without decrypting (the seam enabling overlap).UnlockWithKeychainnow calls it thenUnlock— behavior unchanged.Tests
RetrieveKeychainPassword(not-enabled →ErrKeychainNotEnabled);SyncConflictDetecteddefault; and a-racetest driving the concurrent password branch end-to-end against a real sync-enabled, initialized vault. Full unit + integration suites green;vault+cmdpassgo test -race;golangci-lint v2.5clean.Refs #103 (Tier 1; Tier 2 remains tracked in #103).
🤖 Generated with Claude Code