fix(cli): pass unrecognised functions entries through the lowering (#7318)

.changeset/lower-callables-functions-passthrough.md

@@ -0,0 +1,38 @@
+---
+"@objectstack/cli": patch
+---
+
+fix(cli): stop `lowerCallables` deleting the `functions` entries it does not recognise (#7318)
+
+The map branch of the top-level `functions` lowering REBUILT the map instead of
+editing it: `out` admitted an entry only in the three shapes it knew — a bare
+callable, `{ handler: callable }`, or a plain string ref — and everything else
+was dropped. No error, no warning, no key. Two distinct failures came out of
+that one line.
+
+**A built artifact could not be lowered again.** The already-lowered declaration
+`{ handler: 'syncBilling', effect: 'writes' }` — the shape this very step emits
+for a declared writer, and the one `FlowFunctionLoweredDeclarationSchema` was
+added to accept in #4976 — matched none of the recognised shapes. A second pass
+therefore deleted the key outright, silently un-declaring the writer the first
+pass had gone out of its way to keep. Lowering is now idempotent: lower a
+lowered stack and the `functions` key set and the declared entries are
+unchanged, in both the map and the array spelling.
+
+**A malformed entry was destroyed rather than reported.** The headless husk
+`{ effect: 'writes' }` — a declaration for a function that is not there, which
+is exactly what a plain `JSON.stringify(stack)` leaves where a declared writer
+was (#6293) — reached the lowering and left it as `functions: {}`. The stack
+then parsed GREEN, so `objectstack build` wrote an artifact missing the function
+instead of refusing, and the evidence had been deleted before the parse could
+name it.
+
+Unrecognised entries now ride through under their own key, untouched, and
+`FlowFunctionEntrySchema` decides. The husk is refused where the build actually
+checks — `invalid_union` on `functions`, with the offending key nameable in the
+branch tree, which `formatZodErrors` (#5341) prints in the terminal.
+
+Nothing changes for a stack that was building correctly: bare callables, declared
+callables, pre-existing string refs and the array form all lower exactly as
+before. A stack that was silently shipping a `functions` map missing an entry now
+fails its build, naming `functions` — which is the point.

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

@@ -1,6 +1,7 @@
 // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
 
 import { describe, it, expect } from 'vitest';
+import type { z } from 'zod';
 import { defineStack, normalizeStackInput, ObjectStackDefinitionSchema } from '@objectstack/spec';
 import { FlowFunctionEntrySchema } from '@objectstack/spec/automation';
 import { lowerCallables } from './lower-callables.js';
@@ -254,3 +255,119 @@ describe('lowerCallables → the spec parses what it emits (#4976, #6238)', () =
       .toEqual([{ name: 'syncBilling', handler: 'syncBilling', effect: 'writes' }]);
   });
 });
+
+// ── #7318: the `functions` map branch is a lowering, not a filter ───────────
+//
+// The map branch REBUILT the map from the shapes it recognised, so anything
+// else was deleted before the parse could see it. Two failures came out of that
+// one line, and both are pinned here:
+//
+//   1. Lowering stopped being IDEMPOTENT. The already-lowered declaration
+//      `{ handler: 'syncBilling', effect: 'writes' }` — which #4976 taught
+//      `FlowFunctionEntrySchema` to accept, and which is exactly what the first
+//      pass emits — matched none of the recognised shapes, so a second pass
+//      dropped the key entirely and silently un-declared the writer the first
+//      pass had gone out of its way to keep.
+//   2. A MALFORMED entry was destroyed rather than reported. The headless husk
+//      `{ effect: 'writes' }` (what a plain `JSON.stringify(stack)` leaves
+//      where a declaration was, #6293) left the lowering as `functions: {}` and
+//      the stack then parsed GREEN — the build writing an artifact missing the
+//      function instead of refusing.
+describe('lowerCallables — unrecognised `functions` entries reach the parse (#7318)', () => {
+  const base = {
+    manifest: { id: 'com.example.demo', name: 'demo', version: '1.0.0', type: 'app' as const },
+  };
+
+  /** `objectstack compile`'s first three steps, then `JSON.stringify` — the artifact. */
+  const buildArtifact = (functions: unknown) => {
+    const stack = defineStack({ ...base, functions } as never);
+    const { lowered } = lowerCallables(normalizeStackInput(stack as Record<string, unknown>));
+    return JSON.parse(JSON.stringify(lowered)) as Record<string, unknown>;
+  };
+
+  const functionsOf = (stack: Record<string, unknown>) =>
+    stack.functions as Record<string, unknown>;
+
+  /** Every `path` in a Zod error, including the branches folded inside a union. */
+  const allIssuePaths = (issues: readonly z.core.$ZodIssue[], prefix: PropertyKey[] = []): string[] =>
+    issues.flatMap((issue) => {
+      const path = [...prefix, ...issue.path];
+      const nested = 'errors' in issue && Array.isArray(issue.errors)
+        ? (issue.errors as z.core.$ZodIssue[][]).flatMap((branch) => allIssuePaths(branch, path))
+        : [];
+      return [path.join('.'), ...nested];
+    });
+
+  it('lowering a lowered stack changes nothing — same keys, same declarations', () => {
+    // The artifact carries BOTH lowered shapes: a bare ref and a lowered
+    // declaration. Neither may move, and no key may go missing.
+    const once = buildArtifact({
+      scoreLead: () => ({ score: 1 }),
+      syncBilling: { handler: () => ({ ok: true }), effect: 'writes' },
+    });
+    expect(functionsOf(once)).toEqual({
+      scoreLead: 'scoreLead',
+      syncBilling: { handler: 'syncBilling', effect: 'writes' },
+    });
+
+    const second = lowerCallables(once);
+
+    expect(
+      Object.keys(functionsOf(second.lowered)).sort(),
+      'the key set of an already-lowered `functions` map must survive a second pass',
+    ).toEqual(Object.keys(functionsOf(once)).sort());
+    expect(functionsOf(second.lowered)).toEqual(functionsOf(once));
+    // Nothing was left to lower, so nothing was registered — a lowered artifact
+    // carries its callables in the sibling module, not here.
+    expect(second.count).toBe(0);
+    expect(ObjectStackDefinitionSchema.safeParse(second.lowered).success).toBe(true);
+  });
+
+  it('is idempotent for the ARRAY form too', () => {
+    const once = buildArtifact([{ name: 'syncBilling', handler: () => ({ ok: true }), effect: 'writes' }]);
+    const second = lowerCallables(once);
+    expect(second.lowered.functions).toEqual(once.functions);
+    expect(second.count).toBe(0);
+  });
+
+  it('keeps a pre-existing bare string ref under its own key (legacy bundles)', () => {
+    const { lowered } = lowerCallables({ functions: { legacy: 'legacy' } });
+    expect(lowered.functions).toEqual({ legacy: 'legacy' });
+  });
+
+  it('passes the headless husk through, so the parse refuses it by key', () => {
+    // The card's measured case. `{ sweep: { effect: 'writes' } }` is what
+    // `JSON.stringify` leaves of a declared writer — a declaration for a
+    // function that is not there.
+    const husk = { sweep: { effect: 'writes' } };
+    const { lowered, count } = lowerCallables({ ...base, functions: husk });
+
+    expect(
+      lowered.functions,
+      'the husk must reach the artifact intact — deleting it here is what made the bad build green',
+    ).toEqual(husk);
+    expect(count).toBe(0);
+
+    // Refused at the entry…
+    const entry = FlowFunctionEntrySchema.safeParse(husk.sweep);
+    expect(entry.success).toBe(false);
+    expect(entry.success ? [] : entry.error.issues.map((i) => i.code)).toContain('invalid_union');
+
+    // …and refused by the whole-stack parse the build actually runs, with the
+    // offending key nameable in the tree rather than an `invalid_union` that
+    // stops at `functions`.
+    const result = ObjectStackDefinitionSchema.safeParse(lowered);
+    expect(result.success, 'a stack whose `functions` map holds a husk must NOT parse green').toBe(false);
+    const paths = result.success ? [] : allIssuePaths(result.error.issues);
+    expect(paths).toContain('functions');
+    expect(paths, 'the rejection must name the key it is about').toContain('functions.sweep');
+  });
+
+  it('refuses a declaration whose `handler` is neither callable nor a ref', () => {
+    // Same rule, the other way a declaration goes wrong: the key is kept and
+    // the schema gets to name it.
+    const { lowered } = lowerCallables({ ...base, functions: { sweep: { handler: 42, effect: 'writes' } } });
+    expect(lowered.functions).toEqual({ sweep: { handler: 42, effect: 'writes' } });
+    expect(ObjectStackDefinitionSchema.safeParse(lowered).success).toBe(false);
+  });
+});

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

@@ -165,12 +165,34 @@ export function lowerCallables(input: Record<string, unknown>): LoweringResult {
         taken.add(ref);
         functions[ref] = value.handler as AnyFn;
         out[ref] = { ...value, handler: ref };
+      } else {
+        // NOTHING ELSE IS THIS STEP'S TO JUDGE (#7318). Everything that is not
+        // a callable to lower rides through under its own key, untouched, and
+        // `FlowFunctionEntrySchema` decides whether it is legal.
+        //
+        // Two kinds of value arrive here, and passing both through is the same
+        // decision, not a compromise between two:
+        //
+        //   ALREADY LOWERED — a bare ref (`'scoreLead'`, #4343) or a lowered
+        //   declaration (`{ handler: 'scoreLead', effect: 'writes' }`, #4976).
+        //   Both are shapes the schema accepts, so lowering a lowered stack
+        //   must be a no-op: same key set, same declarations. Rebuilding the
+        //   map around a fixed list of recognised shapes made that false — the
+        //   lowered declaration matched none of them and was deleted, so a
+        //   second pass (a re-lowered artifact, a fixture that lowers what it
+        //   read back) silently un-declared the writer the FIRST pass had
+        //   carefully kept.
+        //
+        //   MALFORMED — the headless husk `{ effect: 'writes' }` that a plain
+        //   `JSON.stringify(stack)` leaves where a declaration was (#6293).
+        //   Deleting it here erased the evidence BEFORE the parse: the artifact
+        //   came out `functions: {}` and validated green, so the build shipped
+        //   an app missing the function instead of refusing. Handed on, it
+        //   reaches `FlowFunctionEntrySchema`, which names it — `invalid_union`
+        //   on this key — and `objectstack build` fails where it should.
+        out[key] = value;
       }
     }
