Browse Source

fix(codemode): preserve collection value identity (#35776)

Aiden Cline 1 month ago
parent
commit
f86e1cc1ff

+ 17 - 2
packages/codemode/src/interpreter/runtime.ts

@@ -530,17 +530,29 @@ const invokeArrayStatic = (name: string, args: Array<unknown>, node: AstNode): u
       if (args[0] instanceof SandboxURLSearchParams) {
         return Array.from(args[0].params.entries(), ([key, value]) => [key, value])
       }
-      const source = boundedData(args[0], "Array.from input")
+      const source = args[0]
+      if (source instanceof SandboxPromise) {
+        throw new InterpreterRuntimeError(
+          "Array.from received an un-awaited Promise; await it before creating the array.",
+          node,
+          "InvalidDataValue",
+        )
+      }
       if (typeof source === "string") return Array.from(source)
       if (Array.isArray(source)) return [...source]
       if (
         source !== null &&
         typeof source === "object" &&
+        (Object.getPrototypeOf(source) === Object.prototype || Object.getPrototypeOf(source) === null) &&
         typeof (source as { length?: unknown }).length === "number"
       ) {
         return Array.from(source as ArrayLike<unknown>)
       }
-      throw new InterpreterRuntimeError("Array.from expects an array, string, Map, Set, or array-like value.", node)
+      throw new InterpreterRuntimeError(
+        "Array.from expects an array, string, Map, Set, or array-like value.",
+        node,
+        "InvalidDataValue",
+      )
     }
     default:
       throw new InterpreterRuntimeError(`Array.${name} is not available in CodeMode.`, node)
