From d3e8e155f38e87343101efd730064d8e4573d99e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 07:17:19 +0000 Subject: [PATCH] =?UTF-8?q?fix(spec,runtime):=20`functions:=20[{=20name,?= =?UTF-8?q?=20handler=20}]`=20=E6=95=B0=E7=BB=84=E5=BD=A2=E6=89=9B?= =?UTF-8?q?=E5=BE=97=E4=BD=8F=20`objectstack=20build`=20(#6238)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 顶层 `functions` 的数组形通不过它自己的 build。`lowerCallables` 一直在降级数组 分支(把 `handler` 和 `name` 双双改写成 ref),但 `stack.zod.ts` 里 `functions` union 的数组成员仍然只收 `handler: z.function()`。于是 `objectstack build` 亲手 产出 `[{ name: 'syncBilling', handler: 'syncBilling', effect: 'writes' }]`,再由 自己拒收:`invalid_union: Invalid input`,路径止步于 `functions`,不点名条目、 不点名键、不给原因。 同一条缝第三次裂开,前两次修复正是这次看着小的原因。#4343 教会 union 认裸 lowered ref;#4976 教会它认 lowered *declaration*。两次都只动了 **map** 成员 —— 数组成员是另一条内联记录(数组条目自带 `name`,还有可选 `packageId`,因此不可能 是 `FlowFunctionEntrySchema` 装进列表),加宽其一从不加宽其二。 ## 前提复核(实测,非推断) 分诊的机制假设「对称加宽 `handler` 即足」**实测证实**。改动前跑真实流水线 (`defineStack` → `normalizeStackInput` → `lowerCallables` → parse)的四格读数: | 格 | 降级产出 | parse | |---|---|---| | map / bare | `{"scoreLead":"scoreLead"}` | ACCEPT | | map / declared | `{"syncBilling":{handler,effect}}` | ACCEPT | | array / bare | `[{"name","handler":"scoreLead"}]` | **REJECT** `invalid_union` @ `functions` | | array / declared | `[{name,handler,effect}]` | **REJECT** | 两个数组格栽在**同一个**成员上,所以一次加宽同时收两种拼写 —— 这点与 map 形不同: 数组条目的 `effect` 本就 optional,bare 与 declared 只差这个键在不在,不需要 #4976 那样的第二个成员。未照抄 #4976 的派生模式,因为两者结构确实不同。 ## 改动 1. **`packages/spec/src/stack.zod.ts`** —— 数组成员的 `handler` 在原有 callable 之外收下 lowered 字符串 ref。四格全部 round-trip。 2. **`packages/runtime/src/load-artifact-bundle.ts`** —— 本次加宽**让其可达**的装载 缝。`mergeRuntimeModule` 的数组分支会掉进 map 重建(`bundle.functions` 是数组时 `existing` 恒为 `{}`),于是合并后的 bundle 变成裸 `{ name: callable }` map, `effect: 'writes'` 被丢弃。函数照样注册、照样运行,写入却被计为零 —— 正是 #4396 修掉的静默 un-declaring 换了扇门进来,也正是让 #4354 broken-sweep 告警对最需要它 的那次 run 保持沉默的状态。实测读数: 改前 [ARRAY] collected: {"syncBilling":{handler, effect:"pure"}} ← 声明丢失 改后 [ARRAY] collected: {"syncBilling":{handler, effect:"writes"}} ← 与 map 形一致 数组形在 parse 这关被拒之前,没有任何构建产物到过这条分支;与其把它当作活陷阱 一起发布,不如同一次改掉。保留数组形状、按条目 `name` 挂回 callable;产物未声明 条目的模块函数照旧注册(map 分支就是这么做的,数组分支不该比 bundle 构建时少发 函数)。⚠️ 这一处越出 issue 字面范围,PM 若要拆分可单独摘出该 hunk 与其两条测试。 ## 验收(= 扩展 #4976 的跨界 round-trip pin) `packages/cli/src/utils/lower-callables.test.ts` 的参数化正是 #4976 预留的槽位 (原文:「SCOPE: the map form … Filed as #6238; extend the parametrisation below when it lands」)。现覆盖 map/array × bare/declared 全四格,外加 packageId 与多条目: **24 passed(原 18)**。数组格由整栈 parse 钉住(数组成员内联于 `ObjectStackDefinitionSchema`,无导出的单条目 schema),与 build 实际做的断言一致。 ## 逆向验证(方向先判后跑,真实读数) - 撤回 union 新成员 → 预判「恰好 6 条数组行转红」→ 实测 **6 failed | 18 passed**, 红的正是 5 条数组整栈 parse + 1 条数组产物断言;map 行全绿。恢复后 24/24 绿。 - 撤回 runtime 数组分支 → 预判「恰好 2 条新增 #6238 runtime 测试转红」→ 实测 **2 failed | 2 passed**,既有两条不受影响。恢复后 4/4 绿。 ## 验证读数 - `@objectstack/spec` 测试 **8808 passed / 343 files**;`@objectstack/cli` **983 / 94**;`@objectstack/runtime` **1613 / 111**。 - typecheck:spec、cli、runtime 三包干净。 - `check:generated` **✓ All 10 generated artifacts are up to date**(10/10,无生成物漂移)。 - `check:spec-parsed-alias` 通过(18 self-test + 1443 别名核对)。 - `pnpm lint` 干净。 - 三个示例应用 `validate` 全部 **exit=0**(crm / showcase / todo;输出中的告警为既有, 与 `functions` 无关)。 - 已 merge 最新 origin/main(82397b68f)后重跑 pin、runtime 测试与 check:generated,均绿。 ## 未做 / 留给裁决 数组形在本仓库**零作者**(examples/apps/packages 无一处以数组形书写)。但它是 spec 里第一等的公开形状 —— `stack.zod.ts` 文档注释明列、`lowerCallables` 与 `collectBundleFunctionEntries` 都专门支持、`composeStacks` 还为 map/array 冲突 报错,且 #4976 的测试文件亲自预留了本次修复的槽位。因此按「修」而非「退役」处理; 退役属 ADR-0087 全套,另一个 program,未在此静默删除。 changeset:`@objectstack/spec` minor(接受面加宽)+ `@objectstack/runtime` patch。 --- .changeset/functions-array-lowered-handler.md | 46 ++++++++++ .../cli/src/utils/lower-callables.test.ts | 92 +++++++++++++------ .../artifact-function-declarations.test.ts | 46 ++++++++++ packages/runtime/src/load-artifact-bundle.ts | 34 ++++++- packages/spec/src/stack.zod.ts | 26 +++++- 5 files changed, 214 insertions(+), 30 deletions(-) create mode 100644 .changeset/functions-array-lowered-handler.md diff --git a/.changeset/functions-array-lowered-handler.md b/.changeset/functions-array-lowered-handler.md new file mode 100644 index 0000000000..62d6dac22d --- /dev/null +++ b/.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. diff --git a/packages/cli/src/utils/lower-callables.test.ts b/packages/cli/src/utils/lower-callables.test.ts index 2c8c3f7d10..448bd78679 100644 --- a/packages/cli/src/utils/lower-callables.test.ts +++ b/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) => { + const buildPipeline = (functions: unknown) => { const stack = defineStack({ ...base, functions } as never); const normalized = normalizeStackInput(stack as Record); return lowerCallables(normalized); }; - const cases: Array<[label: string, functions: Record]> = [ - ['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; - }).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; + }).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' }]); + }); }); diff --git a/packages/runtime/src/artifact-function-declarations.test.ts b/packages/runtime/src/artifact-function-declarations.test.ts index 141e369c70..5b8c423ed9 100644 --- a/packages/runtime/src/artifact-function-declarations.test.ts +++ b/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' } } }; diff --git a/packages/runtime/src/load-artifact-bundle.ts b/packages/runtime/src/load-artifact-bundle.ts index eb318b5ed4..af71b4773b 100644 --- a/packages/runtime/src/load-artifact-bundle.ts +++ b/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: '', 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; + const attached = new Set(); + const mergedEntries = (bundle.functions as unknown[]).map((entry) => { + if (!entry || typeof entry !== 'object' || Array.isArray(entry)) return entry; + const record = entry as Record; + 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 : {}; // The module supplies the CALLABLE; the JSON supplies what the function diff --git a/packages/spec/src/stack.zod.ts b/packages/spec/src/stack.zod.ts index bc6880d8fb..485687e663 100644 --- a/packages/spec/src/stack.zod.ts +++ b/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(), })),