system: make settings stores opaque and transactional

CHANGELOG.md

@@ -35,6 +35,14 @@
   before it can remove or rename shared staging, backup, marker, or journal
   state; the persistent empty lock file is not a crash marker. The prepared
   installation handle is opaque so callers cannot forge its lock identity.
+
+- Make `settings-store` an opaque synchronized handle. The pre-1.0 API no
+  longer exports its constructor, path/lock/data fields, or mutation setters,
+  which allowed callers to bypass atomic persistence and replace the store's
+  lock. Transactions refresh under shared in-process and operating-system
+  locks to prevent stale handles from losing updates; stored JSON trees and
+  snapshots are deeply immutable. Use `make-settings-store` and the existing
+  controlled operations; `settings-store?` remains public.
 - Add a Windows `rivet::system::TrayController` that owns tray window
   subclassing, localized native menu presentation, command dispatch, and
   automatic `TaskbarCreated` recovery. The reference Taskboard now exercises

docs/system-services.md

@@ -77,6 +77,6 @@ The Linux integration binary self-checks the adapter (`RivetIntegration --system
 
 ## Settings, logs, and crashes
 
-`make-settings-store` reads a JSON object and writes updates through an atomic replacement under a semaphore. The application chooses the path, normally its platform Application Support/AppData directory (on Linux, a file under the XDG state or config directories). Secrets do not belong in settings; use secure storage.
+`make-settings-store` returns an opaque handle that reads a JSON object and writes updates through an atomic replacement under a shared in-process semaphore and operating-system file lock. Operations refresh inside the transaction, so multiple handles and cooperating processes do not overwrite one another from stale caches. Snapshots and stored JSON trees are deeply immutable; callers cannot mutate cached state around synchronization. The persistent empty `.lock` sibling carries no transaction state. Use `settings-ref`, `settings-set!`, `settings-remove!`, and `settings-snapshot`. The application chooses the path, normally its platform Application Support/AppData directory (on Linux, a file under the XDG state or config directories). Secrets do not belong in settings; use secure storage.
 
 `rivet-log` emits structured records to `current-rivet-log-sink`. `call-with-crash-reporting` records an escaping Racket exception and invokes `current-rivet-crash-reporter` before re-raising it. Native fatal hooks are deliberately minimal: crash handlers must avoid allocations and network access, then let a helper submit the report on the next launch.

rivet/scribblings/rivet.scrbl

@@ -272,12 +272,11 @@ Native hosts expose equivalent platform APIs in their native system library.}
 @defparam[current-system-adapter adapter system-adapter?]{
 The adapter used by all system-service convenience procedures.}
 
-@defstruct*[settings-store
-            ([path path?]
-             [lock semaphore?]
-             [data hash?])
-            #:mutable]{
-Represents one synchronized, JSON-backed preferences file.}
+@defproc[(settings-store? [value any/c]) boolean?]{
+Recognizes an opaque, synchronized JSON-backed preferences store. Its path,
+lock, and cached data are private so updates cannot bypass the atomic public
+operations. Transactions refresh under an in-process and operating-system
+lock, and returned JSON trees are deeply immutable.}
 
 @defparam[current-rivet-log-sink sink procedure?]{
 Receives each structured log record. The default writes JSON to the current
@@ -297,11 +296,11 @@ secondary-instance activation.}
                                     [body string?]
                                     [#:tag tag (or/c #f string?) #f]) any/c]{
 Displays an operating-system notification.}
-@defproc[(set-tray-menu! [items list?]) any/c]{Configures a Windows tray or macOS menu-bar menu.}
+@defproc[(set-tray-menu! [items list?]) any/c]{Configures a platform tray or menu-bar menu.}
 @defproc[(set-autostart! [enabled? boolean?]) any/c]{Enables or disables login startup.}
 @defproc[(autostart-enabled?) boolean?]{Reports the current login-startup state.}
 @defproc[(secure-store-set! [service string?] [account string?] [secret bytes?]) any/c]{
-Stores a secret through Windows Credential Manager or macOS Keychain.}
+Stores a secret through the platform credential service.}
 @defproc[(secure-store-ref [service string?] [account string?] [default any/c #f]) any/c]{
 Reads a secret, returning @racket[default] when no value is present.}
 @defproc[(secure-store-remove! [service string?] [account string?]) any/c]{Removes a stored secret.}

rivet/system/settings.rkt

@@ -5,14 +5,40 @@
          racket/path
          racket/port)
 
-(provide (struct-out settings-store)
+(provide settings-store?
          make-settings-store
          settings-ref
          settings-set!
          settings-remove!
          settings-snapshot)
 
-(struct settings-store (path lock data) #:mutable #:transparent)
+;; The path, lock, and cached data form one synchronization invariant.  Keep
+;; the representation private so callers cannot replace any one of them and
+;; then continue through the public operations with a corrupted store.
+(struct settings-store (path lock data) #:mutable)
+
+(define store-locks (make-hash))
+(define store-locks-guard (make-semaphore 1))
+
+(define (shared-store-lock path)
+  (define key (path->string (normal-case-path path)))
+  (call-with-semaphore
+   store-locks-guard
+   (lambda ()
+     (hash-ref store-locks key
+               (lambda ()
+                 (define lock (make-semaphore 1))
+                 (hash-set! store-locks key lock)
+                 lock)))))
+
+(define (immutable-settings-value value)
+  (cond
+    [(hash? value)
+     (for/hasheq ([(key item) (in-hash value)])
+       (values key (immutable-settings-value item)))]
+    [(list? value) (map immutable-settings-value value)]
+    [(string? value) (string->immutable-string value)]
+    [else value]))
 
 (define (load-settings path)
   (cond
@@ -24,15 +50,39 @@
        (raise-arguments-error 'make-settings-store
                               "settings file must contain a JSON object"
                               "path" path))
-     value]))
+     (immutable-settings-value value)]))
 
 (define (make-settings-store path)
-  (settings-store (path->complete-path path)
-                  (make-semaphore 1)
-                  (load-settings path)))
+  (define complete-path (path->complete-path path))
+  (settings-store complete-path
+                  (shared-store-lock complete-path)
+                  (load-settings complete-path)))
+
+(define (store-lock-path store)
+  (path-add-extension (settings-store-path store) #".lock"))
 
 (define (with-store-lock store thunk)
-  (call-with-semaphore (settings-store-lock store) thunk))
+  (call-with-semaphore
+   (settings-store-lock store)
+   (lambda ()
+     (define lock-path (store-lock-path store))
+     (make-parent-directory* lock-path)
+     (call-with-file-lock/timeout
+      #f
+      'exclusive
+      thunk
+      (lambda ()
+        (raise-arguments-error
+         'settings-store
+         "another process owns the settings transaction"
+         "lock" lock-path))
+      #:lock-file lock-path
+      #:max-delay 2.0))))
+
+(define (refresh! store)
+  (define data (load-settings (settings-store-path store)))
+  (set-settings-store-data! store data)
+  data)
 
 (define (persist! store data)
   (define path (settings-store-path store))
@@ -54,25 +104,26 @@
   (with-store-lock
    store
    (lambda ()
-     (hash-ref (settings-store-data store) (normalize-key 'settings-ref key) default))))
+     (hash-ref (refresh! store) (normalize-key 'settings-ref key) default))))
 
 (define (settings-set! store key value)
   (with-store-lock
    store
    (lambda ()
+     (define safe-value (immutable-settings-value value))
      (persist! store
-               (hash-set (settings-store-data store)
+               (hash-set (refresh! store)
                          (normalize-key 'settings-set! key)
-                         value))
-     value)))
+                         safe-value))
+     safe-value)))
 
 (define (settings-remove! store key)
   (with-store-lock
    store
    (lambda ()
      (persist! store
-               (hash-remove (settings-store-data store)
+               (hash-remove (refresh! store)
                             (normalize-key 'settings-remove! key))))))
 
 (define (settings-snapshot store)
-  (with-store-lock store (lambda () (settings-store-data store))))
+  (with-store-lock store (lambda () (refresh! store))))

tests/public-api.rkt

@@ -48,11 +48,9 @@
       current-system-adapter install-crash-hook! make-settings-store
       register-activation-handler! rivet-log secure-store-ref
       secure-store-remove! secure-store-set! set-autostart!
-      set-settings-store-data! set-settings-store-lock!
-      set-settings-store-path! set-tray-menu! settings-ref settings-remove!
-      settings-set! settings-snapshot settings-store settings-store-data
-      settings-store-lock settings-store-path settings-store?
-      show-system-notification! struct:settings-store struct:system-adapter
+      set-tray-menu! settings-ref settings-remove! settings-set!
+      settings-snapshot settings-store? show-system-notification!
+      struct:system-adapter
       system-adapter system-adapter-acquire-single-instance
       system-adapter-autostart-enabled system-adapter-capabilities
       system-adapter-install-crash-hook system-adapter-name

tests/system-services.rkt

@@ -14,11 +14,32 @@
     (check-equal? (settings-ref store 'theme "system") "system")
     (settings-set! store 'theme "dark")
     (check-equal? (settings-ref store "theme") "dark")
+    (check-true (settings-store? store))
+    (check-true (immutable? (settings-snapshot store)))
     (define reloaded (make-settings-store settings-path))
     (check-equal? (settings-ref reloaded 'theme) "dark")
+    ;; Two handles refresh under their shared transaction lock instead of
+    ;; overwriting one another from stale construction-time caches.
+    (settings-set! store 'first 1)
+    (settings-set! reloaded 'second 2)
+    (check-equal? (settings-ref store 'second) 2)
+    (check-equal? (settings-ref reloaded 'first) 1)
+    ;; Caller-owned mutable values are copied into an immutable JSON tree.
+    (define mutable-text (string-copy "before"))
+    (define mutable-nested (make-hasheq `((text . ,mutable-text))))
+    (settings-set! store 'nested mutable-nested)
+    (string-set! mutable-text 0 #\X)
+    (hash-set! mutable-nested 'extra #t)
+    (define nested (settings-ref store 'nested))
+    (check-true (immutable? nested))
+    (check-true (immutable? (hash-ref nested 'text)))
+    (check-equal? nested (hasheq 'text "before"))
     (settings-remove! reloaded 'theme)
     (check-false (settings-ref reloaded 'theme #f)))
-  (lambda () (when (file-exists? settings-path) (delete-file settings-path))))
+  (lambda ()
+    (when (file-exists? settings-path) (delete-file settings-path))
+    (define lock-path (path-add-extension settings-path #".lock"))
+    (when (file-exists? lock-path) (delete-file lock-path))))
 
 (define secrets (make-hash))
 (define notifications '())