fix: config commit blanket-clears dirty flags for unsent sections

apps/web/src/sdk-preview/features/config/domain/ConfigEditor.ts

@@ -18,6 +18,8 @@ import {
   buildModuleConfig,
   buildRadioConfig,
 } from "../infrastructure/configBuilders.ts";
+import { configValuesEqual } from "./configEquality.ts";
+import { mergeStagedValue } from "./configMerge.ts";
 import { ConfigMapper } from "../infrastructure/ConfigMapper.ts";
 import type { ModuleConfig, ModuleConfigSection } from "./ModuleConfig.ts";
 import type { RadioConfig, RadioConfigSection } from "./RadioConfig.ts";
@@ -119,15 +121,23 @@ export class ConfigEditor {
     key: K,
     value: RadioConfig[K],
   ): void {
-    this.workingRadio.value = { ...this.workingRadio.peek(), [key]: value };
+    // Forms stage the output of their Zod resolver, which drops every field
+    // the form does not declare. Merge over the device's value so those
+    // fields are not silently reset to their protobuf defaults on commit.
+    const merged = mergeStagedValue(this.baselineRadio.peek()[key], value);
+    this.workingRadio.value = { ...this.workingRadio.peek(), [key]: merged };
     this.recomputeDirty();
   }
 
   public setModuleSection<K extends ModuleConfigSection & string>(
     key: K,
     value: ModuleConfig[K],
   ): void {
-    this.workingModules.value = { ...this.workingModules.peek(), [key]: value };
+    const merged = mergeStagedValue(this.baselineModules.peek()[key], value);
+    this.workingModules.value = {
+      ...this.workingModules.peek(),
+      [key]: merged,
+    };
     this.recomputeDirty();
   }
 