-    // Preserve any pre-existing string entries (legacy bundles).
-    for (const [key, value] of Object.entries(fnsField)) {
-      if (typeof value === 'string') out[key] = value;
-    }
     (lowered as Record<string, unknown>).functions = out;
   }
 

packages/qa/dogfood/test/build-shaped-artifact.ts

@@ -20,12 +20,13 @@
 // The declared entry made a noise ONCE, on the path where the residue is fed
 // straight to the parse: `FlowFunctionEntrySchema` refuses an entry declaring an
 // effect for a function it does not carry, and that red CI job is the only
-// reason anybody learned about this (#4976). Measured here, it is not a general
-// guarantee — put the same husk back through the lowering and it never reaches
-// the schema at all (see the key-for-key check below). The BARE entry never made
-// a noise on any path: it vanishes key and all, the artifact holds
-// `functions: {}`, and the fixture parses green carrying zero of what it
-// advertises.
+// reason anybody learned about this (#4976). Putting the same husk back through
+// the lowering used to silence even that — the map branch deleted the entry
+// before the schema saw it — until #7318 taught the lowering to hand an
+// unrecognised entry on unchanged, so the refusal now happens on both paths.
+// The BARE entry still makes no noise anywhere: it vanishes key and all before
+// this module is ever called, the artifact holds `functions: {}`, and the
+// fixture parses green carrying zero of what it advertises.
 // `showcase-declarative-endpoints.dogfood.test.ts` shipped exactly that for its
 // whole existence. AGENTS.md, "Absence must be loud": a verifier that silently
 // degrades is worse than no verifier.
@@ -174,14 +175,22 @@ export function buildShapedArtifact(stack: Record<string, unknown>): BuildShaped
     );
   }
 
-  // Key-for-key on the `functions` MAP, which the lowering rebuilds rather than
-  // edits: its `out` object admits an entry only in the three shapes it knows
-  // (a callable, `{ handler: callable }`, a string ref), and anything else is
-  // dropped — no error, no warning, no key. Measured on this exact stack: hand
-  // the lowering the `{ effect: 'writes' }` husk `JSON.stringify` leaves behind
-  // and the artifact comes out with `functions: {}`, parsing green, which is the
-  // #6293 failure wearing a different hat. The parse below cannot see it: by the
-  // time it runs, the evidence has been deleted.
+  // Key-for-key on the `functions` MAP. This was the live gate until #7318: the
+  // lowering rebuilt the map and admitted an entry only in the three shapes it
+  // knew (a callable, `{ handler: callable }`, a string ref), dropping anything
+  // else — no error, no warning, no key. Measured on this exact stack then:
+  // hand the lowering the `{ effect: 'writes' }` husk `JSON.stringify` leaves
+  // behind and the artifact came out `functions: {}`, parsing green, which is
+  // the #6293 failure wearing a different hat; the parse below could not see it,
+  // because by the time it ran the evidence had been deleted.
+  //
+  // The producer was fixed at the source — an entry `lowerCallables` does not
+  // recognise now rides through under its own key and the parse below refuses
+  // it by name — so this check no longer has anything to catch on that path.
+  // It is KEPT as the backstop it always was: it is the only assertion that
+  // reconciles the input's `functions` keys against the output's, so a future
+  // lowering that starts deleting again fails here, named, instead of shrinking
+  // this stand-in in silence.
   const inputFns = normalized.functions;
   if (isPlainObject(inputFns)) {
     const kept = new Set(Object.keys((lowering.lowered.functions ?? {}) as Record<string, unknown>));

packages/qa/dogfood/test/showcase-declarative-endpoints.dogfood.test.ts

@@ -194,14 +194,18 @@ describe('[#6293] the stand-in artifact carries what a built one carries', () =>
   it('REFUSES to build an artifact out of that residue instead of quietly shrinking', () => {
     // The reverse verification, kept in the suite rather than done once by hand:
     // feed the helper the very thing this fixture used to write and it must
-    // fail, loudly, naming what went missing. Direction predicted before it was
-    // run — and the mechanism is NOT the one #4976 documented. The schema never
-    // sees the husk: `lowerCallables` rebuilds the `functions` map from the three
-    // shapes it recognises and deletes everything else, so the residue would have
-    // reached the parse as `functions: {}` and passed. The gate that speaks here
-    // is the helper's own key-for-key reconciliation.
+    // fail, loudly. Which gate speaks CHANGED in #7318, and the new one is the
+    // mechanism #4976 documented: `lowerCallables` used to rebuild the
+    // `functions` map from the shapes it recognised and delete everything else,
+    // so the residue reached the parse as `functions: {}` and passed — only the
+    // helper's own key-for-key reconciliation caught it. The lowering now hands
+    // an unrecognised entry ON, so the husk reaches `FlowFunctionEntrySchema`
+    // and the SPEC refuses it, here and in `objectstack build` alike. The
+    // reconciliation stays as the backstop for a producer that starts dropping
+    // again.
     const residue = JSON.parse(JSON.stringify(showcaseStack)) as Record<string, unknown>;
-    expect(() => buildShapedArtifact(residue)).toThrowError(/dropped 1 `functions` entr.*sweepProjectHealth/s);
+    expect(() => buildShapedArtifact(residue))
+      .toThrowError(/does not satisfy ObjectStackDefinitionSchema[\s\S]*functions/);
   });
 });