@@ -2005,6 +2017,9 @@ class Interpreter<R> {
         if (callable.namespace === "Object" && objectMethodsPreservingIdentity.has(callable.name)) {
           return invokeGlobalMethod(callable, args, node)
         }
+        if (callable.namespace === "Array" && (callable.name === "from" || callable.name === "of")) {
+          return invokeGlobalMethod(callable, args, node)
+        }
         return boundedData(invokeGlobalMethod(callable, args, node), `${callable.namespace}.${callable.name} result`)
       }
       if (callable instanceof CoercionFunction) {

+ 18 - 15
packages/codemode/src/stdlib/object.ts

@@ -1,6 +1,6 @@
 import { type AstNode, InterpreterRuntimeError } from "../interpreter/model.js"
 import { isBlockedMember } from "../tool-runtime.js"
-import { isSandboxValue, SandboxMap, SandboxSet, SandboxURLSearchParams } from "../values.js"
+import { isSandboxValue, SandboxMap, SandboxPromise, SandboxSet, SandboxURLSearchParams } from "../values.js"
 import { boundedData, coerceToString } from "./value.js"
 
 export const objectStatics = new Set(["keys", "values", "entries", "hasOwn", "assign", "fromEntries"])
@@ -10,13 +10,23 @@ export const invokeObjectMethod = (name: string, args: Array<unknown>, node: Ast
   if (!objectStatics.has(name)) throw new InterpreterRuntimeError(`Object.${name} is not available in CodeMode.`, node)
   const requireObject = (): Record<string, unknown> => {
     const input = args[0]
-    const value = boundedData(args[0], `Object.${name} input`)
     if (Array.isArray(input)) return input as unknown as Record<string, unknown>
-    if (isSandboxValue(value)) return {}
-    if (value === null || typeof value !== "object") {
-      throw new InterpreterRuntimeError(`Object.${name} expects a data object or array.`, node)
+    if (isSandboxValue(input)) return {}
+    if (input instanceof SandboxPromise) {
+      throw new InterpreterRuntimeError(
+        `Object.${name} received an un-awaited Promise; await it before inspecting the result.`,
+        node,
+        "InvalidDataValue",
+      )
     }
-    return value as Record<string, unknown>
+    if (input === null || typeof input !== "object") {
+      throw new InterpreterRuntimeError(`Object.${name} expects a data object or array.`, node, "InvalidDataValue")
+    }
+    const prototype = Object.getPrototypeOf(input)
+    if (prototype !== null && prototype !== Object.prototype) {
+      throw new InterpreterRuntimeError(`Object.${name} expects a data object or array.`, node, "InvalidDataValue")
+    }
+    return input as Record<string, unknown>
   }
   const guardedSet = (out: Record<string, unknown>, key: string, item: unknown): void => {
     if (isBlockedMember(key)) throw new InterpreterRuntimeError(`Property '${key}' is not available in CodeMode.`, node)
@@ -28,15 +38,8 @@ export const invokeObjectMethod = (name: string, args: Array<unknown>, node: Ast
     guardedSet(out, coerceToString(key), item)
   }
   switch (name) {
-    case "keys": {
-      const value = boundedData(args[0], "Object.keys input")
-      if (isSandboxValue(value)) return []
-      if (Array.isArray(value)) return Object.keys(value)
-      if (value === null || typeof value !== "object") {
-        throw new InterpreterRuntimeError("Object.keys expects a data object or array.", node)
-      }
-      return Object.keys(value)
-    }
+    case "keys":
+      return Object.keys(requireObject())
     case "values":
       return Object.values(requireObject())
     case "entries":

+ 6 - 0
packages/codemode/test/promise.test.ts

@@ -245,6 +245,12 @@ describe("promises at data boundaries", () => {
     expect(diagnostic.message).toContain("await tools.ns.tool(...)")
   })
 
+  test("collection helpers do not let un-awaited promises cross the result boundary", async () => {
+    const diagnostic = await error(`return Array.from([Promise.resolve(1)])`)
+    expect(diagnostic.kind).toBe("InvalidDataValue")
+    expect(diagnostic.message).toContain("un-awaited Promise")
+  })
+
   test("passing an un-awaited promise as a tool argument is a clear diagnostic", async () => {
     const diagnostic = await error(`return await tools.host.sleepy({ id: tools.host.sleepy({ id: 1 }) })`)
     expect(diagnostic.kind).toBe("InvalidDataValue")

+ 84 - 0
packages/codemode/test/stdlib.test.ts

@@ -773,6 +773,43 @@ describe("sandbox values at intra-sandbox checkpoints", () => {
     )
   })
 
+  test("Object.values/entries preserve nested object identity", async () => {
+    expect(
+      await value(`
+      const child = { selected: false }
+      const rows = { a: child }
+      Object.values(rows)[0].selected = true
+      return child.selected
+    `),
+    ).toBe(true)
+    expect(
+      await value(`
+      const child = { selected: false }
+      const rows = { a: child }
+      Object.entries(rows)[0][1].selected = true
+      return child.selected
+    `),
+    ).toBe(true)
+  })
+
+  test("Object enumeration preserves promises and callable references", async () => {
+    expect(
+      await value(`
+      const pending = Promise.resolve(1)
+      const source = { pending }
+      return [Object.keys(source), Object.hasOwn(source, "pending"), await Object.values(source)[0], await Object.entries(source)[0][1]]
+    `),
+    ).toEqual([["pending"], true, 1, 1])
+    expect(await value(`return Object.values({ max: Math.max })[0](1, 2)`)).toBe(2)
+  })
+
+  test("Object enumeration rejects invalid receivers and gives promises an await hint", async () => {
+    const diagnostic = await error(`return Object.keys(Promise.resolve({ a: 1 }))`)
+    expect(diagnostic.kind).toBe("InvalidDataValue")
+    expect(diagnostic.message).toContain("await")
+    expect((await error(`return Object.keys(Math)`)).kind).toBe("InvalidDataValue")
+  })
+
   test("Object.assign keeps Maps usable", async () => {
     expect(await value(`const merged = Object.assign({}, { m: new Map([["a", 1]]) }); return merged.m.get("a")`)).toBe(
       1,
@@ -795,6 +832,53 @@ describe("sandbox values at intra-sandbox checkpoints", () => {
     expect(await value(`return Array.from([new Date(5)])[0].getTime()`)).toBe(5)
   })
 
+  test("Array.from and Array.of preserve nested object identity", async () => {
+    expect(
+      await value(`
+      const child = { selected: false }
+      Array.from([child])[0].selected = true
+      return child.selected
+    `),
+    ).toBe(true)
+    expect(
+      await value(`
+      const child = { selected: false }
+      Array.of(child)[0].selected = true
+      return child.selected
+    `),
+    ).toBe(true)
+  })
+
+  test("Array.from and Array.of preserve promises and callable references", async () => {
+    expect(
+      await value(`
+      const pending = Promise.resolve(1)
+      return [await Array.from([pending])[0], await Array.of(pending)[0]]
+    `),
+    ).toEqual([1, 1])
+    expect(await value(`return [Array.from([Math.max])[0](1, 2), Array.of(Math.max)[0](3, 4)]`)).toEqual([2, 4])
+  })
+
+  test("Array.from preserves identity across supported collection shapes", async () => {
+    expect(
+      await value(`
+      const child = { selected: false }
+      const fromArrayLike = Array.from({ 0: child, length: 1 })
+      const fromMap = Array.from(new Map([["child", child]]))
+      const fromSet = Array.from(new Set([child]))
+      fromArrayLike[0].selected = true
+      return [fromMap[0][1] === child, fromSet[0] === child, child.selected]
+    `),
+    ).toEqual([true, true, true])
+  })
+
+  test("Array.from rejects invalid receivers and gives promises an await hint", async () => {
+    const diagnostic = await error(`return Array.from(Promise.resolve([1]))`)
+    expect(diagnostic.kind).toBe("InvalidDataValue")
+    expect(diagnostic.message).toContain("await")
+    expect((await error(`return Array.from(() => 1)`)).kind).toBe("InvalidDataValue")
+  })
+
   test("regexes stay callable through Object.values", async () => {
     expect(await value(`return Object.values({ r: /ab+/ })[0].test("abb")`)).toBe(true)
   })