Skip to content

fix(core, service-messaging): a failed settings read is retried, not dead-lettered as TEMPLATE_INVALID - #27730

Draft
objectstack-fleet[bot] wants to merge 6 commits into
mainfrom
claude/project-thread-tfwfer
Draft

objectstack-fleet[bot] wants to merge 6 commits into
mainfrom
claude/project-thread-tfwfer

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

domain:server#2 · os-sales · session_01L6bhgUCizoYWQVJWjhdR6F

Requested by Sales · project thread

Fixes #27257

Clause-②: yes (widening) — resolveLocalizationContext's result (@objectstack/core) gains one optional member, readFailed?: true. Nothing is narrowed.
Metadata-protocol: none

维护者速读

  • 改了什么:resolveLocalizationContext 的结果新增可选字段 readFailed: true,只在直连 sys_setting 读取抛错时出现(新读与 30 秒失败记忆命中都带)。消息服务的邮件、站内信渠道据此区分:读失败时不带币种照常渲染;若被拒(TEMPLATE_INVALID),再以 ISO「无币种」码 XXX 作默认试渲染一次(只渲染、不发送):能过,说明唯一的拒绝是缺默认,改记为可重试;仍被拒(如 @path 值错误),原样返回那句拒绝、判永久。读成功且确实没有默认币种时,仍是 TEMPLATE_INVALID、永久失败、带补救说明。
  • 为什么改:原先读失败与"没有默认币种"都返回 currency: undefined,一次瞬时的设置读取故障会让带无参 currency 洞的邮件进死信,永不发送。分诊 6496449802:读失败是瞬时的,要重试;只有读成功且无默认才是永久的。
  • 风险与回滚:扩大一个已发布导出类型(可选字段,Clause-②: yes (widening)),7 个调用方源码兼容、类型检查通过;失败记忆不加长,resolveLocalizationContext's 30s failure cache can answer a deliberate post-bind re-read with the pre-bind value #11877 不变。不含无参 currency 洞的模板在读故障期间照常发送。回滚即 revert。
  • 先看:packages/core/src/security/resolve-authz-context.ts(结果类型与 resolveLocalizationContext 中 value 的构造及失败记忆写入);packages/services/service-messaging/src/email-channel.ts 的 templateRefusalAfterCurrencyReadFault;messaging-service-plugin.ts 的 getTenantCurrency。
  • 两分钟验证:pnpm --filter @objectstack/service-messaging exec vitest run src/template-tenant-currency.test.ts,预期 21 条全过(读失败两渠道判 retryable、记忆命中同样、读成功无默认判 permanent、读失败下与币种无关的拒绝判 permanent)。
  • 需要你做的:在本 PR 上批准这次公开类型扩面(分诊 6506866652 所要求);契约复核与增量复核均已 PASS(6508920767、6509670944),批准后席位落地。

What changes

The resolver says which of the two it is. resolveLocalizationContext already computed backendFailed (the direct sys_setting read threw) but returned only the value, and the failure memo returned the bare value on a hit. The answer now carries readFailed: true in that case. It is built once and the memo stores that same object, so a hit inside the 30 s failure TTL says the read failed too. A settings-service refusal still does not count as a backend fault (#11877 unchanged), and the memo's lifetime is unchanged. On every successful read the member is absent.

Messaging grades from it. getTenantCurrency (messaging-service-plugin.ts) keeps its signature and rejects when the read failed; with no default after a successful read it answers undefined as before. The email and inbox channels catch the rejection, render without a currency, and pass a render failure through templateRefusalAfterCurrencyReadFault (email-channel.ts, shared by both channels, not exported from the package entry): a TEMPLATE_INVALID that followed a failed read triggers ONE trial render (render only, never sent) with the ISO no-currency code XXX as the default. If that render passes, the missing default was the only refusal, and the failure is recorded as the template was not rendered: the organization's default currency could not be read (...); the delivery is retried until that read succeeds, which carries no TEMPLATE_* code, so classifyError grades it retryable. If the trial render still answers TEMPLATE_INVALID (for example an @path value refusal a default cannot cure), that refusal is returned as is and stays permanent. An email service with no renderTemplate cannot tell the two apart and is graded retryable (the helper's docblock says so). Every other failure and every refusal after a successful read is returned unchanged.

Public surface. One optional member on the result type of an exported function of @objectstack/core (reachable from the package root through security/index). No new export name, no input change. EmailChannelOptions / InboxChannelOptions are unchanged in type; their getTenantCurrency JSDoc now states the rejection.

