Merge PR: fix(desktop): stop silent account wipe on keychain-access errors

Merges nostr proposal 5d31b68e into main:
- fix(desktop): stop silent account wipe on keychain errors and upgrade races
- fix(desktop): allow first-launch key bootstrap and stop caching unwritten state

Desktop lost every logged-in account whenever the OS keychain answered
ambiguously. getOrCreateKey() treated any failed lookup as "key absent" and
minted a fresh AES key, orphaning accounts.json.enc; the read path then
renamed the unreadable file to .corrupt.<ts>; and nothing serialised access,
so a Homebrew upgrade race could interleave two instances.

Now: getPrivateKeyOrThrow() distinguishes confirmed-absent from refused or
ambiguous (macOS via /usr/bin/security exit codes) and only the former
rotates; genuine corruption (AEAD tag, bad padding, malformed JSON) is
separated from transient failures, which preserve the file and surface as
StorageCorruption.TransientError; and a cross-process advisory lock plus
in-process mutexes guard the file.

Review follow-ups in the second commit: non-macOS keyring backends throw for
a genuinely absent credential, so a fresh Linux/Windows install could never
mint the key -- creation is now allowed when accounts.json.enc does not yet
exist, where there is no ciphertext to orphan. And the metadata cache is
populated only after the disk write succeeds, so a failed save no longer
leaves the session serving accounts that were never persisted.

Verified on macOS against the real Keychain and the real account store:
security lookup exit 0, accounts load and survive a restart, ciphertext
byte-identical, zero corruption events.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VgVDQQXAg4cmzsWHoJj61k

commons/src/androidMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt

@@ -107,6 +107,26 @@ actual class SecureKeyStorage private actual constructor() {
             }
         }
 
