fix(runner): inline data URIs before handling write redirects (#1911)

fix(runner): inline data URIs before handling write redirects

Move data URI inlining logic to occur before write redirect (alias)
handling in normalizeAndDiff. This ensures the correct evaluation order
when a data URI contains a redirect link.

Previously, when newValue was a data URI containing a redirect, the
code would:
1. Check currentValue for redirects (line 290)
2. Follow currentValue redirect if present
3. Process data URI inlining later (line 318)

This caused the redirect within the data URI to be written to the wrong
location - it would follow the currentValue redirect first, then inline
the data URI, resulting in the redirect being written to the destination
of the currentValue alias instead of to the intended target.

The fix reorders the logic so data URIs are inlined immediately after
unwrapping proxies and cells (line 212), before any redirect handling.
This preserves the invariant that redirect handling at line 290 only
applies when newValue is not itself an alias.

The order is now:
1. Unwrap proxies/cells
2. Inline data URIs (exposing any contained redirects)
3. Handle redirects in newValue
4. Handle redirects in currentValue

This ensures redirects contained in data URIs are properly recognized
and handled as part of newValue, not incorrectly written through a
currentValue redirect.

Author: seefeldb

packages/runner/src/data-updating.ts

@@ -24,6 +24,11 @@ import {
 import { type IRuntime } from "./runtime.ts";
 import { toURI } from "./uri-utils.ts";
 
+const diffLogger = getLogger("normalizeAndDiff", {
+  enabled: false,
+  level: "debug",
+});
+
 /**
  * Traverses newValue and updates `current` and any relevant linked documents.
  *
@@ -86,34 +91,6 @@ type ChangeSet = {
  * @param context - The context of the change.
  * @returns An array of changes that should be written.
  */
-const diffLogger = getLogger("normalizeAndDiff", {
-  enabled: false,
-  level: "debug",
-});
-
-/**
- * Returns true if `target` is the immediate parent of `base` in the same document.
- *
- * Example:
- * - base.path = ["internal", "__#1", "next"]
- * - target.path = ["internal", "__#1"]
- *
- * This is used to decide when to collapse a self/parent link that would create
- * a tight self-loop (e.g., obj.next -> obj) while allowing references to
- * higher ancestors (like an item's `items` pointing to its containing array).
- */
-function isImmediateParent(
-  target: NormalizedFullLink,
-  base: NormalizedFullLink,
-): boolean {
-  return (
-    target.id === base.id &&
-    target.space === base.space &&
-    target.path.length === base.path.length - 1 &&
-    target.path.every((seg, i) => seg === base.path[i])
-  );
-}
-
 export function normalizeAndDiff(
   runtime: IRuntime,
   tx: IExtendedStorageTransaction,
@@ -232,6 +209,54 @@ export function normalizeAndDiff(
     newValue = newValue.getAsLink();
   }
 
+  // Check for links that are data: URIs and inline them, by calling
+  // normalizeAndDiff on the contents of the link.
+  if (isLink(newValue)) {
+    const parsedLink = parseLink(newValue, link);
+    if (parsedLink.id.startsWith("data:")) {
+      diffLogger.debug(() =>
+        `[BRANCH_CELL_LINK] Data link detected, treating as contents at path=${pathStr}`
+      );
+      // Use the tx code to make sure we read it the same way
+      let dataValue: any = tx.readValueOrThrow({
+        ...parsedLink,
+        path: [],
+      }, options);
+      const path = [...parsedLink.path];
+      // If there is a link on the way to `path`, follow it, appending remaining
+      // path to the target link.
+      for (;;) {
+        if (isAnyCellLink(dataValue)) {
+          const dataLink = parseLink(dataValue, parsedLink);
+          dataValue = createSigilLinkFromParsedLink({
+            ...dataLink,
+            path: [...dataLink.path, ...path],
+          });
+          break;
+        }
+        if (path.length > 0) {
+          if (isRecord(dataValue)) {
+            dataValue = dataValue[path.shift()!];
+          } else {
+            dataValue = undefined;
+            break;
+          }
+        } else {
+          break;
+        }
+      }
+      return normalizeAndDiff(
+        runtime,
+        tx,
+        link,
+        dataValue,
+        context,
+        options,
+        seen,
+      );
+    }
+  }
+
   // If we're about to create a reference to ourselves, no-op
   if (areMaybeLinkAndNormalizedLinkSame(newValue, link)) {
     diffLogger.debug(() =>
@@ -318,48 +343,6 @@ export function normalizeAndDiff(
         seen,
       );
     }
-    if (parsedLink.id.startsWith("data:")) {
-      diffLogger.debug(() =>
-        `[BRANCH_CELL_LINK] Data link detected, treating as contents at path=${pathStr}`
-      );
-      // If there is a data link treat it as writing it's contents instead.
-
-      //  Use the tx code to make sure we read it the same way
-      let dataValue: any = tx.readValueOrThrow({
-        ...parsedLink,
-        path: [],
-      }, options);
-      const path = [...parsedLink.path];
-      for (;;) {
-        if (isAnyCellLink(dataValue)) {
-          const dataLink = parseLink(dataValue, parsedLink);
-          dataValue = createSigilLinkFromParsedLink({
-            ...dataLink,
-            path: [...dataLink.path, ...path],
-          });
-          break;
-        }
-        if (path.length > 0) {
-          if (isRecord(dataValue)) {
-            dataValue = dataValue[path.shift()!];
-          } else {
-            dataValue = undefined;
-            break;
-          }
-        } else {
-          break;
-        }
-      }
-      return normalizeAndDiff(
-        runtime,
-        tx,
-        link,
-        dataValue,
-        context,
-        options,
-        seen,
-      );
-    }
     if (
       isAnyCellLink(currentValue) &&
       areLinksSame(newValue, currentValue, link)
@@ -634,3 +617,26 @@ export function addCommonIDfromObjectID(
 
   traverse(obj);
 }
+
+/**
+ * Returns true if `target` is the immediate parent of `base` in the same document.
+ *
+ * Example:
+ * - base.path = ["internal", "__#1", "next"]
+ * - target.path = ["internal", "__#1"]
+ *
+ * This is used to decide when to collapse a self/parent link that would create
+ * a tight self-loop (e.g., obj.next -> obj) while allowing references to
+ * higher ancestors (like an item's `items` pointing to its containing array).
+ */
+function isImmediateParent(
+  target: NormalizedFullLink,
+  base: NormalizedFullLink,
+): boolean {
+  return (
+    target.id === base.id &&
+    target.space === base.space &&
+    target.path.length === base.path.length - 1 &&
+    target.path.every((seg, i) => seg === base.path[i])
+  );
+}

packages/runner/test/data-updating.test.ts

@@ -13,6 +13,7 @@ import {
   areNormalizedLinksSame,
   createSigilLinkFromParsedLink,
   isAnyCellLink,
+  isSigilLink,
   parseLink,
 } from "../src/link-utils.ts";
 import { type IExtendedStorageTransaction } from "../src/storage/interface.ts";
@@ -1095,6 +1096,80 @@ describe("data-updating", () => {
     expect(value.result).toBe(100);
   });
 
+  it("should inline data URI containing redirect without writing redirect to wrong location", () => {
+    // Setup: Create two separate cells - source and destination
+    const sourceCell = runtime.getCell<{ value: number }>(
+      space,
+      "data URI redirect source value",
+      undefined,
+      tx,
+    );
+    sourceCell.set({ value: 99 });
+
+    const destinationCell = runtime.getCell<{ value: number }>(
+      space,
+      "data URI redirect destination value",
+      undefined,
+      tx,
+    );
+    destinationCell.set({ value: 42 });
+
+    // Create a data URI that contains a redirect pointing to sourceCell
+    const redirectAlias = sourceCell.key("value").getAsWriteRedirectLink();
+    const dataCell = runtime.getImmutableCell<any>(
+      space,
+      redirectAlias,
+      undefined,
+      tx,
+    );
+
+    // Create a target cell that currently has an alias to destinationCell
+    const targetCell = runtime.getCell<{ result: any }>(
+      space,
+      "data URI redirect target cell",
+      undefined,
+      tx,
+    );
+    targetCell.setRaw({
+      result: destinationCell.key("value").getAsWriteRedirectLink(),
+    });
+
+    const current = targetCell.key("result").getAsNormalizedFullLink();
+
+    // Write the data cell (which contains a redirect to sourceCell) to the target
+    // Before the fix: data URI was not inlined early enough, and the redirect
+    // would be written to destinationCell.value instead of target.result
+    // After the fix: data URI is inlined first, exposing the redirect, which is
+    // then properly written to target.result
+    const changes = normalizeAndDiff(
+      runtime,
+      tx,
+      current,
+      dataCell.getAsLink(),
+    );
+
+    // Should write the new redirect to target Cell.result
+    // Note: The change writes a redirect (alias object with $alias key)
+    expect(changes.length).toBe(1);
+    expect(changes[0].location.id).toBe(
+      targetCell.getAsNormalizedFullLink().id,
+    );
+    expect(changes[0].location.path).toEqual(["result"]);
+    // The value should be the redirect link
+    expect(isSigilLink(changes[0].value)).toBe(true);
+    const parsedLink = parseLink(changes[0].value);
+    expect(parsedLink?.overwrite).toBe("redirect");
+
+    applyChangeSet(tx, changes);
+
+    // Verify that targetCell now points to sourceCell's value (99), not destinationCell's (42)
+    expect(targetCell.get().result).toBe(99);
+
+    // Verify neither source nor destination cells were modified
+    expect(sourceCell.get()).toEqual({ value: 99 });
+    expect(destinationCell.get()).toEqual({ value: 42 });
+  });
+
   describe("addCommonIDfromObjectID", () => {
     it("should handle arrays", () => {
       const obj = { items: [{ id: "item1", name: "First Item" }] };