fix(review): Settings — типизированный SettingValue + лог битого JSON + Error::Failed + чистый reset

- SettingValue{Str,Int,Bool} в сторе: провод-variant теперь честно поддержан (не string-only).
- битый settings.json → WARN в лог (не молча) + откат к дефолтам.
- отказ записи /data → Error::Failed (не InvalidArgument).
- reset ключа без дефолта → no-op + InvalidArgument (без ложного Changed("")).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Alexander <akotenev2003@gmail.com>

Cargo.lock

@@ -3606,6 +3606,7 @@ version = "0.0.0"
 dependencies = [
  "anyhow",
  "futures-util",
+ "serde",
  "serde_json",
  "shturman-common",
  "shturman-ipc",

crates/core/shturman-settings/Cargo.toml

@@ -9,6 +9,7 @@ shturman-ipc = { path = "../../shturman-ipc" }
 shturman-common = { path = "../../shturman-common" }
 zbus.workspace = true
 tokio.workspace = true
+serde.workspace = true
 serde_json.workspace = true
 anyhow.workspace = true
 tracing.workspace = true

crates/core/shturman-settings/src/service.rs

@@ -1,6 +1,7 @@
-//! `ru.shturman.Settings1` — server-стаб. Хранит строки (v0); namespace-изоляция по `sender` — позже (v3).
+//! `ru.shturman.Settings1` — server-стаб. Значения типизированы (`SettingValue`); конверсия в/из
+//! D-Bus variant — один адаптер на границе (ниже). namespace-изоляция по `sender` — позже (v3).
 
-use crate::store::Store;
+use crate::store::{SettingValue, Store};
 use shturman_ipc::Error;
 use std::sync::Arc;
 use tokio::sync::Mutex;
@@ -20,13 +21,40 @@ impl SettingsService {
     }
 }
 
