Repository navigation
fix(plugin-email): validate the default sender at configuration time and record pre-delivery rejections in sys_email - #14371
Conversation
…and record pre-delivery rejections in sys_email An unsendable OS_EMAIL_FROM was first judged inside normalizeMessage on the first send, which for a fresh deployment is the first user's sign-up -- and better-auth's runInBackgroundOrAwait logs and swallows that throw, so the UI reported a verification email nobody had sent. EmailServicePlugin.init() now refuses a declared defaultFrom no message could be sent from; the mail settings channel refuses the value, keeps the previous sender and says so at error. EmailService also writes a sys_email row at status='failed' for a message rejected before delivery -- the one window that previously produced no record. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…rtEngineUpdateDispatch check:engine-double-contract flagged the double in sender-address-validation.test.ts: a fake looser than ObjectQL.update is how a dead route once shipped with a green suite. Routed through the predicate (the shape email-plugin.template-runtime-write.test.ts already pins) and recorded the new pinned coverage in the ledger with --write. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
What this run could not see
Coarse fallback — 4 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 82fdc295ea025ebed1f4a7f3f43f6ea44978aa00 && git checkout 82fdc295ea025ebed1f4a7f3f43f6ea44978aa00
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin d62f990a985eb15152579ad34c99f43e76e3657a 5170e7a85c80db628cfb9fe9133a9dd5e13d5fc7 && git checkout -B drift-repro d62f990a985eb15152579ad34c99f43e76e3657a && git merge --no-ff 5170e7a85c80db628cfb9fe9133a9dd5e13d5fc7
node scripts/docs-audit/affected-docs.mjs --json d62f990a985eb15152579ad34c99f43e76e3657a
|
…e-delivery rejections `sys_email.error` was declared as "Transport error message when status=failed", but since #14371 EmailService.recordRejectedMessage also writes status=failed rows for messages rejected by normalizeMessage before they reach a transport (prefixed "rejected before delivery: ..."). The declared field help was narrower than what the column actually holds. Widen the description (wording settled in triage, issue comment 5504375428) and regenerate the platform-objects i18n bundle with its own tooling (node scripts/check-i18n-bundles.mjs --write) rather than hand-editing the generated file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
…e-delivery rejections (objectstack-ai#15276) * fix(runtime): type the packages-domain `protocol` service handle so undeclared request keys are compile errors (objectstack-ai#15215) * wip(runtime): type the packages-domain protocol service handle Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N * wip(runtime): add the packages-domain protocol handle typing pin Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N * chore(changeset): patch note for the packages-domain protocol handle typing Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N * docs(permissions): re-anchor the system-context census rows moved by the typing block Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N * docs(permissions): regenerate the system-context census from the merged tree Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N --------- Co-authored-by: Claude <noreply@anthropic.com> * fix(objectql): publish the record's organization on every DataEvent (objectstack-ai#15220) * fix(objectql): publish the record's organization on every DataEvent (objectstack-ai#14970) `DataEventSchema.organizationId` was declared and published by the spec half but populated by nothing, so every `data.record.*` event went out with the key absent — which the contract requires a consumer to read as "this record is behind no organization wall". `publishDataEvent` now resolves it from the row itself: the written record on `created`, the post-state on `updated`, and the by-id branch's already-read pre-image on `deleted`, so no per-event read is bought. The record's organization, never `ExecutionContext.tenantId` — that is the caller's active org, and the two diverge on exactly the system/unscoped write this key most needs to label correctly. Absence keeps one spelling: the key is omitted, never `''` (which the schema refuses outright, dropping the whole event) and never an explicit `undefined` (which survives `parse` as a present key). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ * docs(permissions): re-anchor the system-context census after the engine line shift Mechanical repair by `node scripts/check-system-context-census.mjs --fix`, the only correct writer for this table. Pure line rot: the `eventOrganizationId` helper and its threading shifted every later line in `packages/objectql/src/engine.ts`, so 14 anchors (15 citation sites — one source line is cited twice) pointed at the wrong lines. No population and no classification change: still 106 elevation read sites in 20 packages across 45 files, all anchored; 140 anchors resolve, 27 declared non-read — the same figures as before the shift. `--fix` did not refuse, and the diff is digits and nothing else (12 lines added, 12 removed, identical once digits are stripped). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ --------- Co-authored-by: Claude <noreply@anthropic.com> * ci(reaper): arm the merged-branch reaper for scheduled deletion of claude/ branches (objectstack-ai#15224) * ci(reaper): arm the merged-branch reaper for scheduled deletion of claude/ branches Flips .github/workflows/merged-branch-reaper.yml from report-only to the scheduled weekly deletion the maintainer ruled on 2026-09-04 (issue objectstack-ai#12771, decision batch objectstack-ai#30), reaffirming the 2026-08-31 ruling under the base-ref guard PR objectstack-ai#15144 landed. Deletion is a SEPARATE job (`reap`), because `permissions:` is scoped per job. `sweep` keeps `contents: read` + `pull-requests: read` and remains structurally incapable of deleting a ref; `reap` holds the only `contents: write` in the file, consumes the `reapable` list `sweep` publishes as a job output, and computes no classification of its own. Fences: - `reap` never runs on `pull_request` — the self-exercising run stays a dry run — and its `if:` is an allowlist of `schedule` plus a `workflow_dispatch` on which the operator explicitly set `dry_run: false`. - the new `dry_run` workflow_dispatch input defaults to true, so the manual path is fail-closed. - the base-ref guard, `PREFIX`, `BASE_REF`, the grace window, the schedule and the `is-ancestor` prohibition are all untouched. - the whole deletion list is printed to the run log before the first delete. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zGPuVVX3deAx9LdjK8jCk * test(reaper): pin the deletion hand-off and fence the delete job structurally The contract harness drives the `sweep` classifier and can say nothing about the job that deletes — deletion deliberately lives outside the extracted script, so what the harness judges stays a classification rather than an action. Two additions close that gap. 1. The hand-off. `sweep` now publishes `reapable_branches`, the machine-readable half of the list it prints, and `reap` consumes that and nothing else. Scenarios G1/G2/R1 pin that the list EQUALS the reapable bucket — same members, same order — over a population carrying one branch in every bucket, and mutations M13/M14 drive both directions red (held branches leaking in; the list not published at all). 2. The fence. `reapFenceFailures()` parses the shipped YAML and asserts the delete job's structure: its `if:` excludes `pull_request` and gates `workflow_dispatch` on `inputs.dry_run == false`; it declares `contents: write` and is the ONLY job in the file that does; the top-level grant stays `contents: read`; it still `needs: sweep`. New self-test battery 6 drives six mutations of the workflow text to red, each asserting its anchor was present first. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zGPuVVX3deAx9LdjK8jCk * ci(reaper): put every excluded bucket on the run-log audit line The maintainer's ruling names the run log as the audit trail, and the notice line named three of the seven buckets — reapable, mergedElsewhere, noPr. The other four (open, closedUnmerged, grace, protectedBranch) lived only in the step summary and the uploaded artifact, so the log alone could not answer "what did it hold back, and why". Also retires two strings that stopped being true when the reaper was armed: the summary heading said "DRY RUN. Nothing was deleted." of a run that may now delete in a later job, and the notice said "Nothing was deleted" of the whole run rather than of this job. Both now speak for the `sweep` job only, which is the thing they were ever really asserting — its token grant is `contents: read` and that has not changed. No classification changed: the buckets, the guard, the grace window and the step outputs are byte-identical. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zGPuVVX3deAx9LdjK8jCk --------- Co-authored-by: Claude <noreply@anthropic.com> * fix(service-automation): compile the test layer with tsc, and repair the TS2341 x3 it hid (objectstack-ai#15152) * wip: onboard service-automation typecheck, fix TS2341 residue * wip: onboarding gate registry entry + changeset * fix(scripts): re-measure this entry's provenance totals on the merged tree The `service-knowledge` onboarding landed on `main` between this entry's first reading and this merge, so every absolute in its provenance block (programs, pairs, packages, clean count) was a number about a tree that no longer exists. Re-taken with `--list` on the merge commit itself, all four rows plus the before/after pair, by varying only what the `typecheck` script names: no `typecheck` script absent 120 programs / 293 pairs names tsconfig.json absent 120 programs / 293 pairs names tsconfig.test PRESENT 121 programs / 302 pairs names both (the card) PRESENT 121 programs / 302 pairs before 59 of 78 packages, 120 programs, 293 pairs, 19 clean after 60 of 78 packages, 121 programs, 302 pairs, 18 clean The deltas this block actually claims (+1 package, +1 program, +9 pairs, one per dep) are unchanged; only the absolutes moved, and the block now says which merge moved them. The sibling entries' own blocks keep their own historical readings untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --------- Co-authored-by: Claude <noreply@anthropic.com> * docs(platform-objects): widen sys_email.error description to cover pre-delivery rejections `sys_email.error` was declared as "Transport error message when status=failed", but since objectstack-ai#14371 EmailService.recordRejectedMessage also writes status=failed rows for messages rejected by normalizeMessage before they reach a transport (prefixed "rejected before delivery: ..."). The declared field help was narrower than what the column actually holds. Widen the description (wording settled in triage, issue comment 5504375428) and regenerate the platform-objects i18n bundle with its own tooling (node scripts/check-i18n-bundles.mjs --write) rather than hand-editing the generated file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ * chore(platform-objects): add changeset for sys_email.error description widening Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ --------- Co-authored-by: Litant Ying <litant.dev@proton.me> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> Co-authored-by: os-sales <sales@objectstack.ai>
Fixes #14318
An unsendable
OS_EMAIL_FROMwas first judged on the first send, which for a freshdeployment is the first user's sign-up — and better-auth swallows that throw, so the UI
reported a verification email nobody had sent, and
sys_emailheld no record of it.What was measured on
origin/mainThe card's premise holds, and the chain is exactly as reported:
EmailService.sendInternalcallsnormalizeMessagebefore it inserts thesys_emailrow, and rethrows. So the one failure class that produces no row at allis a message rejected at normalization — every other shape (transport error, template
error at delivery) finalizes a row at
status:'failed'.formatAddressrefusesnoreply@localhostbecauseEMAIL_REGEXrequires a dotteddomain. That refusal is correct and is not relaxed here.
1.7.2sign-up invokesemailVerification.sendVerificationEmailthroughctx.context.runInBackgroundOrAwait(dist/api/routes/sign-up.mjs:254). With noadvanced.backgroundTasks.handlerconfigured — and ObjectStack configures none — thathelper awaits the promise inside a
try/catchthat logsFailed to run background taskand returns normally(
dist/context/create-context.mjs:214-223). Sign-up therefore answers 200 no matter whatthis callback does. That layer is not ours to change, which is why the fix is a
configuration-time refusal rather than more error plumbing.
One correction to the card's second requirement, stated because it changes what this PR
does: the auth half already propagates.
AuthManager'ssendVerificationEmaillets asendTemplatethrow escape and turns a returnedstatus:'failed'into a throw naming therecipient and the cause; better-auth's
/send-verification-emailroute awaitssendVerificationEmailFndirectly (no background task) and rethrows, so the resend pathis honest today. No code was added there — the behaviour is now pinned by a test, and
the report is explicit that those three cases pass on pre-fix
maintoo.The fix
1. The sender is judged where it is configured
EmailServicePlugin.init()refuses a declareddefaultFromno message could ever be sentfrom. Fail-closed on this channel, and the reason is the file's own stated convention
rather than a new judgement:
resolveTransportalready propagatesmakeTransport's throwfor an SMTP provider with no host, on the grounds that "a boot that cannot deliver must
fail loudly rather than degrade to a LogTransport that reports success" — and
resolveEmailCapabilityArgin the CLI throws for an undeliverable provider tag or amissing API key. A named sender is a deployment declaration of the same kind. The error
names the consequence (every send rejected, silently on sign-up) and the fix
(
OS_EMAIL_FROM/config.email.defaultFrom).An absent sender is still accepted: callers that always pass
input.fromare acomplete configuration, and
normalizeMessagealready refuses a send that has neither.applyMailSettings— themailsettings channel — takes that method's opposite, alreadydocumented trade: it never throws, because one save must not kill a running server. An
unsendable saved From address is refused, the previous sender is kept, and the
consequence is stated at
error.errorand notwarnper AGENTS.md's degradation rule:the save succeeds and the settings page shows the address the operator typed, so nothing
looks broken from the outside while every send made with it would be rejected. The
provider=log"mail settings applied" line now reports the address that was actuallyapplied, not the one that was refused.
isSendableAddressshares the oneEMAIL_REGEXwithformatAddress— a second spellingof "valid enough to send" would let a boot pass a check the send path then fails.
formatAddress,EMAIL_REGEXandnormalizeMessagekeep their exact verdicts.2. A send rejected before delivery leaves a
sys_emailrowEmailService.recordRejectedMessagewrites the record for the one window that producednone:
status:'failed',attempt_count: 0, and the reason prefixedrejected before delivery:so theerrorcolumn distinguishes a message that neverreached a transport from one an SMTP host refused — an operator reading the old wording
would be sent to the wrong system.
not through
formatAddress: canonicalization is what threw, and describing the throwmust not throw again.
(none)where the input named nothing, becausefrom_address/to_addresses/subjectarerequired: trueand an empty string would make therejection record itself unwritable.
status === 'queued'—the
sys_emailafterInsertoutbox drain hook andsweepStrandedOutbox— so arejection record can never be mistaken for an outbox entry and delivered.
warn, so a storage failure can neverreplace the caller's real error with its own.
Verification
Union run at
5170e7a85(the final commit).Reverse verification (ablation). Both edited sources were restored to their pre-fix
bytes at the branch point
909a44171and the new suites re-run.d9a384b4 -> 11b7ac28(email-plugin.ts) and0198f7ac -> e031b07a(email-service.ts),and
grep -cfor the removed anchors went to0for both(
assertSendableDefaultFrom,recordRejectedMessage).relative source specifiers, so vitest resolves
src, neverdist.plugin-email/src/sender-address-validation.test.ts: 11 failed, 5 passed(the 5 are the controls that must pass in both directions: boots with a valid sender,
boots with no sender, applies a deliverable saved address, never throws out of the
settings path, writes nothing without persistence). Direction as predicted: red.
plugin-auth/src/verification-email-failure-propagation.test.ts: 3 passed,i.e. GREEN on the pre-fix tree. Reported as observed, not as predicted: those three pin
behaviour that already existed, so they are a regression pin, not evidence of a new fix.
grep -cback to 2 and 2,git diff HEADempty.Tests.
pnpm --filter @objectstack/plugin-email --filter @objectstack/plugin-auth run test—plugin-email 29 files / 460 tests passed; plugin-auth 88 files / 1806 tests passed.
pnpm --filter @objectstack/cli exec vitest runover the fiveserve-email-*/serve-settings-orderingsuites that constructEmailServicePlugin— 5 files / 58 testspassed.
pnpm --filter @objectstack/dogfood exec vitest run test/email-template-materialization.dogfood.test.ts— 1 file / 2 tests passed (boots the real plugin through
dist).pnpm --filter @objectstack/plugin-email --filter @objectstack/plugin-auth run typecheck— both
Done.Typecheck coverage — one honest gap.
tsc --listFilescounts the new plugin-email testfile 1 time (it is in the program) and the new plugin-auth test file 0 times:
packages/plugins/plugin-auth/tsconfig.jsonexcludes**/*.test.ts, pre-existing andledgered. So "plugin-auth typecheck clean" says nothing about that file — read it as NOT
MEASURED, not as green. It is exercised by vitest.
Gates.
node scripts/pm/dispatch-gates.mjs --commandsderived 41 families at thefinal head (34 before the ledger row below joined the change set). 38 green. The other
three, none of them a finding:
pnpm check:engine-double-contractwas genuinely red and is fixed, not waived: thenew fake engine's
update()now routes throughassertEngineUpdateDispatch(the shapeemail-plugin.template-runtime-write.test.tsalready pins), and the new pinned coveragewas recorded with
--write(scripts/engine-double-contract.pinned.json, 1 row added,0 lost). Re-run green.
pnpm check:bash32-floorexits 1 on this host, in its own self-test (8 of 153 cases,all of the form "the probe is shell this host can parse" plus the simulated-3.2 harness):
the host bash IS
3.2.57, so the gate's bash-4 probes cannot be parsed or run here. Itsreal-tree scan in the same run reports
26 shell file(s) ... 0 finding(s), and thisdiff contains no shell file at all. Host artifact, not a verdict about this card.
node scripts/check-test-completeness.mjsandnode scripts/pm/check-half-states.mjsboth exit 3, which each script's own text defines as NOT MEASURED rather than a
finding — the first needs a saved
turbo run testlog to parse, the second a GitHubroute this container has no access to.
Plus
node scripts/check-nul-bytes.mjs(OK, 7826 files) andpnpm lint— the fullrepo scan,
eslint . --no-inline-config, exit 0 in 5m45s, so no narrowing was taken there.Declared narrowing — verification ran UNLOCKED.
scripts/pm/os-verify-lock.shcouldnot take the shared verify lock on this host: no usable
flock. The shared verify lock isdeclared Linux-only (
flockis util-linux, and a stock macOS does not ship it), so everybuild and test command below was run directly, without the lock — a declared narrowing,
not a silent one. No serialization guarantee held for these runs, nor for any sibling agent
in this container while they ran.
Out of scope
Resend behaves once a failure signal exists — is not touched here.
sys_email.error's field description still reads "Transport error message whenstatus=failed". Widening it would pull
packages/platform-objectsinto this diff and adifferent gate family, so the row's own text was made self-describing instead
(
rejected before delivery: ...). Noted, not changed.Generated by Claude Code