fix(objectql): key i18n bundles by (name, locale) so no member is overwritten (#7730) - #7775
Conversation
…rwritten (#7730) `EmailTemplateDefinitionSchema` declares that multiple rows with the same `name` and different `locale` form an i18n bundle, and that a template is resolved by `(name, locale)`. `registerItem` keyed every item by name alone, so the second locale collided with the first and replaced it through the `[Registry] Overwriting email_template: ...` path — a stack authoring en-US and zh-CN copies materialized one row into `sys_email_template`. The storage key now carries the identity the spec declares: a metadata type may declare a discriminator (`ITEM_KEY_DISCRIMINATORS`), and its items are stored under `<packageId>:<name>@<discriminator>`. `email_template`/`locale` is the only entry; every other type keeps name-only identity, byte-identical behaviour, and a pin test asserts it. Reads make the round trip whole rather than moving the defect: a bare-name lookup of a bundled type resolves through the unchanged precedence tiers (ADR-0005 overlay, ADR-0048 prefer-local, first composite) and picks the canonical `en-US` member inside the winning tier, and withdrawal by name takes the whole bundle — matching the consumer side, where `deactivateDeclaredEmailTemplate` sweeps by name across locales. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L3fwc4CLCHphhX4cTcN1KQ
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 1 package(s): 15 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also reference the affected code. These are read-only:
|
github-actions
Bot
added
documentation
⛔ merge queue 构建失败 — 先分诊，再决定要不要重排队列构建 31519403681 红了。队列跑的是全量套件（PR 侧 CI 只跑 affected 子集）， 失败的 job（日志抽取，best effort）：
历史信号：
分诊清单：
Generated by Claude Code · merge-queue-triage workflow (#4859) |
|
Queue kick triaged — not this PR's fault, ⛔ not re-queued yet (engine-core seat #6019, Removed from the queue at 18:03:41Z, Full signature (pulled from the 1.3 MB job log, ⛔ not the triage bot's truncated extract, which quotes only the test name and reads like an assertion failure): Initial reading — a timeout, not an offender. The assertion never ran; no offender list was produced. The test synchronously walks and regexes every Why this PR is not the cause:
Action taken: the measured counter-evidence is recorded on #7706 (the existing card claiming the queue is blocked by a fourth declaration — that premise does not match this build, and a dev dispatched against it would find nothing). Why not re-queued right now: this is this PR's first kick, so one re-queue is sanctioned — but the triage checklist's own item 3 says to wait for the batch ahead to land or fail out before re-queuing, and blind re-queues rebuild the entire downstream queue. If the timeout is load-driven, re-queueing into a busy queue reproduces it and burns everyone. Re-queue is scheduled for the next patrol once the queue has drained; membership will be verified by the The review verdict is unchanged — this PR was and remains ACCEPTed; nothing about its content is in question. Generated by Claude Code |
2 participants
Fixes #7730
The contract this restores — verified, quoted
packages/spec/src/system/email-template.zod.tsL82-86:and the schema header, L20-22: "a single named template is resolved by
(name, locale)". So the declared identity of an email template is a pair, and the spec does state the i18n-bundle semantics the card asserts — this is a restore-invariant, not a contract question.Premise — still valid on
origin/main@8669e5dregistry.tscomputed the item storage key with no locale component, so the second row of a bundle collided with the first and went through the[Registry] Overwriting …path. Confirmed by reverse verification rather than by reading: neutralizing only the key computation (leaving every other change in place) turns 4 of the 18 new tests red, withexpected [ … ] to have a length of 2 but got 1— the reported symptom exactly.What changed
packages/objectql/src/registry.ts, one file:ITEM_KEY_DISCRIMINATORS; its items are stored under<packageId>:<name>@<discriminator>instead of<packageId>:<name>. An item declaring no discriminator is keyed as the canonical member, so{ name }and{ name, locale: 'en-US' }stay one template and re-registration stays idempotent.en-US) member inside the winning tier. That is the localesendTemplatealready falls back to, so the answer is decided by the spec rather than by Map iteration order.getArtifactItemkeeps serving the packaged member over an overlay.unregisterItem,removeOverlayEntry,removeRuntimeShadow), matching the consumer side:deactivateDeclaredEmailTemplatesweepssys_email_templateby name across locales precisely because a delete event carries no locale. Leaving a member registered would make a withdrawn template re-seed. The group is still chosen exactly as before (bare group if present, else the first composite group), so a second package's same-named bundle is untouched — pinned.Why a per-type declaration and not a universal extra key segment, and not duck-typing
item.locale: the key computation is generic to every registered metadata type. Measured onorigin/main,email_templateis the only registered type whose schema declares a top-levellocalethat is part of its identity (locale:acrosspackages/spec/src/**/*.zod.tsotherwise hits SCIM users, execution context, API discovery/protocol payloads and the translation payloads — none of them a registered metadata type). Reading whateveritem.localehappened to be present would silently re-key any type that later grows a locale-ish field; the table makes that a deliberate act. A test pins the table to exactly one entry today, and another pins its canonical locale againstEmailTemplateDefinitionSchema's own parsed default so the literal cannot drift from the spec.Which item types this affects, and which it provably does not
Affected:
email_templateonly. Provably not affected: every other registered type — the discriminator lookup isITEM_KEY_DISCRIMINATORS[type],undefinedfor all of them, and each touched method early-returns into the pre-existing code path unchanged. Pinned from the other side too: twopageitems sharing a name and differing only in alocaleproperty still collapse to one, last-write-wins.Cross-seat surfaces
None.
packages/specis not touched — the spec was read and quoted, not changed. Nopackages/specchange turned out to be needed; if a reviewer wants the canonical locale exported from the schema instead of pinned by test, that is adomain:specdeclaration on #6017 and a follow-up.Consumer sweep
Every consumer of the item storage-key shape is inside
registry.ts—git grepforendsWith(':'/endsWith(\:${…}`)and for${packageId}:${…}composite builders finds no other reader anywhere in the repo (the one non-registry hit is a spec build script matching React block names, unrelated). Thesys_email_templatematerializer readsregistry.listItems('email_template')(values, not keys) and upserts on(name, locale)` — already locale-aware, which is why the registry was the whole defect on that path.One consumer is deliberately out of scope and filed: #7774 —
MetadataProtocol's unscoped list merge dedups bymetaItemKey(packageId, name)with no locale, soGET /meta/email_templatecan collapse the bundle again when ametadataservice is installed. It is not on this issue's reported path (the materializer bypasses it) and its fix needs the discriminator table to live in a packagemetadata-protocoldepends on, which@objectstack/objectqlis not.Verification
pnpm --workspace-concurrency=2 --filter '...@objectstack/objectql...' build— green, before any typecheck....@objectstack/objectql):Scope: 40 of 78 workspace projects,typecheckgreen with noerror TS. The first attempt was a false red —Cannot find module '@objectstack/objectql/core'and'@objectstack/plugin-hono-server'— from unbuiltdist, which is why the closure build above came first and was re-run over the union of consumers and their dependencies.@objectstack/objectql184 files / 3255 tests passed;@objectstack/metadata-protocol72 / 1066 passed;@objectstack/metadata31 / 603 passed;@objectstack/plugin-email20 / 302 passed.packages/objectql/src/registry-i18n-bundle-key.test.ts, 18 cases, passing.pnpm check:durability-log-levelOK ·pnpm check:engine-double-contractOK (150 pinned, 133 debt, 2 exempt) ·node scripts/check-engine-split-ratio.mjsOK (ratio 100.0%) ·node scripts/check-nul-bytes.mjsOK ·eslinton both changed files clean.Reverse verification — two directions, both predicted before running
withDisc→ identity, everything else intact): predicted red on the multi-member pins, green elsewhere. Result: 4 failed / 14 passed —keeps both locales of one name,is what the sys_email_template materializer reads back, the cross-package unregister pin, and the disabled-package pin. The other 14 stay green because with one key there is only one row, so the precedence tiers are trivially satisfied; they are regression guards for the new read path, not discriminators of this bug, and item 2 is what shows they are not vacuous.getItem, key change intact): predicted red on every bare-name read of a bundled type. Result: 7 failed / 11 passed, allexpected undefined to be …— which is the "moved the defect" failure mode in the flesh: the rows exist and nothing can look them up.No refusal/error case is introduced or rewritten by this change, so there is no ADR-0112 envelope assertion to make;
registerItemdoes not refuse, deliberately (#3903 — registration proceeds on spec-conformance failure).Against the same-day churn
registry.tswas modified today by8f1851e(PR #7700, #7557 package-disable propagation), which touchedlistItems— a reader of exactly the collections re-keyed here. Worked from the post-merge code and pinned that gate specifically: a bundle whose owning package is disabled disappears fromlistItemsin full, and both members come back on enable.Generated by Claude Code