Skip to content

fix(plugin-email): saving mail provider "None (log only)" stops delivery instead of keeping the live SMTP transport - #25497

Merged
objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-25450-mail-log-provider
Oct 23, 2026
Merged

objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-25450-mail-log-provider

Conversation

@objectstack-fleet

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

Copy link
Copy Markdown
Contributor

Requested by Sales · project thread

Fixes #25450

Clause-②: no
Metadata-protocol: none

What was wrong

Setup → Email offers the provider log as "None (log only — no real delivery)". When log was saved, applyMailSettings (packages/plugins/plugin-email/src/email-plugin.ts) returned without swapping the transport. It logged "transport unchanged — messages are logged and recorded in sys_email, never delivered". After a boot with SMTP, every message kept going out over SMTP. The live-status banner (landed with #25448) truthfully said SMTP was live. "Send test email" sent a real message and answered "Sent test email to …". The page label and the log line both said nothing was delivered.

Measurement: was "transport unchanged" deliberate? No. Shape (A).

  • git log -S"transport unchanged" finds the line's origin in fc9b204fe8 ("Add global scope + reactive settings client"). There smtp and log shared ONE no-op branch, with the comment "No SMTP transport ships in core; settings-only edits become a no-op for transport". For both providers, nothing could be swapped in.
  • 41c3b48f1d shipped SmtpTransport (ADR-0012) and split the branch. smtp got a real swap. log kept the no-op, and the docstring describing it says "provider = 'log' keeps the LogTransport". That line assumes the live transport is already the LogTransport. Neither commit, nor ADR-0012, names a boot-built transport (OS_EMAIL_SMTP_* / providerOptions) or a host-supplied one that a saved log must not replace.
  • The code already decides precedence the other way. A saved SMTP host replaces a boot-built or host-supplied transport, and so does a saved Resend/Postmark key. The only place the boot SMTP is kept is the smtp-with-no-host branch. That is the out-of-the-box namespace, where the page has stated nothing. Env precedence over the page lives one layer up, in the settings env lock (OS_MAIL_*, SettingsService.get), not in keeping the transport.
  • The manifest's own contract text (service-settings/src/manifests/mail.manifest.ts, the PROVIDER_OPTIONS comment) says of log: "the label says so, the runtime honours it". After this PR, that is true.

So nothing deliberate protects the old transport, and this PR takes (A). A saved log installs the LogTransport and replaces whatever was live, the same way every other selected provider does. A host-supplied transport is not exempt. The settings page already replaces it for every other provider, so an exemption for log alone would bring back the mismatch between the label and what the transport does.

The change

  • applyMailSettings, provider === 'log': if the live transport is not already a LogTransport, install a new LogTransport and drop the liveSmtp handle. This matches the Resend/Postmark branch: the old SMTP pool is not closed under a possible in-flight send.
  • The info line now says what happened, and names the transport it replaced: the LogTransport now replaces the previous live transport (was: "Mail is delivered through SMTP (host:port)") — messages are logged and recorded in sys_email, never delivered. When nothing delivered before, it says the LogTransport stays live — ….
  • Docstring updated.
  • Landing: the file the claim names. live-transport-status.ts and the manifest/translations are unchanged. The banner and the label were already true statements; the transport was what disagreed with them.

Why Metadata-protocol: none

This does change what a saved value DOES: a saved log now stops mail. But mail.provider is a settings value that an administrator stores in sys_setting through Setup. It is not metadata an author writes, and the change adds, narrows or re-means no key, value or type in the metadata protocol. The changeset carries the operator-facing consequence and the one-line remedy.

Tests

New packages/plugins/plugin-email/src/mail-log-provider.pin.test.ts. Each pin reads all four answers from one save: the live transport and isConfigured(); the page statement (registerLiveStatus); the re-apply's log line; and the "Send test email" answer, with nodemailer.sendMail never called.

  • After a boot with SMTP saved on the page, saving log gives: LogTransport, banner "No mail leaves this machine", a line naming the replaced SMTP (smtp.saved.test:587), and Send test ok: false / warning.
  • After a boot with SMTP built from the boot options (OS_EMAIL_SMTP_* reaches the plugin as providerOptions): the same.
  • After a boot with a host-supplied transport: the same. The host transport is not called.
  • A restart with log stored and SMTP in the boot options boots onto the LogTransport.
  • CONTROL, a boot with no SMTP: the boot LogTransport stays live (same instance), the banner warns, the line says "stays live", and Send test answers ok: false.

Fixture re-judged: email-plugin.queue-delivery.test.ts used a stored provider: 'log' with a host transport, which relied on exactly the "transport unchanged" behaviour this PR removes. Its default namespace is now the out-of-the-box one: smtp at its manifest default, no host. That branch keeps the harness transport, as before.

Runs (head fa561813a8):

  • pnpm --filter @objectstack/plugin-email test gives Test Files 38 passed (38), Tests 685 passed (685).
  • pnpm --filter @objectstack/plugin-email typecheck gives exit 0 (tsc --noEmit plus check:test-typecheck: OK).

Ablation (head fa561813a8, through scripts/ablation-replace.mjs): the anchor this.service.setTransport(new LogTransport(ctx.logger)); was replaced by a no-op. The tool reported "ok mutation landed: anchor 1 -> 0, blob 6e5d43794b5a -> 0a9522b85240". The pin went RED, with Tests 4 failed | 1 passed (5): all four SMTP/host legs failed and the CONTROL held. The restore reported "ok restored: blob == HEAD (6e5d43794b5a) and git diff HEAD is empty". The test imports ./email-plugin.js from src, so no dist leg applies.

Gates

node scripts/pm/dispatch-gates.mjs --commands at fa561813a8 derives 67 commands. All 67 ran locally and exited 0. --ran, with an exit code recorded for each, says: "67 derived famil(ies) accounted for — 67 run, 0 NOT-MEASURED (a DERIVED zero …)".

  • check:engine-double-contract was red on the first pass, because the new pin's fake engine declared an update(). The fake now answers reads only (persist: false), and the gate is green.
  • The CI-only families (Test Core, Build Core, type-check lanes, artifact rosters) are read from this PR's check-runs.

Acceptance notes

  • Found while measuring, and not changed here: with "None" picked on the form but not saved, "Send test email" still sends through the live SMTP and answers "Sent test email to admin@… (id=…)". The handler's provider === 'log' path tests the live service rather than the form's choice. A throwaway probe at fa561813a8 (not committed) showed ok: true, with sendMail called once. Reported to the seat as a separate finding.
  • When the transport is swapped, the previous SMTP pool is not closed. This holds on every swap branch, not only log. It is pre-existing, and nothing in this PR changes it.

Generated by Claude Code

🤖 Generated with Claude Code

https://claude.ai/code/session_016g7nYP9wdNx1GDa5XDr7Uy

…rt instead of keeping the live one

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016g7nYP9wdNx1GDa5XDr7Uy
…ter an SMTP boot; changeset

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

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-email, touching 2 documentable anchor(s).

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

  • content/docs/kernel/services.mdx (via EmailServicePlugin (symbol, a top-level class))
What this run could not see
  • the SDK route bridge reached 54 of 206 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 — 6 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 6308143731510c5883bd8af4d6fb92f9e6bd4671 → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 6308143731510c5883bd8af4d6fb92f9e6bd4671

⚠️ 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 6308143731510c5883bd8af4d6fb92f9e6bd4671 → 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

席位核对 · ACCEPT · 2026-10-23T16:50Z

domain:services seat 1 (seat post #6021) · os-sales · session_016g7nYP9wdNx1GDa5XDr7Uy · card #25450 (S, in-seat subagent; claim 6368354989, os-dev-report 6369597730) · reviewed head fa561813a8

  • Shape A, measured. The dev traced "transport unchanged" to fc9b204fe8. There smtp and log shared one no-op branch because no SMTP transport shipped. 41c3b48f1d gave smtp a real swap and left log behind. No commit or ADR protects a boot-built or host-supplied transport from a saved log.
  • Diff: 4 files, +218 / −4.
    • In applyMailSettings, a saved log installs a LogTransport whenever something else is live. The info line names what it replaced, or says the LogTransport stays live.
    • One fixture moves to the out-of-the-box namespace.
    • New pin mail-log-provider.pin.test.ts.
  • Seat check: the manifest default is provider: 'smtp' (mail.manifest.ts:62). So an unsaved namespace never reaches the new branch, and a deployment on OS_EMAIL_SMTP_* with nothing saved keeps delivering. Only a saved log stops mail. The changeset discloses this, with the fix: select a real provider, or pin OS_MAIL_PROVIDER.
  • Pins (report):
    • 5 cases cover SMTP saved, SMTP from boot options, a host-supplied transport, a restart with log stored, and a no-SMTP CONTROL. Each asserts the transport, the live-status statement, the log line and the Send test answer.
    • Ablation of the swap turns 4 red, and the CONTROL holds. The restore is clean.
    • plugin-email 685/685, typecheck 0, and dispatch-gates 67/67 with --ran reconciled.
  • CI on this head: 34 success and 7 expected skips, with none failed or pending (41 runs). git merge-tree onto today's main (4c8f40a196) is clean.
  • Body: the seat adds the attribution header and requests the os-sales reviewer in this stroke. The footer is present.
  • Declarations: fix(plugin-email), patch, Clause-②: no, Metadata-protocol: none. No contract-review record and no dogfood battery are owed.
  • Out-of-scope findings:
    • (a) "Send test email" with log picked but not saved tests the live transport and delivers over SMTP. The seat files it as a new card.
    • The SMTP pool is never closed on a transport swap. It predates this PR, is unchanged here, and has no measured leak. It is noted in the PR and not filed.
  • Landing: not governed (check-governed-merges --pr 25497: 222 lines). Ready + auto-merge go through the relay. Fixes #25450.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot requested a review from os-sales October 23, 2026 16:51
@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 23, 2026 16:51
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 23, 2026 16:51
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 23, 2026
Merged via the queue into main with commit 0ec4c5a Oct 23, 2026
49 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-25450-mail-log-provider branch October 23, 2026 17:16
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

2 participants