fix(spec,runtime): functions: [{ name, handler }] 数组形扛得住 objectstack build (#6238)

.changeset/functions-array-lowered-handler.md

@@ -0,0 +1,46 @@
+---
+"@objectstack/spec": minor
+"@objectstack/runtime": patch
+---
+
+fix(spec,runtime): `functions: [{ name, handler }]` survives `objectstack build` (#6238)
+
+The array form of the top-level `functions` collection could not pass its own
+build. `lowerCallables` has lowered the array branch the whole time — it rewrites
+both `handler` and `name` to the emitted ref — but the array member of the
+`functions` union in `stack.zod.ts` still demanded `handler: z.function()`. So
+`objectstack build` produced
+`[{ name: 'syncBilling', handler: 'syncBilling', effect: 'writes' }]` and then
+rejected it, with `invalid_union: Invalid input` and a path stopping at
+`functions`: no entry named, no key named, no reason given.
+
+This is the third time the same seam has parted, and the first two fixes are why
+this one only looks small. #4343 taught the union the bare lowered ref; #4976
+taught it the lowered *declaration*. Both only ever touched the **map** member —
+the array member is a separate inline record (an array entry names itself, so it
+carries `name` and an optional `packageId` and cannot be `FlowFunctionEntrySchema`
+in a list), and widening one never widened the other.
+
+**The fix.** The array member's `handler` now accepts the lowered string ref
+beside the authored callable. One widening covers both array spellings at once,
+unlike the map form's two separate members: `effect` is already optional on an
+array entry, so the bare and the declared entry differ only in whether that key
+is present. All four cells of map/array × bare/declared now round-trip.
+
+**The load seam, which the fix made reachable.** `mergeRuntimeModule` re-attaches
+each callable from the sibling ESM module to the declaration the JSON carried.
+Its array branch fell through to a map rebuild — `existing` was `{}` whenever
+`bundle.functions` was an array — so the merged bundle came back as a bare
+`{ name: callable }` map with `effect: 'writes'` dropped on the floor. The
+function still registered and still ran, and its writes were counted as none:
+#4396's silent un-declaring arriving by the other door, and exactly the state
+that keeps #4354's broken-sweep alert quiet on the one run that needed it. Since
+the parse rejected the array form until now, no built artifact had ever reached
+that branch; it is fixed in the same change rather than shipped as a live trap.
+The array shape is preserved, callables are attached per entry `name`, and a
+module function the artifact declared no entry for still registers — the map
+branch keeps those, and the array branch must not ship fewer functions than the
+bundle was built with.
+
+Authoring is unchanged and nothing narrows: this widens what the artifact form
+accepts. The map form is still the preferred spelling.

packages/cli/src/utils/lower-callables.test.ts

@@ -142,49 +142,70 @@ describe('lowerCallables — declared `functions` entries (#4396)', () => {
 // hand-written sample is a third copy of the truth and drifts exactly the way
 // the two halves already did.
 //
-// SCOPE: the map form. The ARRAY form (`functions: [{ name, handler }]`) does
-// not round-trip either — in both its bare and declared spellings, since #4343
-// and #4976 each only ever touched the map — and its member lives in
-// `stack.zod.ts` rather than in `FlowFunctionEntrySchema`. Filed as #6238;
-// extend the parametrisation below when it lands.
-describe('lowerCallables → the spec parses what it emits (#4976)', () => {
+// SCOPE: all four cells of map/array × bare/declared. The ARRAY form
+// (`functions: [{ name, handler }]`) was the last one still broken — in BOTH
+// its spellings, because #4343 and #4976 each only ever touched the map, while
+// `lowerCallables` has lowered the array branch the whole time. Its member
+// lives inline in `stack.zod.ts` rather than in `FlowFunctionEntrySchema`
+// (an array entry names itself, so it is a different record, not the same
+// schema in a list), which is why widening one did not widen the other. #6238
+// widened it; the parametrisation below now covers the array form too.
+describe('lowerCallables → the spec parses what it emits (#4976, #6238)', () => {
   const base = {
     manifest: { id: 'com.example.demo', name: 'demo', version: '1.0.0', type: 'app' as const },
   };
 
   /** Exactly what `objectstack compile` does, in the order it does it. */
-  const buildPipeline = (functions: Record<string, unknown>) => {
+  const buildPipeline = (functions: unknown) => {
     const stack = defineStack({ ...base, functions } as never);
     const normalized = normalizeStackInput(stack as Record<string, unknown>);
     return lowerCallables(normalized);
   };
 
-  const cases: Array<[label: string, functions: Record<string, unknown>]> = [
-    ['a bare handler', { scoreLead: () => ({ score: 1 }) }],
-    ['a declared writer', { syncBilling: { handler: () => ({ ok: true }), effect: 'writes' } }],
-    ['a declaration that states the pure default', { scoreLead: { handler: () => ({ score: 1 }), effect: 'pure' } }],
-    ['a declaration that states nothing', { scoreLead: { handler: () => ({ score: 1 }) } }],
-    ['both spellings side by side', {
+  /**
+   * `form` drives the entry-level assertion, which can only run on the map:
+   * `FlowFunctionEntrySchema` is the map entry's schema and is exported, while
+   * the array member is inline in `ObjectStackDefinitionSchema`. The array
+   * cells are pinned by the whole-stack parse below — the assertion the build
+   * actually makes.
+   */
+  const cases: Array<[label: string, form: 'map' | 'array', functions: unknown]> = [
+    ['a bare handler', 'map', { scoreLead: () => ({ score: 1 }) }],
+    ['a declared writer', 'map', { syncBilling: { handler: () => ({ ok: true }), effect: 'writes' } }],
+    ['a declaration that states the pure default', 'map', { scoreLead: { handler: () => ({ score: 1 }), effect: 'pure' } }],
+    ['a declaration that states nothing', 'map', { scoreLead: { handler: () => ({ score: 1 }) } }],
+    ['both spellings side by side', 'map', {
       scoreLead: () => ({ score: 1 }),
       syncBilling: { handler: () => ({ ok: true }), effect: 'writes' },
     }],
+    // ── the array form (#6238) ──
+    ['an array entry with a bare handler', 'array', [{ name: 'scoreLead', handler: () => ({ score: 1 }) }]],
+    ['an array entry declaring a writer', 'array', [{ name: 'syncBilling', handler: () => ({ ok: true }), effect: 'writes' }]],
+    ['an array entry declaring the pure default', 'array', [{ name: 'scoreLead', handler: () => ({ score: 1 }), effect: 'pure' }]],
+    ['an array entry carrying a packageId', 'array', [{ name: 'scoreLead', handler: () => ({ score: 1 }), packageId: 'com.example.pkg' }]],
+    ['several array entries side by side', 'array', [
+      { name: 'scoreLead', handler: () => ({ score: 1 }) },
+      { name: 'syncBilling', handler: () => ({ ok: true }), effect: 'writes' },
+    ]],
   ];
 
-  for (const [label, functions] of cases) {
-    it(`parses every entry it emits for ${label}`, () => {
-      const emitted = (buildPipeline(functions).lowered as {
-        functions: Record<string, unknown>;
-      }).functions;
-
-      for (const [name, entry] of Object.entries(emitted)) {
-        const result = FlowFunctionEntrySchema.safeParse(entry);
-        expect(
-          result.success,
-          `emitted entry '${name}' (${JSON.stringify(entry)}) is not a shape FlowFunctionEntrySchema accepts: `
-          + JSON.stringify(result.success ? [] : result.error.issues),
-        ).toBe(true);
-      }
-    });
+  for (const [label, form, functions] of cases) {
+    if (form === 'map') {
+      it(`parses every entry it emits for ${label}`, () => {
+        const emitted = (buildPipeline(functions).lowered as {
+          functions: Record<string, unknown>;
+        }).functions;
+
+        for (const [name, entry] of Object.entries(emitted)) {
+          const result = FlowFunctionEntrySchema.safeParse(entry);
+          expect(
+            result.success,
+            `emitted entry '${name}' (${JSON.stringify(entry)}) is not a shape FlowFunctionEntrySchema accepts: `
+            + JSON.stringify(result.success ? [] : result.error.issues),
+          ).toBe(true);
+        }
+      });
+    }
 
     it(`parses the whole lowered stack for ${label}`, () => {
       // The assertion the build itself makes (`compile.ts` step 3). Parsing the
@@ -217,4 +238,19 @@ describe('lowerCallables → the spec parses what it emits (#4976)', () => {
     expect(JSON.parse(JSON.stringify(lowered)).functions.syncBilling)
       .toEqual({ handler: 'syncBilling', effect: 'writes' });
   });
+
+  it('carries an ARRAY entry\'s declaration into the artifact too (#6238)', () => {
+    // Same guarantee, other spelling. The array branch of `lowerCallables`
+    // rewrites `name` to the ref as well as `handler`, so both keys must come
+    // back as the ref and `effect` must survive beside them.
+    const { lowered } = buildPipeline([
+      { name: 'syncBilling', handler: () => ({ ok: true }), effect: 'writes' },
+    ]);
+    const parsed = ObjectStackDefinitionSchema.parse(lowered) as {
+      functions: Array<{ name: string; handler: string; effect: string }>;
+    };
+    expect(parsed.functions).toEqual([{ name: 'syncBilling', handler: 'syncBilling', effect: 'writes' }]);
+    expect(JSON.parse(JSON.stringify(lowered)).functions)
+      .toEqual([{ name: 'syncBilling', handler: 'syncBilling', effect: 'writes' }]);
+  });
 });

packages/runtime/src/artifact-function-declarations.test.ts

@@ -62,6 +62,52 @@ describe('mergeRuntimeModule — declared functions', () => {
         expect((entries.syncBilling.handler as () => unknown)()).toEqual({ ok: true });
     });
 
+    it('re-attaches into the ARRAY form without dropping what it declared (#6238)', async () => {
+        // The array spelling reaches this seam for the first time now that
+        // #6238 lets it past the parse. Rebuilding it as a map would attach the
+        // callable and drop `effect` beside it — the same silent un-declaring
+        // #4396 fixed for the map form, arriving by the other door.
+        const bundle: any = {
+            runtimeModule: './objectstack-runtime.mjs',
+            // What `lowerCallables`'s array branch emits: `name` and `handler`
+            // both rewritten to the ref, the declaration kept beside them.
+            functions: [
+                { name: 'scoreLead', handler: 'scoreLead' },
+                { name: 'syncBilling', handler: 'syncBilling', effect: 'writes' },
+            ],
+        };
+
+        await mergeRuntimeModule(bundle, artifactPath);
+
+        expect(Array.isArray(bundle.functions)).toBe(true);
+        const [scoreLead, syncBilling] = bundle.functions;
+        expect(typeof scoreLead.handler).toBe('function');
+        expect(typeof syncBilling.handler).toBe('function');
+        expect(syncBilling.effect).toBe('writes');
+        expect(syncBilling.name).toBe('syncBilling');
+
+        const entries = collectBundleFunctionEntries(bundle);
+        expect(entries.scoreLead.effect).toBe('pure');
+        expect(entries.syncBilling.effect).toBe('writes');
+        expect((entries.syncBilling.handler as () => unknown)()).toEqual({ ok: true });
+    });
+
+    it('registers a module function the array form declared no entry for (#6238)', async () => {
+        // The map branch keeps these; the array branch must not ship fewer
+        // functions than the bundle was built with.
+        const bundle: any = {
+            runtimeModule: './objectstack-runtime.mjs',
+            functions: [{ name: 'syncBilling', handler: 'syncBilling', effect: 'writes' }],
+        };
+
+        await mergeRuntimeModule(bundle, artifactPath);
+
+        const entries = collectBundleFunctionEntries(bundle);
+        expect(Object.keys(entries).sort()).toEqual(['scoreLead', 'syncBilling']);
+        expect(entries.syncBilling.effect).toBe('writes');
+        expect((entries.scoreLead.handler as () => unknown)()).toEqual({ score: 1 });
+    });
+
     it('leaves a bundle with no runtimeModule alone', async () => {
         const handler = () => ({ ok: true });
         const bundle: any = { functions: { syncBilling: { handler, effect: 'writes' } } };

packages/runtime/src/load-artifact-bundle.ts

@@ -142,7 +142,39 @@ export async function mergeRuntimeModule(bundle: any, artifactAbsPath: string, t
             console.warn(`${tag} runtime module '${moduleAbsPath}' exported no \`functions\` map`);
             return;
         }
-        const existing = (bundle.functions && typeof bundle.functions === 'object' && !Array.isArray(bundle.functions))
+        // The ARRAY form (`[{ name, handler: '<ref>', effect }]`) carries its
+        // declaration exactly like the map form, but names itself by an entry's
+        // `name` instead of by a map key. Rebuilding it as a map below would
+        // attach the callable and drop everything standing beside it —
+        // `effect: 'writes'` included — which is #4396's silent un-declaring in
+        // the other spelling: the function still registers, still runs, and its
+        // writes are still counted as none, so #4354's broken-sweep alert stays
+        // quiet on the one run that needed it. Unreachable until #6238 let the
+        // array form past the parse; reachable now, so it is handled here.
+        if (Array.isArray(bundle.functions)) {
+            const moduleFns = fns as Record<string, unknown>;
+            const attached = new Set<string>();
+            const mergedEntries = (bundle.functions as unknown[]).map((entry) => {
+                if (!entry || typeof entry !== 'object' || Array.isArray(entry)) return entry;
+                const record = entry as Record<string, unknown>;
+                const name = typeof record.name === 'string' ? record.name : undefined;
+                if (name === undefined) return entry;
+                const fn = moduleFns[name];
+                if (typeof fn !== 'function') return entry;
+                attached.add(name);
+                return { ...record, handler: fn };
+            });
+            // A module function the artifact declared no entry for still has to
+            // register — the map branch keeps those, and dropping them here
+            // would make the array form quietly ship fewer functions than it
+            // was built with.
+            for (const [name, fn] of Object.entries(moduleFns)) {
+                if (typeof fn === 'function' && !attached.has(name)) mergedEntries.push({ name, handler: fn });
+            }
+            bundle.functions = mergedEntries;
+            return;
+        }
+        const existing = (bundle.functions && typeof bundle.functions === 'object')
             ? bundle.functions as Record<string, unknown>
             : {};
         // The module supplies the CALLABLE; the JSON supplies what the function

packages/spec/src/stack.zod.ts

@@ -441,12 +441,36 @@ export const ObjectStackDefinitionSchema = lazySchema(() => z.object({
    * (what it declared rides along in the artifact and is re-attached on load).
    * The `AppPlugin` registers them on the engine before binding hooks so
    * `string` handlers resolve at startup.
+   *
+   * BOTH shapes therefore reach this schema twice: once as authored, once
+   * lowered. All four combinations (map/array × bare/declared) are accepted —
+   * the map's two lowered forms since #4343 and #4976, the array's since #6238.
+   * `packages/cli`'s `lower-callables.test.ts` pins every cell against what the
+   * lowering actually emits, rather than against a belief about it.
    */
   functions: z.union([
     z.record(z.string(), FlowFunctionEntrySchema),
+    // The array member is NOT `FlowFunctionEntrySchema` in a list: an array
+    // entry carries its own `name` (and an optional `packageId`) because it has
+    // no map key to be named by, so the two shapes are genuinely different
+    // records rather than one reused schema.
+    //
+    // `handler` accepts the lowered string ref for the same reason the map form
+    // does (#4343, #4976), and this member was the last place it did not:
+    // `lowerCallables` lowers the array branch too (`next.handler = ref`,
+    // `next.name = ref`), so `objectstack build` emitted
+    // `[{ name: 'syncBilling', handler: 'syncBilling', effect: 'writes' }]` into
+    // a schema that still demanded a callable — `invalid_union: Invalid input`,
+    // path stopping at `functions`, naming neither the entry nor the key. One
+    // widening covers BOTH array spellings at once, unlike the map form's two
+    // separate members: `effect` is already optional here, so the bare and the
+    // declared entry differ only in whether that key is present.
     z.array(z.object({
       name: z.string(),
-      handler: z.function(),
+      handler: z.union([
+        z.function(),
+        z.string().min(1).describe('The lowered handler ref (built artifacts) — the callable rides in the sibling ESM module'),
+      ]).describe('The function invoked by name — the authored callable, or the ref `objectstack build` lowered it to'),
       packageId: z.string().optional(),
       effect: FlowFunctionEffectSchema.optional(),
     })),