fix(#1498, #1496): bind value-shaped controls, and decode the change event instead of the node

.github/workflows/clients.yml

@@ -128,7 +128,7 @@ jobs:
       - run: pytest -q
 
   node-sdk:
-    name: "@meshweaver/client (typecheck)"
+    name: "@meshweaver/client (typecheck + test)"
     runs-on: ubuntu-latest
     defaults:
       run:
@@ -142,7 +142,12 @@ jobs:
           node-version: 22
       - run: npm ci --no-audit --no-fund
       - run: npm run typecheck
-      - run: npm test # node --test: the js/ts Code-node execution core (type stripping → vm → REPL value)
+      # 🚨 The tests MUST run here (issue #1496). This package defined `test` and shipped
+      # test/worker.test.mjs, but the job ran `typecheck` ONLY — so the SDK carried a decode that
+      # returned an empty node for every subscription, and nothing in CI could have noticed. A test
+      # nobody runs is documentation, and the defect it would have caught silently dropped thread
+      # message ids in production.
+      - run: npm test # node --test: the js/ts Code-node execution core (type stripping → vm → REPL value) + the change-event decode
 
   grpc-web:
     name: "@meshweaver/client-web (typecheck + test)"

clients/grpc-web/src/changeFold.test.ts

@@ -0,0 +1,87 @@
+import { describe, expect, it } from "vitest";
+import { foldChange } from "./changeFold";
+import { applyJsonPatch } from "./jsonPatch";
+
+// THE DECODE GUARD (issue #1496).
+//
+// A subscription delivers `DataChangedEvent { changeType, change, streamId }` — NOT a node. The
+// Node SDK yielded `meshNodeFromChange(delivery.message)`, i.e. it decoded the EVENT as the NODE,
+// and because `meshNodeFromChange` looks up four keys the event does not have, every emission was
+// `{ path: undefined, name: undefined, nodeType: undefined, content: {} }`. Nothing threw:
+//   * `get()` resolved to "the node exists but is empty" rather than erroring, and
+//   * `submitMessage` read `content["userMessageIds"]` off that, saw `[]`, and sent a patch that
+//     REPLACED the thread's id list with one id — dropping every earlier message (RFC 7396 arrays).
+//
+// It survived because `clients/typescript` had no test for `Mesh` at all and its CI job ran only
+// `npm run typecheck`. Both halves are fixed: the fold is shared and mirrored, and the node-sdk job
+// now runs `npm test`.
+
+const node = (name: string, extra: Record<string, unknown> = {}) => ({
+  path: "acme/Thing",
+  name,
+  nodeType: "Markdown",
+  content: { title: name, userMessageIds: ["m1", "m2"] },
+  ...extra,
+});
+
+describe("foldChange decodes the DataChangedEvent, not the event as the node", () => {
+  it("Full replaces the state with the node the change carries", () => {
+    const out = foldChange({ changeType: "Full", change: node("first"), streamId: "s" }, null, applyJsonPatch);
+
+    expect(out).not.toBeNull();
+    expect(out!["path"]).toBe("acme/Thing");
+    expect(out!["name"]).toBe("first");
+    // The regression in one line: decoding the EVENT would leave content empty.
+    expect((out!["content"] as Record<string, unknown>)["userMessageIds"]).toEqual(["m1", "m2"]);
+  });
+
+  it("accepts a change delivered as a JSON STRING (RawJson on the wire)", () => {
+    const out = foldChange({ changeType: "Full", change: JSON.stringify(node("stringy")) }, null, applyJsonPatch);
+
+    expect(out!["name"]).toBe("stringy");
+  });
+
+  it("PascalCase members decode identically", () => {
+    const out = foldChange({ ChangeType: "Full", Change: node("pascal") }, null, applyJsonPatch);
+
+    expect(out!["name"]).toBe("pascal");
+  });
+
+  it("Patch folds an RFC 6902 array onto the state so far, at the node root", () => {
+    const first = foldChange({ changeType: "Full", change: node("first") }, null, applyJsonPatch);
+    const out = foldChange(
+      { changeType: "Patch", change: [{ op: "replace", path: "/name", value: "second" }] },
+      first,
+      applyJsonPatch,
+    );
+
+    expect(out!["name"]).toBe("second");
+    // The rest of the node survives the patch — a fold, not a replacement.
+    expect(out!["path"]).toBe("acme/Thing");
+  });
+
+  it("the numeric ChangeType ordinal for Patch folds too", () => {
+    const first = foldChange({ changeType: "Full", change: node("first") }, null, applyJsonPatch);
+    const out = foldChange(
+      { changeType: "1", change: [{ op: "replace", path: "/name", value: "n" }] },
+      first,
+      applyJsonPatch,
+    );
+
+    expect(out!["name"]).toBe("n");
+  });
+
+  it("NoUpdate emits nothing", () => {
+    expect(foldChange({ changeType: "NoUpdate", change: {} }, node("prev"), applyJsonPatch)).toBeNull();
+  });
+
+  it("a null change emits nothing", () => {
+    expect(foldChange({ changeType: "Full", change: null }, null, applyJsonPatch)).toBeNull();
+  });
+
+  it("a message with no `change` member is taken as the node itself (test transports)", () => {
+    const out = foldChange(node("flat"), null, applyJsonPatch);
+
+    expect(out!["name"]).toBe("flat");
+  });
+});

clients/grpc-web/src/changeFold.ts

@@ -0,0 +1,81 @@
+// THE CHANGE FOLD — how either TypeScript SDK turns a DataChangedEvent into the node's state.
+//
+// A subscription does NOT deliver nodes. It delivers `DataChangedEvent { changeType, change,
+// streamId }`, where `change` is the payload and `changeType` says how to apply it:
+//   * "Full"  — `change` IS the whole node JSON; it replaces the accumulated state.
+//   * "Patch" — `change` is an RFC 6902 patch ARRAY applied at the NODE ROOT
+//               (JsonSynchronizationStream.CreateSingleObjectPatch), folded onto the state so far.
+//   * "NoUpdate" — nothing changed; emit nothing.
+// `change` may arrive as a JSON STRING (RawJson on the wire) rather than an object.
+//
+// 🚨 Why this is a shared module and not two implementations. The Node SDK used to yield
+// `meshNodeFromChange(delivery.message)` — i.e. it treated the EVENT as the node. `DataChangedEvent`
+// has none of `path` / `name` / `nodeType` / `content`, so every emission was
+// `{ path: undefined, name: undefined, nodeType: undefined, content: {} }` and every consequence was
+// silent (issue #1496): `get()` resolved to "the node exists but is empty" rather than erroring, and
+// `submitMessage` read `content["userMessageIds"]` off that value, saw `[]`, and sent a patch that
+// REPLACED the thread's id list with a single id — dropping every earlier message under RFC 7396
+// array semantics. The browser twin had the fold right the whole time; only the copy without a test
+// was wrong.
+//
+// So, exactly like wire.ts: this file is import-free and pure, and it is mirrored BYTE-IDENTICALLY
+// into the OTHER TypeScript SDK's copy of the same path. (Side-neutral on purpose — the two copies
+// are byte-equal, so a sentence naming one package's path is wrong when read in the other.)
+// wireContract.test.ts asserts they are equal, so a fix can never again land in one SDK only.
+//
+// 🚨 IMPORT-FREE IS LOAD-BEARING, not tidiness. The two SDKs resolve modules differently — the Node
+// SDK is NodeNext (a `.js` specifier is REQUIRED) while grpc-web is bundled — and grpc-web's source
+// is additionally bundled by METRO for the React Native web export, which does not remap `.js` to
+// `.ts` at all. A single import here therefore has no spelling that works everywhere: `./jsonPatch`
+// breaks the Node SDK's build and `./jsonPatch.js` breaks the RN bundle ("Unable to resolve module
+// ./changeFold.js"). Taking the patch applier as a PARAMETER removes the question — and it is the
+// same reason wire.ts is import-free.
+
+/** The accumulated node JSON a fold carries between emissions. */
+export type NodeState = Record<string, unknown>;
+
+/** One RFC 6902 operation, as the mesh's sync streams deliver them. */
+export interface PatchOperation {
+  op: string;
+  path: string;
+  value?: unknown;
+  from?: string;
+}
+
+/** Applies an RFC 6902 patch array at the node root — each SDK passes its own `applyJsonPatch`. */
+export type ApplyPatch = (state: unknown, ops: PatchOperation[]) => unknown;
+
+/**
+ * Fold one delivery message onto the state so far.
+ *
+ * @param message    the DataChangedEvent as received (camelCase or PascalCase members).
+ * @param previous   the node state accumulated from earlier deliveries, or null at stream start.
+ * @param applyPatch the SDK's RFC 6902 applier (see ApplyPatch — this module stays import-free).
+ * @returns the new node state, or null when the delivery carries no update (emit nothing).
+ */
+export function foldChange(
+  message: Record<string, unknown>,
+  previous: NodeState | null,
+  applyPatch: ApplyPatch,
+): NodeState | null {
+  // 🚨 ABSENT is not NULL. Probing with `message["change"] ?? message["Change"]` and testing the
+  // result for `undefined` collapses the two: a DataChangedEvent carrying an explicit null Change
+  // reads as "no change member at all" and the whole EVENT is then returned as the node — the same
+  // class of decode error this module exists to end. The membership test keeps them apart.
+  const hasChange = "change" in message || "Change" in message;
+
+  // No `change` member at all: the node's fields are flat on the message itself. That is what
+  // in-memory fakes and the legacy shapes produce, and it stays supported so a test transport does
+  // not have to model the event envelope.
+  if (!hasChange) return message as NodeState;
+
+  const rawChange = message["change"] ?? message["Change"];
+  const change = typeof rawChange === "string" ? (JSON.parse(rawChange) as unknown) : rawChange;
+  const changeType = String(message["changeType"] ?? message["ChangeType"] ?? "Full");
+  if (change == null || changeType === "NoUpdate") return null;
+
+  // "1" is ChangeType.Patch's ordinal — a serializer that writes the enum numerically still folds.
+  return changeType === "Patch" || changeType === "1"
+    ? (applyPatch(previous ?? {}, change as PatchOperation[]) as NodeState)
+    : (change as NodeState);
+}

clients/grpc-web/src/mesh.ts

@@ -6,7 +6,8 @@
 // underneath is already correct + tested.
 
 import { connect as connectTransport, MeshWebConnection, type ConnectOptions } from "./connection";
-import { applyJsonPatch, type PatchOperation } from "./jsonPatch";
+import { foldChange, type NodeState } from "./changeFold";
+import { applyJsonPatch } from "./jsonPatch";
 import { meshNodeFromChange, type MeshNode } from "./types";
 import { newId } from "./envelope";
 import {
@@ -131,22 +132,13 @@ export class Mesh {
   async *watch(path: string): AsyncIterableIterator<MeshNode> {
     const streamId = newId();
     const sub = subscribeRequest(path, streamId);
-    let node: Record<string, unknown> | null = null;
+    // The fold itself now lives in changeFold.ts, mirrored byte-identically into the Node SDK —
+    // that copy had the decode wrong end to end (#1496) and nothing compared the two.
+    let node: NodeState | null = null;
     for await (const delivery of this.conn.watch(path, streamId, sub.type, sub.message)) {
-      const m = delivery.message;
-      const rawChange = m["change"] ?? m["Change"];
-      if (rawChange === undefined) {
-        // Flat node fields on the message itself (in-memory fakes / legacy shapes).
-        yield meshNodeFromChange(m);
-        continue;
-      }
-      const change = typeof rawChange === "string" ? JSON.parse(rawChange) : rawChange;
-      const changeType = String(m["changeType"] ?? m["ChangeType"] ?? "Full");
-      if (change == null || changeType === "NoUpdate") continue;
-      node =
-        changeType === "Patch" || changeType === "1"
-          ? (applyJsonPatch(node ?? {}, change as PatchOperation[]) as Record<string, unknown>)
-          : (change as Record<string, unknown>);
+      const next = foldChange(delivery.message, node, applyJsonPatch);
+      if (next === null) continue;
+      node = next;
       yield meshNodeFromChange(node);
     }
   }

clients/grpc-web/src/wireContract.test.ts

@@ -163,6 +163,33 @@ describe("every field the SDK sends exists on the C# record it names", () => {
 const WEB_WIRE = "src/wire.ts";
 const NODE_WIRE = "../typescript/src/wire.ts";
 
+// Every file the two SDKs must carry ONE copy of. `wire.ts` is the original (#1475); `jsonPatch.ts`
+// and `changeFold.ts` joined it for #1496, where the Node SDK's own decode treated the
+// DataChangedEvent AS the node and no test compared it against the browser twin that had it right.
+const MIRRORED = [
+  ["src/wire.ts", "../typescript/src/wire.ts"],
+  ["src/jsonPatch.ts", "../typescript/src/jsonPatch.ts"],
+  ["src/changeFold.ts", "../typescript/src/changeFold.ts"],
+] as const;
+
+describe("the two TypeScript SDKs share ONE copy of every mirrored module", () => {
+  for (const [web, node] of MIRRORED) {
+    it(`${node} is byte-identical to ${web}`, () => {
+      const a: string[] = readFileSync(resolve(pkgRoot, web), "utf8").split("\n");
+      const b: string[] = readFileSync(resolve(pkgRoot, node), "utf8").split("\n");
+      const at = a.findIndex((line: string, i: number) => line !== b[i]);
+      expect(
+        at === -1 && a.length === b.length,
+        at === -1
+          ? `${node} has ${b.length} lines vs ${a.length} in ${web} — copy one over the other.`
+          : `${node} diverged from ${web} at line ${at + 1}:\n` +
+            `  grpc-web: ${a[at]}\n  node    : ${b[at] ?? "(missing)"}\n` +
+            `The two SDKs must carry ONE copy — copy one file over the other.`,
+      ).toBe(true);
+    });
+  }
+});
+
 describe("the two TypeScript SDKs share ONE copy of the wire shapes", () => {
   // #1475 shipped in both SDKs at once because each hand-mirrored the other. Byte-identical copies
   // (the discipline clients/react/src/i18n uses for the server string catalog) make a one-sided fix

clients/react-native/src/editors.test.tsx

@@ -39,9 +39,14 @@ const tree: AreaTree = {
     date: { $type: "Date", label: "When", data: ptr("/data/when") },
     dt: { $type: "DateTime", label: "When exactly", data: ptr("/data/when") },
     combo: { $type: "Combobox", label: "City", data: ptr("/data/city"), options: [{ value: "Zurich", text: "Zurich" }, { value: "Bern", text: "Bern" }] },
-    search: { $type: "SearchBox", data: ptr("/data/query"), placeholder: "Find" },
-    code: { $type: "CodeEditor", label: "Code", data: ptr("/data/code") },
-    mdedit: { $type: "MarkdownEditor", label: "Notes", data: ptr("/data/md") },
+    search: { $type: "SearchBox", value: ptr("/data/city"), placeholder: "Find" },
+    // 🚨 `value`, NOT `data` (issue #1498). CodeEditorControl and MarkdownEditorControl carry their
+    // text on `Value` — that is what the SERVER sends and what the Blazor views bind. Building this
+    // tree with `data` was the harness encoding the bug: it exercised the one shape the pack
+    // happened to support instead of the shape it actually receives, so an empty editor on RN
+    // passed here. SearchBoxControl carries `Value` too, so `search` below is value-shaped as well.
+    code: { $type: "CodeEditor", label: "Code", value: ptr("/data/code") },
+    mdedit: { $type: "MarkdownEditor", label: "Notes", value: ptr("/data/md") },
     diff: { $type: "DiffEditor", original: "old line", data: "new line" },
     sample: { $type: "CodeSample", data: "readonly();" },
     err: { $type: "Exception", message: "Boom happened" },
@@ -126,6 +131,15 @@ describe("RN leaf pack renders every editor + resolves its binding", () => {
     expect(inputValues.some((v) => v.includes("# Notes"))).toBe(true);
   });
 
+  // The regression guard for #1498. The three assertions above already cover it now that the tree
+  // is value-shaped, but this one names the defect so a future `data`-only binding helper cannot
+  // quietly reintroduce it: a control whose C# record calls the member `Value` must render its
+  // text, not an empty box.
+  it("resolves `value`-shaped controls — the shape the server actually sends (#1498)", () => {
+    expect(inputValues).not.toContain("");
+    expect(inputValues).toContain("Zurich"); // SearchBox, bound via `value`
+  });
+
   it("DiffEditor shows original and modified", () => {
     expect(hasText("old line")).toBe(true);
     expect(hasText("new line")).toBe(true);

clients/react/src/controls/common.ts

@@ -28,7 +28,17 @@ export interface Field {
 
 /** Bound form field: resolves the value, and writes edits back via UpdatePointer to its /data pointer. */
 export function useField(control: UiControl): Field {
-  const bound = control.data ?? control.isChecked;
+  // 🚨 `value` FIRST (issue #1498). `data ?? isChecked` is right for the form fields whose C# record
+  // names the member `Data`, but several controls name it `Value` — CodeEditorControl,
+  // MarkdownEditorControl, SearchBoxControl — and the Blazor views bind `ViewModel.Value`. A control
+  // arriving from the server with `value` and no `data` therefore resolved to undefined and rendered
+  // an EMPTY editor, with nothing on screen to say the text had simply not been read.
+  //
+  // This is the same precedence the web pack's monaco leaf already applies locally
+  // (`control.value ?? control.data`); putting it in the shared helper is what stops the two packs
+  // from disagreeing about it again. It is a no-op for every `Data`-shaped control, since `value` is
+  // then undefined.
+  const bound = control.value ?? control.data ?? control.isChecked;
   const value = useResolve(bound);
   const pointer = useBindingPointer(bound);
   const disabled = !!useResolve(control.disabled);