Skip to content

email-template-render (a): declared i18n bundle collides — Registry keys email_template without locale, only the last locale survives #7730

Closed

Description

@huangyiirene

Symptom

A stack declaring two emailTemplates with the same name and different locale (an i18n bundle) does not materialize all rows. The registry logs [Registry] Overwriting email_template: packageId:name and only 3 of 4 authored templates reach sys_email_template — the en-US twin is dropped. The spec states that multiple rows with the same name but different locale form an i18n bundle, so both should coexist.

Root cause

packages/objectql/src/registry.ts (~line 1826) computes the storage key without locale:

const storageKey = packageId ? + "${packageId}:${baseName}" + : baseName;

A second row with the same name but a different locale collides on that key and overwrites the first (the [Registry] Overwriting ... path at ~line 1828). Locale is not part of any item key, so the registry cannot hold an i18n bundle for a single name. This contradicts the spec's own "same name + different locale = i18n bundle" statement. The key computation is generic to every item type, so nothing email-specific rescues it.

Stale-premise check: identical in src, in the executed dist, and on origin/main (byte-for-byte with the 92f26f75 pin) — not fixed upstream.

Reproduction

  1. Author a stack declaring two emailTemplates with the same name, one locale: 'en-US' and one locale: 'zh-CN'.
  2. Boot; watch the registry log and query sys_email_template.

Expected: both locales materialize as an i18n bundle. Actual: [Registry] Overwriting email_template is logged and only the last-registered locale survives.

Source

Extracted from the QA run #7690 (framework 92f26f7, console 09987b680).

