Serialize configuration and log updates with bounded file locks

AGENTS.md

@@ -0,0 +1,19 @@
+# Development on the shared desktop
+
+The user reported a GNOME/Wayland freeze associated with high memory pressure,
+nearly exhausted swap, and an AppIndicator extension reload. Treat this as a
+development constraint; a larger swap file is not a reason to increase load.
+
+- Routine project work must not reload, enable, or disable desktop extensions,
+  restart GNOME Shell, or close/manage the user's terminal windows.
+- Run one local compilation at a time with Nim `--parallelBuild:1` and reduced
+  priority (`nice -n 10` on Linux). Before starting, inspect `MemAvailable` and
+  `/proc/pressure/memory`; defer compilation to CI if available memory is below
+  6 GiB or the memory `full avg10` stall percentage is at least 2.
+- Use focused local tests. Keep persistence test workers bounded to four small
+  processes; run full native platform suites and heavier matrices in CI.
+- Put development executables in `.ci/` and select them explicitly for tests.
+  Preserve the user's configuration, credentials, and active installed binary
+  during development. Test settings and keys in temporary configuration roots.
+- When another session is changing the same checkout, use an isolated worktree
+  and stage only the files owned by the current task.

STATE_PERSISTENCE.md

@@ -0,0 +1,34 @@
+# Configuration and log concurrency
+
+This follow-up addresses the configuration/log concurrency issue recorded in
+[the v3 review](CODE_REVIEW-v3.md). Concurrent `get set` operations now lock the
+entire read, validation, and write sequence, so changing different options does
+not discard another process's update. Key writes, full config saves, and reset
+use the same settings lock. Invalid settings release their lock without saving.
+
+Logs serialize append, retention, and cleaning. Bounded retention writes a
+private replacement containing only the retained entries. Historical multiline
+entries are scanned incrementally; counting and cleaning do not load the entire
+file, and trimming keeps only the requested tail plus the current entry.
+
+Locks use POSIX `flock` or Windows `LockFileEx`, with a five-second monotonic
+wait budget for lock contention. The OS releases ownership when a process exits, including after a
+crash. Persistent, empty `.settings.lock` and `get.log.lock` sidecars are retained
+so an unlink/recreate race cannot create two independent locks. POSIX lock files
+are private, regular files and cannot follow a symbolic link. Windows readers
+also take the settings lock to prevent replacement racing an open reader.
+
+These are cooperating-process locks. They serialize writers using this version;
+older binaries and external editors do not participate. A full config save or
+reset intentionally replaces all settings. Two-file reset is not a crash-atomic
+database transaction, and power-loss durability is not claimed. Unlimited logs
+still grow on disk when `log-max-entries=0`; retained memory depends on the
+configured entry count and the largest historical entry.
+
+The bounded worker regressions cover concurrent setters, reset, whole log
+entries with/without retention, serialized cleaning, lock deadlines, owner
+termination, and symbolic-link rejection. Local work follows [AGENTS.md](AGENTS.md)
+to limit pressure on the shared desktop.
+
+API behavior was checked against the [flock manual](https://man7.org/linux/man-pages/man2/flock.2.html)
+and [Microsoft LockFileEx documentation](https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-lockfileex).

src/config.nim

@@ -21,6 +21,7 @@ when defined(windows):
   import std/base64
 
 import harness_types
+import file_lock
 import style
 import utils
 
@@ -700,7 +701,7 @@ when defined(getTest):
 ## .. code-block:: nim
 ##   runnableExamples:
 ##     discard
-proc saveKey*(key: Option[string]) =
+proc implSaveKeyUnlocked(key: Option[string]) =
   let path = getKeyFilePath()
   if key.isNone:
     if fileExists(path):
@@ -713,14 +714,23 @@ proc saveKey*(key: Option[string]) =
   else:
     writePrivateFile(path, value)
 
+proc implSettingsLockPath(): string =
+  result = getAppConfigDir() / ".settings.lock"
+
+proc saveKey*(key: Option[string]) =
+  ## Serialize key updates with config setters and reset.
+  let lock = acquireFileLock(implSettingsLockPath())
+  defer: releaseFileLock(lock)
+  implSaveKeyUnlocked(key)
+
 ## Loads the API key from platform-specific secure storage.
 ##
 ## :returns: The stored key, or none if absent.
 ##
 ## .. code-block:: nim
 ##   runnableExamples:
 ##     discard
-proc loadKey*(): Option[string] =
+proc implLoadKeyUnlocked(): Option[string] =
   let path = getKeyFilePath()
   if not fileExists(path):
     return none(string)
@@ -738,6 +748,12 @@ proc loadKey*(): Option[string] =
   else:
     result = some(content)
 
+proc loadKey*(): Option[string] =
+  when defined(windows):
+    let lock = acquireFileLock(implSettingsLockPath())
+    defer: releaseFileLock(lock)
+  result = implLoadKeyUnlocked()
+
 # ---------------------------------------------------------------------------
 # Public API — config persistence
 # ---------------------------------------------------------------------------
@@ -750,7 +766,7 @@ proc loadKey*(): Option[string] =
 ## .. code-block:: nim
 ##   runnableExamples:
 ##     discard
-proc loadConfig*(): Config =
+proc implLoadConfigUnlocked(): Config =
   let path = getConfigFilePath()
   if not fileExists(path):
     return defaultConfig()
@@ -770,18 +786,30 @@ proc loadConfig*(): Config =
       " using defaults")
     result = defaults
 
+proc loadConfig*(): Config =
+  # Readers also cooperate so Windows replacement never races an open reader.
+  when defined(windows):
+    let lock = acquireFileLock(implSettingsLockPath())
+    defer: releaseFileLock(lock)
+  result = implLoadConfigUnlocked()
+
 ## Writes the configuration to disk as pretty-printed JSON.
 ##
 ## :param cfg: The configuration to persist.
 ##
 ## .. code-block:: nim
 ##   runnableExamples:
 ##     discard
-proc saveConfig*(cfg: Config) =
+proc implSaveConfigUnlocked(cfg: Config) =
   let path = getConfigFilePath()
   let node = implConfigToJson(cfg)
   writePrivateFile(path, pretty(node, 2) & "\n")
 
+proc saveConfig*(cfg: Config) =
+  let lock = acquireFileLock(implSettingsLockPath())
+  defer: releaseFileLock(lock)
+  implSaveConfigUnlocked(cfg)
+
 # ---------------------------------------------------------------------------
 # Public API — display
 # ---------------------------------------------------------------------------
@@ -897,8 +925,10 @@ proc displayConfig*(sk: StyleKind = skSimp) =
 ##   runnableExamples:
 ##     discard
 proc resetConfig*() =
-  saveConfig(defaultConfig())
-  saveKey(none(string))
+  let lock = acquireFileLock(implSettingsLockPath())
+  defer: releaseFileLock(lock)
+  implSaveConfigUnlocked(defaultConfig())
+  implSaveKeyUnlocked(none(string))
 
 # ---------------------------------------------------------------------------
 # Public API — set by name