@@ -140,26 +150,54 @@ export class ConfigEditor {
 
   /**
    * Send every dirty section to the device inside a beginEdit/commitEdit pair.
-   * On success the baseline is replaced with the working copy (optimistic);
-   * inbound config packets after commit reconcile. Any failure aborts and
-   * returns the error — the baseline is left untouched.
+   *
+   * The payload is frozen synchronously before the first `await`, and on
+   * success only the sections that were actually transmitted are promoted into
+   * the baseline. Edits staged while the transaction was in flight stay dirty
+   * and go out on the next commit instead of being silently marked as saved.
+   * Any failure aborts and returns the error — the baseline is left untouched.
    */
   public async commit(): Promise<ResultType<void, MeshError>> {
     if (!this._isDirty.peek()) {
       return Result.ok(undefined);
     }
 
-    const begin = await beginEditSettings(this.client);
-    if (Result.isError(begin)) {
-      return Result.err(begin.error);
-    }
-
     const radio = this.workingRadio.peek();
+    const radioPayload: Array<
+      [RadioConfigSection, NonNullable<RadioConfig[RadioConfigSection]>]
+    > = [];
     for (const section of this._dirtyRadioSections.peek()) {
       const value = radio[section];
       if (value === undefined) {
         continue;
       }
+      radioPayload.push([section, value]);
+    }
+
+    const modules = this.workingModules.peek();
+    const modulePayload: Array<
+      [ModuleConfigSection, NonNullable<ModuleConfig[ModuleConfigSection]>]
+    > = [];
+    for (const section of this._dirtyModuleSections.peek()) {
+      const value = modules[section];
+      if (value === undefined) {
+        continue;
+      }
+      modulePayload.push([section, value]);
+    }
+
+    if (radioPayload.length === 0 && modulePayload.length === 0) {
+      // An empty begin/commit pair makes the device rewrite its config
+      // unchanged and report success — a "save" that saved nothing.
+      return Result.ok(undefined);
+    }
+
+    const begin = await beginEditSettings(this.client);
+    if (Result.isError(begin)) {
+      return Result.err(begin.error);
+    }
+
+    for (const [section, value] of radioPayload) {
       const result = await setConfig(
         this.client,
         buildRadioConfig(section, value),
@@ -169,12 +207,7 @@ export class ConfigEditor {
       }
     }
 
-    const modules = this.workingModules.peek();
-    for (const section of this._dirtyModuleSections.peek()) {
-      const value = modules[section];
-      if (value === undefined) {
-        continue;
-      }
+    for (const [section, value] of modulePayload) {
       const result = await setModuleConfig(
         this.client,
         buildModuleConfig(section, value),
@@ -189,11 +222,37 @@ export class ConfigEditor {
       return Result.err(commit.error);
     }
 
-    this.baselineRadio.value = this.workingRadio.peek();
-    this.baselineModules.value = this.workingModules.peek();
-    this._dirtyRadioSections.value = [];
-    this._dirtyModuleSections.value = [];
-    this._isDirty.value = false;
+    // Promote only what went on the wire, and only where the working copy is
+    // still the exact object we transmitted (setters always replace the whole
+    // section, so reference identity is an exact "untouched since" test).
+    if (radioPayload.length > 0) {
+      const current = this.workingRadio.peek();
+      const baseline: Record<string, unknown> = {
+        ...this.baselineRadio.peek(),
+      };
+      for (const [section, value] of radioPayload) {
+        if (current[section] === value) {
+          baseline[section] = value;
+        }
+      }
+      this.baselineRadio.value = baseline as RadioConfig;
+    }
+    if (modulePayload.length > 0) {
+      const current = this.workingModules.peek();
+      const baseline: Record<string, unknown> = {
+        ...this.baselineModules.peek(),
+      };
+      for (const [section, value] of modulePayload) {
+        if (current[section] === value) {
+          baseline[section] = value;
+        }
+      }
+      this.baselineModules.value = baseline as ModuleConfig;
+    }
+
+    // Recompute instead of blanket-clearing, so anything staged mid-flight
+    // stays flagged as pending.
+    this.recomputeDirty();
     return Result.ok(undefined);
   }
 
@@ -206,7 +265,7 @@ export class ConfigEditor {
       ...Object.keys(radioWorking),
     ])) {
       const section = key as RadioConfigSection;
-      if (!shallowEqual(radioBase[section], radioWorking[section])) {
+      if (!configValuesEqual(radioBase[section], radioWorking[section])) {
         radioDirty.push(section);
       }
     }
@@ -219,7 +278,7 @@ export class ConfigEditor {
       ...Object.keys(moduleWorking),
     ])) {
       const section = key as ModuleConfigSection;
-      if (!shallowEqual(moduleBase[section], moduleWorking[section])) {
+      if (!configValuesEqual(moduleBase[section], moduleWorking[section])) {
         moduleDirty.push(section);
       }
     }
@@ -229,38 +288,3 @@ export class ConfigEditor {
     this._isDirty.value = radioDirty.length > 0 || moduleDirty.length > 0;
   }
 }
-
-/** Recursive value-equality used for dirty detection (matches the SDK helper). */
-function shallowEqual(a: unknown, b: unknown): boolean {
-  if (a === b) {
-    return true;
-  }
-  if (a === undefined || b === undefined || a === null || b === null) {
-    return false;
-  }
-  if (typeof a !== "object" || typeof b !== "object") {
-    return false;
-  }
-  const ao = a as Record<string, unknown>;
-  const bo = b as Record<string, unknown>;
-  const aKeys = Object.keys(ao);
-  const bKeys = Object.keys(bo);
-  if (aKeys.length !== bKeys.length) {
-    return false;
-  }
-  for (const k of aKeys) {
-    const av = ao[k];
-    const bv = bo[k];
-    if (av === bv) {
-      continue;
-    }
-    if (typeof av === "object" && typeof bv === "object") {
-      if (!shallowEqual(av, bv)) {
-        return false;
-      }
-    } else {
-      return false;
-    }
-  }
-  return true;
-}

apps/web/src/sdk-preview/features/config/domain/configEquality.ts

@@ -0,0 +1,140 @@
+/**
+ * Value-equality used by {@link ConfigEditor} (mirror of the `@meshtastic/sdk` helper) to decide whether a config
+ * section still matches the device's baseline.
+ *
+ * This has to compare two things that are *never* structurally identical even
+ * when they mean exactly the same thing:
+ *
+ * - the **baseline**, which is a `@bufbuild/protobuf` message: it carries a
+ *   `$typeName` marker and every singular scalar field is materialised as an
+ *   own property holding its zero value (`false` / `0` / `""`), and
+ * - the **working copy**, which is whatever the UI staged. Web forms hand over
+ *   plain objects produced by a Zod resolver, so they have no `$typeName`, and
+ *   fields the form does not render are simply absent.
+ *
+ * A naive key-count/`Object.keys(a)` comparison therefore reports "changed"
+ * forever, which pins the working copy (the editor refuses to let inbound
+ * device config overwrite a section it believes is dirty) and makes the UI
+ * report stale values as if they were saved. It also gets `undefined` vs
+ * `false` wrong in both directions.
+ *
+ * The rules implemented here mirror protobuf's own semantics:
+ *
+ * - `$typeName` is metadata, not data.
+ * - Keys are compared over the *union* of both sides, so a field that only one
+ *   side carries is never skipped.
+ * - An absent field is equal to that field's protobuf default
+ *   (`undefined` == `false` / `0` / `0n` / `""` / empty bytes / empty list),
+ *   and *only* to its default — an absent field is never equal to `true`.
+ * - An absent sub-message is equal to a sub-message whose fields are all
+ *   defaults.
+ * - `Uint8Array` is compared byte-wise, repeated fields element-wise.
+ */
+
+function isAbsent(value: unknown): boolean {
+  return value === undefined || value === null;
+}
+
+/** True when `value` is the protobuf zero value for its type (or absent). */
+function isProtobufDefault(value: unknown): boolean {
+  if (isAbsent(value)) {
+    return true;
+  }
+  if (value instanceof Uint8Array) {
+    return value.byteLength === 0;
+  }
+  if (Array.isArray(value)) {
+    return value.length === 0;
+  }
+  if (typeof value === "object") {
+    return Object.entries(value as Record<string, unknown>).every(
+      ([key, entry]) => key === "$typeName" || isProtobufDefault(entry),
+    );
+  }
+  return value === false || value === 0 || value === 0n || value === "";
+}
+
+function bytesEqual(a: Uint8Array, b: Uint8Array): boolean {
+  if (a.byteLength !== b.byteLength) {
+    return false;
+  }
+  for (let i = 0; i < a.byteLength; i++) {
+    if (a[i] !== b[i]) {
+      return false;
+    }
+  }
+  return true;
+}
+
+function isRecord(value: unknown): value is Record<string, unknown> {
+  return (
+    typeof value === "object" &&
+    value !== null &&
+    !Array.isArray(value) &&
+    !(value instanceof Uint8Array)
+  );
+}
+
+const EMPTY_RECORD: Record<string, unknown> = {};
+
+export function configValuesEqual(a: unknown, b: unknown): boolean {
+  if (a === b) {
+    return true;
+  }
+
+  if (a instanceof Uint8Array || b instanceof Uint8Array) {
+    if (a instanceof Uint8Array && b instanceof Uint8Array) {
+      return bytesEqual(a, b);
+    }
+    // One side is absent: equal only if the present side is empty bytes.
+    const present = a instanceof Uint8Array ? a : (b as Uint8Array);
+    const other = a instanceof Uint8Array ? b : a;
+    return present.byteLength === 0 && isAbsent(other);
+  }
+
+  if (Array.isArray(a) || Array.isArray(b)) {
+    if (!Array.isArray(a) && !isAbsent(a)) {
+      return false;
+    }
+    if (!Array.isArray(b) && !isAbsent(b)) {
+      return false;
+    }
+    const left = Array.isArray(a) ? a : [];
+    const right = Array.isArray(b) ? b : [];
+    if (left.length !== right.length) {
+      return false;
+    }
+    return left.every((value, index) => configValuesEqual(value, right[index]));
+  }
+
+  if (isRecord(a) || isRecord(b)) {
+    if (!isRecord(a) && !isAbsent(a)) {
+      return false;
+    }
+    if (!isRecord(b) && !isAbsent(b)) {
+      return false;
+    }
+    const left = isRecord(a) ? a : EMPTY_RECORD;
+    const right = isRecord(b) ? b : EMPTY_RECORD;
+    const keys = new Set([...Object.keys(left), ...Object.keys(right)]);
+    for (const key of keys) {
+      if (key === "$typeName") {
+        continue;
+      }
+      if (!configValuesEqual(left[key], right[key])) {
+        return false;
+      }
+    }
+    return true;
+  }
+
+  // Scalars. An absent field carries its protobuf default, so `undefined`
+  // equals `false`/`0`/`""` — but never `true`, `1`, or a non-empty string.
+  if (isAbsent(a)) {
+    return isProtobufDefault(b);
+  }
+  if (isAbsent(b)) {
+    return isProtobufDefault(a);
+  }
+  return false;
+}