fix(plugin-email): refuse a present-but-unreadable smtp_port instead of silently sending on 587

[os-elon]

Fixes#13190

smtpOptionsFromMailSettings coerced smtp_port with Number(...) and then omitted the key whenever the result was not finite. SmtpTransport then applied its built-in 587, so a configured smtp_port: 'abc' became a working-looking connection to a port nobody chose, and describe() — the diagnostic surface — reported 587 as though it had been selected. Nothing threw, nothing warned.

The refusal already existed one layer down. This PR stops hiding the value from it, and adds no second, parallel refusal.

- ...(port != null && Number.isFinite(port) ? { port } : {}),+ ...(port !== undefined ? { port } : {}),

port is undefined only when smtp_port is absent or the empty string, so that one token is the whole absent / present but unreadable distinction. The refusal itself is untouched: isValidSmtpPort and the generated formatInvalidSmtpPortNotice from smtp-port-contract.ts (the module PR #13191 landed this morning) do the rejecting, and no bound and no sentence is re-spelled anywhere in this diff.

The buckets, now distinct — and all five pinned

stored smtp_port	before	after
absent	587	587 — unchanged, a legitimate default
''	587	587 — unchanged, deliberately: an empty field spells "not set"
'abc' / 'Infinity' / {} / []	587, in silence	refused: SmtpTransport: invalid port 'NaN' (expected 1-65535)
'99999'	refused	refused — unchanged
'465' / 465	465	465 — unchanged

Row 2 is the one that would break working deployments if it drifted, so it is pinned beside the refusal rather than left to the reader.

Reachability is measured, not argued

The card said its own reachability was read off the resolver's precedence rather than measured. It is measured now, end to end, against the built dist at 3322527fa, and both routes reach smtpOptionsFromMailSettings with a value the form would have rejected:

ROUTE 1 — OS_MAIL_SMTP_HOST=smtp.x OS_MAIL_SMTP_PORT=abc
getNamespace('mail').values.smtp_port -> { value: 'abc', source: 'env', locked: true }
smtpOptionsFromMailSettings(values) -> { host: 'smtp.x' } // port GONE
SmtpTransport(...).describe() -> { host: 'smtp.x', port: 587 }
ROUTE 2 — an ordinary stored value
await svc.set('mail','smtp_port','abc') -> ACCEPTED { value: 'abc', source: 'global' }
smtpOptionsFromMailSettings(values) -> { host: 'smtp.x' }
SmtpTransport(...).describe().port -> 587
CONTROL — the same declaration is live in both doors
OS_MAIL_SMTP_PORT=99999 -> "env override ... is outside the declared range ... IGNORED"
await svc.set('mail','smtp_port',99999) -> SETTINGS_VALIDATION, "must be within the declared range"
OS_MAIL_SMTP_PORT=2525 -> { value: 2525, source: 'env' }

⚠️One sub-premise of the card is falsified, in the direction that raises the severity. The card states that service-settings really does enforce the bound so "the FORM refuses abc". Route 2 above shows the server-side save path accepts'abc' today — this is not only a legacy-row story. The mechanism: firstRangeViolation seeds from numericValue, which returns null for a value it cannot read, and a null reading is no violation. A window rejects what is outside it; it cannot reject what is not on the line at all. That layer is filed separately as #13314 and is not touched here — the card's own ruling is that the refusal belongs at the transport, and it does regardless of what the settings layer decides to do.

Ablation — the refusal test does fail against main's line

Implementation committed first, then the fixed line mutated back to exactly what origin/main carries, then restored:

HEAD blob for .../transports/smtp.ts = 842aaf0e7b79889a371c6196fbcf070038770934
on-disk check: injected(main line)=1 removed(fixed line)=0 blob=797206a0a2a6e69ab4ffcade8921d34496ed6dc7
x passes a present-but-unreadable port through so the guard refuses it by name
x refuses every unreadable form, and still maps every readable one
AssertionError: expected { host: 'smtp.x' } to have property "port"
AssertionError: smtp_port="abc": expected [Function] to throw an error
Test Files 1 failed (1) Tests 2 failed | 21 passed (23)
restore: blob=842aaf0e7b79889a371c6196fbcf070038770934 head=842aaf0e... (git diff HEAD empty)

Exactly the two refusal assertions go red and the two fall-back pins stay green, which is the direction that had to hold: main already falls back correctly for absent and '', and only the third bucket moves. No build leg — smtp.test.ts imports ./smtp.js relatively, inside its own package, so vitest reads src and there is no dist for a stale artifact to hide in. The mutation was confirmed on disk by grep counts and a changed blob hash before the run, never by the editor's exit code; restore was proven by blob equality against the HEAD blob plus an empty git diff HEAD, not by the trap having fired.

Verification

All at f87d9577e, the final commit.

check	verdict
pnpm --filter @objectstack/plugin-email test	Test Files 28 passed (28) · Tests 440 passed (440)
pnpm --filter @objectstack/plugin-email typecheck	exit 0, @objectstack/plugin-email@17.2.0 typecheck echoed
tsc --noEmit --listFiles	smtp.ts 1 · smtp.test.ts 1 · smtp-port-contract.ts 1 — the new tests really are inside the program the typecheck reads
pnpm lint (eslint . --no-inline-config, whole repo)	exit 0
26 gate families derived from the changed set	green

The gate list came from node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack against the real changed set, not from a copied list. Two families are NOT MEASURED locally and are named rather than counted as passes:

• node scripts/check-test-completeness.mjs — PREREQUISITE NOT MET (exit 3). It grades a saved turbo run test log and the derivation names it with no argument; its own text says to record it as NOT MEASURED. CI tees the log and passes the path.
• pnpm check:dual-build-cjs-loads (exit 3) and pnpm check:type-check-debt --re-measure (throws) — both refuse without a full workspace build, which does not fit this container's foreground ceiling once the shared verify-lock queue is included. Declared narrowing, not a skip. Exposure is bounded and stated: this diff changes no package's entry points or exports, and plugin-email carries no DEBT/TEST_DEBT ledger entry at all — so the only way it could move the ratchet is a type error in the new test code, which typecheck above answers directly with the test files proven inside the program. CI runs both.

Scope

• ⛔ SmtpTransport accepts a non-integer port its own message promises to reject, then fails at connect time under an internal name #13189 (a non-integer port such as 587.5) is a separate card and is not addressed here; it stays open. It does not interact: isValidSmtpPort accepts a non-integer in range exactly as before, and this PR changes only which values reach it.
• Filed while working, unassigned, not touched here: A type: 'number' settings specifier accepts a non-numeric string on BOTH doors — the declared window cannot reject what it cannot read #13314 (the settings layer above) and The SMTP port refusal names 'NaN' rather than the value the operator configured, on the one path that actually produces it #13315 (the refusal names 'NaN' rather than the operator's spelling — a diagnostic-quality gap this change makes reachable, and one the card's dispatch explicitly kept out of scope).

Reviewer's attention, two points

1. Behaviour change. A deployment carrying an unreadable port was delivering mail on 587 and will now refuse to build the SMTP transport — reporting at boot and under Send test email — instead of sending. That is the point of the card, and the boot path still never throws (the previous transport is kept and the reason is logged at error). Shipped as a patch per dispatch. A reviewer could argue this is an accept-set narrowing on a published export and wants the repo's minor + breaking-marker convention instead; the counter-argument is that no one can be relying on smtp_port: 'abc' meaning 587, since the setting was being ignored. Flagged rather than decided.
2. Clause ②. The dispatch declared it no and I concur, having looked for the reading that would flip it: the accept set narrows ('abc' stops being silently accepted), SmtpTransportOptions is not edited in any direction, and no packages/spec file is touched. The one honest wrinkle: smtpOptionsFromMailSettings is a published export, and for the unreadable input its return value gains a port: NaN member. An external consumer spelling opts.port ?? 587 would now get NaN — but that consumer is the defect being repaired, in miniature.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see

• the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 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.

Coarse fallback — 4 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 56c093c4ddb83bb6f0cb6d1b670a369e27284468 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 9b822d922ad186e864170730093c176951397ae4 — the merge of head f87d9577e4ad499e77e61963c5e988578249fc13 into base 56c093c4ddb83bb6f0cb6d1b670a369e27284468, 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 9b822d922ad186e864170730093c176951397ae4 && git checkout 9b822d922ad186e864170730093c176951397ae4
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 56c093c4ddb83bb6f0cb6d1b670a369e27284468 f87d9577e4ad499e77e61963c5e988578249fc13 && git checkout -B drift-repro 56c093c4ddb83bb6f0cb6d1b670a369e27284468 && git merge --no-ff f87d9577e4ad499e77e61963c5e988578249fc13
node scripts/docs-audit/affected-docs.mjs --json 56c093c4ddb83bb6f0cb6d1b670a369e27284468

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