fix(runner): avoid infinite recursion in function cacher (#1776)

* fix(runner): avoid infinite recursion in function cacher

* ignore literals, better types

* simplify things given the new isRecord check

* don't make seen a default input so we don't accidentally recurse without it

* Runner: stop ref recursion & fix anonymous cell naming

- prevent the runner’s function cache walk from re-entering cells, shadow refs, and opaque refs
- tighten opaque-ref typing so JSON conversion uses the shared OpaqueRefMethods surface
- teach the recipe factory to pull cell names from export() so unnamed internal cells get stable, meaningful aliases
- refresh recipe.test.ts expectations to cover the corrected alias naming and avoid the long-standing double naming bug

* re-arrange for clarity

* undo the name fix, since it caused other issues

* Avoid duplicate auto cell names

* re-enable names and fix tests

Author: seefeldb

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

@@ -42,7 +42,7 @@ export function toJSONWithLegacyAliases(
   // If this is an external reference, just copy the reference as is.
   if (isOpaqueRef(value)) {
     const { external } = value.export();
-    if (external) return external;
+    if (external) return external as JSONValue;
   }
 
   if (isOpaqueRef(value) || isShadowRef(value)) {
@@ -81,7 +81,7 @@ export function toJSONWithLegacyAliases(
             `Shadow ref alias with parent cell not found in current frame`,
           );
         }
-        return value;
+        return value as JSONValue;
       }
       if (!paths.has(cell)) throw new Error(`Cell not found in paths`);
       return {
@@ -108,10 +108,10 @@ export function toJSONWithLegacyAliases(
   }
 
   if (isRecord(value) || isRecipe(value)) {
-    const valueToProcess =
-      (isRecipe(value) && typeof (value as toJSON).toJSON === "function")
-        ? (value as toJSON).toJSON() as Record<string, any>
-        : (value as Record<string, any>);
+    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;

packages/runner/src/builder/recipe.ts

@@ -156,7 +156,7 @@ function factoryFromRecipe<T, R>(
       if (isOpaqueRef(value)) value = value.unsafe_getExternal();
       if (
         (isOpaqueRef(value) || isShadowRef(value)) && !cells.has(value) &&
-        !shadows.has(value)
+        !shadows.has(value as ShadowRef)
       ) {
         if (isOpaqueRef(value) && value.export().frame !== getTopFrame()) {
           value = createShadowRef(value.export().value);
@@ -190,12 +190,26 @@ function factoryFromRecipe<T, R>(
 
   // Fill in reasonable names for all cells, where possible:
 
+  const usedNames = new Set<string>();
+  cells.forEach((cell) => {
+    const existingName = cell.export().name;
+    if (existingName) usedNames.add(existingName);
+  });
+
   // First from results
   if (isRecord(outputs)) {
     Object.entries(outputs).forEach(([key, value]: [string, unknown]) => {
       if (isOpaqueRef(value)) {
         const ref = value; // Typescript needs this to avoid type errors
-        if (!ref.export().path.length && !ref.export().name) ref.setName(key);
+        const exported = ref.export();
+        if (
+          !exported.path.length &&
+          !exported.name &&
+          !usedNames.has(key)
+        ) {
+          ref.setName(key);
+          usedNames.add(key);
+        }
       }
     });
   }
@@ -207,9 +221,11 @@ function factoryFromRecipe<T, R>(
       if (isRecord(node.inputs)) {
         Object.entries(node.inputs).forEach(([key, input]) => {
           if (
-            isOpaqueRef(input) && input.cell === cell && !cell.export().name
+            isOpaqueRef(input) && input.export().cell === cell &&
+            !cell.export().name && !usedNames.has(key)
           ) {
             cell.setName(key);
+            usedNames.add(key);
           }
         });
       }

packages/runner/src/builder/types.ts

@@ -24,6 +24,7 @@ import type {
   NavigateToFunction,
   Opaque,
   OpaqueRef,
+  OpaqueRefMethods,
   Recipe,
   RecipeFunction,
   RenderFunction,
@@ -132,11 +133,13 @@ declare module "@commontools/api" {
   }
 }
 
-export type { OpaqueRefMethods } from "@commontools/api";
+export type { OpaqueRefMethods };
 
 export const isOpaqueRefMarker = Symbol("isOpaqueRef");
 
-export function isOpaqueRef<T = any>(value: unknown): value is OpaqueRef<T> {
+export function isOpaqueRef<T = any>(
+  value: unknown,
+): value is OpaqueRefMethods<T> {
   return !!value &&
     typeof (value as OpaqueRef<T>)[isOpaqueRefMarker] === "boolean";
 }

packages/runner/src/runner.ts

@@ -6,6 +6,7 @@ import {
   isModule,
   isOpaqueRef,
   isRecipe,
+  isShadowRef,
   isStreamValue,
   type JSONSchema,
   type JSONValue,
@@ -24,7 +25,7 @@ import {
   pushFrameFromCause,
   recipeFromFrame,
 } from "./builder/recipe.ts";
-import { type Cell } from "./cell.ts";
+import { type Cell, isCell } from "./cell.ts";
 import { type Action } from "./scheduler.ts";
 import { diffAndUpdate } from "./data-updating.ts";
 import {
@@ -332,7 +333,7 @@ export class Runner implements IRunner {
     }
 
     // Discover and cache all JavaScript functions in the recipe before start
-    this.discoverAndCacheFunctions(recipe);
+    this.discoverAndCacheFunctions(recipe, new Set());
 
     return { resultCell, recipe, processCell, needsStart: true };
   }
@@ -401,7 +402,7 @@ export class Runner implements IRunner {
     this.allCancels.add(cancel);
 
     // Re-discover functions to be safe (idempotent)
-    this.discoverAndCacheFunctions(recipe);
+    this.discoverAndCacheFunctions(recipe, new Set());
 
     for (const node of recipe.nodes) {
       this.instantiateNode(
@@ -665,14 +666,18 @@ export class Runner implements IRunner {
    *
    * @param recipe The recipe to discover functions from
    */
-  private discoverAndCacheFunctions(recipe: Recipe): void {
+  private discoverAndCacheFunctions(
+    recipe: Recipe,
+    seen: Set<object>,
+  ): void {
+    if (seen.has(recipe)) return;
+    seen.add(recipe);
+
     for (const node of recipe.nodes) {
-      this.discoverAndCacheFunctionsFromModule(node.module);
+      this.discoverAndCacheFunctionsFromModule(node.module, seen);
 
       // Also check inputs for nested recipes (e.g., in map operations)
-      if (isRecord(node.inputs)) {
-        this.discoverAndCacheFunctionsFromValue(node.inputs);
-      }
+      this.discoverAndCacheFunctionsFromValue(node.inputs, seen);
     }
   }
 
@@ -681,7 +686,13 @@ export class Runner implements IRunner {
    *
    * @param module The module to process
    */
-  private discoverAndCacheFunctionsFromModule(module: Module): void {
+  private discoverAndCacheFunctionsFromModule(
+    module: Module,
+    seen: Set<object>,
+  ): void {
+    if (seen.has(module)) return;
+    seen.add(module);
+
     if (!isModule(module)) return;
 
     switch (module.type) {
@@ -698,7 +709,7 @@ export class Runner implements IRunner {
       case "recipe":
         // Recursively discover functions in nested recipes
         if (isRecipe(module.implementation)) {
-          this.discoverAndCacheFunctions(module.implementation);
+          this.discoverAndCacheFunctions(module.implementation, seen);
         }
         break;
 
@@ -708,7 +719,7 @@ export class Runner implements IRunner {
           const referencedModule = this.runtime.moduleRegistry.getModule(
             module.implementation as string,
           );
-          this.discoverAndCacheFunctionsFromModule(referencedModule);
+          this.discoverAndCacheFunctionsFromModule(referencedModule, seen);
         } catch (error) {
           console.warn(
             `Failed to resolve module reference for implementation "${module.implementation}":`,
@@ -725,22 +736,43 @@ export class Runner implements IRunner {
    *
    * @param value The value to search for recipes
    */
-  private discoverAndCacheFunctionsFromValue(value: JSONValue): void {
+  private discoverAndCacheFunctionsFromValue(
+    value: JSONValue,
+    seen: Set<object>,
+  ): void {
     if (isRecipe(value)) {
-      this.discoverAndCacheFunctions(value);
-    } else if (isModule(value)) {
-      this.discoverAndCacheFunctionsFromModule(value);
-    } else if (isRecord(value)) {
-      // Recursively search in objects and arrays
-      if (Array.isArray(value)) {
-        for (const item of value) {
-          this.discoverAndCacheFunctionsFromValue(item);
-        }
-      } else {
-        for (const key in value) {
-          this.discoverAndCacheFunctionsFromValue(value[key] as JSONValue);
-        }
+      this.discoverAndCacheFunctions(value, seen);
+      return;
+    }
+
+    if (isModule(value)) {
+      this.discoverAndCacheFunctionsFromModule(value, seen);
+      return;
+    }
+
+    if (
+      !isRecord(value) || isOpaqueRef(value) || isShadowRef(value) ||
+      isCell(value)
+    ) {
+      return;
+    }
+
+    if (seen.has(value)) return;
+    seen.add(value);
+
+    // Recursively search in objects and arrays
+    if (Array.isArray(value)) {
+      for (const item of value) {
+        this.discoverAndCacheFunctionsFromValue(item, seen);
       }
+      return;
+    }
+
+    for (const key in value) {
+      this.discoverAndCacheFunctionsFromValue(
+        value[key] as JSONValue,
+        seen,
+      );
     }
   }
 

packages/runner/test/recipe.test.ts

@@ -71,9 +71,11 @@ describe("complex recipe function", () => {
     );
     expect(nodes[0].inputs).toEqual({ $alias: { path: ["argument", "x"] } });
     expect(nodes[0].outputs).toEqual({
-      $alias: { path: ["internal", "__#0"] },
+      $alias: { path: ["internal", "defaultValue"] },
+    });
+    expect(nodes[1].inputs).toEqual({
+      $alias: { path: ["internal", "defaultValue"] },
     });
-    expect(nodes[1].inputs).toEqual({ $alias: { path: ["internal", "__#0"] } });
     expect(nodes[1].outputs).toEqual({
       $alias: { path: ["internal", "double"] },
     });