+/// `SettingValue` → D-Bus variant (для `Get`/`Changed`).
+fn to_value(sv: &SettingValue) -> Value<'static> {
+    match sv {
+        SettingValue::Str(s) => Value::from(s.clone()),
+        SettingValue::Bool(b) => Value::from(*b),
+        SettingValue::Int(i) => Value::from(*i),
+    }
+}
+
+/// D-Bus variant → `SettingValue` (для `Set`). v0: str/int/bool; иначе — `InvalidArgument`.
+fn from_value(v: &Value<'_>) -> Result<SettingValue, Error> {
+    match v {
+        Value::Str(s) => Ok(SettingValue::Str(s.to_string())),
+        Value::Bool(b) => Ok(SettingValue::Bool(*b)),
+        Value::I64(i) => Ok(SettingValue::Int(*i)),
+        Value::I32(i) => Ok(SettingValue::Int(*i as i64)),
+        Value::I16(i) => Ok(SettingValue::Int(*i as i64)),
+        Value::U32(u) => Ok(SettingValue::Int(*u as i64)),
+        Value::U16(u) => Ok(SettingValue::Int(*u as i64)),
+        Value::U8(u) => Ok(SettingValue::Int(*u as i64)),
+        other => Err(Error::InvalidArgument(format!(
+            "неподдерживаемый тип значения (v0: str/int/bool): {}",
+            other.value_signature()
+        ))),
+    }
+}
+
 #[interface(name = "ru.shturman.Settings1")]
 impl SettingsService {
     async fn get(&self, key: &str) -> Result<OwnedValue, Error> {
         let store = self.store.lock().await;
         match store.get(key) {
-            Some(v) => OwnedValue::try_from(Value::from(v.to_string()))
-                .map_err(|e| Error::InvalidArgument(format!("value convert: {e}"))),
+            Some(sv) => OwnedValue::try_from(to_value(sv))
+                .map_err(|e| Error::Failed(format!("value convert: {e}"))),
             None => Err(Error::InvalidArgument(format!("unknown key: {key}"))),
         }
     }
@@ -37,20 +65,14 @@ impl SettingsService {
         value: Value<'_>,
         #[zbus(signal_context)] ctx: SignalContext<'_>,
     ) -> Result<(), Error> {
-        let s = match value {
-            Value::Str(s) => s.as_str().to_string(),
-            _ => {
-                return Err(Error::InvalidArgument(
-                    "v0 поддерживает только строковые значения".into(),
-                ))
-            }
-        };
+        let sv = from_value(&value)?;
         self.store
             .lock()
             .await
-            .set(&key, &s)
-            .map_err(|e| Error::InvalidArgument(format!("persist: {e}")))?;
-        let _ = Self::changed(&ctx, &key, &Value::from(s)).await;
+            .set(&key, sv.clone())
+            // отказ записи в /data — внутренний сбой, НЕ InvalidArgument (плохой ввод)
+            .map_err(|e| Error::Failed(format!("persist: {e}")))?;
+        let _ = Self::changed(&ctx, &key, &to_value(&sv)).await;
         Ok(())
     }
 
@@ -68,9 +90,17 @@ impl SettingsService {
             .lock()
             .await
             .reset(&key)
-            .map_err(|e| Error::InvalidArgument(format!("persist: {e}")))?;
-        let _ = Self::changed(&ctx, &key, &Value::from(new.unwrap_or_default())).await;
-        Ok(())
+            .map_err(|e| Error::Failed(format!("persist: {e}")))?;
+        match new {
+            Some(sv) => {
+                let _ = Self::changed(&ctx, &key, &to_value(&sv)).await;
+                Ok(())
+            }
+            // нет дефолта — reset неприменим; не эмитим ложный Changed("")
+            None => Err(Error::InvalidArgument(format!(
+                "reset: у ключа '{key}' нет дефолта"
+            ))),
+        }
     }
 
     #[zbus(signal)]

crates/core/shturman-settings/src/store.rs

@@ -1,39 +1,65 @@
-//! Стор настроек: `/data/settings/settings.json` (строковые значения в v0), durable-write.
-//! Единственный писатель поддерева `/data/settings/` (architecture §3).
+//! Стор настроек: `/data/settings/settings.json`, durable-write. Единственный писатель
+//! поддерева `/data/settings/` (architecture §3). Значения типизированы (`SettingValue`) —
+//! провод D-Bus отдаёт variant, и стор честно его поддерживает (str/int/bool).
 
+use serde::{Deserialize, Serialize};
 use shturman_common::{write_atomic, Layout};
 use std::collections::BTreeMap;
 use std::io;
 
-/// Дефолты v0 (data-model §2: канон единиц; конвертация — на презентации).
-const DEFAULTS: &[(&str, &str)] = &[("ui.theme", "auto"), ("ui.units", "metric")];
+/// Значение настройки. На проводе — D-Bus variant; на диске — нативный JSON (untagged).
+#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
+#[serde(untagged)]
+pub enum SettingValue {
+    Bool(bool),
+    Int(i64),
+    Str(String),
+}
 
-fn defaults_map() -> BTreeMap<String, String> {
-    DEFAULTS
+fn defaults_map() -> BTreeMap<String, SettingValue> {
+    [("ui.theme", "auto"), ("ui.units", "metric")]
         .iter()
-        .map(|(k, v)| (k.to_string(), v.to_string()))
+        .map(|(k, v)| (k.to_string(), SettingValue::Str(v.to_string())))
         .collect()
 }
 
-fn serialize(map: &BTreeMap<String, String>) -> String {
+/// Дефолт ключа (если есть) — единый источник для seed и `reset` (data-model §2: канон единиц).
+fn default_for(key: &str) -> Option<SettingValue> {
+    match key {
+        "ui.theme" => Some(SettingValue::Str("auto".into())),
+        "ui.units" => Some(SettingValue::Str("metric".into())),
+        _ => None,
+    }
+}
+
+fn serialize(map: &BTreeMap<String, SettingValue>) -> String {
     serde_json::to_string_pretty(map).unwrap_or_else(|_| "{}".to_string())
 }
 
-/// Key-value стор настроек (строки v0).
+/// Key-value стор настроек.
 pub struct Store {
     layout: Layout,
-    map: BTreeMap<String, String>,
+    map: BTreeMap<String, SettingValue>,
 }
 
 impl Store {
     /// Загрузить из `settings.json`; если файла нет — посеять дефолты (durable-write).
-    /// Битый JSON → дефолты в памяти (перезапишутся при следующем `set`).
+    /// Битый JSON → дефолты в памяти + **WARN в лог** (не молча); перезапишутся при следующем `set`.
     pub fn load_or_seed(layout: Layout) -> io::Result<Self> {
         std::fs::create_dir_all(layout.settings())?;
         let path = layout.settings().join("settings.json");
         let map = if path.exists() {
-            serde_json::from_str(&std::fs::read_to_string(&path)?)
-                .unwrap_or_else(|_| defaults_map())
+            let raw = std::fs::read_to_string(&path)?;
+            match serde_json::from_str(&raw) {
+                Ok(m) => m,
+                Err(e) => {
+                    tracing::warn!(
+                        error = %e, path = %path.display(),
+                        "битый settings.json — откат к дефолтам (сохранится при следующем set)"
+                    );
+                    defaults_map()
+                }
+            }
         } else {
             let d = defaults_map();
             write_atomic(&path, serialize(&d).as_bytes())?;
@@ -42,31 +68,26 @@ impl Store {
         Ok(Self { layout, map })
     }
 
-    pub fn get(&self, key: &str) -> Option<&str> {
-        self.map.get(key).map(|s| s.as_str())
+    pub fn get(&self, key: &str) -> Option<&SettingValue> {
+        self.map.get(key)
     }
 
-    pub fn set(&mut self, key: &str, value: &str) -> io::Result<()> {
-        self.map.insert(key.to_string(), value.to_string());
+    pub fn set(&mut self, key: &str, value: SettingValue) -> io::Result<()> {
+        self.map.insert(key.to_string(), value);
         self.persist()
     }
 
-    /// Сбросить ключ к дефолту (если есть) или удалить. Возвращает значение после сброса (для `Changed`).
-    pub fn reset(&mut self, key: &str) -> io::Result<Option<String>> {
-        let default = DEFAULTS
-            .iter()
-            .find(|(k, _)| *k == key)
-            .map(|(_, v)| v.to_string());
-        match &default {
+    /// Сбросить к дефолту. `Some(default)` — у ключа есть дефолт (восстановлен + persist);
+    /// `None` — дефолта нет (reset неприменим, стор не трогаем — без ложного `Changed("")`).
+    pub fn reset(&mut self, key: &str) -> io::Result<Option<SettingValue>> {
+        match default_for(key) {
             Some(v) => {
                 self.map.insert(key.to_string(), v.clone());
+                self.persist()?;
+                Ok(Some(v))
             }
-            None => {
-                self.map.remove(key);
-            }
+            None => Ok(None),
         }
-        self.persist()?;
-        Ok(self.map.get(key).cloned())
     }
 
     pub fn list(&self, prefix: &str) -> Vec<String> {
@@ -94,12 +115,16 @@ mod tests {
         (d, l)
     }
 
+    fn s(v: &str) -> SettingValue {
+        SettingValue::Str(v.to_string())
+    }
+
     #[test]
     fn seeds_defaults_when_empty() {
         let (_d, l) = layout();
-        let s = Store::load_or_seed(l.clone()).unwrap();
-        assert_eq!(s.get("ui.theme"), Some("auto"));
-        assert_eq!(s.get("ui.units"), Some("metric"));
+        let st = Store::load_or_seed(l.clone()).unwrap();
+        assert_eq!(st.get("ui.theme"), Some(&s("auto")));
+        assert_eq!(st.get("ui.units"), Some(&s("metric")));
         assert!(l.settings().join("settings.json").exists());
     }
 
@@ -107,28 +132,39 @@ mod tests {
     fn set_get_persists_across_reload() {
         let (_d, l) = layout();
         {
-            let mut s = Store::load_or_seed(l.clone()).unwrap();
-            s.set("ui.theme", "night").unwrap();
+            let mut st = Store::load_or_seed(l.clone()).unwrap();
+            st.set("ui.theme", s("night")).unwrap();
+            st.set("ui.brightness", SettingValue::Int(80)).unwrap();
         }
-        let s2 = Store::load_or_seed(l.clone()).unwrap();
-        assert_eq!(s2.get("ui.theme"), Some("night"));
+        let st2 = Store::load_or_seed(l.clone()).unwrap();
+        assert_eq!(st2.get("ui.theme"), Some(&s("night")));
+        assert_eq!(st2.get("ui.brightness"), Some(&SettingValue::Int(80)));
     }
 
     #[test]
     fn reset_restores_default() {
         let (_d, l) = layout();
-        let mut s = Store::load_or_seed(l).unwrap();
-        s.set("ui.theme", "night").unwrap();
-        s.reset("ui.theme").unwrap();
-        assert_eq!(s.get("ui.theme"), Some("auto"));
+        let mut st = Store::load_or_seed(l).unwrap();
+        st.set("ui.theme", s("night")).unwrap();
+        assert_eq!(st.reset("ui.theme").unwrap(), Some(s("auto")));
+        assert_eq!(st.get("ui.theme"), Some(&s("auto")));
+    }
+
+    #[test]
+    fn reset_unknown_key_is_noop() {
+        let (_d, l) = layout();
+        let mut st = Store::load_or_seed(l).unwrap();
+        st.set("net.proxy", s("x")).unwrap();
+        assert_eq!(st.reset("net.proxy").unwrap(), None); // нет дефолта — no-op
+        assert_eq!(st.get("net.proxy"), Some(&s("x"))); // значение не тронуто
     }
 
     #[test]
     fn list_by_prefix() {
         let (_d, l) = layout();
-        let mut s = Store::load_or_seed(l).unwrap();
-        s.set("net.proxy", "x").unwrap();
-        let ui = s.list("ui.");
+        let mut st = Store::load_or_seed(l).unwrap();
+        st.set("net.proxy", s("x")).unwrap();
+        let ui = st.list("ui.");
         assert!(ui.contains(&"ui.theme".to_string()));
         assert!(!ui.contains(&"net.proxy".to_string()));
     }
@@ -136,7 +172,7 @@ mod tests {
     #[test]
     fn unknown_key_is_none() {
         let (_d, l) = layout();
-        let s = Store::load_or_seed(l).unwrap();
-        assert_eq!(s.get("nope"), None);
+        let st = Store::load_or_seed(l).unwrap();
+        assert_eq!(st.get("nope"), None);
     }
 }

crates/shturman-ipc/src/error.rs

@@ -17,4 +17,7 @@ pub enum Error {
     ReadOnly(String),
     InvalidArgument(String),
     Unsupported(String),
+    /// Внутренний сбой сервиса (напр. отказ записи в `/data`) — НЕ путать с `InvalidArgument`
+    /// (плохой ввод клиента). Аддитивно к набору ipc §2 (синхронизировать ipc.md при реализации).
+    Failed(String),
 }