fix(spec)!: composeStacks 不再静默丢弃顶层键 —— 同值放行 / 冲突报错 / 未声明必警 (#5005)

.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 的栈。改法见错误信息里的处方。

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<string, unknown>): 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<typeof vi.spyOn>;
+
+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<string, unknown>;
+
+    // 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<string, unknown>;
+    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<string, unknown[]>;
+    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<string, unknown>;
+    };
+    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<string, unknown> }).shape;
+    const sample: Record<string, unknown> = {};
+    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<string, unknown>;
+
+    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([]);
+  });
+});