Repository navigation
Commit 3cf7f61
fix(metadata-protocol)!: refuse a keyed seed whose externalId names no field of its object (#29066)
domain:server#1 · os-project-manager · session_01R1q2xdtYgU6FKrZ7LgjPCC
<!-- claude-projects-attribution: {"github_login":"os-project-manager"}
-->
_Requested by **Leehom** · [project
thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1X7V7Gq8G5nk7gPLT3UNF6Sk)_
Fixes #28952
Clause-②: no (narrowing)
Metadata-protocol: narrowed
## 维护者速读
- **改了什么**:种子(seed)在 `upsert` / `update` / `ignore` 模式下,如果
`externalId`(组合键的任一成员;没写时默认是 `name`)不是目标对象的字段,保存、发布和恢复版本时都会被拒绝(`422
INVALID_METADATA`,和对象数据源写错时同一种拒绝格式,指出是哪个字段、哪个对象),什么都不写入。启动时加载种子从不拒绝,只在某个数据集的每条记录都取不到键值时打一行
warn。
- **为什么改**:原来这种种子保存时显示有效,每次发布都回报「成功、新增 1
条」,表里却每发布一次就多一份重复数据,没有任何提示,只有最终用户会发现。
- **风险与回滚**:低到中。这是对保存/发布入口的收紧:以前能存进去的这类种子现在会被拒。实测本仓库 4 个示例、平台包和 hotcrm
里没有一个种子命中。`insert` / `replace` 模式不受影响。回滚即撤销本 PR。
-
**先看**:`packages/metadata-protocol/src/protocol-save-door-refusals.ts`(新增的
`seedExternalIdSaveRefusal`,判断对象字段的逻辑在这里);`packages/metadata-protocol/src/protocol.ts`(保存入口和发布晋升里的两处调用);`packages/metadata-protocol/src/seed-loader.ts`(启动加载的那行
warn)。
- **两分钟验证**:`pnpm --filter @objectstack/metadata-protocol exec vitest
run src/protocol.seed-external-id-refusal.test.ts`,预期 22/22 通过(main 上其中
12 条拒绝用例是红的,对照组是绿的)。
- **需要你做的**:无,席位落地。
## What changes
**The refusal.** New `seedExternalIdSaveRefusal` in
`packages/metadata-protocol/src/protocol-save-door-refusals.ts`, beside
the other save-door refusals, with its class method in `protocol.ts`.
- **Judged.** A `seed` in a keyed mode (`upsert`, `update`, `ignore`).
Each member of `externalId` is checked; an undeclared `externalId` is
`SeedSchema`'s default, `name`, the same default the loader applies
(`dataset.externalId || 'name'`). `insert` and `replace` never match by
key and are not judged.
- **Against what.** The object as the publish will have it: a draft of
it in the same package batch (`request.pending.objects`, the batch
closure `publishPackageDrafts` already collects), else the engine's
registered definition (`engine.registry.getObject`), whose fields
include extension fields. The columns the registry injects count,
through the spec's `resolveInjectedSystemColumns` (so `id` and the audit
and tenant columns are fields).
- **Not refused when the object cannot be resolved.** On a draft save, a
seed whose object is itself still a draft is left for its publish, where
the batch's object draft is judged (the order's "object resolved only at
publish" case). A seed whose object is neither registered nor drafted is
not judged at all: the load already reports an object it cannot find.
- **Envelope.** The save door's envelope for a body it judges, the one
`unknownDatasourceSaveRefusal` (#26724) answers: `422 INVALID_METADATA`,
message `seed/NAME failed spec validation: ` plus
`specValidationFindings` (the REST faces carry the headline and the
prose lives in `issues[]`), and `issues[]` with one finding per missing
member at `externalId` (`externalId.N` for a composite member), `code:
'custom'`. Each finding opens "Seed 'NAME' keys on externalId member
'FIELD', which is not a field of object 'OBJECT'" (or, undeclared,
"declares no externalId, so it keys on the default 'name'"), then the
consequence and the remedy, under the 500-character REST cap. No new
error code.
**Where it is asked.** The same three places
`unknownDatasourceSaveRefusal` is:
- `saveMetaItem`, draft and publish mode, after the schema and before
the authoring gate and every write. A `migrate-stored` re-save (the
platform healing its own storage) is not judged.
- `promoteDraftForPublish`, on the draft body, before
`repo.promoteDraft` writes and before any seed row is loaded. It covers
a seed draft stored before this change, one whose object was a draft at
its save, and one whose object has since lost the field.
`publishPackageDrafts` aborts the batch on it (`outcome: 'refused'`),
and `publishMetaItem` refuses it.
- The restore doors, through `refuseRestoredBodyTheSaveDoorRefuses`
(`rollbackMetaItem`, `revertCommit`).
**The boot loader warns, never refuses.** In
`SeedLoaderService.loadDataset` (`seed-loader.ts`), a keyed dataset
whose every record has no value for its key logs one `warn` per dataset,
naming the object, the key and the mode. Its writes are unchanged. A
replay of such a stored seed on this head still inserts every record
again (`upsert`, `ignore`) or skips it (`update`); the loader test pins
that, so the behaviour the doors now stop is stated rather than implied.
**Changeset** `.changeset/28952-seed-external-id-refusal.md`: `minor` on
`@objectstack/metadata-protocol`, both declaration lines, the BREAKING
banner and the ADR-0087 `not-required (no-migration-prescription)`
marker.
## Tests
- **Step 1, the measurement (before any refusal).** Every seed module in
`examples/app-showcase`, `app-crm`, `app-todo`, `app-multi-package`, the
platform packages and hotcrm (`d4ffe99`) was imported and each keyed
seed's `externalId` checked against its object's fields plus the
injected columns. 47 keyed seeds (showcase 19, crm 5, todo 1, hotcrm 22;
the platform packages and app-multi-package ship none), all with a
declared `externalId`: **0 dangling**, 0 relying on an implicit `name`
the object lacks. Two targeted platform objects the scan could not load
were read by hand: `sys_business_unit` keyed on `id` (the primary key)
and hotcrm's `sys_activity` keyed on `['source_object', 'source_id']`
(both declared in `plugin-audit`).
- **New
`packages/metadata-protocol/src/protocol.seed-external-id-refusal.test.ts`**
(22 tests): the refusal at a draft save into a package, a draft save
with no package and a publish save; each keyed mode; a composite with
one bad member (`externalId.1`); an undeclared key on an object with no
`name`; `publishMetaItem` and `publishPackageDrafts` on a seed draft
stored before the rule (draft stays a draft, the batch promotes nothing,
no seed row written); an object drafted in the same batch that lacks the
field; the restore door. Controls: `name`, `code`, `id`, a composite of
declared fields and an undeclared key on an object with `name` each
save, publish (and load: the double records the seed row) and save live;
`insert` and `replace`; an object drafted in the same batch that
declares the field; an object neither registered nor drafted; a
`migrate-stored` re-save.
- **New
`packages/metadata-protocol/src/seed-loader-empty-key-warn.test.ts`** (5
tests): one `warn` per load naming object and key, `success: true`, and
a replay inserting the row again (2 rows); a composite member no record
carries; controls: a declared key converging (`inserted: 1`, then
`inserted: 0, updated: 0`) with no warn, an `insert` dataset and a
dataset where some record carries the key.
- **`protocol.restore-doors-save-refusal-family.test.ts`**: one more
`pinBothDoors` case, a seed version keyed on a field its object has
since lost, refused at `rollbackMetaItem` and `revertCommit` with
nothing stored; its controls restore the accepted version. The harness's
registry gained an `objects` map (empty for every other case).
- **New dogfood case
`packages/qa/dogfood/test/seed-external-id-refusal.dogfood.test.ts`** (4
tests, the card's repro over the real showcase and REST): the draft `PUT
/api/v1/meta/seed/r_ghost` refused 422 `INVALID_METADATA` at
`externalId` with nothing in `_drafts`; the composite refused at
`externalId.1`; control: `externalId: 'name'` publishes `inserted: 1`,
republishes `inserted: 0, updated: 0`, and `GET
/api/v1/data/seedkey_other` serves one `G1`; control: an `insert` seed
saves.
- **Red on base `62b3ec36`.** The three in-process files with the four
source files restored to base: 16 failed, 47 passed, every red a refusal
or warn pin and every control green. The dogfood case on a base build of
`@objectstack/metadata-protocol`: 3 failed, 1 passed. Its control failed
too, `inserted: 2` instead of `1`, because the ghost seed the base had
accepted was published in the same batch: the card's duplication,
measured.
<details><summary>Ablation (one-off, from committed HEAD)</summary>
The save door's check in `saveMetaItem` was disabled with `node
scripts/ablation-replace.mjs` (`if (seedKeyRefusal) throw
seedKeyRefusal;` → `if (seedKeyRefusal)
console.debug('ABLATION_28952_SAVE_DOOR');`): anchor 1 → 0, blob
`96642a86ae58` → `18dc7463b075`. `@objectstack/metadata-protocol` was
rebuilt and `scripts/ablation-dist-preflight.mjs` read the marker in 2
built files. Readings: the dogfood case 3 failed, 1 passed (the card and
composite refusals, and the control, whose publish now carried the ghost
seed and was refused by the publish promotion);
`protocol.seed-external-id-refusal.test.ts` 8 failed, 14 passed (the
save-door cases; the publish-door cases stay green, as they should).
Restored: blob equals HEAD `96642a86ae58`, `git diff HEAD` empty;
rebuilt, the preflight `--absent` read the marker in 0 of 24 built files
with the tree clean; the dogfood case passed 4/4 again.
A first attempt used `void 'ABLATION_28952_SAVE_DOOR'` as the marker.
The bundler drops that expression, so the preflight read it as absent
from dist even though the dogfood case went red; it was redone with the
`console.debug` marker above, and only the second run's readings are
quoted.
</details>
## Acceptance notes
- **Envelope.** The order first named `400 VALIDATION_ERROR`; the seat
then ruled one door, one shape, so the refusal answers with the
unknown-datasource refusal's `422 INVALID_METADATA` envelope at all
three sites (commit `3d04cc7e`).
- **"The publish probe".** The L3 probe pass (`runBuildProbes`,
`probes.checked.seeds`) runs after the publish has written and by
contract never fails it, so it cannot refuse "before any write". The
publish-side refusal therefore sits in `promoteDraftForPublish`, where
every publish path promotes a draft and the sibling refusals already
sit. `build-probes.ts` is unchanged.
- **Implicit `name`.** An undeclared `externalId` is judged as `name`,
because the loader keys on `name` and the duplication is the same. The
measurement found no shipped seed relying on it.
- **Extension fields in the same batch.** An object's registered
definition includes its extension fields. An `object_extension` drafted
in the same package batch is not in the batch closure this reads, so a
seed keyed on a field only that pending extension adds would be refused
at the batch publish. No shipped seed does this; noted, not filed.
- **Loader warn level.** The order fixed `warn`. Under AGENTS.md's
degradation rule the duplicate rows are a data-consistency outcome, but
the doors now refuse such a seed at authoring and the boot must not
fail, so the remaining case is a stored seed from before this change.
## Verification
Head `d9200288` (base `62b3ec36`). `origin/main` moved 9 commits since
the base, none touching `packages/metadata-protocol`,
`packages/qa/dogfood` or the double ledger; the branch was not merged
with it, and the merge queue tests the merged tree.
<details><summary>Package tests, typecheck, lint</summary>
- `pnpm --filter @objectstack/metadata-protocol test`: 299 files passed
(4 skipped), 22137 tests passed (26 skipped).
- `pnpm --filter @objectstack/metadata-protocol --filter
@objectstack/dogfood typecheck`: exit 0 (the metadata-protocol
`tsconfig` includes `src/**/*`, test files too).
- `pnpm exec eslint --no-inline-config packages/metadata-protocol/src
packages/qa/dogfood/test/seed-external-id-refusal.dogfood.test.ts`: exit
0.
</details>
<details><summary>Dogfood battery (a narrowing)</summary>
`pnpm --filter @objectstack/dogfood test` under the verify lock, after
the `@objectstack/dogfood^...` closure build: VERDICT command-exit 0,
Test Files 384 passed (3 skipped), Tests 3167 passed (28 skipped), at
`b8d8083b`. The one later commit, `d9200288`, changes only
`scripts/engine-double-contract.pinned.json`, which no dogfood case
reads.
</details>
<details><summary>Derived gates</summary>
`node scripts/pm/dispatch-gates.mjs --commands` on this diff named 73
families (65 before the ledger commit, 8 more after it). All 73 exit 0,
and `--ran` answers: 73 derived, 73 run, 0 NOT-MEASURED, 0 UNRUN.
- First pass, three non-zero exits, each fixed and re-run:
`check:engine-double-contract` exit 1 (the two new test doubles were not
pinned; `--write`, committed in `d9200288`, then exit 0);
`check-plugin-teardown-shape --self-test` exit 3 (shallow clone; the
fixture commit fetched, then exit 0); `check:dual-build-cjs-loads` exit
3 (prerequisite: not every package had `dist/`; after a full `turbo run
build`, 118/118 tasks, exit 0: 115 require entry points across 70
packages load).
- The 8 families the ledger commit added (`check:agent-test-spelling`,
`check:bash32-floor`, `check:cli-command-ids`, `check:entry-guard`,
`check:parse-guard`, `check:pnpm-filter-targets`,
`check:pnpm-flag-spellings`, `check:scripts-symbol-anchors`): exit 0 at
`d9200288`.
- The other 62 ran at `b8d8083b`. CI runs the full farm on this head.
</details>
---
_Generated by [Claude
Code](https://claude.ai/code/session_01FEkQ5pEjzdJdM36Fgh7YyY)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 692e0dc commit 3cf7f61
10 files changed
Lines changed: 941 additions & 5 deletions
File tree
- .changeset
- packages
- metadata-protocol/src
- qa/dogfood/test
- scripts
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
90 | 90 | | |
91 | 91 | | |
92 | 92 | | |
| 93 | + | |
93 | 94 | | |
94 | 95 | | |
95 | 96 | | |
| |||
123 | 124 | | |
124 | 125 | | |
125 | 126 | | |
126 | | - | |
| 127 | + | |
127 | 128 | | |
128 | 129 | | |
129 | 130 | | |
| |||
140 | 141 | | |
141 | 142 | | |
142 | 143 | | |
| 144 | + | |
| 145 | + | |
143 | 146 | | |
144 | 147 | | |
145 | 148 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
62 | 62 | | |
63 | 63 | | |
64 | 64 | | |
65 | | - | |
| 65 | + | |
66 | 66 | | |
67 | 67 | | |
68 | 68 | | |
| |||
677 | 677 | | |
678 | 678 | | |
679 | 679 | | |
| 680 | + | |
| 681 | + | |
| 682 | + | |
| 683 | + | |
| 684 | + | |
| 685 | + | |
| 686 | + | |
| 687 | + | |
| 688 | + | |
| 689 | + | |
| 690 | + | |
| 691 | + | |
| 692 | + | |
| 693 | + | |
| 694 | + | |
| 695 | + | |
| 696 | + | |
| 697 | + | |
| 698 | + | |
| 699 | + | |
| 700 | + | |
| 701 | + | |
| 702 | + | |
| 703 | + | |
| 704 | + | |
| 705 | + | |
| 706 | + | |
| 707 | + | |
| 708 | + | |
| 709 | + | |
| 710 | + | |
| 711 | + | |
| 712 | + | |
| 713 | + | |
| 714 | + | |
| 715 | + | |
| 716 | + | |
| 717 | + | |
| 718 | + | |
| 719 | + | |
| 720 | + | |
| 721 | + | |
| 722 | + | |
| 723 | + | |
| 724 | + | |
| 725 | + | |
| 726 | + | |
| 727 | + | |
| 728 | + | |
| 729 | + | |
| 730 | + | |
| 731 | + | |
| 732 | + | |
| 733 | + | |
| 734 | + | |
| 735 | + | |
| 736 | + | |
| 737 | + | |
| 738 | + | |
| 739 | + | |
| 740 | + | |
| 741 | + | |
| 742 | + | |
| 743 | + | |
| 744 | + | |
| 745 | + | |
| 746 | + | |
| 747 | + | |
| 748 | + | |
| 749 | + | |
| 750 | + | |
| 751 | + | |
| 752 | + | |
| 753 | + | |
| 754 | + | |
| 755 | + | |
| 756 | + | |
| 757 | + | |
| 758 | + | |
| 759 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
11 | | - | |
| 11 | + | |
| 12 | + | |
12 | 13 | | |
13 | 14 | | |
14 | 15 | | |
| |||
95 | 96 | | |
96 | 97 | | |
97 | 98 | | |
| 99 | + | |
| 100 | + | |
98 | 101 | | |
99 | 102 | | |
100 | 103 | | |
| |||
188 | 191 | | |
189 | 192 | | |
190 | 193 | | |
191 | | - | |
| 194 | + | |
192 | 195 | | |
193 | 196 | | |
194 | 197 | | |
| |||
200 | 203 | | |
201 | 204 | | |
202 | 205 | | |
| 206 | + | |
203 | 207 | | |
204 | 208 | | |
205 | 209 | | |
| |||
502 | 506 | | |
503 | 507 | | |
504 | 508 | | |
| 509 | + | |
| 510 | + | |
| 511 | + | |
| 512 | + | |
| 513 | + | |
| 514 | + | |
| 515 | + | |
| 516 | + | |
| 517 | + | |
| 518 | + | |
| 519 | + | |
| 520 | + | |
| 521 | + | |
| 522 | + | |
| 523 | + | |
| 524 | + | |
| 525 | + | |
| 526 | + | |
| 527 | + | |
| 528 | + | |
| 529 | + | |
| 530 | + | |
| 531 | + | |
| 532 | + | |
| 533 | + | |
505 | 534 | | |
506 | 535 | | |
507 | 536 | | |
| |||
0 commit comments