Skip to content

fix(plugin-email): a non-string recipient display name answers the send door's 400, not 500 - #29955

Merged
objectstack-fleet[bot] merged 2 commits into
mainfrom
claude/project-thread-mhk9pz
Nov 3, 2026
Merged

objectstack-fleet[bot] merged 2 commits into
mainfrom
claude/project-thread-mhk9pz

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Requested by Yibo Wang · project thread

Fixes #29891

Clause-②: no
Metadata-protocol: none

维护者速读

  • 改了什么:POST /api/v1/email/send 里 to / cc / bcc / replyTo(含列表项)的显示名不是字符串时,现在返回 400 VALIDATION_FAILED("the display name of … must be a string"),不再是 500 EMAIL_SEND_FAILED;拒收记录照常写入 sys_email。
  • 为什么改:拒收判断本来就对(formatCallerAddress,fix(plugin-email): a malformed recipient address answers 400 VALIDATION_FAILED, not 500 #29250),但随后写拒收行的 rawAddressText 对调用方原始 name 调 .trim(),抛 TypeError,把 400 变成 500。
  • 风险与回滚:只改一个内部函数(未导出),非字符串显示名不写进拒收行(error 列已写明原因);字符串、缺省、null 显示名行为不变。回滚即 revert。
  • 先看:packages/plugins/plugin-email/src/email-service.ts 的 rawAddressText;新 pin packages/rest/src/email-send-display-name.test.ts。
  • 两分钟验证:pnpm --filter @objectstack/rest exec vitest run --project local src/email-send-display-name.test.ts,预期 19 passed。
  • 需要你做的:无,席位落地。

What changes

  • @objectstack/plugin-email (patch). rawAddressText, the writer of the rejection row in EmailService.recordRejectedMessage, reads each address part as unknown input: a string display name is trimmed and kept, any other type is left out of the row, and a non-object, non-string entry records nothing. It still judges nothing. formatCallerAddress stays the one place a caller's recipient shape is refused (it already refused non-string names, fix(plugin-email): a malformed recipient address answers 400 VALIDATION_FAILED, not 500 #29250), so no second validation was added.
  • @objectstack/rest (tests only, no changeset). A new pin drives the real route (HonoHttpServer + RestServer) over the real EmailServicePlugin (log provider) with sys_email persistence wired, which is the stock showcase shape. email-send-door-bounds.test.ts uses a mail double and never reaches the writer. Supporting changes: a devDependency on @objectstack/plugin-email (lockfile importer row), an anchored source alias in vitest.config.ts (check:test-source-alias), the double's update() routed through assertEngineUpdateDispatch, and the regenerated scripts/engine-double-contract.pinned.json row (check:engine-double-contract).

Same-pattern sweep (what I checked)

Every caller-supplied field the rejection row reads: to, cc, bcc, replyTo and from all go through the one rawAddressText, so the fix covers all five. subject is String(...); text / html are typeof-guarded; address is String(...); relatedObject / relatedId / sentBy / organizationId are stored, not trimmed. On the validation side, formatCallerAddress covers all four recipient fields. formatAddress (the sender's path) still calls obj.name?.trim(), but the REST door replaces a caller from with the configured sender, so a caller's non-string sender name never reaches it through this route. Left as is.

Why

Reproduced on 21510bba (main) through the new pin before the fix: every non-string shape in every field answered 500 (see the ablation below). The card's own reproduction: {"to":{"address":"ok@example.com","name":42},"subject":"x","text":"x"}.

Pins

packages/rest/src/email-send-display-name.test.ts, 19 cases:

  • number / object / array / boolean display name × to / cc / bcc / replyTo: 400 VALIDATION_FAILED with the exact message naming the field, one rejection row with status: 'failed', error: 'rejected before delivery: VALIDATION_FAILED: …' and the bare address in the field's column (16);
  • a to list entry with an object name: 400, and the row lists both entries (1);
  • null name sends (200), and a string name sends (200) (2).

Ablation, once. Restored const name = a.name?.trim() in rawAddressText and rebuilt: 17 of 19 failed with AssertionError: expected 500 to be 400 (all except the two 200 controls). Restored the fix: 19 passed.

Deviation from the order (null). The order listed null among the shapes that answer 400. formatCallerAddress deliberately treats null as "no name" (name !== undefined && name !== null), and null?.trim() never threw, so null already answered 200 before this PR. Making it 400 would refuse input the door accepts today (a narrowing), so null is pinned at 200 instead.

Verification

  • pnpm --filter @objectstack/plugin-email test, pnpm --filter @objectstack/rest test: exit 0.
  • typecheck and check:test-typecheck for both packages: exit 0.
  • pnpm lint: exit 0.
  • Full turbo run build --concurrency=2: exit 0.
  • node scripts/pm/dispatch-gates.mjs --commands on the final head: 82 derived, 82 run, all exit 0; --ran: "82 run, 0 NOT-MEASURED". In the first round (before commit 2), check:engine-double-contract and check:test-source-alias were red and are fixed by commit 2. check:dual-build-cjs-loads exited 3 until the full build ran.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WGufzj6ujX8a8Krdcs92Ye

…nd door's 400, not 500

The rejection-row writer (rawAddressText in recordRejectedMessage) called
.trim() on the caller's raw display name after formatCallerAddress had
already raised VALIDATION_FAILED, so POST /api/v1/email/send answered
500 EMAIL_SEND_FAILED. The writer now reads the name as unknown input and
omits a non-string name from the row; formatCallerAddress stays the one
place recipient shape is judged.

Pinned through the real REST route over the real EmailServicePlugin with
sys_email persistence (new rest devDependency on @objectstack/plugin-email).

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WGufzj6ujX8a8Krdcs92Ye
…ineUpdateDispatch and alias plugin-email to source

check:engine-double-contract pins every engine double's update() to the
real dispatch predicate (ledger regenerated with --write), and
check:test-source-alias requires a cross-package value import in a test
to resolve to the checkout's source, not dist.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WGufzj6ujX8a8Krdcs92Ye
@github-actions github-actions Bot added size/m dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation tests tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface labels Nov 3, 2026
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 2 changed package(s); no hand-written page names any of them. ⚠️ 1 changed file(s) yielded no anchor (packages/rest/vitest.config.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/rest/vitest.config.ts) — pages documenting those are invisible to this run
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 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; 103 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 — 23 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 1f6bc5aadc9e463373eaf500e268d071263f4ddc → packageMentionDocs.

Which tree this was computed on

This run read content/docs from b146c255ce2a62b1c4c5c1e95db2b01a2d8074b9 — the merge of head 86cb5d384c1e52eed02b11726093ab7cd12db6d4 into base 1f6bc5aadc9e463373eaf500e268d071263f4ddc, 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 b146c255ce2a62b1c4c5c1e95db2b01a2d8074b9 && git checkout b146c255ce2a62b1c4c5c1e95db2b01a2d8074b9
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 1f6bc5aadc9e463373eaf500e268d071263f4ddc 86cb5d384c1e52eed02b11726093ab7cd12db6d4 && git checkout -B drift-repro 1f6bc5aadc9e463373eaf500e268d071263f4ddc && git merge --no-ff 86cb5d384c1e52eed02b11726093ab7cd12db6d4

node scripts/docs-audit/affected-docs.mjs --json 1f6bc5aadc9e463373eaf500e268d071263f4ddc

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#6 · os-support-ai · session_01WGufzibcN9oHa8kDbTTnL5

Contract review

Served-tier: TIER_DEFAULT
Head-sha: 86cb5d384c1e52eed02b11726093ab7cd12db6d4
Local-runs: none

Face: declaration (Clause-②: no, Metadata-protocol: none). No published schema and no governed path in the committed tree (7 files). Read: the full diff, card #29891 and its triage audit 6601834563, and the dev's report.

① Derived judgments

  • Fix site: rawAddressText (unexported, the sys_email rejection-row writer) reads the display name as unknown; a non-string name is left out of the row, and a non-object, non-string entry records nothing. formatCallerAddress (fix(plugin-email): a malformed recipient address answers 400 VALIDATION_FAILED, not 500 #29250) stays the single judge of recipient shape and already raised the 400; the 500 came only from the writer's .trim(). Correct, and there is no second validation.
  • Sweep: to/cc/bcc/replyTo/from share that one writer; subject/text/html/related never trim raw input; the sender formatAddress trims only configured input, because the REST door replaces a caller from. Adequate.
  • null name (deviation from the order): formatCallerAddress treats null as no name and it never answered 500; refusing it would narrow the door. Pinned at 200. Accepted. The order's list was wrong there.
  • Pin: real route + real EmailServicePlugin + sys_email persistence, 19 cases; ablated (old a.name?.trim()) → 17/19 red "expected 500 to be 400", restored green.

② Semver level

'@objectstack/plugin-email': patch. No exported surface moves (rawAddressText is module-private); @objectstack/rest gains only a devDependency, a test alias and a test, so it ships nothing. Consistent with both declaration lines.

③ Boundary flags

  • No open_questions. The rest devDependency on plugin-email is one-way (plugin-email does not depend on rest); engine-double-contract.pinned.json gains the new test's row.

Implemented-by: claude/project-thread-mhk9pz
Reviewed-by: session_01WGufzibcN9oHa8kDbTTnL5

VERDICT: PASS

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#6 · os-support-ai · session_01WGufzibcN9oHa8kDbTTnL5

席位核对

Verdict: ACCEPT at head 86cb5d384c1e52eed02b11726093ab7cd12db6d4 · 2026-11-03T17:31Z

  • Shape: 7 files (plugin-email rejection-row writer, a rest route pin with its devDependency, alias, lockfile and engine-double row, one patch changeset); no published schema, no governed path ⇒ not Tier H. TIER_DEFAULT review PASS 6610977523 at this head.
  • Premise measured: reproduced by the dev on main (17/19 cases answered 500 EMAIL_SEND_FAILED with the old writer); the 400 was already raised by formatCallerAddress.
  • Gates: plugin-email and rest test/typecheck/test-typecheck exit 0, pnpm lint exit 0, dispatch-gates 82/82 exit 0. Landing waits for every check on this head to be green or an expected skip.
  • Accepted deviations: harness-pinned branch claude/project-thread-mhk9pz; null pinned at 200, not 400 (refusing it would narrow the door). The signed trailer is appended by the seat through the relay.
  • Landing: on all-green at this head, the seat sends pr_ready + automerge_enable through the relay.

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review November 3, 2026 18:16
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Nov 3, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 3, 2026
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Nov 3, 2026
Merged via the queue into main with commit 509c40a Nov 3, 2026
69 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-mhk9pz branch November 3, 2026 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation size/m tests tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface

Projects

None yet

1 participant