Tests

  • packages/core/src/security/resolve-localization-read-failed.test.ts (new): a throwing direct read answers the built-ins with readFailed: true; the memo's hit keeps it with no second read; a successful read with no currency has no readFailed; a settings refusal in front of a successful direct read has none (resolveLocalizationContext's 30s failure cache can answer a deliberate post-bind re-read with the pre-bind value #11877); past the TTL a recovered read answers the stored default with no kind (the memo is not lengthened).
  • resolve-authz-context.test.ts and resolve-localization-cache.test.ts: two existing failure-memo pins that compared the whole answer with toEqual now include readFailed: true (moved pins, same behaviour otherwise).
  • packages/services/service-messaging/src/template-tenant-currency.test.ts: through the plugin's own wiring (MessagingServicePlugin.init, kernel:ready, getChannel), for both the email and inbox channels: a failed read renders without currency and the refusal is retryable with no TEMPLATE_ text; a second delivery served from the failure memo is retryable again with no second read; a successful read with no default gives exactly the render's TEMPLATE_INVALID sentence (with its remedy), graded permanent; a template that needs no default is delivered during a read fault; TEMPLATE_NOT_FOUND after a failed read stays permanent; a read that finds CNY formats in it; under a failed read, an @path refusal stays permanent as is, and a template with both a missing default and an @path refusal returns the @path refusal, permanent, nothing sent (2 pins per channel); the retryable pins assert the trial render used XXX and sent nothing.

Acceptance notes

  • Branch name. The card thread's harness-pinned branch claude/project-thread-tfwfer names no issue; claim 6507228661 is the identity record.
  • Same family, not here. plugin-auth (withMailTenantCurrency wiring) and service-automation (setTenantCurrencyResolver wiring) also read a failed read as no default; triage filed them as i18n: plugin-auth and service-automation read a failed tenant-settings read as "no default", like #27257; adopt readFailed once it lands (automation fails the notify run) #27710, Blocked-by: #27257, which reads the new member.
  • A persistent fault. Where the direct read fails for a long time (a sys_setting table that does not exist yet), a delivery whose template has a no-argument currency hole is now retried along the retry ladder before it dead-letters, instead of dead-lettering at once. A template without such a hole is unaffected.
  • The PR body's attribution lines name this card thread (the harness requires its own thread link), not the seat thread order-common.md names.

Verification

Final head 4397f06d (S1 fix of contract review 6508920767 at 56a2a5cf; 4397f06d changes only the changeset prose). Seat-relayed readings from the card thread at 56a2a5cf: ablations A4 (never re-grade) 6 red, A5 (re-grade every refusal) 3 red, A6 (ignore the trial render's answer) 4 red, each restored blob == HEAD; @objectstack/service-messaging 62 files / 755 tests passed, typecheck exit 0, eslint 0; derived gates 66/66 exit 0, --ran 0 NOT-MEASURED. CI on 4397f06d: all required checks green. Delta contract review 6509670944: PASS.

First head aef05cf6 (merges origin/main at ecd944f5). The full runs below were taken on 55cc3270, before the merge; after it, the scoped re-checks of the last bullet.

  • Ablations (scripts/ablation-replace.mjs, wrap mode, on 55cc3270; each leg red, then restored with blob == HEAD and git diff HEAD empty):
    • A1, no kind on the fresh answer (value = read): core 3 of 5 red, messaging 4 of 17 red.
    • A2, the memo stores the bare value (value: read in the failure entry): core 1 red (the memo-hit pin), messaging 2 red (the memo-hit pins).
    • A3, the plugin never rejects: messaging 4 red.
    • A4, the channel never re-grades: messaging 4 red.
    • A5, the channel re-grades every TEMPLATE_INVALID: messaging 3 red (both "no default stays permanent" pins and the existing permanent pin).
    • The first A4 attempt was refused by the tool (its replacement text already occurred in the file) and restored; it was rerun with a distinct replacement.
  • Tests and types (under os-verify-lock, on 55cc3270): @objectstack/core test 127 files / 2853 tests passed; @objectstack/service-messaging test 62 / 751 passed; typecheck exit 0 for core, service-messaging and the callers mcp, plugin-audit, plugin-auth, plugin-hono-server, service-automation, rest.
  • Gates: node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 66 families; each was run with its exit code recorded. --ran: 66 derived, 66 run, 0 NOT-MEASURED, 0 UNRUN, all exit 0. The first run of check:objectql-double-limit flagged both new sys_setting doubles as unjudged; they now hold the caller's bound, and the gate is green. check:i18n and check:dual-build-cjs-loads needed the full turbo run build (114 tasks) first.
  • Lint: pnpm lint exit 0; ESLint on the changed files, 0 errors.
  • After the merge (aef05cf6): rebuilt @objectstack/core... and service-messaging; typecheck exit 0 for both; @objectstack/core src/security 60 files / 1164 tests and @objectstack/service-messaging 62 / 751 passed.
  • Open PRs: none of the 26 open PRs touches these files.
  • Dogfood battery: not run, because the title has no ! and there is no (narrowing) arm.

Generated by Claude Code

🤖 Generated with Claude Code

https://claude.ai/code/session_01L6bhgUg7mjioboFdpaj1fQ

…not dead-lettered as TEMPLATE_INVALID

resolveLocalizationContext marks an answer whose direct sys_setting read
threw with readFailed: true, on the fresh read and on the failure memo's
hit alike. Messaging's getTenantCurrency rejects on it, and the email and
inbox channels render without a currency and grade a TEMPLATE_INVALID that
followed a failed read as retryable.

Claude-Session: https://claude.ai/code/session_01L6bhgUg7mjioboFdpaj1fQ
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 29, 2026
@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/core, @objectstack/service-messaging, touching 10 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/automation/webhooks.mdx (via MessagingServicePlugin (symbol, a top-level class))

⛔ 3 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v14.mdx (via MessagingServicePlugin (symbol, a top-level class))
  • content/docs/releases/v17/17-5.mdx (via MessagingServicePlugin (symbol, a top-level class))
  • content/docs/releases/v18/18-2.mdx (via resolveLocalizationContext (symbol, a top-level function))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 3 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 58 of 210 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 39 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 481ade61c9c8369a78a0350090f6b204900a06c6 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from b9125d677dc95a4990015e83c728cf4c84011601 — the merge of head 4397f06d5f45b7ffbdc6ed0ba68749b26fadfe0b into base 481ade61c9c8369a78a0350090f6b204900a06c6, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin b9125d677dc95a4990015e83c728cf4c84011601 && git checkout b9125d677dc95a4990015e83c728cf4c84011601
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 481ade61c9c8369a78a0350090f6b204900a06c6 4397f06d5f45b7ffbdc6ed0ba68749b26fadfe0b && git checkout -B drift-repro 481ade61c9c8369a78a0350090f6b204900a06c6 && git merge --no-ff 4397f06d5f45b7ffbdc6ed0ba68749b26fadfe0b

node scripts/docs-audit/affected-docs.mjs --json 481ade61c9c8369a78a0350090f6b204900a06c6

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 481ade61c9c8369a78a0350090f6b204900a06c6 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#2 · os-sales · session_01L6bhgUCizoYWQVJWjhdR6F

Contract-review-claim: session_01L6bhgUCizoYWQVJWjhdR6F · head aef05cf · 2026-10-29T21:21Z

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#2 · os-sales · session_01L6bhgUCizoYWQVJWjhdR6F

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: aef05cf6dafd9b1b109ff4cf909049cbf3ba46b0
Local-runs: none
Scope: .changeset/27257-localization-read-failed.md, packages/core/src/security/resolve-authz-context.ts, packages/core/src/security/resolve-authz-context.test.ts, packages/core/src/security/resolve-localization-cache.test.ts, packages/core/src/security/resolve-localization-read-failed.test.ts, packages/services/service-messaging/src/email-channel.ts, packages/services/service-messaging/src/inbox-channel.ts, packages/services/service-messaging/src/messaging-service-plugin.ts, packages/services/service-messaging/src/template-tenant-currency.test.ts + packages/core/src/security/assemble-execution-context.ts (EntryLocalization, entryFields), packages/core/src/security/request-execution-context.ts, packages/core/src/security/organization-reach.ts and localizationFallbackReach / resolveGuestOrganization in the resolver file (the reach the direct read is scoped by), packages/core/src/security/index.ts (the export path), packages/mcp/src/plugin.ts (resolveStdioExecutionContext, the localization memo), packages/plugins/plugin-audit/src/audit-plugin.ts, packages/plugins/plugin-auth/src/auth-plugin.ts and mail-tenant-currency.ts, packages/plugins/plugin-hono-server/src/current-user-endpoints.ts (resolveCurrentUserLocalization), packages/services/service-automation/src/plugin.ts, run-resume.ts and engine.ts (setTenantCurrencyResolver), packages/services/service-messaging/src/dispatcher.ts, backoff.ts, channel.ts, index.ts, sms-channel.ts (the grading, the retry ladder, the package entry, the other channels), packages/formula/src/template-engine.ts (currencyDefaultRefusal, the sentence the channel's rewrite stands in for), content/docs/automation/email-templates.mdx and content/docs/kernel/runtime-services/email-service.mdx (read for the claims they make; prose, not reviewed). Written at 2026-10-29T21:37Z; the check-run and review readings below were taken at 2026-10-29T21:35Z.

① Derived judgments

Each accept-set and public-surface change the diff implies, read against the cited code on this head:

  1. @objectstack/core — resolveLocalizationContext's result gains readFailed?: true — right, and minimal. The alias LocalizationResult is not exported; the surface is the return type of the exported function (security/index → package root). One optional key, literal true, absent on every successful answer; no input change, no new export name. The seven callers, each read: request-execution-context.ts assigns the answer to EntryLocalization (structurally assignable; the key is carried at runtime) and the shared assembler entryFields copies timezone / locale / currency by name, so ExecutionContext never carries it; mcp/plugin.ts memoizes the promise and hands it to resolveStdioExecutionContext, which calls that same assembler; plugin-audit destructures locale; plugin-auth reads .currency; the hono /auth/me/localization handler copies the three fields into its response; service-automation reads .currency; service-messaging is this PR. None spreads, deep-equals, serialises or persists the object, so no HTTP response, i18n payload, run context or delivery row sees the key except where this PR writes a sentence about it. Type Check · workspace / source gates / consumer gates and TypeScript Type Check are green on this head.
  2. The failure memo keeps the kind, and only the kind it should — right. The flagged object is built once (backendFailed ? { ...read, readFailed: true } : read) and stored as the failure entry's value; the hit path returns hit.value, so a hit inside the 30 s TTL carries the flag (pinned twice in core, twice in messaging). The success entry stores the unflagged read, so no success is ever cached with the flag. LOCALIZATION_FAILURE_CACHE_TTL_MS, localizationEntryIsLive (a failure entry stays live until its TTL) and putLocalizationEntry are untouched. backendFailed is set at the one sys_setting catch only; the five settings-service legs still set nothing (resolveLocalizationContext's 30s failure cache can answer a deliberate post-bind re-read with the pre-bind value #11877, pinned with getMany throwing in front of a successful empty direct read). A stale failure returned after a later success is unreachable within one key — a live hit returns before any read, and another key is another entry; the up-to-30 s "still failed" window after the backend recovers existed before this PR and now lands on the ladder's 1 s / 10 s / 60 s retries, the third of which is past the TTL. A non-cacheable ql gets the flag on the fresh answer and nothing stored.
  3. @objectstack/service-messaging — getTenantCurrency rejects on readFailed, answers undefined on a successful read with no default — right. The public EmailChannelOptions.getTenantCurrency / InboxChannelOptions.getTenantCurrency signatures are unchanged (their JSDoc now states the rejection). Both channels read the currency BEFORE the render try, so a rejection cannot be mistaken for a render refusal, then render with the currency key omitted — the same render a failed read produced before this PR. templateRefusalAfterCurrencyReadFault is a module export of email-channel.ts that index.ts (the package's one entry) does not re-export: no new published symbol.
  4. Grading — right, bounded, and the same on both channels. The rewritten row error carries no TEMPLATE_* token and does not start with TRANSPORT_NOT_CONFIGURED:, so classifyError answers retryable; classifyDeliveryAttempt then walks nextRetryDelayMs's schedule (about 1 s, 10 s, 1 m, 10 m, 1 h) and answers dead on the sixth attempt — there is no endless loop, worst case about 71 minutes before dead-letter. The dispatcher consults classifyError on the single and the digest path alike; the digest path has no template arm. sms-channel.ts and the webhook / http outbox never read a currency and are untouched. Every other refusal (TEMPLATE_NOT_FOUND / TEMPLATE_INACTIVE / MISSING_VARIABLES / TEMPLATE_UNSUPPORTED, and a TEMPLATE_INVALID after a successful read) keeps its grade — TEMPLATE_NOT_FOUND during a fault and "no default after a successful read" are pinned permanent on both channels.
  5. What a reader of the row now sees. During a fault a delivery whose template needs no default is sent (pinned); one whose template has a no-argument currency hole is retried and the row says the organization's default currency could not be read. A TEMPLATE_INVALID of a cause a default cannot cure is rewritten too — ④ S1.
  6. The docs' claims still hold. email-templates.mdx and email-service.mdx say the render refuses such a hole with TEMPLATE_INVALID and sends nothing, and that the refusal clears when the default is set or the hole names its code — true of the render before and after; neither page states the outbox's grade of that refusal during a settings fault, so no page moves. The drift check's webhooks.mdx row is the class-symbol anchor, not a claim this diff touches.

② Semver level

patch on @objectstack/core and @objectstack/service-messaging (both private: false, both at 18.2.0), with Clause-②: yes (widening) and Metadata-protocol: none line-initial on the PR body and in .changeset/27257-localization-read-failed.md, and a Before / Now / Unchanged body. Matches the diff. AGENTS.md Post-Task Checklist step 3 sets the level by the kind alone: none takes patch, and "a non-protocol change, widening or breaking, ships patch" — so core is patch, not minor: nothing a metadata author can write, no accepted value and no meaning of the hole moves; what moves is the outbox's grade of a render refusal during a settings fault, and one optional member on a runtime result type. No BREAKING banner, so no ADR-0087 disposition marker is owed (check-adr-0087-registration judges declared-breaking changesets only); no Skills-docs: line is owed (kind none, no (narrowing) arm). No content/docs/releases/ or CHANGELOG.md edit. Check Changeset is green on this head.

③ Boundary flags

  • Deviation (report 6508594911): the PR body's attribution lines name the card thread and its session, not the seat thread. Accepted — the harness writes them; the claim 6507228661 declares the branch claude/project-thread-tfwfer as the identity record, and the branch names no issue for the same reason (the Duplicate Fix Guard warning is expected and is not a claim).
  • Landing: the PR body names the maintainer's approval of the widening as the one action needed. Read at the reading time above, the PR carries no review of any state (the reviews list is empty) and no GOVERNED_APPROVERS approval; the PR is draft, base main, head unchanged. That approval is the landing precondition triage 6506866652 set, not this verdict — this record does not stand in for it.
  • open_questions: none.
  • out_of_scope_findings[0] — i18n: plugin-auth and service-automation read a failed tenant-settings read as "no default", like #27257; adopt readFailed once it lands (automation fails the notify run) #27710 (plugin-auth and service-automation fold a failed read into "no default"). Noted; this PR's shape serves it: both siblings hold the result object (auth-plugin.ts returns .currency of it, plugin.ts in automation does the same) and both wrappers (withCurrency in mail-tenant-currency.ts, withTenantCurrency in run-resume.ts) already catch a rejection, so i18n: plugin-auth and service-automation read a failed tenant-settings read as "no default", like #27257; adopt readFailed once it lands (automation fails the notify run) #27710 is a two-site change on this member with no further core change. The memo-hit semantics (①.2) reach them unchanged.
  • Acceptance note "a persistent fault": a no-argument-currency template on an environment whose sys_setting read keeps failing now walks the retry ladder before it dead-letters. Accepted as the card's own trade, with ④ S1 on what the final row says.
  • Ablations (five legs, report and PR body). Read against the pins, each count is the one the pins predict: A1 reds the three fresh-read / memo pins of the new core file and the four fresh-read / memo-hit pins in messaging; A2 reds the memo-hit pins alone; A5 reds the two "no default stays permanent" pins plus the pre-existing permanent pin (line 35 of the messaging file). The core counts are over the NEW file alone (5 tests); the two moved pins in the existing files (toEqual of the whole memoized answer) would also red under A1 and A2. Not re-run here (Local-runs: none).
  • Premise: holds — the removed lines of the diff show the resolver returning only value and memoizing the bare value, and the plugin folding both cases into undefined.
  • Face and tier, for the seat. Read off the committed tree by reviewFace's rule, this diff has no packages/spec/src/** path and no governed path, and its changeset declares yes (widening), so its face is the declaration face, whose exact token under contract-review.md is TIER_DEFAULT. This record is served at CONTRACT_REVIEW_TIER per the seat's brief and carries ④; no queue guard reads it (no governed path), so the landing lift is the maintainer's approval above, and nothing mechanical is owed a different token.
  • Open PRs touching these files: the dev reports none of 26. Not re-measured here.
  • Check-runs on this head, read at the reading time above — none red. Green (36): Lint & Repo Gates, TypeScript Type Check, Build Core, Governed Surface Queue Guard, Test UI, Type Check UI, Build & E2E UI, Live E2E UI, Check Changeset, Check PR Size, Dogfood Regression Gate, Flag docs affected by code changes, PR-body guards, Plan Test Core shards, Test Core shards 1, 2, 6, 7, 8, every Lint · job (ESLint, PM dispatch-gates, docs & code guards, repo & tooling gates, CI / release / engine gates, the three scoped jobs), every Type Check · job (workspace, source gates, consumer gates, debt ledger), Build UI Deps, Auto Label, filter. Still in progress (3): Test Core (3/8), (4/8), (5/8) — the required Test Core context is not answered until they complete; nothing here stands in for it. Skipped (12): the opt-in harnesses, Build Docs, Lint UI, the sharded dogfood / Test UI matrix rows, Temporal Conformance, UI Published Artifacts, the schedule gate, and the second Auto Label / Check PR Size runs.

④ Design judgment

For each in-scope file, against the code it cites and the scenario it is accepted against (a transient sys_setting fault must retry; a successful read with no default must dead-letter with the remedy):

  • resolve-authz-context.ts — holds. Tried: (i) a hit returns the stored object, so the kind survives the memo; (ii) a success after a failure inside the TTL is unreachable for one key; (iii) a settings service that answers manifest defaults (source: 'default', timezone truthy) returns early unflagged — right, the service answered and "no default" is its word; (iv) a settings refusal in front of a throwing direct read is flagged — the 新环境日志被 sys_setting 'no such table' ERROR 刷屏:本地化读取先于建表,真错误被噪音淹没 #10221 environment, pinned; (v) a ql with no find reads nothing and is unflagged — a composition, not a fault; (vi) the object identity shared by memo and callers predates this PR. One residual, S2 below.
  • resolve-authz-context.test.ts, resolve-localization-cache.test.ts — hold. Two moved pins, each a whole-answer toEqual on the memoized failure, so each pins the memo-hit kind as well.
  • resolve-localization-read-failed.test.ts — holds. Five pins: fresh flag, memo-hit flag with one read, successful-empty unflagged, resolveLocalizationContext's 30s failure cache can answer a deliberate post-bind re-read with the pre-bind value #11877 unflagged, and the TTL pin on ONE engine whose backend is swapped under fake timers (the WeakMap key stays stable, so the pin measures the memo and not a new engine). Both doubles hold the caller's limit after the filter.
  • email-channel.ts — holds, with S1. The read sits before the render try; currency undefined omits the key, as before; the helper's docblock states the broad match and its self-correction.
  • inbox-channel.ts — holds. Mirrors the email arm through the shared helper; the new value import from email-channel.ts adds the edge inbox → email with no cycle (email imports nothing from inbox).
  • messaging-service-plugin.ts — holds. Dropping the blanket try/catch is right: the resolver never throws by contract, and a getData() throw now surfaces as a rejection the channel grades retryable instead of being folded into a permanent "no default".
  • template-tenant-currency.test.ts — holds. Twelve pins through MessagingServicePlugin.init + kernel:ready + getChannel, both channels, each on its own engine object. The REFUSAL constant paraphrases currencyDefaultRefusal's sentence; fine for a token match, and the thing to replace if S1 narrows on prose.
  • .changeset/27257-localization-read-failed.md — right (②).

Findings:

  • S1 — should-fix — templateRefusalAfterCurrencyReadFault rewrites every TEMPLATE_INVALID after a failed read, and drops the render's diagnosis. TEMPLATE_INVALID has three causes in plugin-email's renderer: a hole the grammar refuses, an @path value that is not a code, and the missing default. Only the third is cured by the read succeeding. A template of the first two kinds, during a persistent fault (the fresh environment the acceptance notes name), now burns the whole ladder and dead-letters after about 71 minutes with a row error that names the settings read and not the hole — the sentence the row used to carry. Bounded, documented in the helper's docblock, and self-correcting once the read succeeds, so not a FAIL. Remedy: keep the render's sentence in the rewritten error with its code spelled out of classifyError's reach (strip the leading code and quote the rest as the render's refusal), or match the no-default cause alone — currencyDefaultRefusal is one sentence in @objectstack/formula, which messaging does not depend on, so a stable fragment or a sub-code from the renderer is the handle.
  • S2 — should-fix, as a follow-up card beside i18n: plugin-auth and service-automation read a failed tenant-settings read as "no default", like #27257; adopt readFailed once it lands (automation fails the notify run) #27710 (outside the shape triage accepted) — the organization-less leg of the same fault is still graded permanent. localizationFallbackReach turns an AuthzStoreUnavailableError from the sys_organization probe (resolveGuestOrganization → tryFind, a failed read other than a missing table) into unowned; no sys_setting read then runs, backendFailed stays false, and the answer is "no default" with no readFailed. For a delivery that names no organization under a posture with no wall, the same transient database fault the card is about still dead-letters a no-argument-currency template. The accepted shape scopes the flag to the direct sys_setting read, so this is not a miss against the card's grade; set readFailed on that leg too (without populating the memo, which 新环境日志被 sys_setting 'no such table' ERROR 刷屏:本地化读取先于建表,真错误被噪音淹没 #10221 reserves for the direct read), or carry it on i18n: plugin-auth and service-automation read a failed tenant-settings read as "no default", like #27257; adopt readFailed once it lands (automation fails the notify run) #27710.

No must-fix. The widening is the one key triage accepted, absent on every successful answer; the memo keeps the kind without lengthening; a failed read retries along a bounded ladder on both channels; a successful read with no default dead-letters with the render's remedy; #11877 and the TTL are untouched; nothing outside the two channels changes behaviour.

Implemented-by: claude/project-thread-tfwfer
Reviewed-by: session_01L6bhgUCizoYWQVJWjhdR6F

VERDICT: PASS

…after a failed currency read

A TEMPLATE_INVALID that follows a failed tenant-currency read is rendered
once more, without sending, with the no-currency code XXX as the default.
If that render passes, the missing default was the only refusal and the
delivery is retried; if it still refuses, that refusal stands, permanent.

Claude-Session: https://claude.ai/code/session_01L6bhgUg7mjioboFdpaj1fQ
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#2 · os-sales · session_01L6bhgUCizoYWQVJWjhdR6F

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 4397f06d5f45b7ffbdc6ed0ba68749b26fadfe0b
Local-runs: none
Scope: packages/services/service-messaging/src/email-channel.ts, packages/services/service-messaging/src/inbox-channel.ts, packages/services/service-messaging/src/template-tenant-currency.test.ts, .changeset/27257-localization-read-failed.md (the four paths that moved between aef05cf6 and this head; the other five files of the PR are byte-identical to the head record 6508920767 reviewed, and that record's judgments on them carry) + packages/plugins/plugin-email/src/email-service.ts (resolveAndRenderTemplate, renderTemplate, sendTemplate — the probe's path), packages/plugins/plugin-email/src/template-loader.ts (load, the read the probe repeats), packages/plugins/plugin-email/src/template-engine.ts (every consumer of the supplied currency), packages/formula/src/template-engine.ts (the currency formatter, isCurrencyCode, currencyDefaultRefusal), packages/spec/src/contracts/email-service.ts (IEmailService.renderTemplate, RenderTemplateInput), packages/services/service-messaging/src/index.ts (the package entry). Delta review on the head that answers S1 of record 6508920767; written at 2026-10-29T22:20Z, the check-run and review readings below taken at 2026-10-29T22:16Z. Commits since the reviewed head: 56a2a5cf (code), 4397f06d (changeset text only); merge-base with main unchanged.

① Derived judgments

  1. templateRefusalAfterCurrencyReadFault is now async and takes renderWithDefault — right, and the only behaviour change of this head. It still returns the message untouched unless a read fault stands AND the message carries TEMPLATE_INVALID. Then it renders once more with the ISO "no currency" code XXX: a probe that renders means the missing default was the only refusal, and the row is rewritten without a TEMPLATE_* token (retryable, as before); a probe that answers TEMPLATE_INVALID returns THAT sentence, which classifyError grades permanent; no probe, or a probe failing any other way, keeps the retryable rewrite. This closes S1 of 6508920767 exactly as that finding asked: a refusal a default cannot cure keeps its own sentence and its permanent grade.
  2. The probe has no side effect — right. The email channel's closure calls email.renderTemplate (bound), never sendTemplate; the inbox channel awaits the probe and discards its value (title / bodyMd are assigned only inside the first try). IEmailService.renderTemplate runs resolveAndRenderTemplate and returns rendered: the template loader's load (a registry read through protocol.getMetaItemsForExecution, no write), the required-variables check, and the three face renders. No transport, queue, persistence, outbox, hook, counter or audit row sits on that path — the only write-shaped calls in email-service.ts (persistence.insert) are in the send paths, which the probe never enters. Cost: one extra metadata read and render per attempt that already failed, bounded by the retry ladder.
  3. XXX is safe in every formatter path — right. Formula's currency formatter accepts an argument or default by /^[A-Z]{3}$/ (so currencyDefaultRefusal answers null for XXX) and formats with new Intl.NumberFormat(locale, { style: 'currency', currency: code }); ECMA-402's well-formedness check asks for three ASCII letters and nothing else, so no locale and no ICU build throws on XXX (CLDR renders it with the generic sign). The mail renderer's other consumer is the currency:@path branch, which judges the send's own value and never reads the default; a non-numeric amount falls to baseString. So a probe refusal is only ever one the default cannot cure, and the permanent grade is never reached because of XXX itself. The constant flows into the probe closure alone and can reach no sent mail or stored row.
  4. The no-renderTemplate fallback (treated as retryable) — acceptable. IEmailService.renderTemplate is a required member of the spec contract, so the fallback reaches only a structural third-party sender that implements sendTemplate without it; there the head keeps the previous head's broad grade, and the docblock says so. The inbox channel's renderTemplate! is guarded by its earlier TEMPLATE_UNSUPPORTED refusal. A probe failing in another way (the registry read failing under the same backend fault, a template retired between the two renders) is graded retryable — right, since nothing shows the template at fault.
  5. Refusal order — right. The real renderer refuses at the first offending hole; the both pin covers a template whose first refusal is the missing default and whose probe then meets the @path refusal: the standing refusal is returned, permanent, nothing sent. A template whose @path refusal comes first never reaches the probe's retryable arm.
  6. Public surface unchanged. The helper is still a module export index.ts does not re-export; EmailChannelOptions / InboxChannelOptions / EmailSenderSurface are unchanged in type (④ S3 on the last); callers are the email and inbox channels only (grep of packages/ and ui/).
  7. The changeset's Now paragraph now states the probe — matches the tree.

② Semver level

Unchanged from 6508920767: patch on @objectstack/core and @objectstack/service-messaging, Clause-②: yes (widening) and Metadata-protocol: none line-initial in the PR body and the changeset; the head's only changeset edit is prose. Matches the diff. No ADR-0087 marker owed, no Skills-docs: line owed. Check Changeset is green on this head.

③ Boundary flags

  • S1 of 6508920767 — fixed on this head, verified by reading (①.1–①.5). The dev's account of the delta (ablation legs A4 / A5 / A6 red and restored at 56a2a5cf, four new pins) reached this review through the seat; it is not on the board — no report comment on [finding] resolveLocalizationContext cannot tell "no tenant default" from "the settings read failed", so a transient sys_setting fault dead-letters a mail with a {{ x | currency }} hole as a permanent TEMPLATE_INVALID #27257 since 6508594911 and no PR comment since the record — and was not re-run here (Local-runs: none). The pins themselves were read: four new (path-value and both, per channel) and the two retryable pins now assert two render calls, calls[1].currency === 'XXX' and sent() === 0.
  • S2 of 6508920767 — filed as core: an organization-probe store fault in localizationFallbackReach reads as unowned, so the delivery's missing default currency is graded permanent with no readFailed #27733 (pm:blocked), not on this PR. Right: outside the shape triage accepted.
  • S4 (should-fix, below): the PR body describes the previous head. The maintainer quick-read still says a TEMPLATE_INVALID after a failed read is re-recorded retryable (若因此被拒则改记为可重试), the two-minute verification expects 17 passing tests (now 21), "What changes", "Tests" and "Verification" (Head aef05cf6, legs A1–A5) predate the probe, and the body names neither the probe render nor XXX. The maintainer approves the widening off this body.
  • Maintainer approval: not present. At the reading time the PR carries no review of any state; draft, base main, head 4397f06d. The landing precondition of triage 6506866652 is unchanged and this record does not stand in for it.
  • Face and tier: as in 6508920767 — the committed tree's face is the declaration face (no packages/spec/src/**, no governed path, yes (widening)); served at CONTRACT_REVIEW_TIER per the seat's brief, with ④; no queue guard reads it.
  • Commits: both carry the model-free trailer pair; no model identifier in the diff.
  • Check-runs on this head at the reading time — all complete, none red. Green (38): Lint & Repo Gates, TypeScript Type Check, Test Core and all eight shards, Build Core, Governed Surface Queue Guard, Type Check UI, Test UI, Build & E2E UI, Live E2E UI, Check Changeset, Check PR Size, Dogfood Regression Gate, Flag docs affected by code changes, PR-body guards, Plan Test Core shards, every Lint · and Type Check · job, Build UI Deps, Auto Label, filter. In progress: none. Skipped (10): Lint UI, Build Docs, the opt-in harnesses, the sharded dogfood / Test UI matrix rows, Temporal Conformance, UI Published Artifacts, the schedule gate.

④ Design judgment

  • email-channel.ts — holds. Tried: the probe cannot send (①.2); XXX cannot throw or be formatted into a sent mail (①.3); a probe that fails for a reason other than TEMPLATE_INVALID lands retryable, the right side when the fault is the backend's; the closure captures the same templateName, data and templateLocale as the failed render, and the loader keys on name and locale alone (RenderTemplateInput has no organization), so the probe resolves the row the first render resolved; renderTemplate?.bind(email) keeps the service's this. One finding, S3.
  • inbox-channel.ts — holds. Mirrors the email arm; the probe's value is discarded; the non-null assertion is behind the arm's own TEMPLATE_UNSUPPORTED refusal.
  • template-tenant-currency.test.ts — holds. Twenty-one pins through the plugin's wiring, both channels; the sent() counter is what proves the email probe never sends; REFUSAL and PATH_REFUSAL paraphrase the renderer's sentences, which is fine now that the helper judges the code token, not the prose.
  • .changeset — right.

Findings:

  • S3 — should-fix — EmailSenderSurface still declares no currency on sendTemplate or renderTemplate, so the channel smuggles it past the type. Both channels pass the probe as ...{ currency: probe }, and the first render passes ...(currency ? { currency } : {}) (the shape spec(formula): a currency hole with no argument still formats in US dollars — refuse it at authoring in every template dialect (follow-up of #26660) #26939 landed): a spread inside an object literal escapes TypeScript's excess-property check, which is the only reason these calls compile against an input type that does not name the key. A typed implementer of the surface never learns the key is sent. Declare currency?: string on both input shapes of EmailSenderSurface (a widening the PR already declares) and pass it plainly. Pre-existing shape, so not a FAIL.
  • S4 — should-fix — bring the PR body to this head before the maintainer is asked (③): the quick-read's grading sentence, the 17-test expectation, "What changes", "Tests" and "Verification" all describe aef05cf6; the body is what the approval is given on, and after the merge it is the record of what landed.

No must-fix. The probe is render-only and discarded, XXX is well-formed everywhere it can reach, a refusal a default cannot cure now keeps its sentence and its permanent grade, the card's case still retries, and nothing outside the two channels changed.

Implemented-by: claude/project-thread-tfwfer
Reviewed-by: session_01L6bhgUCizoYWQVJWjhdR6F

VERDICT: PASS

This branch has not been deployed

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

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

1 participant