+    /**
+     * Android backend: EncryptedSharedPreferences.contains + getString has no
+     * ambiguous-error state comparable to macOS Keychain user-cancel/deny, so
+     * "key not present" and "key present" are the only two null outcomes.
+     * Any thrown exception is a genuine failure and propagates.
+     */
+    actual suspend fun getPrivateKeyOrThrow(npub: String): String? =
+        withContext(Dispatchers.IO) {
+            try {
+                val key = KEY_PREFIX + npub
+                if (!encryptedPrefs.contains(key)) {
+                    null
+                } else {
+                    encryptedPrefs.getString(key, null)
+                }
+            } catch (e: Exception) {
+                throw SecureStorageException("Failed to retrieve private key", e)
+            }
+        }
+
     actual suspend fun deletePrivateKey(npub: String): Boolean =
         withContext(Dispatchers.IO) {
             try {

commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt

@@ -76,12 +76,34 @@ expect class SecureKeyStorage private constructor() {
      * **Security Warning:** The returned String cannot be securely zeroed from memory (JVM limitation).
      * Dereference the returned value immediately after use to minimize exposure time.
      *
+     * Callers that MUST distinguish "key does not exist" from "backend refused/locked/failed"
+     * (for example, before generating a replacement key on disk) should use
+     * [getPrivateKeyOrThrow] instead. This method returns null on any error and cannot
+     * safely be used as an "is this the first launch?" probe.
+     *
      * @param npub The public key in npub (Bech32) format
      * @return The private key in hexadecimal format, or null if not found
      * @throws SecureStorageException if retrieval operation fails
      */
     suspend fun getPrivateKey(npub: String): String?
 
+    /**
+     * Retrieves a private key for the given npub, distinguishing "definitively absent"
+     * from any other failure mode.
+     *
+     * On success, returns the key. When the backend confirms the item does not exist,
+     * returns null. Any other outcome (backend unavailable, user denied the OS prompt,
+     * keychain locked, I/O error) throws [SecureStorageException]. This is the safe
+     * primitive for compare-and-swap style flows where a null must not be interpreted
+     * as permission to generate and persist a replacement.
+     *
+     * @param npub The public key in npub (Bech32) format
+     * @return The private key in hexadecimal format, or null only when the backend
+     *   confirms the item does not exist
+     * @throws SecureStorageException on any ambiguous or transient failure
+     */
+    suspend fun getPrivateKeyOrThrow(npub: String): String?
+
     /**
      * Deletes a private key for the given npub.
      *

commons/src/iosMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.ios.kt

@@ -34,6 +34,8 @@ actual class SecureKeyStorage private actual constructor() {
 
     actual suspend fun getPrivateKey(npub: String): String? = throw SecureStorageException("Keychain Services binding pending (iOS Phase 4)")
 
+    actual suspend fun getPrivateKeyOrThrow(npub: String): String? = throw SecureStorageException("Keychain Services binding pending (iOS Phase 4)")
+
     actual suspend fun deletePrivateKey(npub: String): Boolean = throw SecureStorageException("Keychain Services binding pending (iOS Phase 4)")
 
     actual suspend fun hasPrivateKey(npub: String): Boolean = throw SecureStorageException("Keychain Services binding pending (iOS Phase 4)")

commons/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt

@@ -149,6 +149,75 @@ actual class SecureKeyStorage private actual constructor() {
             }
         }
 
+    /**
+     * Strict variant that distinguishes "backend confirms item not found" from every
+     * other outcome. This matters on macOS: `javakeyring` collapses `errSecItemNotFound`
+     * (-25300), `errSecAuthFailed` (-25293), `errSecUserCanceled` (-128), and
+     * `errSecInteractionNotAllowed` (-25308) into the same `PasswordAccessException`.
+     * A caller that mistook "user clicked Deny" for "first launch, generate a fresh
+     * key" would silently rotate the metadata AES key and permanently destroy the
+     * accounts.json.enc it was supposed to unlock.
+     *
+     * On macOS this shells out to `/usr/bin/security find-generic-password`, whose
+     * exit codes are documented and unambiguous (44 = not found, 128 = user cancel /
+     * dialog dismissed, others = backend failure). On Windows / Linux, javakeyring
+     * has no such ambiguity for the equivalent flows in practice, but we still treat
+     * any `PasswordAccessException` here as ambiguous (throw) to keep the contract
+     * strict on the getOrCreate path.
+     */
+    actual suspend fun getPrivateKeyOrThrow(npub: String): String? =
+        withContext(Dispatchers.IO) {
+            try {
+                if (!keyringAvailable) {
+                    return@withContext getFromFallback(npub)
+                }
+                if (isMacOs()) {
+                    return@withContext getFromMacSecurityCli(SERVICE_NAME, npub)
+                }
+                try {
+                    keyring().getPassword(SERVICE_NAME, npub)
+                } catch (e: PasswordAccessException) {
+                    // Non-mac backends: keep the strict contract by refusing to
+                    // treat this as "definitively absent". A caller that needs a
+                    // permissive lookup should use getPrivateKey() instead.
+                    throw SecureStorageException(
+                        "Keyring backend refused access or returned ambiguous not-found",
+                        e,
+                    )
+                }
+            } catch (e: SecureStorageException) {
+                throw e
+            } catch (e: BackendNotSupportedException) {
+                keyringAvailable = false
+                println("OS keyring not available, using fallback encrypted storage")
+                getFromFallback(npub)
+            } catch (e: Exception) {
+                throw SecureStorageException("Failed to retrieve private key (strict)", e)
+            }
+        }
+
+    /**
+     * Test seam: overridable strategy for the strict macOS lookup. Production wires
+     * to [defaultMacSecurityLookup] which spawns `/usr/bin/security`. Tests replace
+     * this with a stub so unit tests run hermetically on any OS.
+     */
+    internal var macSecurityLookup: (String, String) -> MacSecurityResult =
+        ::defaultMacSecurityLookup
+
+    private fun getFromMacSecurityCli(
+        service: String,
+        account: String,
+    ): String? {
+        val result = macSecurityLookup(service, account)
+        return when (result) {
+            is MacSecurityResult.Found -> result.password
+            is MacSecurityResult.NotFound -> null
+            is MacSecurityResult.Ambiguous -> throw SecureStorageException(
+                "macOS Keychain access failed (${result.reason}, exit=${result.exitCode})",
+            )
+        }
+    }
+
     actual suspend fun deletePrivateKey(npub: String): Boolean =
         withContext(Dispatchers.IO) {
             try {
@@ -466,6 +535,98 @@ internal interface KeyringHandle {
     )
 }
 
+/**
+ * Outcome of a strict macOS `/usr/bin/security find-generic-password` lookup.
+ * Kept as a sealed hierarchy so [SecureKeyStorage.getPrivateKeyOrThrow] can
+ * cleanly translate to `null` versus `SecureStorageException`.
+ */
+internal sealed class MacSecurityResult {
+    data class Found(
+        val password: String,
+    ) : MacSecurityResult()
+
+    object NotFound : MacSecurityResult()
+
+    /**
+     * Any exit code other than 0 (found) or 44 (item not found). Reason is a short
+     * human string derived from stderr / documented codes:
+     *   128 = user cancelled or dismissed the Keychain Access dialog
+     *   -25293 (errSecAuthFailed) surfaces as exit 51 in practice
+     *   -25308 (errSecInteractionNotAllowed) surfaces when Keychain is locked
+     */
+    data class Ambiguous(
+        val exitCode: Int,
+        val reason: String,
+    ) : MacSecurityResult()
+}
+
+/**
+ * Pure parser split out for testability on non-macOS CI runners. Maps the
+ * documented exit code contract of `/usr/bin/security find-generic-password`
+ * to a [MacSecurityResult]. `stdout` is the raw password body (`-w` prints it
+ * followed by a newline; strip the trailing newline only). `stderr` is used
+ * as a hint for the ambiguous [MacSecurityResult.Ambiguous.reason] string.
+ */
+internal fun parseMacSecurityFindResult(
+    exitCode: Int,
+    stdout: String,
+    stderr: String,
+): MacSecurityResult =
+    when (exitCode) {
+        0 -> MacSecurityResult.Found(stdout.trimEnd('\n', '\r'))
+        44 -> MacSecurityResult.NotFound
+        else -> {
+            val reason =
+                when {
+                    exitCode == 128 -> "user cancelled Keychain dialog"
+                    stderr.contains("-25293") -> "errSecAuthFailed"
+                    stderr.contains("-25308") -> "errSecInteractionNotAllowed"
+                    stderr.contains("-128") -> "user cancelled Keychain dialog"
+                    stderr.isNotBlank() ->
+                        stderr
+                            .lineSequence()
+                            .first()
+                            .trim()
+                            .take(120)
+                    else -> "unknown"
+                }
+            MacSecurityResult.Ambiguous(exitCode, reason)
+        }
+    }
+
+private fun isMacOs(): Boolean = System.getProperty("os.name").orEmpty().startsWith("Mac")
+
+/**
+ * Production implementation: spawn `/usr/bin/security` and read exit code + streams.
+ * Kept package-private so tests can also reach it if they want to run the real path
+ * on a mac host, but production always goes through the [SecureKeyStorage.macSecurityLookup]
+ * indirection.
+ */
+internal fun defaultMacSecurityLookup(
+    service: String,
+    account: String,
+): MacSecurityResult {
+    val process =
+        try {
+            ProcessBuilder(
+                "/usr/bin/security",
+                "find-generic-password",
+                "-s",
+                service,
+                "-a",
+                account,
+                "-w",
+            ).redirectErrorStream(false).start()
+        } catch (e: Exception) {
+            return MacSecurityResult.Ambiguous(-1, "failed to spawn /usr/bin/security: ${e.message ?: e::class.simpleName ?: "unknown"}")
+        }
+    process.outputStream.close()
+    val stdout = process.inputStream.bufferedReader().use { it.readText() }
+    val stderr = process.errorStream.bufferedReader().use { it.readText() }
+    val exitCode = process.waitFor()
+    return parseMacSecurityFindResult(exitCode, stdout, stderr)
+}
+
 internal class RealKeyringHandle(
     private val keyring: Keyring,
 ) : KeyringHandle {

commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorageOrThrowTest.kt

@@ -0,0 +1,212 @@
+/*
+ * Copyright (c) 2025 Vitor Pamplona
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of
+ * this software and associated documentation files (the "Software"), to deal in
+ * the Software without restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
+ * Software, and to permit persons to whom the Software is furnished to do so,
+ * subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
+ * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+ * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
+ * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+ * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+package com.vitorpamplona.amethyst.commons.keystorage
+
+import com.github.javakeyring.PasswordAccessException
+import kotlinx.coroutines.runBlocking
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNull
+import org.junit.Assert.assertTrue
+import org.junit.Assert.fail
+import org.junit.Test
+
+/**
+ * Unit tests for the strict `getPrivateKeyOrThrow` lookup and its pure macOS
+ * `/usr/bin/security` exit-code parser.
+ *
+ * These tests must never touch the OS keychain and must run on any host, so:
+ *  - the macOS integration paths are gated behind [MacSecurityResult] stubs
+ *    injected via `SecureKeyStorage.macSecurityLookup`;
+ *  - the parser test operates on captured stdout/stderr/exitCode triples;
+ *  - non-mac backends are exercised via the `KeyringHandle` test seam already
+ *    used by [SecureKeyStorageKeyringCacheTest].
+ */
+class SecureKeyStorageOrThrowTest {
+    private class ExplodingKeyring(
+        private val onGet: () -> Nothing,
+    ) : KeyringHandle {
+        override fun getPassword(
+            service: String,
+            account: String,
+        ): String = onGet()
+
+        override fun setPassword(
+            service: String,
+            account: String,
+            password: String,
+        ) {
+            // unused in these tests
+        }
+
+        override fun deletePassword(
+            service: String,
+            account: String,
+        ) {
+            // unused in these tests
+        }
+    }
+
+    private class StaticKeyring(
+        private val map: Map<Pair<String, String>, String>,
+    ) : KeyringHandle {
+        override fun getPassword(
+            service: String,
+            account: String,
+        ): String = map[service to account] ?: throw PasswordAccessException("no entry")
+
+        override fun setPassword(
+            service: String,
+            account: String,
+            password: String,
+        ) {}
+
+        override fun deletePassword(
+            service: String,
+            account: String,
+        ) {}
+    }
+
+    // --- macOS security(1) parser ---
+
+    @Test
+    fun `parser exit 0 returns Found with trimmed password`() {
+        val result = parseMacSecurityFindResult(0, "hunter2\n", "")
+        assertTrue(result is MacSecurityResult.Found)
+        assertEquals("hunter2", (result as MacSecurityResult.Found).password)
+    }
+
+    @Test
+    fun `parser exit 0 preserves internal newlines and only strips trailing`() {
+        val result = parseMacSecurityFindResult(0, "line1\nline2\n", "")
+        assertEquals("line1\nline2", (result as MacSecurityResult.Found).password)
+    }
+
+    @Test
+    fun `parser exit 44 returns NotFound`() {
+        val result =
+            parseMacSecurityFindResult(
+                44,
+                "",
+                "security: SecKeychainSearchCopyNext: The specified item could not be found in the keychain.\n",
+            )
+        assertTrue(result is MacSecurityResult.NotFound)
+    }
+
+    @Test
+    fun `parser exit 128 flagged as user-cancelled`() {
+        val result = parseMacSecurityFindResult(128, "", "security: dismissed\n")
+        assertTrue(result is MacSecurityResult.Ambiguous)
+        val ambig = result as MacSecurityResult.Ambiguous
+        assertEquals(128, ambig.exitCode)
+        assertTrue(ambig.reason.contains("cancel"))
+    }
+
+    @Test
+    fun `parser stderr -25293 mapped to errSecAuthFailed`() {
+        val result = parseMacSecurityFindResult(51, "", "security: SecKeychainItemCopyContent (-25293)\n")
+        val ambig = result as MacSecurityResult.Ambiguous
+        assertEquals("errSecAuthFailed", ambig.reason)
+    }
+
+    @Test
+    fun `parser unknown exit falls back to first stderr line`() {
+        val result = parseMacSecurityFindResult(9999, "", "security: mystery: line 1\nline 2\n")
+        val ambig = result as MacSecurityResult.Ambiguous
+        assertEquals("security: mystery: line 1", ambig.reason)
+    }
+
+    // --- getPrivateKeyOrThrow: strict semantics via injected macOS lookup ---
+    // (Enabled unconditionally: the macSecurityLookup indirection is exercised
+    // via a stub, so no `security` binary is invoked. The `isMacOs()` check
+    // means this test only takes the mac path on macOS runners; on Linux it
+    // takes the javakeyring path, which we validate separately below.)
+
+    private fun newStorage(): SecureKeyStorage = SecureKeyStorage.create()
+
+    @Test
+    fun `mac lookup Found returns password without ambiguity`() =
+        runBlocking {
+            if (!System.getProperty("os.name").orEmpty().startsWith("Mac")) return@runBlocking
+            val storage = newStorage()
+            storage.macSecurityLookup = { _, _ -> MacSecurityResult.Found("secretval") }
+            assertEquals("secretval", storage.getPrivateKeyOrThrow("account-metadata-key"))
+        }
+
+    @Test
+    fun `mac lookup NotFound returns null`() =
+        runBlocking {
+            if (!System.getProperty("os.name").orEmpty().startsWith("Mac")) return@runBlocking
+            val storage = newStorage()
+            storage.macSecurityLookup = { _, _ -> MacSecurityResult.NotFound }
+            assertNull(storage.getPrivateKeyOrThrow("account-metadata-key"))
+        }
+
+    @Test
+    fun `mac lookup Ambiguous throws SecureStorageException with reason`() =
+        runBlocking {
+            if (!System.getProperty("os.name").orEmpty().startsWith("Mac")) return@runBlocking
+            val storage = newStorage()
+            storage.macSecurityLookup = { _, _ ->
+                MacSecurityResult.Ambiguous(128, "user cancelled Keychain dialog")
+            }
+            try {
+                storage.getPrivateKeyOrThrow("account-metadata-key")
+                fail("Expected SecureStorageException")
+            } catch (e: SecureStorageException) {
+                assertTrue(e.message?.contains("user cancelled") == true)
+                assertTrue(e.message?.contains("128") == true)
+            }
+        }
+
+    // --- non-mac backend: PasswordAccessException must throw, never null ---
+
+    @Test
+    fun `non-mac keyring PasswordAccessException throws not returns null`() =
+        runBlocking {
+            if (System.getProperty("os.name").orEmpty().startsWith("Mac")) return@runBlocking
+            val storage = newStorage()
+            storage.keyringFactory = { ExplodingKeyring { throw PasswordAccessException("locked") } }
+            try {
+                storage.getPrivateKeyOrThrow("account-metadata-key")
+                fail("Expected SecureStorageException")
+            } catch (e: SecureStorageException) {
+                assertTrue(
+                    e.message?.contains("ambiguous", ignoreCase = true) == true ||
+                        e.message?.contains("refused", ignoreCase = true) == true,
+                )
+            }
+        }
+
+    @Test
+    fun `non-mac keyring hit returns password`() =
+        runBlocking {
+            if (System.getProperty("os.name").orEmpty().startsWith("Mac")) return@runBlocking
+            val storage = newStorage()
+            storage.keyringFactory = {
+                StaticKeyring(
+                    mapOf(
+                        ("amethyst-desktop" to "account-metadata-key") to "abc123",
+                    ),
+                )
+            }
+            assertEquals("abc123", storage.getPrivateKeyOrThrow("account-metadata-key"))
+        }
+}

desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorage.kt

@@ -23,12 +23,16 @@ package com.vitorpamplona.amethyst.desktop.account
 import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper
 import com.fasterxml.jackson.module.kotlin.readValue
 import com.vitorpamplona.amethyst.commons.keystorage.SecureKeyStorage
+import com.vitorpamplona.amethyst.commons.keystorage.SecureStorageException
 import com.vitorpamplona.amethyst.commons.model.account.AccountInfo
 import com.vitorpamplona.amethyst.commons.model.account.AccountStorage
 import com.vitorpamplona.amethyst.commons.model.account.SignerType
 import com.vitorpamplona.amethyst.commons.util.deleteOrWarn
 import com.vitorpamplona.quartz.utils.Log
+import kotlinx.coroutines.sync.Mutex
+import kotlinx.coroutines.sync.withLock
 import java.io.File
+import java.io.RandomAccessFile
 import java.nio.file.Files
 import java.nio.file.StandardCopyOption
 import java.nio.file.attribute.PosixFilePermission
@@ -58,6 +62,16 @@ sealed class StorageCorruption(
     class JsonMalformed(
         backupPath: String?,
     ) : StorageCorruption(backupPath)
+
+    /**
+     * A transient failure surfaced from the read path (I/O error, keychain refused
+     * or otherwise ambiguous access, OOM, etc). No backup was written and the
+     * on-disk file is untouched. Callers should retry or surface an error UI rather
+     * than treating this as data loss. See [DesktopAccountStorage.readMetadataFromDisk].
+     */
+    class TransientError(
+        val cause: Throwable,
+    ) : StorageCorruption(backupPath = null)
 }
 
 class DesktopAccountStorage(
@@ -68,6 +82,7 @@ class DesktopAccountStorage(
     companion object {
         private const val METADATA_KEY_ALIAS = "account-metadata-key"
         private const val ACCOUNTS_FILE = "accounts.json.enc"
+        private const val ACCOUNTS_LOCK_FILE = "accounts.json.enc.lock"
         private const val AES_KEY_SIZE = 32 // 256 bits
         private const val GCM_IV_SIZE = 12
         private const val GCM_TAG_BITS = 128
@@ -76,38 +91,51 @@ class DesktopAccountStorage(
     private val mapper = jacksonObjectMapper()
     private val amethystDir by lazy { File(homeDir, ".amethyst") }
 
-    // In-memory cache — read from disk once, then serve from memory
+    // In-memory cache: read from disk once, then serve from memory
     private var cachedMetadata: AccountMetadata? = null
 
+    // In-process mutex around the cross-process file lock. Two callers inside
+    // the same JVM would otherwise fail with OverlappingFileLockException from
+    // FileChannel.lock(), since JVM file locks are per-JVM not per-thread.
+    private val fileLockMutex = Mutex()
+
+    // Guards read-modify-write cycles on [cachedMetadata]. Distinct from
+    // [fileLockMutex] so we can hold it across a full read + mutate + write
+    // sequence (the file lock is taken and released inside each disk op).
+    private val stateMutex = Mutex()
+
     // --- AccountStorage interface ---
 
     override suspend fun loadAccounts(): List<AccountInfo> = getCachedMetadata().accounts.map { it.toAccountInfo() }
 
-    override suspend fun saveAccount(info: AccountInfo) {
-        val metadata = getCachedMetadata()
-        val dto = AccountInfoDto.from(info)
-        val updated = metadata.accounts.filter { it.npub != info.npub } + dto
-        writeCachedMetadata(metadata.copy(accounts = updated))
-    }
+    override suspend fun saveAccount(info: AccountInfo) =
+        stateMutex.withLock {
+            val metadata = getCachedMetadata()
+            val dto = AccountInfoDto.from(info)
+            val updated = metadata.accounts.filter { it.npub != info.npub } + dto
+            writeCachedMetadata(metadata.copy(accounts = updated))
+        }
 
-    override suspend fun deleteAccount(npub: String) {
-        val metadata = getCachedMetadata()
-        val updated = metadata.accounts.filter { it.npub != npub }
-        val newActive =
-            if (metadata.activeNpub == npub) {
-                updated.firstOrNull()?.npub
-            } else {
-                metadata.activeNpub
-            }
-        writeCachedMetadata(metadata.copy(accounts = updated, activeNpub = newActive))
-    }
+    override suspend fun deleteAccount(npub: String) =
+        stateMutex.withLock {
+            val metadata = getCachedMetadata()
+            val updated = metadata.accounts.filter { it.npub != npub }
+            val newActive =
+                if (metadata.activeNpub == npub) {
+                    updated.firstOrNull()?.npub
+                } else {
+                    metadata.activeNpub
+                }
+            writeCachedMetadata(metadata.copy(accounts = updated, activeNpub = newActive))
+        }
 
     override suspend fun currentAccount(): String? = getCachedMetadata().activeNpub
 
-    override suspend fun setCurrentAccount(npub: String) {
-        val metadata = getCachedMetadata()
-        writeCachedMetadata(metadata.copy(activeNpub = npub))
-    }
+    override suspend fun setCurrentAccount(npub: String) =
+        stateMutex.withLock {
+            val metadata = getCachedMetadata()
+            writeCachedMetadata(metadata.copy(activeNpub = npub))
+        }
 
     // --- Cached I/O ---
 
@@ -118,9 +146,17 @@ class DesktopAccountStorage(
         return loaded
     }
 
+    /**
+     * Persists first, caches second.
+     *
+     * If the disk write fails (keychain refused, I/O error, disk full) the in-memory
+     * cache must NOT be left claiming a state that was never written: the rest of the
+     * session would serve accounts that vanish on the next launch, and the user would
+     * see a successful save that silently did nothing.
+     */
     private suspend fun writeCachedMetadata(metadata: AccountMetadata) {
-        cachedMetadata = metadata
         writeMetadataToDisk(metadata)
+        cachedMetadata = metadata
     }
 
     // --- Encrypted file I/O ---
@@ -129,9 +165,17 @@ class DesktopAccountStorage(
         val file = getAccountsFile()
         if (!file.exists()) return AccountMetadata()
 
+        ensureDir()
+        return withAccountsFileLock {
+            readMetadataFromDiskLocked(file)
+        }
+    }
+
+    private suspend fun readMetadataFromDiskLocked(file: File): AccountMetadata {
         val encrypted = file.readBytes()
         if (encrypted.size < GCM_IV_SIZE) {
-            val backup = backupCorruptFile(file)
+            // Genuinely unusable: not enough bytes for the IV. Back up and reset.
+            val backup = backupCorruptFile(file, ".corrupt")
             onCorruption(StorageCorruption.FileCorrupted(backup))
             return AccountMetadata()
         }
@@ -140,31 +184,43 @@ class DesktopAccountStorage(
             val decrypted = decrypt(encrypted)
             mapper.readValue<AccountMetadata>(decrypted)
         } catch (e: javax.crypto.AEADBadTagException) {
-            Log.e("DesktopAccountStorage", "GCM auth tag mismatch — file corrupted or key lost", e)
-            val backup = backupCorruptFile(file)
+            // Genuine ciphertext corruption or lost/rotated AES key.
+            Log.e("DesktopAccountStorage", "GCM auth tag mismatch, file corrupted or key lost", e)
+            val backup = backupCorruptFile(file, ".corrupt")
             onCorruption(StorageCorruption.FileCorrupted(backup))
             AccountMetadata()
         } catch (e: javax.crypto.BadPaddingException) {
-            Log.e("DesktopAccountStorage", "Decryption failed — file corrupted", e)
-            val backup = backupCorruptFile(file)
+            // Genuine ciphertext corruption.
+            Log.e("DesktopAccountStorage", "Decryption failed, file corrupted", e)
+            val backup = backupCorruptFile(file, ".corrupt")
             onCorruption(StorageCorruption.FileCorrupted(backup))
             AccountMetadata()
         } catch (e: com.fasterxml.jackson.core.JacksonException) {
+            // Schema mismatch: decrypted cleanly but the JSON does not fit our shape.
+            // Distinct suffix so operators can tell it apart from ciphertext corruption.
             Log.e("DesktopAccountStorage", "JSON malformed after decryption", e)
-            val backup = backupCorruptFile(file)
+            val backup = backupCorruptFile(file, ".jsonerror")
             onCorruption(StorageCorruption.JsonMalformed(backup))
             AccountMetadata()
+        } catch (e: kotlin.coroutines.cancellation.CancellationException) {
+            throw e
         } catch (e: Exception) {
-            Log.e("DesktopAccountStorage", "Failed to read accounts metadata", e)
-            val backup = backupCorruptFile(file)
-            onCorruption(StorageCorruption.FileCorrupted(backup))
-            AccountMetadata()
+            // Transient failure: I/O error, keychain refused / ambiguous, OOM, etc.
+            // DO NOT rename the on-disk file; the ciphertext is intact and the next
+            // launch may succeed (for example after the user re-approves the
+            // Keychain Access prompt). Surface up for the caller to decide.
+            Log.e("DesktopAccountStorage", "Transient error reading accounts metadata; file preserved", e)
+            onCorruption(StorageCorruption.TransientError(e))
+            throw e
         }
     }
 
-    private fun backupCorruptFile(file: File): String? =
+    private fun backupCorruptFile(
+        file: File,
+        suffix: String,
+    ): String? =
         try {
-            val backup = File(file.parent, "accounts.json.enc.corrupt.${System.currentTimeMillis()}")
+            val backup = File(file.parent, "${file.name}$suffix.${System.currentTimeMillis()}")
             java.nio.file.Files
                 .copy(file.toPath(), backup.toPath())
             file.deleteOrWarn("DesktopAccountStorage", "corrupt accounts file")
@@ -178,31 +234,103 @@ class DesktopAccountStorage(
         val json = mapper.writeValueAsBytes(metadata)
         val encrypted = encrypt(json)
 
-        // Atomic write via temp file
         val file = getAccountsFile()
-        val temp = File(amethystDir, "${ACCOUNTS_FILE}.tmp")
-        temp.writeBytes(encrypted)
-        Files.move(temp.toPath(), file.toPath(), StandardCopyOption.REPLACE_EXISTING)
-
-        setFilePermissions(file)
+        withAccountsFileLock {
+            // Atomic write via temp file, under the cross-process lock so two
+            // Amethyst instances (Homebrew upgrade race, accidental double-launch)
+            // cannot interleave writes and truncate the file.
+            val temp = File(amethystDir, "$ACCOUNTS_FILE.tmp")
+            temp.writeBytes(encrypted)
+            Files.move(
+                temp.toPath(),
+                file.toPath(),
+                StandardCopyOption.REPLACE_EXISTING,
+                StandardCopyOption.ATOMIC_MOVE,
+            )
+            setFilePermissions(file)
+        }
     }
 
+    /**
+     * Cross-process advisory lock + in-process mutex around the accounts.json.enc
+     * read/write critical section. The mutex is required because JVM
+     * `FileChannel.lock()` is a per-JVM lock and would throw
+     * `OverlappingFileLockException` on the second acquire from the same JVM.
+     * The channel lock is required to keep two Amethyst processes serial (upgrade
+     * race, accidental double-launch, cron-style relaunch).
+     *
+     * Mirrors the pattern used in SecureKeyStorage.withFileLock; kept private
+     * to this class so the two lock lifecycles stay independent.
+     */
+    private suspend inline fun <T> withAccountsFileLock(crossinline block: suspend () -> T): T =
+        fileLockMutex.withLock {
+            val lockFile = File(amethystDir, ACCOUNTS_LOCK_FILE)
+            if (!lockFile.exists()) {
+                lockFile.createNewFile()
+                setFilePermissions(lockFile)
+            }
+            RandomAccessFile(lockFile, "rw").use { raf ->
+                raf.channel.lock().use { _ ->
+                    block()
+                }
+            }
+        }
+
     private fun getAccountsFile() = File(amethystDir, ACCOUNTS_FILE)
 
     // --- AES-256-GCM encryption ---
 
     private var cachedKey: ByteArray? = null
 
+    /**
+     * Reads (or creates on first launch) the metadata AES key.
+     *
+     * Distinguishes:
+     *   - key exists in keychain: use it
+     *   - keychain confirms definitively absent: generate + persist a fresh key
+     *   - any other outcome (user cancelled/denied prompt, keychain locked,
+     *     backend transient error): propagate the exception, do NOT rotate --
+     *     unless there is no accounts.json.enc yet, in which case there is no
+     *     ciphertext to orphan and we bootstrap a fresh key (see below).
+     *
+     * Rotating the AES key on an ambiguous miss silently destroys the ability
+     * to decrypt the existing accounts.json.enc, wiping the logged-in accounts
+     * on next launch. That is the bug this method exists to prevent.
+     */
     private suspend fun getOrCreateKey(): ByteArray {
         cachedKey?.let { return it }
 
-        val existing = secureStorage.getPrivateKey(METADATA_KEY_ALIAS)
+        val existing =
+            try {
+                secureStorage.getPrivateKeyOrThrow(METADATA_KEY_ALIAS)
+            } catch (e: SecureStorageException) {
+                // Bootstrap escape. Every non-macOS backend java-keyring ships
+                // (Windows Credential Store, Freedesktop Secret Service, KWallet)
+                // throws PasswordAccessException for a *genuinely absent* credential,
+                // so the strict lookup structurally cannot report "definitively
+                // absent" there. Without this branch a fresh Linux/Windows install
+                // could never mint the key and could never persist an account.
+                //
+                // Minting is only safe while there is no accounts.json.enc: with no
+                // ciphertext on disk there is nothing a new key can orphan. Once the
+                // file exists the strict contract applies and we propagate.
+                if (getAccountsFile().exists()) throw e
+                Log.w(
+                    "DesktopAccountStorage",
+                    "Keychain lookup failed and no accounts file exists; bootstrapping a fresh metadata key",
+                    e,
+                )
+                null
+            }
+
         if (existing != null) {
             val key = Base64.getDecoder().decode(existing)
             cachedKey = key
             return key
         }
 
+        // Definitively absent (or bootstrapping with nothing on disk): safe to
+        // create and persist a fresh key.
         val key = ByteArray(AES_KEY_SIZE).also { SecureRandom().nextBytes(it) }
         secureStorage.savePrivateKey(METADATA_KEY_ALIAS, Base64.getEncoder().encodeToString(key))
         cachedKey = key

desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerKeyLoginTest.kt

@@ -58,6 +58,7 @@ class AccountManagerKeyLoginTest {
         val keySlot = slot<String>()
         val valueSlot = slot<String>()
         coEvery { storage.getPrivateKey(capture(keySlot)) } answers { keyStore[keySlot.captured] }
+        coEvery { storage.getPrivateKeyOrThrow(capture(keySlot)) } answers { keyStore[keySlot.captured] }
         coEvery { storage.savePrivateKey(capture(keySlot), capture(valueSlot)) } answers {
             keyStore[keySlot.captured] = valueSlot.captured
         }

desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLoadAccountTest.kt

@@ -49,6 +49,7 @@ class AccountManagerLoadAccountTest {
         storage = mockk(relaxed = true)
         // Return null so DesktopAccountStorage generates a fresh AES key
         coEvery { storage.getPrivateKey("account-metadata-key") } returns null
+        coEvery { storage.getPrivateKeyOrThrow("account-metadata-key") } returns null
         tempDir = createTempDirectory("acctmgr-load-test").toFile()
         amethystDir = File(tempDir, ".amethyst")
         amethystDir.mkdirs()

desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLoadStateTransitionsTest.kt

@@ -63,6 +63,7 @@ class AccountManagerLoadStateTransitionsTest {
     fun setup() {
         storage = mockk(relaxed = true)
         coEvery { storage.getPrivateKey("account-metadata-key") } returns null
+        coEvery { storage.getPrivateKeyOrThrow("account-metadata-key") } returns null
         tempDir = createTempDirectory("acctmgr-load-state").toFile()
         File(tempDir, ".amethyst").mkdirs()
         manager = AccountManager(storage, tempDir)

desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLogoutTest.kt

@@ -46,6 +46,7 @@ class AccountManagerLogoutTest {
     fun setup() {
         storage = mockk(relaxed = true)
         coEvery { storage.getPrivateKey("account-metadata-key") } returns null
+        coEvery { storage.getPrivateKeyOrThrow("account-metadata-key") } returns null
         tempDir = createTempDirectory("acctmgr-logout-test").toFile()
         manager = AccountManager(storage, tempDir)
     }

desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerNip46IsolationTest.kt

@@ -56,6 +56,7 @@ class AccountManagerNip46IsolationTest {
     fun setup() {
         storage = mockk(relaxed = true)
         coEvery { storage.getPrivateKey("account-metadata-key") } returns null
+        coEvery { storage.getPrivateKeyOrThrow("account-metadata-key") } returns null
         tempDir = createTempDirectory("acctmgr-nip46-iso-test").toFile()
         amethystDir = File(tempDir, ".amethyst")
         amethystDir.mkdirs()

desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerStateTransitionTest.kt

@@ -56,6 +56,7 @@ class AccountManagerStateTransitionTest {
     fun setup() {
         storage = mockk(relaxed = true)
         coEvery { storage.getPrivateKey("account-metadata-key") } returns null
+        coEvery { storage.getPrivateKeyOrThrow("account-metadata-key") } returns null
         tempDir = createTempDirectory("acctmgr-state-test").toFile()
         amethystDir = File(tempDir, ".amethyst")
         amethystDir.mkdirs()

desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorageTest.kt

@@ -21,18 +21,28 @@
 package com.vitorpamplona.amethyst.desktop.account
 
 import com.vitorpamplona.amethyst.commons.keystorage.SecureKeyStorage
+import com.vitorpamplona.amethyst.commons.keystorage.SecureStorageException
 import com.vitorpamplona.amethyst.commons.model.account.AccountInfo
 import com.vitorpamplona.amethyst.commons.model.account.SignerType
 import io.mockk.coEvery
 import io.mockk.coVerify
 import io.mockk.mockk
 import io.mockk.slot
+import kotlinx.coroutines.asCoroutineDispatcher
+import kotlinx.coroutines.async
+import kotlinx.coroutines.awaitAll
+import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.test.runTest
+import kotlinx.coroutines.withContext
 import java.io.File
+import java.util.concurrent.Executors
 import kotlin.test.AfterTest
 import kotlin.test.BeforeTest
 import kotlin.test.Test
 import kotlin.test.assertEquals
+import kotlin.test.assertFails
+import kotlin.test.assertFalse
+import kotlin.test.assertNotNull
 import kotlin.test.assertNull
 import kotlin.test.assertTrue
 
@@ -56,6 +66,9 @@ class DesktopAccountStorageTest {
         coEvery { secureStorage.getPrivateKey(capture(keySlot)) } answers {
             keyStore[keySlot.captured]
         }
+        coEvery { secureStorage.getPrivateKeyOrThrow(capture(keySlot)) } answers {
+            keyStore[keySlot.captured]
+        }
         coEvery { secureStorage.savePrivateKey(capture(keySlot), capture(valueSlot)) } answers {
             keyStore[keySlot.captured] = valueSlot.captured
         }
@@ -197,4 +210,269 @@ class DesktopAccountStorageTest {
             // Encrypted content should NOT contain the npub in plaintext
             assertTrue(!content.contains("npub1secret"))
         }
+
+    // --- Bug 1: silent AES key rotation ---
+
+    @Test
+    fun `getOrCreateKey keyring throws ambiguous error does not rotate key or touch file`() =
+        runTest {
+            // First launch: seed a real metadata key + an existing accounts.json.enc
+            storage.saveAccount(AccountInfo("npub1existing", SignerType.Internal))
+            val file = File(File(tempDir, ".amethyst"), "accounts.json.enc")
+            val originalBytes = file.readBytes()
+            val originalMetadataKey = keyStore["account-metadata-key"]
+            assertNotNull(originalMetadataKey)
+
+            // Fresh storage instance simulating a relaunch: the keychain now
+            // returns an ambiguous error (user cancelled the Keychain dialog).
+            val throwingStorage: SecureKeyStorage = mockk()
+            coEvery { throwingStorage.getPrivateKeyOrThrow("account-metadata-key") } throws
+                SecureStorageException("user cancelled Keychain dialog")
+            // Legacy permissive read should still return the key; production
+            // must not fall back to it on the getOrCreate path.
+            coEvery { throwingStorage.getPrivateKey(any()) } answers { keyStore[firstArg()] }
+            coEvery { throwingStorage.hasPrivateKey(any()) } answers { keyStore.containsKey(firstArg()) }
+
+            val relaunched = DesktopAccountStorage(throwingStorage, tempDir)
+
+            // Any operation that needs the metadata key must fail loudly, not
+            // rotate the key or write a fresh empty file.
+            assertFails { runBlocking { relaunched.loadAccounts() } }
+
+            // Bug 1 invariant: no new savePrivateKey call for the metadata key.
+            coVerify(exactly = 0) {
+                throwingStorage.savePrivateKey("account-metadata-key", any())
+            }
+            // Bug 1 + Bug 2 invariant: on-disk ciphertext untouched.
+            assertTrue(file.exists())
+            assertContentEquals(originalBytes, file.readBytes())
+            // Bug 1 invariant: keyStore metadata key unchanged.
+            assertEquals(originalMetadataKey, keyStore["account-metadata-key"])
+        }
+
+    @Test
+    fun `getOrCreateKey keyring returns definitive not-found creates and persists new key`() =
+        runTest {
+            // Happy path first launch: getPrivateKeyOrThrow returns null,
+            // storage generates + persists a fresh AES key exactly once.
+            assertNull(keyStore["account-metadata-key"])
+
+            storage.saveAccount(AccountInfo("npub1first", SignerType.Internal))
+
+            assertNotNull(keyStore["account-metadata-key"])
+            coVerify(exactly = 1) {
+                secureStorage.savePrivateKey("account-metadata-key", any())
+            }
+        }
+
+    @Test
+    fun `getOrCreateKey ambiguous error with no accounts file bootstraps a fresh key`() =
+        runTest {
+            // Every non-macOS backend java-keyring ships (Windows Credential Store,
+            // Freedesktop Secret Service, KWallet) throws PasswordAccessException for a
+            // *genuinely absent* credential, which the strict lookup surfaces as
+            // SecureStorageException. With no accounts.json.enc there is no ciphertext
+            // a new key could orphan, so a fresh install must still be able to mint one
+            // -- otherwise Linux/Windows can never persist an account at all.
+            val saved = mutableMapOf<String, String>()
+            val throwingStorage: SecureKeyStorage = mockk()
+            coEvery { throwingStorage.getPrivateKeyOrThrow("account-metadata-key") } throws
+                SecureStorageException("Keyring backend refused access or returned ambiguous not-found")
+            coEvery { throwingStorage.savePrivateKey(any(), any()) } answers {
+                saved[firstArg()] = secondArg()
+            }
+            coEvery { throwingStorage.getPrivateKey(any()) } answers { saved[firstArg()] }
+            coEvery { throwingStorage.hasPrivateKey(any()) } answers { saved.containsKey(firstArg()) }
+
+            val file = File(File(tempDir, ".amethyst"), "accounts.json.enc")
+            assertFalse(file.exists())
+
+            val fresh = DesktopAccountStorage(throwingStorage, tempDir)
+            fresh.saveAccount(AccountInfo("npub1freshinstall", SignerType.Internal))
+
+            assertNotNull(saved["account-metadata-key"])
+            assertTrue(file.exists())
+            assertEquals(listOf("npub1freshinstall"), fresh.loadAccounts().map { it.npub })
+            // The escape is bootstrap-only: once the file exists the strict contract
+            // applies again -- pinned by `getOrCreateKey keyring throws ambiguous error
+            // does not rotate key or touch file` above.
+        }
+
+    // --- Cache must never claim a state that was not persisted ---
+
+    @Test
+    fun `failed disk write does not poison the in-memory cache`() =
+        runTest {
+            storage.saveAccount(AccountInfo("npub1persisted", SignerType.Internal))
+            assertEquals(listOf("npub1persisted"), storage.loadAccounts().map { it.npub })
+
+            // Block the atomic-write temp path so writeMetadataToDisk fails.
+            val temp = File(File(tempDir, ".amethyst"), "accounts.json.enc.tmp")
+            assertTrue(temp.mkdirs())
+
+            assertFails {
+                runBlocking {
+                    storage.saveAccount(AccountInfo("npub1phantom", SignerType.Internal))
+                }
+            }
+
+            // Same instance: the cache must still reflect only what reached the disk,
+            // not the account the failed save handed it.
+            assertEquals(listOf("npub1persisted"), storage.loadAccounts().map { it.npub })
+
+            // And the on-disk file agrees.
+            temp.delete()
+            val relaunched = DesktopAccountStorage(secureStorage, tempDir)
+            assertEquals(listOf("npub1persisted"), relaunched.loadAccounts().map { it.npub })
+        }
+
+    // --- Bug 2: read failure must not silently reset the file ---
+
+    @Test
+    fun `readMetadataFromDisk transient IO error does not backup file`() =
+        runTest {
+            // Seed a real file we can inspect.
+            storage.saveAccount(AccountInfo("npub1existing", SignerType.Internal))
+            val amethystDir = File(tempDir, ".amethyst")
+            val file = File(amethystDir, "accounts.json.enc")
+            val originalBytes = file.readBytes()
+            val originalName = file.name
+
+            // Fresh storage that surfaces a transient error from the keychain.
+            val throwingStorage: SecureKeyStorage = mockk()
+            coEvery { throwingStorage.getPrivateKeyOrThrow("account-metadata-key") } throws
+                SecureStorageException("transient keychain error")
+            coEvery { throwingStorage.getPrivateKey(any()) } returns null
+            coEvery { throwingStorage.hasPrivateKey(any()) } returns false
+
+            val corruptions = mutableListOf<StorageCorruption>()
+            val relaunched =
+                DesktopAccountStorage(throwingStorage, tempDir, onCorruption = { corruptions += it })
+
+            assertFails { runBlocking { relaunched.loadAccounts() } }
+
+            // File preserved, no .corrupt.* or .jsonerror.* sibling created.
+            assertTrue(file.exists())
+            assertContentEquals(originalBytes, file.readBytes())
+            val siblings = amethystDir.listFiles().orEmpty().map { it.name }
+            assertFalse(siblings.any { it != originalName && it.startsWith("accounts.json.enc") && (it.contains(".corrupt.") || it.contains(".jsonerror.")) })
+
+            // The callback fired with the transient subtype so the app can retry.
+            assertTrue(corruptions.any { it is StorageCorruption.TransientError })
+        }
+
+    @Test
+    fun `readMetadataFromDisk gcm tag mismatch backs up and resets`() =
+        runTest {
+            // Seed a valid file so we have a real metadata key in the mock keystore.
+            storage.saveAccount(AccountInfo("npub1a", SignerType.Internal))
+            val amethystDir = File(tempDir, ".amethyst")
+            val file = File(amethystDir, "accounts.json.enc")
+            assertTrue(file.exists())
+
+            // Overwrite with random bytes that pass the length check but fail
+            // GCM auth tag verification. Prefix with a fresh IV, then garbage.
+            val garbage = ByteArray(64) { it.toByte() }
+            file.writeBytes(garbage)
+
+            val corruptions = mutableListOf<StorageCorruption>()
+            val relaunched =
+                DesktopAccountStorage(secureStorage, tempDir, onCorruption = { corruptions += it })
+
+            val loaded = relaunched.loadAccounts()
+            assertTrue(loaded.isEmpty())
+
+            // Backup exists with the .corrupt.<ts> suffix; original file was
+            // removed (and will be re-created on next save).
+            val siblings = amethystDir.listFiles().orEmpty().map { it.name }
+            assertTrue(siblings.any { it.startsWith("accounts.json.enc.corrupt.") })
+            assertTrue(corruptions.any { it is StorageCorruption.FileCorrupted })
+        }
+
+    @Test
+    fun `readMetadataFromDisk json malformed uses jsonerror suffix`() =
+        runTest {
+            // Build an accounts.json.enc whose plaintext decrypts fine but is
+            // not the expected AccountMetadata shape. Easiest path: reuse the
+            // production encrypt via a lightweight helper storage that lets us
+            // control the plaintext.
+            storage.saveAccount(AccountInfo("npub1a", SignerType.Internal))
+            val amethystDir = File(tempDir, ".amethyst")
+            val file = File(amethystDir, "accounts.json.enc")
+
+            // Encrypt an unrelated JSON payload with the same AES key the mock
+            // keystore holds so decryption succeeds but Jackson rejects the shape.
+            val key =
+                java.util.Base64
+                    .getDecoder()
+                    .decode(keyStore["account-metadata-key"]!!)
+            val iv = ByteArray(12) { 7 }
+            val cipher = javax.crypto.Cipher.getInstance("AES/GCM/NoPadding")
+            cipher.init(
+                javax.crypto.Cipher.ENCRYPT_MODE,
+                javax.crypto.spec.SecretKeySpec(key, "AES"),
+                javax.crypto.spec.GCMParameterSpec(128, iv),
+            )
+            val badPayload = cipher.doFinal("\"not an object\"".toByteArray())
+            file.writeBytes(iv + badPayload)
+
+            val corruptions = mutableListOf<StorageCorruption>()
+            val relaunched =
+                DesktopAccountStorage(secureStorage, tempDir, onCorruption = { corruptions += it })
+
+            val loaded = relaunched.loadAccounts()
+            assertTrue(loaded.isEmpty())
+
+            val siblings = amethystDir.listFiles().orEmpty().map { it.name }
+            assertTrue(siblings.any { it.startsWith("accounts.json.enc.jsonerror.") })
+            assertTrue(corruptions.any { it is StorageCorruption.JsonMalformed })
+        }
+
+    // --- Bug 3: cross-process file lock ---
+
+    @Test
+    fun `writeMetadataToDisk concurrent saves serialize under file lock`() {
+        val executor = Executors.newFixedThreadPool(4)
+        try {
+            runBlocking {
+                withContext(executor.asCoroutineDispatcher()) {
+                    val jobs =
+                        (1..8).map { idx ->
+                            async {
+                                storage.saveAccount(
+                                    AccountInfo(
+                                        npub = "npub1parallel$idx",
+                                        signerType = SignerType.Internal,
+                                    ),
+                                )
+                            }
+                        }
+                    jobs.awaitAll()
+                }
+            }
+            // All eight accounts present, file not truncated.
+            val loaded = runBlocking { storage.loadAccounts() }
+            assertEquals(8, loaded.size)
+            val npubs = loaded.map { it.npub }.toSet()
+            assertEquals((1..8).map { "npub1parallel$it" }.toSet(), npubs)
+
+            // Lock sidecar exists and is respected.
+            val lockFile = File(File(tempDir, ".amethyst"), "accounts.json.enc.lock")
+            assertTrue(lockFile.exists())
+        } finally {
+            executor.shutdownNow()
+        }
+    }
+
+    private fun assertContentEquals(
+        expected: ByteArray,
+        actual: ByteArray,
+    ) {
+        assertEquals(expected.size, actual.size, "byte size mismatch")
+        for (i in expected.indices) {
+            if (expected[i] != actual[i]) {
+                throw AssertionError("byte differs at index $i: expected=${expected[i]} actual=${actual[i]}")
+            }
+        }
+    }
 }

desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/benchmark/LaunchScenario.kt

@@ -95,6 +95,7 @@ object LaunchScenario {
             val tempHome = createTempDirectory("launch-scenario").toFile()
             val storage = mockk<SecureKeyStorage>(relaxed = true)
             coEvery { storage.getPrivateKey(any()) } returns null
+            coEvery { storage.getPrivateKeyOrThrow(any()) } returns null
             File(tempHome, ".amethyst").mkdirs()
 
             val account = AccountManager(storage, tempHome)

desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/ui/AppStateMachineTest.kt

@@ -89,6 +89,7 @@ class AppStateMachineTest {
         File(tempDir, ".amethyst").mkdirs()
         storage = mockk(relaxed = true)
         coEvery { storage.getPrivateKey(any()) } returns null
+        coEvery { storage.getPrivateKeyOrThrow(any()) } returns null
         harnessScope = CoroutineScope(Dispatchers.Default + SupervisorJob())
         relay = LaunchFixtureRelay.open(LaunchFixture.build(noteCount = 0).events)
     }