From 6a6713e460b8f7400b5d096079c753310c8debd9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 01:04:30 +0000 Subject: [PATCH 1/2] fix(spec)!: composeStacks no longer silently drops top-level keys (#5005) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit composeStacks built its result from an empty object, filling in manifest, i18n, objects and a hand-maintained array whitelist. Anything absent from that whitelist was DELETED — no error, no warning, and `undefined` at the consumer is indistinguishable from "the author never wrote it". Composition is the platform's app-packaging/install story, so the silence reached security config: `api.enforceProjectMembership` (the per-environment 403 gate) and `server.security.rateLimit` (#4910) both vanished the moment a stack was composed with any other one, as did `functions` (every declarative handler), seven declared array collections (datasourceMapping, datasets, jobs, emailTemplates, docs, books, tiers) and `runtimeModule`. Per the 2026-08-04 maintainer verdict: 1. same value in several stacks composes fine (deep equality); 2. differing values THROW, naming the key, both source stacks and the two ways out — NOT last-wins (a silent security downgrade: an add-on package switching off an earlier stack's 403 gate) and NOT deep-merge (a third value neither author wrote); 3. a top-level key with no declared composition rule WARNS and is composed by the default, so the next new key reports itself instead of being found by accident the way `server:` was. Array keys keep their concat semantics unchanged. `functions` merges by name (composing CRM + Todo must yield both packages' handlers) and throws on a duplicate name; the map and array forms are merged in kind, never converted (an array entry carries `packageId` the map entry cannot hold). `i18n` keeps its pre-existing last-wins — it is the one key here that already had a working strategy, and #5005's subject is keys that were dropped. The whitelist made forgetting the default; the replacement is a total disposition table typed `Record< keyof ObjectStackDefinition, ... >`, so a new top-level key does not compile until someone states what composing it means. The runtime warn covers what the type cannot see (`strict: false`, hand-built stack objects). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Ehu85kbvMcrNTUJjwxvLJ9 --- .../compose-stacks-no-silent-key-loss.md | 52 +++ .../spec/src/compose-stacks-key-loss.test.ts | 346 +++++++++++++++ packages/spec/src/stack.zod.ts | 408 ++++++++++++++++-- 3 files changed, 772 insertions(+), 34 deletions(-) create mode 100644 .changeset/compose-stacks-no-silent-key-loss.md create mode 100644 packages/spec/src/compose-stacks-key-loss.test.ts diff --git a/.changeset/compose-stacks-no-silent-key-loss.md b/.changeset/compose-stacks-no-silent-key-loss.md new file mode 100644 index 0000000000..e8c2ea746a --- /dev/null +++ b/.changeset/compose-stacks-no-silent-key-loss.md @@ -0,0 +1,52 @@ +--- +"@objectstack/spec": major +--- + +fix(spec)!: `composeStacks` 不再静默丢弃顶层键 —— 同值放行、冲突报错、未声明规则必警 (#5005) + +`composeStacks` 从一个空对象开始逐项填充:`manifest`、`i18n`、`objects`,再加一份 +手工维护的数组白名单。**不在白名单里的顶层键不是"原样保留",而是被删除** —— +不报错、不告警,消费方看到的 `undefined` 与"作者从没写过"完全无法区分。 + +组合栈是平台的应用打包/安装承载,所以这份静默一路蔓延到了安全配置: + +| 顶层键 | 谁消费 | 组合后(修复前) | +|:--|:--|:--| +| `api`(含 `enforceProjectMembership` 每环境成员 403 闸门) | `objectstack serve` → REST + dispatcher | **丢** | +| `server`(`security.rateLimit` / `trustProxy`,#4910) | `objectstack serve` → 入站限流器 | **丢** | +| `functions`(声明式 hook / action / script 节点按名解析的 handler) | `AppPlugin` 启动绑定 | **丢** | +| `datasourceMapping`、`datasets`、`jobs`、`emailTemplates`、`docs`、`books`、`tiers` | 各自运行时 | **丢**(声明为数组,却漏进白名单) | +| `runtimeModule` | 构建产物的 ESM handler bundle | **丢** | + +`stacks.length === 1` 时 `composeStacks` 原样返回,所以单栈一切正常 —— 只有真正 +≥2 个栈才丢,这是它至今没被发现的原因。ADR-0109 当年也只是给 `tools` 单独补了 +一行白名单,并没有堵住这一类。 + +## 新语义(维护者 2026-08-04 裁决) + +1. **同值放行** —— 多个栈声明同一个非数组顶层键且值深相等,照常合成。 +2. **冲突报错**,错误信息点名冲突键、两个来源栈(manifest id,无 manifest 时用 + `stack #N`)与两条出路(改一致 / 只在应当拥有它的那个栈里保留)。 + ⛔ **不做 last-wins** —— 后组合的包无声关掉前一个栈的 403 闸门或收紧过的限流 + 预算,正是本单要消灭的静默安全降级;⛔ **不做 deep-merge** —— 那会造出一个两 + 位作者都没写过的第三种值。 +3. **未声明规则的顶层键必警** —— 按默认规则合成(数组拼接,其余按单值规则)**并** + 点名告警指向 #5005,而不是消失。 + +数组键的拼接语义一字不变。`functions` 按名合并(组合 CRM + Todo 必须两边的 +handler 都在),重名报错而非择一;两种书写形态(map / array)不互转(array 条目 +带 `packageId`,map 条目没有位置放它),混用报错。`i18n` 保留既有 last-wins —— +它是这里唯一本来就有明确策略的键,本单主题是"被丢掉的键",不动它。 + +## 结构性保证 + +顶层键的处置表类型是 `Record< keyof ObjectStackDefinition, ComposeDisposition >`, +**新增一个顶层键而没说清它怎么合成,`tsc --noEmit` 直接不过**。白名单让"忘记"成为 +默认,处置表让它成为编译错误;运行时那条 warn 兜住类型看不见的入口 +(`strict: false`、手搓 stack 对象)。 + +## 破坏性 + +组合两个对 `api` / `server` / `runtimeModule` 声明了**不同**值的栈,过去静默丢弃、 +现在抛错;`functions` 重名同理。这正是要的:过去"成功"的那次组合,产出的是一个 +少了闸门或少了 handler 的栈。改法见错误信息里的处方。 diff --git a/packages/spec/src/compose-stacks-key-loss.test.ts b/packages/spec/src/compose-stacks-key-loss.test.ts new file mode 100644 index 0000000000..c84283c4b6 --- /dev/null +++ b/packages/spec/src/compose-stacks-key-loss.test.ts @@ -0,0 +1,346 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #5005 — `composeStacks` must never silently drop a top-level key. + * + * Composition is the platform's app-packaging / install story: an author writes + * `server.security.rateLimit` or `api.enforceProjectMembership`, it works in a + * single stack, and then composing that stack with any other one made it vanish + * with no error, no warning, and no way to tell "dropped" apart from "never + * written". This file pins the three rules adjudicated on 2026-08-04: + * + * 1. same value in several stacks → composes fine (pass-through); + * 2. different values → ERROR naming the key, the two source stacks, and the fix + * (NOT last-wins — a silent security downgrade; NOT deep-merge — a new class + * of silent ambiguity); + * 3. a top-level key with no declared composition rule → WARN, always, so the + * next new key self-reports instead of being discovered by accident. + * + * Plus the control tests: array keys keep their concat semantics, and every key + * the schema declares is classified (the static pin that makes rule 3 rare). + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; + +import { + composeStacks, + defineStack, + ObjectStackDefinitionSchema, + type ObjectStackDefinition, +} from './stack.zod'; + +// ─── Helpers ──────────────────────────────────────────────────────── + +function raw(overrides: Record): ObjectStackDefinition { + return defineStack(overrides as never, { strict: false }); +} + +const manifestA = { id: 'com.example.base', name: 'base', version: '1.0.0', type: 'app' as const }; +const manifestB = { id: 'com.example.addon', name: 'addon', version: '1.0.0', type: 'app' as const }; + +let warnSpy: ReturnType; + +beforeEach(() => { + warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); +}); + +afterEach(() => { + warnSpy.mockRestore(); +}); + +// ─── Rule 1 — same value passes through ───────────────────────────── + +describe('#5005 rule 1 — non-array top-level keys survive composition', () => { + it('keeps `api` when only one stack declares it (the issue\'s own repro)', () => { + const a = raw({ manifest: manifestA, api: { enforceProjectMembership: true } }); + const b = raw({ manifest: manifestB }); + + // On origin/main this was `undefined` — the 403 gate silently disappeared. + expect(composeStacks([a, b]).api).toEqual({ enforceProjectMembership: true }); + // Order must not matter: the gate survives from either position. + expect(composeStacks([b, a]).api).toEqual({ enforceProjectMembership: true }); + }); + + it('keeps `server` when only one stack declares it (#4910 rate limiting)', () => { + const a = raw({ + manifest: manifestA, + server: { security: { rateLimit: { enabled: true, maxRequests: 5 } } }, + }); + const b = raw({ manifest: manifestB }); + + expect(composeStacks([a, b]).server).toEqual({ + security: { rateLimit: { enabled: true, maxRequests: 5 } }, + }); + }); + + it('composes fine when several stacks declare the SAME value (deep equality)', () => { + const value = { enableProjectScoping: true, projectResolution: 'required' as const }; + const a = raw({ manifest: manifestA, api: { ...value } }); + const b = raw({ manifest: manifestB, api: { ...value } }); + const c = raw({ api: { ...value } }); + + const composed = composeStacks([a, b, c]); + expect(composed.api).toEqual(value); + // Same value is not a conflict, so it must not warn either. + expect(warnSpy).not.toHaveBeenCalled(); + }); + + it('treats an absent key and an explicitly-undefined key alike', () => { + const a = raw({ manifest: manifestA, api: { enforceProjectMembership: true } }); + const b = raw({ manifest: manifestB, api: undefined }); + + expect(composeStacks([a, b]).api).toEqual({ enforceProjectMembership: true }); + }); + + it('omits the key entirely when no stack declares it', () => { + const composed = composeStacks([raw({ manifest: manifestA }), raw({ manifest: manifestB })]); + expect('api' in composed).toBe(false); + expect('server' in composed).toBe(false); + }); +}); + +// ─── Rule 2 — conflict is an error with a prescription ────────────── + +describe('#5005 rule 2 — conflicting values throw a prescriptive error', () => { + it('throws naming the key, both source stacks and the fix', () => { + const a = raw({ manifest: manifestA, api: { enforceProjectMembership: true } }); + const b = raw({ manifest: manifestB, api: { enforceProjectMembership: false } }); + + let message = ''; + try { + composeStacks([a, b]); + throw new Error('composeStacks should have thrown'); + } catch (err) { + message = (err as Error).message; + } + + // Names the conflicting key… + expect(message).toContain("top-level key 'api'"); + // …names both source stacks by their manifest identity… + expect(message).toContain('com.example.base'); + expect(message).toContain('com.example.addon'); + // …and prescribes the two ways out. + expect(message).toMatch(/identical/i); + expect(message).toMatch(/remove it from/i); + }); + + it('is NOT last-wins — the earlier stack\'s stricter gate is never silently dropped', () => { + const strict = raw({ manifest: manifestA, api: { enforceProjectMembership: true } }); + const lax = raw({ manifest: manifestB, api: { enforceProjectMembership: false } }); + + expect(() => composeStacks([strict, lax])).toThrow(/conflict/i); + expect(() => composeStacks([lax, strict])).toThrow(/conflict/i); + }); + + it('is NOT deep-merge — disjoint sub-keys still conflict', () => { + const a = raw({ manifest: manifestA, api: { enableProjectScoping: true } }); + const b = raw({ manifest: manifestB, api: { enforceProjectMembership: true } }); + + expect(() => composeStacks([a, b])).toThrow(/conflict/i); + }); + + it('conflicts on `server` too', () => { + const a = raw({ manifest: manifestA, server: { security: { rateLimit: { maxRequests: 5 } } } }); + const b = raw({ manifest: manifestB, server: { security: { rateLimit: { maxRequests: 500 } } } }); + + expect(() => composeStacks([a, b])).toThrow(/top-level key 'server'/); + }); + + it('falls back to a positional label when a stack has no manifest', () => { + const a = raw({ api: { enforceProjectMembership: true } }); + const b = raw({ api: { enforceProjectMembership: false } }); + + expect(() => composeStacks([a, b])).toThrow(/stack #0/); + expect(() => composeStacks([a, b])).toThrow(/stack #1/); + }); +}); + +// ─── Rule 3 — unhandled keys warn ─────────────────────────────────── + +describe('#5005 rule 3 — a key with no declared rule warns', () => { + it('warns once, names the key and points at #5005', () => { + // A key the schema does not declare: reaches composeStacks only via + // `strict: false`, which is exactly how a NEW key looks before someone + // remembers to teach the composer about it. + const a = raw({ manifest: manifestA, futureThing: { enabled: true } }); + const b = raw({ manifest: manifestB }); + + const composed = composeStacks([a, b]) as Record; + + // One warning that both names the key AND points at #5005 — not two + // unrelated ones (`defineStack` also warns about undeclared keys). + const warnings = warnSpy.mock.calls.map((c) => String(c[0])); + expect( + warnings.some((w) => w.includes('composeStacks') && w.includes("'futureThing'") && w.includes('#5005')), + ).toBe(true); + // …and it is composed by the default rule rather than dropped. + expect(composed.futureThing).toEqual({ enabled: true }); + }); + + it('applies the default rule to an unknown ARRAY key (concat) and still warns', () => { + const a = raw({ manifest: manifestA, futureList: [1, 2] }); + const b = raw({ manifest: manifestB, futureList: [3] }); + + const composed = composeStacks([a, b]) as Record; + expect(composed.futureList).toEqual([1, 2, 3]); + expect(warnSpy.mock.calls.map((c) => String(c[0])).some((w) => w.includes("'futureList'"))).toBe(true); + }); + + it('warns rather than skipping a collection key that holds a non-array value', () => { + const a = raw({ manifest: manifestA, views: [{ name: 'v1' }] }); + const b = raw({ manifest: manifestB, views: 'not-an-array' }); + + const composed = composeStacks([a, b]); + expect(composed.views).toHaveLength(1); + expect( + warnSpy.mock.calls + .map((c) => String(c[0])) + .some((w) => w.includes('composeStacks') && w.includes("'views'") && w.includes('#5005')), + ).toBe(true); + }); + + it('does NOT warn for keys the composer has a declared rule for', () => { + const a = raw({ manifest: manifestA, api: { enableProjectScoping: true }, jobs: [] }); + const b = raw({ manifest: manifestB, server: { trustProxy: true } }); + + composeStacks([a, b]); + expect(warnSpy).not.toHaveBeenCalled(); + }); +}); + +// ─── Control — array keys keep today's concat semantics ───────────── + +describe('#5005 control — array keys still concatenate', () => { + it('concatenates in stack order, unchanged', () => { + const a = raw({ + manifest: manifestA, + apps: [{ name: 'a1' }], + views: [{ name: 'v1' }], + requires: ['ai'], + }); + const b = raw({ + manifest: manifestB, + apps: [{ name: 'a2' }], + views: [{ name: 'v2' }], + requires: ['automation'], + }); + + const composed = composeStacks([a, b]); + expect(composed.apps?.map((x) => x.name)).toEqual(['a1', 'a2']); + expect(composed.views?.map((x) => x.name)).toEqual(['v1', 'v2']); + expect(composed.requires).toEqual(['ai', 'automation']); + }); + + it('concatenates the array keys that were previously dropped outright', () => { + // These are declared `z.array(...)` on ObjectStackDefinitionSchema but were + // missing from the concat list, so composition deleted them (#5005). + const a = raw({ + manifest: manifestA, + jobs: [{ name: 'j1' }], + docs: [{ name: 'd1' }], + books: [{ name: 'b1' }], + datasets: [{ name: 'ds1' }], + emailTemplates: [{ name: 'e1' }], + datasourceMapping: [{ namespace: 'crm', datasource: 'memory' }], + tiers: ['core'], + }); + const b = raw({ + manifest: manifestB, + jobs: [{ name: 'j2' }], + docs: [{ name: 'd2' }], + books: [{ name: 'b2' }], + datasets: [{ name: 'ds2' }], + emailTemplates: [{ name: 'e2' }], + datasourceMapping: [{ default: true, datasource: 'turso' }], + tiers: ['ai'], + }); + + const composed = composeStacks([a, b]) as Record; + expect(composed.jobs).toHaveLength(2); + expect(composed.docs).toHaveLength(2); + expect(composed.books).toHaveLength(2); + expect(composed.datasets).toHaveLength(2); + expect(composed.emailTemplates).toHaveLength(2); + expect(composed.datasourceMapping).toHaveLength(2); + expect(composed.tiers).toEqual(['core', 'ai']); + }); + + it('merges `functions` by name — the handler collection is not an opaque scalar', () => { + const h1 = () => 'one'; + const h2 = () => 'two'; + const a = raw({ manifest: manifestA, functions: { handler_one: h1 } }); + const b = raw({ manifest: manifestB, functions: { handler_two: h2 } }); + + const composed = composeStacks([a, b]) as unknown as { + functions: Record; + }; + expect(Object.keys(composed.functions).sort()).toEqual(['handler_one', 'handler_two']); + }); + + it('errors on a duplicate function NAME rather than picking a winner', () => { + const a = raw({ manifest: manifestA, functions: { dup: () => 'a' } }); + const b = raw({ manifest: manifestB, functions: { dup: () => 'b' } }); + + expect(() => composeStacks([a, b])).toThrow(/dup/); + }); +}); + +// ─── Control — the pre-existing bespoke strategies are untouched ──── + +describe('#5005 control — manifest / objects / i18n strategies unchanged', () => { + it('manifest still follows the `manifest` option', () => { + const a = raw({ manifest: manifestA }); + const b = raw({ manifest: manifestB }); + expect(composeStacks([a, b]).manifest?.id).toBe('com.example.addon'); + expect(composeStacks([a, b], { manifest: 'first' }).manifest?.id).toBe('com.example.base'); + expect(composeStacks([a, b], { manifest: 0 }).manifest?.id).toBe('com.example.base'); + }); + + it('objects still follow `objectConflict`', () => { + const a = raw({ manifest: manifestA, objects: [{ name: 'task', fields: { a: {} } }] }); + const b = raw({ manifest: manifestB, objects: [{ name: 'task', fields: { b: {} } }] }); + + expect(() => composeStacks([a, b])).toThrow(/object 'task'/); + expect(composeStacks([a, b], { objectConflict: 'override' }).objects).toHaveLength(1); + }); + + it('i18n keeps its pre-existing last-wins (deliberately out of scope for #5005)', () => { + const a = raw({ manifest: manifestA, i18n: { defaultLocale: 'en' } }); + const b = raw({ manifest: manifestB, i18n: { defaultLocale: 'zh-CN' } }); + + expect(composeStacks([a, b]).i18n).toEqual({ defaultLocale: 'zh-CN' }); + }); +}); + +// ─── Structural pin — every declared key has a rule ───────────────── + +describe('#5005 structural pin — the schema and the composer cannot drift', () => { + /** + * The disposition table is typed `Record< keyof ObjectStackDefinition, … >`, + * so a key added to the schema without a composition rule is already a + * `tsc --noEmit` error. This is the runtime half: it catches the case the + * type cannot see — the schema shape and the TS type drifting apart — by + * asserting the observable contract instead of the table's contents. + */ + it('composes a stack declaring EVERY schema key with no drops and no warnings', () => { + const shape = (ObjectStackDefinitionSchema as unknown as { shape: Record }).shape; + const sample: Record = {}; + for (const key of Object.keys(shape)) { + if (key === 'manifest' || key === 'objects' || key === 'functions') continue; + sample[key] = key === 'runtimeModule' ? './rt.mjs' : []; + } + + const a = raw({ manifest: manifestA, ...sample }); + const b = raw({ manifest: manifestB, ...sample }); + const composed = composeStacks([a, b]) as Record; + + for (const key of Object.keys(sample)) { + expect(composed, `top-level key '${key}' was dropped by composeStacks`).toHaveProperty(key); + } + + // Zero warnings ⇒ every schema key hit a DECLARED rule, not the default. + // A new top-level key that nobody taught composeStacks about fails here, + // naming itself — that is the whole point of #5005 rule 3. + expect(warnSpy.mock.calls.map((c) => String(c[0]))).toEqual([]); + }); +}); diff --git a/packages/spec/src/stack.zod.ts b/packages/spec/src/stack.zod.ts index a275829747..d8f14b1419 100644 --- a/packages/spec/src/stack.zod.ts +++ b/packages/spec/src/stack.zod.ts @@ -1271,40 +1271,326 @@ export const ComposeStacksOptionsSchema = lazySchema(() => z.object({ export type ComposeStacksOptions = z.input; +/** + * How {@link composeStacks} treats one top-level key (#5005). + * + * - `'concat'` — array collection; concatenated in stack order. + * - `'single'` — one scalar/object value; identical declarations pass + * through, differing ones are a composition ERROR. + * - `'manifest'` — chosen by the `manifest` option. + * - `'objects'` — merged by the `objectConflict` strategy. + * - `'i18n'` — last-wins (pre-existing; see the table note below). + * - `'functions'` — named-handler collection; merged by name. + * @internal + */ +type ComposeDisposition = 'concat' | 'single' | 'manifest' | 'objects' | 'i18n' | 'functions'; + +/** + * The composition rule for EVERY top-level key of `ObjectStackDefinition` + * (#5005). + * + * ## Why a total table and not a list + * + * `composeStacks` used to build its result from an empty object by filling in + * `manifest`, `i18n`, `objects` and a hand-maintained array whitelist. Anything + * absent from that whitelist was not "left alone" — it was **deleted**, with no + * error, no warning, and no way for a consumer to tell "the composer dropped it" + * apart from "the author never wrote it". Composition is the platform's + * app-packaging / install story, so that silence reached real security config: + * `api.enforceProjectMembership` (the per-environment 403 gate) and, as of + * #4910, `server.security.rateLimit` both vanished the moment a stack was + * composed with any other one. Seven declared array collections + * (`datasourceMapping`, `datasets`, `jobs`, `emailTemplates`, `docs`, `books`, + * `tiers`) and the whole `functions` handler map went the same way; `tools` + * escaped the same fate only because ADR-0109 noticed and patched the list. + * + * A whitelist makes forgetting the default. This table makes it a **type + * error**: it is `Record< keyof ObjectStackDefinition, … >`, so a new top-level + * key does not compile until someone states what composing it means. That is + * the structural half of the fix; {@link composeStacks} carries the runtime + * half (an undeclared key warns rather than disappearing), so a key that + * reaches composition without a rule — via `strict: false`, or a raw object — + * still reports itself. + * + * ## Note on `i18n` + * + * `i18n` keeps its pre-existing last-wins. It is the one key here that already + * had a deliberate, working strategy, so #5005 (whose subject is keys that were + * *dropped*) deliberately leaves it alone rather than breaking compositions + * that rely on it. It is nonetheless the same silent-override shape the + * maintainer rejected for `api`/`server` — tracked separately. + * + * @internal + */ +const COMPOSE_KEY_DISPOSITIONS: Record = { + // ── Bespoke strategies (unchanged by #5005) ── + manifest: 'manifest', + objects: 'objects', + i18n: 'i18n', + functions: 'functions', + + // ── Array collections — concatenated in stack order ── + datasources: 'concat', + datasourceMapping: 'concat', + translations: 'concat', + objectExtensions: 'concat', + apps: 'concat', + views: 'concat', + pages: 'concat', + dashboards: 'concat', + reports: 'concat', + datasets: 'concat', + actions: 'concat', + themes: 'concat', + flows: 'concat', + jobs: 'concat', + emailTemplates: 'concat', + docs: 'concat', + books: 'concat', + positions: 'concat', + permissions: 'concat', + capabilities: 'concat', + sharingRules: 'concat', + apis: 'concat', + webhooks: 'concat', + agents: 'concat', + tools: 'concat', + skills: 'concat', + hooks: 'concat', + mappings: 'concat', + analyticsCubes: 'concat', + connectors: 'concat', + data: 'concat', + plugins: 'concat', + requires: 'concat', + tiers: 'concat', + devPlugins: 'concat', + + // ── Single-valued configuration — same value passes, difference throws ── + api: 'single', + server: 'single', + runtimeModule: 'single', +}; + /** * All array fields on `ObjectStackDefinition` that are simply concatenated. + * Derived from {@link COMPOSE_KEY_DISPOSITIONS} so the two cannot drift. + * @internal + */ +const CONCAT_ARRAY_FIELDS = (Object.keys(COMPOSE_KEY_DISPOSITIONS) as (keyof ObjectStackDefinition)[]) + .filter((key) => COMPOSE_KEY_DISPOSITIONS[key] === 'concat'); + +/** + * Structural deep equality for the "same value composes fine" rule (#5005). + * + * Deliberately strict and deliberately small: it decides only whether two + * authored declarations are the SAME, never how to reconcile two different + * ones. Keys explicitly set to `undefined` are treated as absent, matching how + * the composer reads a declaration in the first place. Callables compare by + * reference (`Object.is`) — two distinct closures are two distinct + * declarations, which is the honest answer for a config value. + * @internal + */ +function deepEqualDeclarations(a: unknown, b: unknown): boolean { + if (Object.is(a, b)) return true; + if (typeof a !== 'object' || typeof b !== 'object' || a === null || b === null) return false; + if (Array.isArray(a) !== Array.isArray(b)) return false; + + if (Array.isArray(a) && Array.isArray(b)) { + return a.length === b.length && a.every((item, i) => deepEqualDeclarations(item, b[i])); + } + + const left = a as Record; + const right = b as Record; + const leftKeys = Object.keys(left).filter((k) => left[k] !== undefined); + const rightKeys = Object.keys(right).filter((k) => right[k] !== undefined); + if (leftKeys.length !== rightKeys.length) return false; + return leftKeys.every((k) => deepEqualDeclarations(left[k], right[k])); +} + +/** + * Name a stack the way its author would recognise it (#5005). + * + * A composition error is only actionable if it says WHICH stacks disagree, and + * at compose time the only identity a stack carries is its manifest. Falls back + * to the positional index for the manifest-less stacks that composition also + * accepts. + * @internal + */ +function stackLabel(stack: ObjectStackDefinition, index: number): string { + const id = stack.manifest?.id ?? stack.manifest?.name; + return id ? `'${id}' (stack #${index})` : `stack #${index}`; +} + +/** + * Compose a single-valued (non-array) top-level key across stacks (#5005). + * + * Same value everywhere ⇒ that value. Any disagreement ⇒ throw, naming the key, + * both source stacks and the two ways out. NOT last-wins: silently preferring + * the later stack is a security downgrade — it is precisely how an earlier + * stack's `enforceProjectMembership` 403 gate or a tighter rate-limit budget + * would be switched off by an add-on package. NOT a deep merge either: that + * invents a third value neither author wrote. + * @internal + */ +function composeSingleValue( + stacks: ObjectStackDefinition[], + key: string, +): { declared: boolean; value: unknown } { + let holder = -1; + + for (let i = 0; i < stacks.length; i++) { + const value = (stacks[i] as Record)[key]; + if (value === undefined) continue; + if (holder === -1) { + holder = i; + continue; + } + const held = (stacks[holder] as Record)[key]; + if (deepEqualDeclarations(held, value)) continue; + + throw new Error( + `composeStacks conflict: top-level key '${key}' is declared with different values by ` + + `${stackLabel(stacks[holder], holder)} and ${stackLabel(stacks[i], i)}.\n` + + `composeStacks does not pick a winner for single-valued top-level configuration: ` + + `overriding would silently disable whichever stack declared the stricter setting ` + + `(an 'api.enforceProjectMembership' 403 gate, a 'server.security.rateLimit' budget), ` + + `and deep-merging would produce a value neither stack declared.\n` + + `Fix: make the two '${key}' declarations identical, or remove it from every stack ` + + `except the one that should own it.`, + ); + } + + return holder === -1 + ? { declared: false, value: undefined } + : { declared: true, value: (stacks[holder] as Record)[key] }; +} + +/** + * Compose the `functions` handler collection across stacks (#5005). + * + * `functions` is a named collection, not an opaque config blob: composing CRM + + * Todo must yield BOTH packages' handlers, or every declarative hook, action + * and script node that resolves a handler by name breaks at boot. So it merges + * by name — and a name declared twice throws rather than picking a winner, + * matching `objectConflict: 'error'`. + * + * The two authored shapes (map and array) are merged in kind. They are NOT + * converted into one another: an array entry carries a `packageId` that the map + * entry has no place for, so a conversion would drop provenance. Mixing the two + * shapes across composed stacks therefore throws with that instruction. * @internal */ -const CONCAT_ARRAY_FIELDS = [ - 'datasources', - 'translations', - 'objectExtensions', - 'apps', - 'views', - 'pages', - 'dashboards', - 'reports', - 'actions', - 'themes', - 'flows', - 'positions', - 'permissions', - 'capabilities', - 'sharingRules', - 'apis', - 'webhooks', - 'agents', - 'skills', - 'tools', - 'hooks', - 'mappings', - 'analyticsCubes', - 'connectors', - 'data', - 'plugins', - 'devPlugins', - 'requires', -] as const satisfies readonly (keyof ObjectStackDefinition)[]; +function composeFunctions( + stacks: ObjectStackDefinition[], +): { declared: boolean; value: unknown } { + type ArrayEntry = { name: string }; + const declaring: { index: number; value: unknown }[] = []; + + for (let i = 0; i < stacks.length; i++) { + const value = (stacks[i] as Record).functions; + if (value !== undefined) declaring.push({ index: i, value }); + } + if (declaring.length === 0) return { declared: false, value: undefined }; + if (declaring.length === 1) return { declared: true, value: declaring[0].value }; + + const arrayForm = declaring.filter((d) => Array.isArray(d.value)); + if (arrayForm.length !== 0 && arrayForm.length !== declaring.length) { + const mapSide = declaring.find((d) => !Array.isArray(d.value))!; + throw new Error( + `composeStacks conflict: top-level key 'functions' is declared in the map form by ` + + `${stackLabel(stacks[mapSide.index], mapSide.index)} and in the array form by ` + + `${stackLabel(stacks[arrayForm[0].index], arrayForm[0].index)}.\n` + + `The two shapes cannot be merged without losing information (an array entry carries ` + + `'packageId', the map entry does not).\n` + + `Fix: author 'functions' in the same shape in both stacks — the map form ` + + `({ my_handler: fn }) is preferred.`, + ); + } + + const seen = new Map(); + const claim = (name: string, index: number): void => { + const first = seen.get(name); + if (first !== undefined) { + throw new Error( + `composeStacks conflict: function '${name}' is defined by both ` + + `${stackLabel(stacks[first], first)} and ${stackLabel(stacks[index], index)}.\n` + + `Handlers are resolved by name at boot, so one would silently shadow the other.\n` + + `Fix: rename one of them (prefix it with its package, e.g. 'crm_${name}'), or ` + + `declare it in exactly one stack.`, + ); + } + seen.set(name, index); + }; + + if (arrayForm.length === declaring.length) { + const merged: ArrayEntry[] = []; + for (const { index, value } of declaring) { + for (const entry of value as ArrayEntry[]) { + claim(entry.name, index); + merged.push(entry); + } + } + return { declared: true, value: merged }; + } + + const merged: Record = {}; + for (const { index, value } of declaring) { + for (const [name, handler] of Object.entries(value as Record)) { + claim(name, index); + merged[name] = handler; + } + } + return { declared: true, value: merged }; +} + +const warnedUncomposedStackKeys = new Set(); + +/** + * Report a top-level key that reached composition with no declared rule (#5005). + * + * The invariant this restores: composition is never silent about a key it did + * not know what to do with. It still composes the key by the default rule + * (arrays concatenate, everything else follows the single-value rule) so + * nothing is lost — but the NEXT top-level key someone adds without teaching + * `composeStacks` about it announces itself here, instead of being discovered + * by accident during unrelated work the way `server:` was. + * + * Warn-once per key, like the other authoring-time notices in this module. + * @internal + */ +const warnedMalformedCollectionKeys = new Set(); + +/** + * Report a collection key that carried a non-array value (#5005). + * + * The concat rule can only concatenate arrays, so such a value is skipped — + * and skipping it silently is the same defect in miniature. Only reachable + * with an unparsed stack (`strict: false`, hand-built object); the strict + * `defineStack` path rejects the shape outright. + * @internal + */ +function warnMalformedCollectionKey(key: string): void { + if (warnedMalformedCollectionKeys.has(key)) return; + warnedMalformedCollectionKeys.add(key); + console.warn( + `composeStacks: top-level key '${key}' is a collection (concatenated across stacks) but at ` + + `least one stack carries a non-array value for it — that value cannot be composed and was ` + + `skipped. Author it as an array, or run the stack through strict \`defineStack\` to have ` + + `the shape rejected where it is written. See objectstack-ai/objectstack#5005.`, + ); +} + +function warnUncomposedStackKey(key: string, rule: ComposeDisposition): void { + if (warnedUncomposedStackKeys.has(key)) return; + warnedUncomposedStackKeys.add(key); + console.warn( + `composeStacks: top-level key '${key}' has no declared composition rule — composed with ` + + `the default (${rule === 'concat' ? 'arrays are concatenated' : 'single value; conflicting declarations throw'}). ` + + `Declare what composing it means in COMPOSE_KEY_DISPOSITIONS (packages/spec/src/stack.zod.ts) ` + + `in the same change that declares the key — see objectstack-ai/objectstack#5005.`, + ); +} /** * Merge objects from multiple stacks according to the chosen conflict strategy. @@ -1424,7 +1710,8 @@ export function composeStacks( // 1. Manifest — pick based on strategy composed.manifest = selectManifest(stacks, opts.manifest); - // 2. i18n — last-wins (single object, not array) + // 2. i18n — last-wins (single object, not array). Pre-existing strategy, + // deliberately untouched by #5005; see COMPOSE_KEY_DISPOSITIONS. for (let i = stacks.length - 1; i >= 0; i--) { if (stacks[i].i18n) { composed.i18n = stacks[i].i18n; @@ -1438,14 +1725,67 @@ export function composeStacks( composed.objects = objects; } - // 4. All other array fields — simple concatenation + // 4. Array collections — simple concatenation, in stack order. for (const field of CONCAT_ARRAY_FIELDS) { - const arrays = stacks + const declared = stacks .map((s) => (s as Record)[field]) - .filter((v): v is unknown[] => Array.isArray(v)); + .filter((v) => v !== undefined); + const arrays = declared.filter((v): v is unknown[] => Array.isArray(v)); if (arrays.length > 0) { composed[field] = arrays.flat(); } + // A collection key holding something that is not an array cannot be + // concatenated. `defineStack` rejects that shape, so this is only + // reachable via `strict: false` or a hand-built stack object — but + // dropping it without a word is the exact defect #5005 closes. + if (declared.length !== arrays.length) { + warnMalformedCollectionKey(field); + } + } + + // 5. Named handler functions — merged by name (#5005). + const functions = composeFunctions(stacks); + if (functions.declared) { + composed.functions = functions.value; + } + + // 6. Every remaining top-level key (#5005). + // + // This loop is the reason composition can no longer eat a key. It walks + // what the STACKS actually carry rather than a whitelist, so a key with a + // declared rule gets it, and a key without one is composed by the default + // AND reported — instead of being deleted in silence the way `api:` and + // `server:` were. + const remainingKeys: string[] = []; + for (const stack of stacks) { + for (const key of Object.keys(stack as Record)) { + if ((stack as Record)[key] === undefined) continue; + if (key in composed) continue; + const rule = COMPOSE_KEY_DISPOSITIONS[key as keyof ObjectStackDefinition]; + // Handled above (a declared key whose value happened to be absent from + // every stack lands here too — nothing to compose, so skip it). + if (rule !== undefined && rule !== 'single') continue; + if (!remainingKeys.includes(key)) remainingKeys.push(key); + } + } + + for (const key of remainingKeys) { + const rule = COMPOSE_KEY_DISPOSITIONS[key as keyof ObjectStackDefinition]; + if (rule === undefined) { + // Not declared on ObjectStackDefinitionSchema at all — reachable via + // `defineStack(..., { strict: false })` or a hand-built stack object. + const isArray = stacks.some((s) => Array.isArray((s as Record)[key])); + warnUncomposedStackKey(key, isArray ? 'concat' : 'single'); + if (isArray) { + composed[key] = stacks + .map((s) => (s as Record)[key]) + .filter((v): v is unknown[] => Array.isArray(v)) + .flat(); + continue; + } + } + const single = composeSingleValue(stacks, key); + if (single.declared) composed[key] = single.value; } return mergeActionsIntoObjects(composed as ObjectStackDefinition); From bb39cf9fb1fd229582ed014883e25ea0ec3d2bd9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 01:08:34 +0000 Subject: [PATCH 2/2] docs(spec): restore docblock ordering around the composeStacks warn helpers The warn-helper insertion left `warnUncomposedStackKey`'s docblock attached to `warnedMalformedCollectionKeys`. Comment-only; no behaviour change. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Ehu85kbvMcrNTUJjwxvLJ9 --- packages/spec/src/stack.zod.ts | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/spec/src/stack.zod.ts b/packages/spec/src/stack.zod.ts index d8f14b1419..47ae961cf6 100644 --- a/packages/spec/src/stack.zod.ts +++ b/packages/spec/src/stack.zod.ts @@ -1544,21 +1544,6 @@ function composeFunctions( return { declared: true, value: merged }; } -const warnedUncomposedStackKeys = new Set(); - -/** - * Report a top-level key that reached composition with no declared rule (#5005). - * - * The invariant this restores: composition is never silent about a key it did - * not know what to do with. It still composes the key by the default rule - * (arrays concatenate, everything else follows the single-value rule) so - * nothing is lost — but the NEXT top-level key someone adds without teaching - * `composeStacks` about it announces itself here, instead of being discovered - * by accident during unrelated work the way `server:` was. - * - * Warn-once per key, like the other authoring-time notices in this module. - * @internal - */ const warnedMalformedCollectionKeys = new Set(); /** @@ -1581,6 +1566,21 @@ function warnMalformedCollectionKey(key: string): void { ); } +const warnedUncomposedStackKeys = new Set(); + +/** + * Report a top-level key that reached composition with no declared rule (#5005). + * + * The invariant this restores: composition is never silent about a key it did + * not know what to do with. It still composes the key by the default rule + * (arrays concatenate, everything else follows the single-value rule) so + * nothing is lost — but the NEXT top-level key someone adds without teaching + * `composeStacks` about it announces itself here, instead of being discovered + * by accident during unrelated work the way `server:` was. + * + * Warn-once per key, like the other authoring-time notices in this module. + * @internal + */ function warnUncomposedStackKey(key: string, rule: ComposeDisposition): void { if (warnedUncomposedStackKeys.has(key)) return; warnedUncomposedStackKeys.add(key);