refactor(metadata-protocol): 删除 saveMetaItem 中已不可达的 legacy raw-engine 写入分支 (#5264)

.changeset/save-legacy-raw-engine-branch-removed.md

@@ -0,0 +1,29 @@
+---
+"@objectstack/metadata-protocol": patch
+---
+
+refactor(metadata-protocol): 删除 `saveMetaItem` 里已不可达的 legacy raw-engine 写入分支 (#5264)
+
+`saveMetaItem` 过去有两条持久化路径:repository 写入路径(追加
+`sys_metadata_history`、发 watch 事件、带单调 `seq`),以及其后的 legacy
+raw-engine 分支(直接 `engine.insert` / `engine.update` 写 `sys_metadata`,
+没有 history 行、没有 watch 事件、没有 `seq`,回执形如
+`Saved customization overlay (env-wide) — type=…`)。后者的进入条件是
+`isOverlayAllowed(type) || isRuntimeCreateAllowed(type)` 为假。
+
+**没有行为变化 —— 这条分支在运行时已经到不了。** #5086(PR #5263)把
+code-only 类型的拒绝提到了同一方法更早的位置,并且不再以 `environmentId`
+为条件:它抛错的判据与上面那个条件恰好互为反面,读的还是同一个规范化后的
+类型键(`canonicalizeMetaRequestType` 在方法开头折叠单复数,两个标志读取器
+内部又各自折叠一次)。`OS_METADATA_WRITABLE` 也不是缺口:在那里解锁一个
+类型会让 `isOverlayAllowed` 为真,从而走回 repository 路径。因此凡是能走到
+分叉点的写入,一律走 repository 路径。
+
+保留 `useRepoPath` 的代价不是多几行代码,而是它是一份 grep 得到、读起来
+像活代码的样板:照它推理会得出「`sys_metadata` 存在一个不写 history 的
+合法写入口」——现在没有了。
+
+`deleteMetaItem` 里结构对称的那条 legacy 分支**一行未动**:它在
+control-plane kernel(`environmentId === undefined`)上删除 code-only 遗留行
+时仍然可达且必要(#5263 特意没有收紧删除侧,因为删除是修复动作),该分支上
+新增了说明它为何还活着的注释。

packages/metadata-protocol/src/protocol.code-only-types.test.ts

@@ -85,6 +85,11 @@ function makeStubEngine(artifacts: Array<{ type: string; name: string }> = []) {
     const artifactKeys = new Set(artifacts.map((a) => `${a.type}|${a.name}`));
     const keyOf = (w: Record<string, unknown>) =>
         `${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}`;
+    // #5264 — every write the engine is asked for, in order. `rows` alone
+    // cannot tell the two persistence routes apart (both end at
+    // `insert('sys_metadata', …)`); the tell is whether a
+    // `sys_metadata_history` append came with it. See the #5264 block below.
+    const writes: Array<{ op: 'insert' | 'update' | 'delete'; table: string }> = [];
     const engine: any = {
         async findOne(_t: string, opts: { where: Record<string, unknown> }) {
             for (const row of rows.values()) {
@@ -96,14 +101,15 @@ function makeStubEngine(artifacts: Array<{ type: string; name: string }> = []) {
         },
         async find() { return []; },
         async insert(_t: string, data: Record<string, unknown>) {
+            writes.push({ op: 'insert', table: _t });
             if (_t !== 'sys_metadata') return { id: 'side_effect_skip' };
             nextId += 1;
             const row = { id: `r_${nextId}`, ...(data as any) } as Row;
             rows.set(keyOf(data), row);
             return { id: row.id };
         },
-        async update() { return { id: null }; },
-        async delete() { return { deleted: 0 }; },
+        async update(_t: string) { writes.push({ op: 'update', table: _t }); return { id: null }; },
+        async delete(_t: string) { writes.push({ op: 'delete', table: _t }); return { deleted: 0 }; },
         registry: {
             registerItem: () => {},
             registerObject: () => {},
@@ -115,7 +121,7 @@ function makeStubEngine(artifacts: Array<{ type: string; name: string }> = []) {
                 artifactKeys.has(`${type}|${name}`) ? { name, _packageId: 'showcase' } : undefined,
         },
     };
-    return { engine, rows };
+    return { engine, rows, writes };
 }
 
 /**
@@ -132,13 +138,13 @@ const KERNELS: Array<{ label: string; environmentId?: string }> = [
 ];
 
 function makeProtocol(environmentId?: string, artifacts?: Array<{ type: string; name: string }>) {
-    const { engine, rows } = makeStubEngine(artifacts);
+    const { engine, rows, writes } = makeStubEngine(artifacts);
     const protocol = new ObjectStackProtocolImplementation(
         engine,
         () => new Map(),
         environmentId,
     ) as any;
-    return { protocol, rows };
+    return { protocol, rows, writes };
 }
 
 const metaRows = (rows: Map<string, Row>) => Array.from(rows.values());
@@ -321,4 +327,117 @@ describe('code-only metadata types are refused on every kernel (#5086)', () => {
             });
         }
     });
+
+    // ── #5264 — one persistence route, and the proof it is the only one ───
+    //
+    // `saveMetaItem` used to end in a legacy raw-engine branch: `engine.insert`
+    // / `engine.update` straight into `sys_metadata`, no `sys_metadata_history`
+    // append, no watch event, no `seq`. It ran exactly when
+    // `isOverlayAllowed(type) || isRuntimeCreateAllowed(type)` was false —
+    // which is the predicate the #5086 gate above throws on, unconditionally,
+    // over the same canonicalized type key. #5264 removed the branch.
+    //
+    // These pins are about the RECEIPT, not the verdict, because the receipt is
+    // the only thing that told the two routes apart from outside: the legacy
+    // one answered `200 {"success":true,"message":"Saved customization overlay
+    // (env-wide) — type=…"}` with no `state=`, no `[seq=…]`, and no history
+    // row. That is precisely the answer #5086 caught the showcase giving for a
+    // `job`. They are green before the removal too — a branch nothing reaches
+    // is what "dead" means — so they are not a regression test for the
+    // deletion; they are the guard that stops a second historyless write path
+    // from being introduced, and they fail loudly if the #5086 gate is ever
+    // narrowed back to `environmentId !== undefined`.
+    describe('#5264 — saveMetaItem persists through the repository or not at all', () => {
+        for (const type of CODE_ONLY_TYPES) {
+            it(`asks the engine for NOTHING when refusing ${type} on a control-plane kernel`, async () => {
+                // The exact condition the deleted branch claimed for itself:
+                // "control-plane bootstrap (environmentId === undefined) for
+                // non-overlay-allowed types". Driven here on purpose — the
+                // refusal lands first, so the write never becomes a write.
+                const probe = PROBES[type]!;
+                const { protocol, rows, writes } = makeProtocol(undefined);
+
+                const err = await protocol
+                    .saveMetaItem({ type, name: probe.name, item: probe.item })
+                    .then(() => null, (e: any) => e);
+
+                expect(err?.status).toBe(403);
+                expect(metaRows(rows)).toEqual([]);
+                // Stronger than "no row landed": no write was even attempted,
+                // so there is nothing for a raw-engine path to have done.
+                expect(writes).toEqual([]);
+            });
+        }
+
+        // One savable type per shape the two-tier model distinguishes.
+        const ACCEPTED: Array<{ type: string; item: Record<string, unknown> }> = [
+            {
+                type: 'view', // allowOrgOverride + allowRuntimeCreate
+                item: {
+                    name: 'rc3_receipt_view',
+                    label: 'Receipt',
+                    object: 'task',
+                    columns: [{ field: 'name', label: 'Name' }],
+                },
+            },
+            {
+                type: 'hook', // allowRuntimeCreate only
+                item: { name: 'rc3_receipt_view', object: 'task', events: ['beforeUpdate'] },
+            },
+            {
+                type: 'theme', // no static registry entry (plugin-registered)
+                item: { name: 'rc3_receipt_view', label: 'Receipt', tokens: {} },
+            },
+        ];
+
+        for (const { label, environmentId } of KERNELS) {
+            for (const { type, item } of ACCEPTED) {
+                it(`answers a ${type} save with a repository receipt on a ${label}`, async () => {
+                    const { protocol, writes } = makeProtocol(environmentId);
+
+                    const result = await protocol.saveMetaItem({
+                        type,
+                        name: 'rc3_receipt_view',
+                        item,
+                        ...(environmentId ? { organizationId: 'org_alpha' } : {}),
+                    });
+
+                    expect(result.success).toBe(true);
+                    // `seq` and `state` exist only on the repository receipt.
+                    expect(typeof result.seq).toBe('number');
+                    expect(result.state).toBe('active');
+                    expect(result.message).toContain('[seq=');
+                    // And the change log really was appended — the legacy
+                    // branch's defining omission.
+                    expect(writes.some((w) => w.table === 'sys_metadata_history')).toBe(true);
+                });
+            }
+        }
+
+        for (const type of CODE_ONLY_TYPES) {
+            it(`routes ${type} back through the repository once OS_METADATA_WRITABLE unlocks it`, async () => {
+                // The last leg of the unreachability argument: the escape hatch
+                // does not open a second door. Unlocking a type makes
+                // `isOverlayAllowed` true, which is the same predicate the
+                // repository path is chosen by — so an unlocked save is a
+                // repository save, receipt and history row included.
+                const probe = PROBES[type]!;
+                process.env.OS_METADATA_WRITABLE = type;
+                ObjectStackProtocolImplementation.resetEnvWritableCache();
+                resetEnvWritableMetadataTypes();
+
+                const { protocol, writes } = makeProtocol(undefined);
+                const result = await protocol.saveMetaItem({
+                    type,
+                    name: probe.name,
+                    item: probe.item,
+                });
+
+                expect(result.success).toBe(true);
+                expect(typeof result.seq).toBe('number');
+                expect(result.message).toContain('[seq=');
+                expect(writes.some((w) => w.table === 'sys_metadata_history')).toBe(true);
+            });
+        }
+    });
 });

packages/metadata-protocol/src/protocol.stored-migration.test.ts

@@ -463,9 +463,14 @@ describe('migrateStoredMetadata — what it declines to touch, loudly (#4327)',
     });
 
     it('skips a type with no repository write path rather than rewriting it without history', async () => {
-        // `agent` is allowOrgOverride:false + allowRuntimeCreate:false, so
-        // `saveMetaItem` would take the legacy raw-engine branch: no history row
-        // and a forced `state: 'active'`. Declining beats a silent half-write.
+        // `agent` is allowOrgOverride:false + allowRuntimeCreate:false. The
+        // skip is what this test pins, and it is unchanged; only its rationale
+        // moved. It used to be "`saveMetaItem` would take the legacy raw-engine
+        // branch: no history row and a forced `state: 'active'`" — #5086
+        // (PR #5263) then made `saveMetaItem` refuse a code-only type with 403
+        // before persistence, and #5264 deleted the now-unreachable branch. So
+        // today the pass declines a write that would be refused anyway, which
+        // keeps it a reported `skipped` row instead of `failed` noise.
         const { engine, tables } = makeStubEngine([
             { type: 'agent', name: 'legacy_agent', metadata: { name: 'legacy_agent', label: 'Legacy' } },
         ]);