fix(runner): preserve metadata when rewriting legacy aliases (commontoolsinc#1803)

fix: preserve metadata when rewriting legacy aliases

- ensure toJSONWithLegacyAliases no longer drops schema info while normalizing alias depths
- add explanatory comments for the whole function
- extend json-utils tests to cover regression scenario

Author: seefeldb

packages/runner/src/builder/json-utils.ts

@@ -29,7 +29,10 @@ export function toJSONWithLegacyAliases(
   ignoreSelfAliases: boolean = false,
   path: PropertyKey[] = [],
 ): JSONValue | undefined {
-  // Convert regular cells to opaque refs
+  // Turn strongly typed builder values into legacy JSON structures while
+  // preserving alias metadata for consumers that still rely on it.
+
+  // Convert regular cells and results from Cell.get() to opaque refs
   if (canBeOpaqueRef(value)) value = makeOpaqueRef(value);
 
   // Verify that opaque refs are not in a parent frame
@@ -45,12 +48,13 @@ export function toJSONWithLegacyAliases(
     if (external) return external as JSONValue;
   }
 
+  // Otherwise it's an internal reference. Extract the schema and output a link.
   if (isOpaqueRef(value) || isShadowRef(value)) {
     const pathToCell = paths.get(value);
     if (pathToCell) {
       if (ignoreSelfAliases && deepEqual(path, pathToCell)) return undefined;
 
-      // Get schema from exported value if available
+      // Add schema from exported value if available
       const exported = isOpaqueRef(value) ? value.export() : undefined;
       return {
         $alias: {
@@ -67,8 +71,13 @@ export function toJSONWithLegacyAliases(
     } else throw new Error(`Cell not found in paths`);
   }
 
+  // If we encounter a link, it's from a nested recipe.
   if (isLegacyAlias(value)) {
     const alias = (value as LegacyAlias).$alias;
+    // If this was a shadow ref, i.e. a closed over reference, see whether
+    // we're now at the level that it should be resolved to the actual cell.
+    // (i.e. we're generating the recipe from which the closed over reference
+    // was captured)
     if (isShadowRef(alias.cell)) {
       const cell = alias.cell.shadowOf;
       if (cell.export().frame !== getTopFrame()) {
@@ -81,18 +90,30 @@ export function toJSONWithLegacyAliases(
             `Shadow ref alias with parent cell not found in current frame`,
           );
         }
+        // If we're not at the top level, just emit it again. This will be
+        // converted once the higher level recipe is being processed.
         return value as JSONValue;
       }
       if (!paths.has(cell)) throw new Error(`Cell not found in paths`);
+      // If in top frame, it's an alias to another cell on the process cell. So
+      // we emit the alias without the cell reference (it will be filled in
+      // later with the process cell) and concatenate the path.
+      const { cell: _, ...aliasWithoutCell } = alias;
       return {
         $alias: {
+          // Keep any extra metadata (schema, rootSchema, etc.) that might have
+          // been attached to the legacy alias originally.
+          ...aliasWithoutCell,
           path: [...paths.get(cell)!, ...alias.path] as (string | number)[],
         },
       } satisfies LegacyAlias;
     } else if (!("cell" in alias) || typeof alias.cell === "number") {
+      // If we encounter an existing alias and it isn't an absolute reference
+      // with a cell id, then increase the nesting level.
       return {
         $alias: {
-          cell: ((alias.cell as number) ?? 0) + 1,
+          ...alias, // Preserve existing metadata.
+          cell: ((alias.cell as number) ?? 0) + 1, // Increase nesting level.
           path: alias.path as (string | number)[],
         },
       } satisfies LegacyAlias;
@@ -101,20 +122,23 @@ export function toJSONWithLegacyAliases(
     }
   }
 
+  // If this is an array, process each element recursively.
   if (Array.isArray(value)) {
     return (value as Opaque<any>).map((v: Opaque<any>, i: number) =>
       toJSONWithLegacyAliases(v, paths, ignoreSelfAliases, [...path, i])
     );
   }
 
+  // If this is an object or a recipe, process each key recursively.
   if (isRecord(value) || isRecipe(value)) {
+    // If this is a recipe, call its toJSON method to get the properly
+    // serialized version.
     const valueToProcess = (isRecipe(value) &&
         typeof (value as unknown as toJSON).toJSON === "function")
       ? (value as unknown as toJSON).toJSON() as Record<string, any>
       : (value as Record<string, any>);
 
     const result: any = {};
-    let hasValue = false;
     for (const key in valueToProcess as any) {
       const jsonValue = toJSONWithLegacyAliases(
         valueToProcess[key],
@@ -124,13 +148,13 @@ export function toJSONWithLegacyAliases(
       );
       if (jsonValue !== undefined) {
         result[key] = jsonValue;
-        hasValue = true;
       }
     }
 
+    // Retain the original recipe reference for downstream processing.
     if (isRecipe(value)) result[unsafe_originalRecipe] = value;
 
-    return hasValue || Object.keys(result).length === 0 ? result : undefined;
+    return result;
   }
 
   return value;

packages/runner/test/json-utils.test.ts

@@ -1,11 +1,23 @@
 import { afterEach, beforeEach, describe, it } from "@std/testing/bdd";
 import { expect } from "@std/expect";
-import { createJsonSchema } from "../src/builder/json-utils.ts";
-import { Runtime } from "../src/runtime.ts";
-import type { JSONSchema } from "../src/builder/types.ts";
+
 import { Identity } from "@commontools/identity";
 import { StorageManager } from "@commontools/runner/storage/cache.deno";
 
+import {
+  createJsonSchema,
+  toJSONWithLegacyAliases,
+} from "../src/builder/json-utils.ts";
+import {
+  isOpaqueRefMarker,
+  type JSONSchema,
+  type Opaque,
+  type OpaqueRef,
+  type ShadowRef,
+} from "../src/builder/types.ts";
+import type { LegacyAlias } from "../src/sigil-types.ts";
+import { Runtime } from "../src/runtime.ts";
+
 const signer = await Identity.fromPassphrase("test operator");
 const space = signer.did();
 
@@ -317,3 +329,60 @@ describe("createJsonSchema", () => {
     });
   });
 });
+
+describe("toJSONWithLegacyAliases", () => {
+  it("preserves metadata when expanding shadow ref aliases", () => {
+    const cell = {
+      export: () => ({ frame: undefined }),
+      [isOpaqueRefMarker]: true,
+    } as unknown as OpaqueRef<unknown>;
+    const paths = new Map<OpaqueRef<unknown>, PropertyKey[]>([
+      [cell, ["root"]],
+    ]);
+    const schema = { type: "string" as const };
+    const alias: LegacyAlias = {
+      $alias: {
+        cell: { shadowOf: cell } as ShadowRef,
+        path: ["child"],
+        schema,
+      },
+    };
+
+    const result = toJSONWithLegacyAliases(
+      alias as unknown as Opaque<LegacyAlias>,
+      paths,
+    );
+
+    expect(result).toEqual({
+      $alias: {
+        path: ["root", "child"],
+        schema,
+      },
+    });
+  });
+
+  it("increments numeric alias cells without dropping schemas", () => {
+    const alias: LegacyAlias = {
+      $alias: {
+        cell: 2,
+        path: ["child"],
+        schema: { type: "boolean" as const },
+        rootSchema: { type: "boolean" as const },
+      },
+    };
+
+    const result = toJSONWithLegacyAliases(
+      alias as unknown as Opaque<LegacyAlias>,
+      new Map(),
+    );
+
+    expect(result).toEqual({
+      $alias: {
+        cell: 3,
+        path: ["child"],
+        schema: { type: "boolean" as const },
+        rootSchema: { type: "boolean" as const },
+      },
+    });
+  });
+});