Activity

  1. self-assigned this
    on Aug 11, 2026
  2. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    Contributor

    Claim: PM loop round 1
    Session: session_01PzrsfP5pa6xAjXJX7Fhsst
    Branch: claude/issue-7730-registry-locale-key
    Worktree: objectstack-issue-7730
    Domain: domain:engine-core
    File surface: packages/objectql/src/registry.ts (region: the item storage-key computation and its overwrite path, ~L1826–1879) + tests + changeset. If the fix turns out to need a packages/spec change, that is a cross-seat surface — declare on #6017 and proceed, ⛔ never silently. Stop on breach; explain in the report.
    Container & model: M (the key computation is generic to every item type, so the blast radius is all registered metadata, not just email templates), mode:cloud, model: opus
    Serial constraints cleared: registry.ts is free — the seat's hot-file queue records no live claim on it, and the one PR that touched it today, PR #7700 (8f1851e, #7557 package-disable propagation), is MERGED. Batch mates this round are #7673 (engine.ts) and #7737 (plugin.ts) — file-disjoint.

    Premise re-verified on origin/main @ 08cd163: registry.ts:1826 still computes const storageKey = packageId ? + "${packageId}:${baseName}" + : baseName; with no locale component, and the overwrite path still logs at L1829. Premise holds.


    Generated by Claude Code

  3. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    Contributor

    Dispatched — PM loop round 1, engine-core seat #6019.

    • Backend: mode:cloud (M ⇒ its own container, per the 2026-08-10 sizing ruling), model claude-opus-5.
    • Cloud session: session_01L3fwc4CLCHphhX4cTcN1KQ · outcome branch claude/issue-7730-registry-locale-key.
    • Dispatched as a restore-invariant (declared ≠ enforced), so whether to fix it is not open — but the dispatch requires the dev to first verify that the spec actually declares the i18n-bundle semantics and quote file:line. If it does not, the card stops being a restore-invariant and becomes a contract question ⇒ needs_decision rather than invented semantics.
    • Two measurements were requested before any key-shape change, because the key computation is generic to every item type: whether locale is meaningful beyond email_template, and who reads these keys — a registry that stores both rows but whose lookup returns an arbitrary one has moved the defect, not fixed it.
    • Flagged: registry.ts was modified today by PR fix(engine-core): propagate package disable to metadata listings and the data plane (#7557) #7700 (8f1851e), which touched listItems — a reader of exactly the collections being re-keyed.

    Generated by Claude Code

  4. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    Contributor
    {
      "issue": 7730,
      "status": "done",
      "branch": "claude/issue-7730-registry-locale-key",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/7775",
      "premise_still_valid": true,
      "summary": "Contract verified first: packages/spec/src/system/email-template.zod.ts L82-86 does state 'Multiple rows with the same name but different locale form an i18n bundle', and the schema header L20-22 says a template 'is resolved by (name, locale)' — so this is a restore-invariant, not a contract question, and NO packages/spec change was needed (spec read and quoted, not touched; no cross-seat declaration owed on #6017). SchemaRegistry.registerItem now keys an item by the identity its type declares: a per-type table (ITEM_KEY_DISCRIMINATORS, email_template/locale the only entry) makes the storage key '<packageId>:<name>@<locale>', so bundle members coexist instead of the second overwriting the first. The measurement the card asked for: locale is meaningful for NO other registered metadata type (email_template is the only registered type whose schema declares an identity-bearing top-level locale), which is why this is a per-type key strategy and not a universal segment nor a duck-typed item.locale read; and every key-shape reader lives inside registry.ts, so the read side was made whole in the same change — bare-name lookup resolves through the SAME precedence tiers as before (ADR-0005 overlay, ADR-0048 prefer-local, first composite) over bundle groups, picking the canonical en-US member inside the winning tier (the locale sendTemplate already falls back to) rather than an arbitrary one, getArtifactItem still prefers the packaged member, and withdrawal by name takes the whole bundle to match deactivateDeclaredEmailTemplate's by-name-across-locales sweep. One reader is deliberately out of scope and filed: #7774.",
      "tests": "Build closure first: `pnpm --workspace-concurrency=2 --filter '...@objectstack/objectql...' build` green (the first typecheck attempt was a false red from unbuilt dist — \"Cannot find module '@objectstack/objectql/core'\", \"'@objectstack/plugin-hono-server'\" — which is why the closure was built over consumers AND their dependencies before re-running). Consumer sweep direction = PREFIX (downstream consumers, '...@objectstack/objectql'): `Scope: 40 of 78 workspace projects`, typecheck green, zero `error TS`. Suites: @objectstack/objectql `Test Files 184 passed (184) / Tests 3255 passed (3255)`; @objectstack/metadata-protocol `72 passed (72) / 1066 passed (1066)`; @objectstack/metadata `31 passed (31) / 603 passed (603)`; @objectstack/plugin-email `20 passed (20) / 302 passed (302)`. New file packages/objectql/src/registry-i18n-bundle-key.test.ts: `Test Files 1 passed (1) / Tests 18 passed (18)`. Gates: `pnpm check:durability-log-level` OK (24 durability-critical seams, all loud); `pnpm check:engine-double-contract` OK (150 pinned, 133 debt, 2 exempt); `node scripts/check-engine-split-ratio.mjs` OK (ratio 100.0%); `node scripts/check-nul-bytes.mjs` OK (7130 files, no raw control bytes) plus a `grep -naP` self-scan of all three changed files, no hits; `eslint` on both changed source files clean. REVERSE VERIFICATION, both directions predicted before running: (1) neutralize ONLY the key computation, everything else intact — predicted red on the multi-member pins and green elsewhere; got `Tests 4 failed | 14 passed`, failures `expected [ … ] to have a length of 2 but got 1` (the reported symptom) plus the cross-package-unregister and disabled-package pins. The other 14 stay green honestly: with one key there is only one row, so the precedence tiers are trivially satisfied — those cases are regression guards for the new read path, not discriminators of the bug, which is what (2) exists to show. (2) neutralize ONLY the read side, key change intact — predicted every bare-name read of a bundled type goes undefined; got `Tests 7 failed | 11 passed`, all `expected undefined to be …`. That is the 'moved the defect rather than fixed it' failure in the flesh, and it is now pinned. NOT APPLICABLE, stated rather than fabricated: no refusal/error case is written or rewritten by this change, so there is no ADR-0112 code+status envelope assertion to make — registerItem deliberately does not refuse (#3903, registration proceeds on spec-conformance failure). Same-day churn checked as instructed: worked from post-merge origin/main @ 8669e5d (8f1851e / PR #7700 included) and pinned that gate specifically — a bundle whose owning package is disabled leaves `listItems` in full and both members return on enable.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #7774 (label `finding`): MetadataProtocol's unscoped metadata list merge dedups by metaItemKey(packageId, name) with no locale (protocol.ts:988, used at :4363/:4377), so GET /meta/email_template can collapse an i18n bundle back to one row when a `metadata` service is installed. Masked until now because the registry itself held only one locale; NOT on this issue's reported path (bootstrapDeclaredEmailTemplates reads registry.listItems directly and never goes through this merge). Left out because the fix needs the discriminator table to live in a package @objectstack/metadata-protocol depends on — it does not depend on @objectstack/objectql — so sharing it is a home decision (@objectstack/metadata-core is the obvious candidate), not a dependency to add in passing."
      ]
    }

    Generated by Claude Code

  5. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    Contributor

    ACCEPT — PR #7775 reviewed (engine-core seat #6019, session_01PzrsfP5pa6xAjXJX7Fhsst). Verified against GitHub, not against the report's claims.

    Scope check — 3 files: registry.ts (+231/-5), the new pin suite (+250), the changeset. No content/docs/releases/ edit, changeset present, no ADR or skills path (ordinary landing path). Exactly the declared file surface, no breach.

    The contract was verified before the fix, which is what the dispatch asked for. packages/spec/src/system/email-template.zod.ts L82-86 does state that same-name/different-locale rows form an i18n bundle, and the header at L20-22 says a template is resolved by (name, locale). So this really is a restore-invariant and not a contract question — and, importantly, packages/spec was read and quoted, not touched, so no cross-seat declaration is owed on #6017. That was the pre-declared possibility in the claim; it did not materialize.

    Both measurements I asked for came back, and they changed the design:

    1. Is locale meaningful beyond email_template? Measured: no — it is the only registered type whose schema declares an identity-bearing top-level locale. Hence a per-type discriminator table (ITEM_KEY_DISCRIMINATORS) rather than a universal extra key segment, and notably not a duck-typed item.locale read. That is the right call: a universal segment would have been undefined for every other type, and duck-typing would have made the key shape depend on payload contents.
    2. Who reads these keys? All key-shape readers live inside registry.ts, and the read side was made whole in the same change — bare-name lookup resolves through the same precedence tiers as before (ADR-0005 overlay, ADR-0048 prefer-local, first composite) and picks the canonical en-US member inside the winning tier rather than an arbitrary one; withdrawal by name takes the whole bundle, matching deactivateDeclaredEmailTemplate's by-name sweep.

    Reverse verification — two directions, both predicted before running. This is the part that earns the accept. (1) Neutralizing only the key computation: 4 red, expected […] to have a length of 2 but got 1 — the reported symptom. (2) Neutralizing only the read side, key change intact: 7 red, all expected undefined to be …. Direction (2) is precisely the "stored both rows but the lookup got worse" failure I warned about in the dispatch, demonstrated and now pinned. The report is also honest that 14 of the 18 stay green under direction (1) because with one key there is only one row — those are regression guards, not discriminators — rather than presenting 18/18 as if all of it were load-bearing.

    ADR-0112 handled correctly by exception. The report states not applicable with a reason — registerItem deliberately does not refuse (#3903, registration proceeds on spec-conformance failure), so no refusal case is written or rewritten. Declining to fabricate an envelope assertion where there is no refusal is the right answer; a manufactured toThrow() here would have been a permanently green test.

    Same-day churn handled: worked from post-merge origin/main @ 8669e5d (includes 8f1851e / PR #7700) and specifically pinned that interaction — a bundle whose owning package is disabled leaves listItems in full, and both members return on re-enable. That was the exact collision risk I flagged, since #7700 touched listItems, a reader of the collections being re-keyed.

    Out-of-scope finding #7774 verified filed — MetadataProtocol's unscoped list merge dedups by metaItemKey(packageId, name) with no locale (protocol.ts:988), so GET /meta/email_template can collapse a bundle back to one row. Correctly not absorbed: the fix needs the discriminator table to live in a package @objectstack/metadata-protocol depends on (it does not depend on objectql), which is a module-home decision, not a dependency to add in passing. It lands in the domain:metadata seat's area — I am not routing it, the triage seat owns that.

    Landing status: ⛔ not flipped to ready yet. ESLint is completed: success, but TypeScript Type Check is still in_progress — and in_progress is not success. A timer is armed to re-read the job conclusions and flip + enqueue once they are actually green. Fixes #7730 on the first line is correct: #7774 is a distinct defect on a different path, so this card closes in full.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions