runner(schema): respect schema additionalProperties during validation (commontoolsinc#1809)

* runner(schema): respect schema additionalProperties during validation

- Skip the additional-property traversal unless the schema explicitly enables additionalProperties, so we stop including all other keys with `any` schemas.
- Emit a contextual warning if resolving a schema for an allowed additional key still returns undefined, making downstream schema drift visible.

This fixes CT-897 by no longer overtriggering.

* empty object implies additional properties

* add tests

* fix formatting

* remove no longer needed as any cast

* fix cli integration to no longer use path that is not in schema

* clean up support for computed result schemas for charms

Author: seefeldb

packages/charm/src/manager.ts

@@ -339,22 +339,7 @@ export class CharmManager {
       throw e;
     }
 
-    let resultSchema: JSONSchema | undefined = recipe?.resultSchema;
-
-    // If there is no result schema, create one from top level properties that omits UI, NAME
-    if (!resultSchema) {
-      const resultValue = charm.get();
-      if (isObject(resultValue)) {
-        resultSchema = {
-          type: "object",
-          properties: Object.fromEntries(
-            Object.keys(resultValue).filter((key) => !key.startsWith("$")).map((
-              key,
-            ) => [key, {}]), // Empty schema == any
-          ),
-        };
-      }
-    }
+    const resultSchema = this.#getResultSchema(charm, recipe);
 
     if (runIt) {
       // Make sure the charm is running. This is re-entrant and has no effect if
@@ -369,6 +354,34 @@ export class CharmManager {
     }
   }
 
+  #getResultSchema(
+    charm: Cell<unknown>,
+    recipe: Recipe,
+  ): JSONSchema | undefined {
+    if (
+      isRecord(recipe.resultSchema) &&
+      Object.keys(recipe.resultSchema).length > 0
+    ) return recipe.resultSchema;
+
+    // Ignore default cell schema to get to other values
+    const resultValue = charm.asSchema().get();
+    if (isObject(resultValue)) {
+      const keys = Object.keys(resultValue).filter((key) =>
+        !key.startsWith("$")
+      );
+
+      // Only generate a schema for charms that have more than $ props
+      if (keys.length > 0) {
+        return {
+          type: "object",
+          properties: Object.fromEntries(keys.map((key) => [key, true])),
+        };
+      }
+    }
+
+    return undefined;
+  }
+
   getLineage(charm: Cell<Charm>) {
     return charm.getSourceCell(charmSourceCellSchema)?.key("lineage").get() ??
       [];
@@ -786,10 +799,22 @@ export class CharmManager {
     if (!recipeId) throw new Error("charm missing recipe ID");
     const recipe = this.runtime.recipeManager.recipeById(recipeId);
     if (!recipe) throw new Error(`Recipe ${recipeId} not loaded`);
-    // FIXME(ja): return should be Cell<Schema<T>> I think?
     return source.key("argument").asSchema<T>(recipe.argumentSchema);
   }
 
+  getResult<T = unknown>(
+    charm: Cell<Charm | T>,
+  ): Cell<T> {
+    const source = charm.getSourceCell(processSchema);
+    const recipeId = source?.get()?.[TYPE]!;
+    if (!recipeId) throw new Error("charm missing recipe ID");
+    const recipe = this.runtime.recipeManager.recipeById(recipeId);
+    if (!recipe) throw new Error(`Recipe ${recipeId} not loaded`);
+    const resultSchema = this.#getResultSchema(charm, recipe);
+    // FIXME(ja): return should be Cell<Schema<T>> I think?
+    return charm.asSchema<T>(resultSchema);
+  }
+
   // note: removing a charm doesn't clean up the charm's cells
   // Now moves the charm to trash instead of just removing it
   async remove(idOrCharm: string | EntityId | Cell<Charm>) {

packages/charm/src/ops/charm-controller.ts

@@ -61,7 +61,7 @@ class CharmPropIo implements CharmCellIo {
     if (this.#type === "input") {
       return this.#cc.manager().getArgument(this.#cc.getCell());
     } else if (this.#type === "result") {
-      return this.#cc.getCell();
+      return this.#cc.manager().getResult(this.#cc.getCell());
     }
     throw new Error(`Unknown property type "${this.#type}"`);
   }

packages/cli/integration/integration.sh

@@ -155,8 +155,8 @@ test_value "Nested path set" "userData/user/name" '"Jane"' '"Jane"'
 echo "Testing --input flag operations..."
 
 # Test input flag operations
-test_json_value "Input flag set" "config" '{"inputConfig":"test"}' "--input"
-test_value "Nested input path" "config/inputConfig" '"inputValue"' '"inputValue"' "--input"
+test_json_value "Input flag set" "userData" '{"user":{"name":"test"}}' "--input"
+test_value "Nested input path" "userData/user/name" '"inputValue"' '"inputValue"' "--input"
 
 # Check space has new charm with correct inputs and title
 TITLE="Simple counter 2: 10"

packages/html/src/render.ts

@@ -90,7 +90,7 @@ export const renderImpl = (
     return cancel;
   }
   parent.append(root);
-  logger.debug("Rendered", root);
+  logger.debug("Rendered root", root);
   return () => {
     root.remove();
     cancel();

packages/runner/src/runner.ts

@@ -1259,7 +1259,7 @@ export class Runner implements IRunner {
     // TODO(seefeld): Make sure to not cancel after a recipe is elevated to a
     // charm, e.g. via navigateTo. Nothing is cancelling right now, so leaving
     // this as TODO.
-    addCancel(this.cancels.get(this.getDocKey(resultCell.getSourceCell()!)));
+    addCancel(() => this.stop(resultCell));
   }
 }
 

packages/runner/src/schema.ts

@@ -1,4 +1,5 @@
 import { JSONSchemaObj } from "@commontools/api";
+import { getLogger } from "@commontools/utils/logger";
 import { isObject, isRecord } from "@commontools/utils/types";
 import { JSONSchemaMutable } from "@commontools/runner";
 import { ContextualFlowControl } from "./cfc.ts";
@@ -16,6 +17,11 @@ import {
 } from "./query-result-proxy.ts";
 import { toCell, toOpaqueRef } from "./back-to-cell.ts";
 
+const logger = getLogger("validateAndTransform", {
+  enabled: true,
+  level: "debug",
+});
+
 /**
  * Schemas are mostly a subset of JSONSchema.
  *
@@ -677,23 +683,35 @@ export function validateAndTransform(
     }
 
     // Handle additional properties if defined
-    for (const key of keys) {
-      if (!resolvedSchema.properties || !(key in resolvedSchema.properties)) {
-        const childSchema = runtime.cfc.getSchemaAtPath(
-          resolvedSchema,
-          [key],
-          rootSchema,
-        );
-        if (childSchema === undefined) {
-          continue;
+    if (resolvedSchema.additionalProperties || !resolvedSchema.properties) {
+      for (const key of keys) {
+        // Skip properties that were already processed above:
+        if (!resolvedSchema.properties || !(key in resolvedSchema.properties)) {
+          // Will use additionalProperties if present
+          const childSchema = runtime.cfc.getSchemaAtPath(
+            resolvedSchema,
+            [key],
+            rootSchema,
+          );
+          if (childSchema === undefined) {
+            // This should never happen
+            logger.warn(() => [
+              "validateAndTransform: unexpected undefined schema for additional property",
+              key,
+              resolvedSchema,
+              rootSchema,
+              link,
+            ]);
+            continue;
+          }
+          result[key] = validateAndTransform(
+            runtime,
+            tx,
+            { ...link, path: [...link.path, key], schema: childSchema },
+            synced,
+            seen,
+          );
         }
-        result[key] = validateAndTransform(
-          runtime,
-          tx,
-          { ...link, path: [...link.path, key], schema: childSchema },
-          synced,
-          seen,
-        );
       }
     }