@@ -939,7 +969,9 @@ proc setConfigOption*(
       saveKey(some(value))
     return
 
-  var cfg = loadConfig()
+  let lock = acquireFileLock(implSettingsLockPath())
+  defer: releaseFileLock(lock)
+  var cfg = implLoadConfigUnlocked()
   case name
   of "url":
     cfg.url = value
@@ -1056,7 +1088,7 @@ proc setConfigOption*(
   else:
     raise newException(GetError,
       fmt"unknown option '{name}'")
-  saveConfig(cfg)
+  implSaveConfigUnlocked(cfg)
 
 # ---------------------------------------------------------------------------
 # Public API — readiness check

src/file_lock.nim

@@ -0,0 +1,92 @@
+## Short, crash-released cross-process locks for configuration and logs.
+## Sidecar files stay in place: unlinking a lock could create two lock domains.
+
+import std/[monotimes, os, times]
+import utils
+
+when defined(windows):
+  import std/[widestrs, winlean]
+
+  const
+    LOCKFILE_FAIL_IMMEDIATELY = 1
+    LOCKFILE_EXCLUSIVE_LOCK = 2
+
+  proc lockFileEx(handle: Handle, flags, reserved, low, high: DWORD,
+                 overlapped: POVERLAPPED): WINBOOL {.
+    stdcall, dynlib: "kernel32", importc: "LockFileEx".}
+  proc unlockFileEx(handle: Handle, reserved, low, high: DWORD,
+                   overlapped: POVERLAPPED): WINBOOL {.
+    stdcall, dynlib: "kernel32", importc: "UnlockFileEx".}
+elif defined(posix):
+  import std/posix
+
+  proc flock(fd: cint, operation: cint): cint {.importc, header: "<sys/file.h>".}
+  var
+    lockExclusive {.importc: "LOCK_EX", header: "<sys/file.h>".}: cint
+    lockNonblocking {.importc: "LOCK_NB", header: "<sys/file.h>".}: cint
+    openNoFollow {.importc: "O_NOFOLLOW", header: "<fcntl.h>".}: cint
+    openCloseOnExec {.importc: "O_CLOEXEC", header: "<fcntl.h>".}: cint
+else:
+  {.error: "State locks require Windows or POSIX file locking".}
+
+type
+  FileLockError* = object of GetError
+  FileLock* = object
+    when defined(windows):
+      handle: Handle
+    else:
+      fd: cint
+
+const FILE_LOCK_WAIT_MS* = 5_000
+
+proc acquireFileLock*(path: string, timeoutMs = FILE_LOCK_WAIT_MS): FileLock =
+  ## Acquire an exclusive, non-inheritable lock with a monotonic deadline.
+  let started = getMonoTime()
+  var acquired = false
+  when defined(windows):
+    let handle = createFileW(newWideCString(path), GENERIC_READ or GENERIC_WRITE,
+      FILE_SHARE_READ or FILE_SHARE_WRITE, nil, OPEN_ALWAYS,
+      FILE_ATTRIBUTE_NORMAL, 0)
+    if handle == INVALID_HANDLE_VALUE:
+      raise newException(FileLockError, "cannot open state lock: " & osErrorMsg(osLastError()))
+    defer:
+      if not acquired: discard closeHandle(handle)
+  else:
+    let fd = posix.open(path.cstring,
+      O_RDWR or O_CREAT or O_NONBLOCK or openNoFollow or openCloseOnExec, Mode(0o600))
+    if fd < 0:
+      raise newException(FileLockError, "cannot open state lock: " & osErrorMsg(osLastError()))
+    defer:
+      if not acquired: discard posix.close(fd)
+    var info: Stat
+    if fstat(fd, info) != 0 or not S_ISREG(info.st_mode) or fchmod(fd, Mode(0o600)) != 0:
+      raise newException(FileLockError, "state lock must be a private regular file")
+
+  while true:
+    when defined(windows):
+      var overlapped: OVERLAPPED
+      # Fail immediately on contention; retry only within the common deadline.
+      if lockFileEx(handle, LOCKFILE_FAIL_IMMEDIATELY or LOCKFILE_EXCLUSIVE_LOCK,
+                    0, 1, 0, addr overlapped) != 0:
+        acquired = true
+        return FileLock(handle: handle)
+      if getLastError() != ERROR_LOCK_VIOLATION:
+        raise newException(FileLockError, "cannot acquire state lock: " & osErrorMsg(osLastError()))
+    else:
+      if flock(fd, lockExclusive or lockNonblocking) == 0:
+        acquired = true
+        return FileLock(fd: fd)
+      if errno notin [EAGAIN, EWOULDBLOCK, EINTR]:
+        raise newException(FileLockError, "cannot acquire state lock: " & osErrorMsg(osLastError()))
+    if (getMonoTime() - started).inMilliseconds >= timeoutMs:
+      raise newException(FileLockError, "configuration or log is busy; retry the operation")
+    sleep(10)
+
+proc releaseFileLock*(lock: FileLock) =
+  ## Release exactly once, normally from a defer/finally clause.
+  when defined(windows):
+    var overlapped: OVERLAPPED
+    discard unlockFileEx(lock.handle, 0, 1, 0, addr overlapped)
+    discard closeHandle(lock.handle)
+  else:
+    discard posix.close(lock.fd)