From 1c6c253298286595466628a68b6d2b571337b411 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 29 Aug 2026 08:30:21 +0000 Subject: [PATCH 1/2] refactor(plugin-email): declare the SMTP port range once and generate its refusal The bound 1-65535 was hand-written three times across two packages: the enforcement in SmtpTransport, the `(expected 1-65535)` literal on the very next line, and `min: 1, max: 65535` on the mail settings form's `smtp_port` field. The first two were adjacent lines -- changing the check without changing the sentence produced a refusal that misstated its own rule, and nothing failed. `transports/smtp-port-contract.ts` now owns the constants, the predicate and the sentence. The message is GENERATED from the constants rather than re-spelled, so the second spelling no longer exists to drift. The settings manifest keeps its own numbers on purpose: service-settings does not depend on plugin-email, and plugin-email depends on it only as a test-only devDependency, so importing the constant there would add a runtime edge from a service to a plugin and invert the layering. The two are held equal by a cross-package assertion over that existing devDependency instead -- the same mechanism that already holds the provider dropdown equal to EMAIL_TRANSPORT_PROVIDERS. Behaviour-preserving: the accept set is pinned against the previous inline expression, kept verbatim as the oracle. The floor stays 1 and is pinned explicitly -- it is deliberately NOT the CLI's listen range, which floors at 0. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012WkdHQwHr2KQmaX7P1BHzi --- .../smtp-port-contract-single-source.md | 14 + .../src/transports/smtp-port-contract.test.ts | 257 ++++++++++++++++++ .../src/transports/smtp-port-contract.ts | 106 ++++++++ .../plugin-email/src/transports/smtp.ts | 12 +- .../src/manifests/mail.manifest.ts | 14 + 5 files changed, 401 insertions(+), 2 deletions(-) create mode 100644 .changeset/smtp-port-contract-single-source.md create mode 100644 packages/plugins/plugin-email/src/transports/smtp-port-contract.test.ts create mode 100644 packages/plugins/plugin-email/src/transports/smtp-port-contract.ts diff --git a/.changeset/smtp-port-contract-single-source.md b/.changeset/smtp-port-contract-single-source.md new file mode 100644 index 0000000000..deae144466 --- /dev/null +++ b/.changeset/smtp-port-contract-single-source.md @@ -0,0 +1,14 @@ +--- +"@objectstack/plugin-email": patch +"@objectstack/service-settings": patch +--- + +The SMTP port range `1-65535` is now declared once and the refusal is generated from it (#12993). It had been hand-written three times across two packages: the enforcement in `SmtpTransport`, the `(expected 1-65535)` literal in the very next line's message, and `min: 1, max: 65535` on the mail settings form's `smtp_port` field — which `@objectstack/service-settings` really does enforce (`declaredBounds` / `validatePatch`), so it is a second door rather than decoration. The first two were adjacent lines, the cheapest possible drift: changing the check without changing the sentence yields a refusal that misstates its own rule, and nothing fails. + +`transports/smtp-port-contract.ts` now owns `SMTP_PORT_MIN` / `SMTP_PORT_MAX`, the predicate that applies them and the sentence that states them. The message text is **generated** rather than re-spelled, so the second spelling no longer exists to drift — the stronger of the two repairs the card named, since it deletes the drift instead of checking for it. + +Nothing is accepted or refused that was not before. The move is pinned as behaviour-preserving against the previous inline expression, kept verbatim as the oracle, over a table that includes both edges, the non-finite values and the non-integers. + +The settings manifest keeps its own numbers deliberately. `@objectstack/service-settings` does not depend on `@objectstack/plugin-email`, and the plugin depends on the service only as a test-only devDependency; making the manifest import the constant would add a runtime edge from a service to a plugin, invert the layering and pull `nodemailer` into the settings service's install closure. So the two are held equal by a cross-package assertion over that existing devDependency instead — the same mechanism that already holds the provider dropdown equal to `EMAIL_TRANSPORT_PROVIDERS` — and no new dependency edge is created in either direction. + +This range is **not** the CLI's listen range and must never be merged with it: `os serve` floors at `0` because port 0 asks the OS to choose one to listen on, while an SMTP port is a destination and floors at `1`. Collapsing them onto one constant would silently make `0` a legal SMTP port, so the floor is pinned explicitly. diff --git a/packages/plugins/plugin-email/src/transports/smtp-port-contract.test.ts b/packages/plugins/plugin-email/src/transports/smtp-port-contract.test.ts new file mode 100644 index 0000000000..b22e58fb18 --- /dev/null +++ b/packages/plugins/plugin-email/src/transports/smtp-port-contract.test.ts @@ -0,0 +1,257 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #12993 — the SMTP port range is declared ONCE, and every door states it from + * there. + * + * ## The criterion this file exists to falsify + * + * The bound was written by hand three times: the enforcement in `smtp.ts`, the + * `(expected 1-65535)` literal on the next line, and `min: 1, max: 65535` on + * the mail manifest's `smtp_port` field in `@objectstack/service-settings`. + * The card named two possible repairs and observed that generating the message + * from the constants is "strictly stronger — it deletes the drift instead of + * checking for it". So the criterion for sites 1-2 is a **zero**: no second + * declaration and no second literal anywhere in this package's code. + * + * ⭐ The risk with any zero is that it was produced by a scan which would have + * found nothing whatever the source said. Every zero below therefore sits next + * to a POSITIVE CONTROL over the same corpus with the same regex — a constant + * known to be there (`DEFAULT_PORT`, in the very file the bound moved out of), + * plus a live-regex check and a masking control. A zero next to a hit is a + * measurement; a zero on its own is only a grep that ran. + * + * ## Site 3 is pinned as a MIRROR, and that is a measured choice + * + * `@objectstack/service-settings` does not depend on this package, and this + * package depends on it only as a **devDependency — test-only, no runtime + * edge**. Making the manifest import the constant would add a runtime edge + * from a service to a plugin, invert the layering and close a cycle. So the + * manifest keeps its numbers and this file holds them equal, which is the + * mechanism `mail-manifest-providers.contract.test.ts` already uses for the + * provider dropdown over that same devDependency. + * + * ## ⛔ The floor is 1 and must never become 0 + * + * The CLI's listen range floors at 0 ("let the OS choose"). This one floors at + * 1, because 0 is not a destination. The last two cases exist to make a future + * "unification" of the two ranges fail loudly rather than silently make `0` a + * legal SMTP port. + */ + +import { describe, it, expect } from 'vitest'; +import { readdirSync, readFileSync, statSync } from 'node:fs'; +import { join, resolve } from 'node:path'; + +// The one code/prose separator in this repo, typed by the hand-written +// `.d.mts` beside it. This file asks "is this a DECLARATION or a sentence +// about one", which is precisely the question that module answers. +import { maskComments } from '../../../../../scripts/js-comment-mask.mjs'; + +import { mailSettingsManifest } from '@objectstack/service-settings'; + +import { + SMTP_PORT_MIN, + SMTP_PORT_MAX, + SMTP_PORT_RANGE_TEXT, + isValidSmtpPort, + formatInvalidSmtpPortNotice, +} from './smtp-port-contract.js'; +import { SmtpTransport } from './smtp.js'; + +/** + * …/packages/plugins/plugin-email/src/transports + * + * ⛔ Seeded from `__dirname`, NOT `fileURLToPath(import.meta.url)` — the same + * choice `plugin-auth/src/managed-extension-fields.test.ts` documents at + * length, for the reason that did not move: this package is CJS-typed (no + * `"type": "module"`, it publishes `dist/index.js` as CommonJS), so under + * `module: NodeNext` the meta-property is a **TS1470** however well it runs + * under vitest — and this package's tests ARE in front of `tsc`, because its + * tsconfig `include` covers the whole `src` tree, test files and all. Measured, + * not assumed: the first draft of this file failed + * `pnpm --filter @objectstack/plugin-email typecheck` on exactly that line. + * `__dirname` type-checks under this package's own config and is defined at + * runtime by vitest's transform. + */ +const HERE = __dirname; +/** …/packages/plugins/plugin-email/src — this package's whole source tree. */ +const SRC = resolve(HERE, '..'); + +const CONTRACT = 'transports/smtp-port-contract.ts'; +const ENFORCEMENT = 'transports/smtp.ts'; + +/** Every `.ts` file under `src`, as package-relative paths. */ +function everySourceFile(dir = SRC, prefix = ''): string[] { + const found: string[] = []; + for (const entry of readdirSync(dir)) { + const full = join(dir, entry); + if (statSync(full).isDirectory()) { + found.push(...everySourceFile(full, `${prefix}${entry}/`)); + } else if (entry.endsWith('.ts')) { + found.push(`${prefix}${entry}`); + } + } + return found; +} + +const FILES = everySourceFile(); +/** Path → source with COMMENT spans blanked, so prose cannot answer for code. */ +const CODE = new Map(FILES.map((p) => [p, maskComments(readFileSync(join(SRC, p), 'utf8'))])); + +const read = (p: string): string => { + const source = CODE.get(p); + if (source === undefined) throw new Error(`${p} is not in plugin-email/src — the corpus moved`); + return source; +}; + +/** A declaration of `name`, in code — `export` prefix and all. */ +const declarationOf = (name: string): RegExp => + new RegExp(String.raw`(?:^|[^\w$])(?:const|let|var)\s+${name}\b`); + +/** The `smtp_port` specifier as the settings service actually ships it. */ +function smtpPortSpecifier(): Record { + const spec = (mailSettingsManifest.specifiers as Array>).find( + (s) => s.key === 'smtp_port', + ); + expect(spec, 'mail manifest must declare an `smtp_port` specifier').toBeDefined(); + return spec!; +} + +describe('#12993 — one SMTP port range, every door states it from there', () => { + it('declares the bound in exactly one file, and the scan proves it can see a neighbour', () => { + const declares = (name: string): string[] => + FILES.filter((p) => declarationOf(name).test(read(p))); + + // ── The ZERO: no second declaration of either bound in this package ── + expect(declares('SMTP_PORT_MIN'), 'SMTP_PORT_MIN is declared outside the contract module') + .toEqual([CONTRACT]); + expect(declares('SMTP_PORT_MAX'), 'SMTP_PORT_MAX is declared outside the contract module') + .toEqual([CONTRACT]); + + // ── The POSITIVE CONTROL: same scan, same corpus, a constant that IS + // there — and deliberately one that lives in `smtp.ts`, the file the bound + // moved OUT of, so a scan that had stopped seeing that file cannot pass. + expect(declares('DEFAULT_PORT'), 'the scan found nothing at all — the zeros above measure nothing') + .toEqual([ENFORCEMENT]); + + // …and the control is independent of the terms under test in BOTH + // directions, asserted rather than eyeballed: a control that is a + // substring of the term under test proves nothing about either. + for (const term of ['SMTP_PORT_MIN', 'SMTP_PORT_MAX']) { + expect('DEFAULT_PORT'.includes(term), `the control contains ${term}`).toBe(false); + expect(term.includes('DEFAULT_PORT'), `${term} contains the control`).toBe(false); + } + + // The corpus itself has to be real, or `FILES.filter` filters nothing. + expect(FILES.length, 'no plugin-email sources were scanned').toBeGreaterThan(40); + expect(FILES, 'the contract module is not in the scanned corpus').toContain(CONTRACT); + expect(FILES, 'the enforcement is not in the scanned corpus').toContain(ENFORCEMENT); + }); + + it('writes the ceiling nowhere but the contract module — prose does not count as a copy', () => { + // Tests are excluded on purpose: an expectation that read the bound from + // the module would assert `x === x` and pin nothing, so `65535` in a + // `.test.ts` is the point rather than a violation. + const numeric = /(? p !== CONTRACT && !p.endsWith('.test.ts')) + .filter((p) => numeric.test(read(p))); + expect(offenders, 'the port ceiling is written as a literal outside the contract module') + .toEqual([]); + + // Control for the line above — the same regex, over a string that has one. + expect(numeric.test('const x = 65535;'), 'the numeric scan is a dead regex').toBe(true); + + // ⭐ And the MASKING control. `smtp.ts` still explains the range in prose + // (that is where the deleted literal used to live), so the raw file + // contains `65535` while its CODE does not. Without this pair, the zero + // above would also be produced by a mask that blanked everything. + const rawEnforcement = readFileSync(join(SRC, ENFORCEMENT), 'utf8'); + expect(rawEnforcement, 'smtp.ts no longer explains the range at all').toContain('65535'); + expect(numeric.test(read(ENFORCEMENT)), 'a prose sentence is being read as a declaration') + .toBe(false); + }); + + it('GENERATES the refusal from the constants instead of re-spelling it', () => { + // The card's stronger option, made executable: site 2 does not exist as an + // independent spelling, so there is nothing left to drift. + expect(SMTP_PORT_RANGE_TEXT).toBe(`${SMTP_PORT_MIN}-${SMTP_PORT_MAX}`); + expect(formatInvalidSmtpPortNotice('abc')) + .toBe(`SmtpTransport: invalid port 'abc' (expected ${SMTP_PORT_MIN}-${SMTP_PORT_MAX})`); + + // The ceiling is typed exactly once even inside its own module — if the + // notice re-spelled the range, this would be 2. + const inContract = read(CONTRACT).match(/(? new SmtpTransport({ host: 'smtp.example.test', port: 99999 })) + .toThrow(formatInvalidSmtpPortNotice(99999)); + }); + + it('holds the settings form equal to the transport across the package boundary', () => { + // `@objectstack/service-settings` is a devDependency here — test-only, no + // runtime edge — so the manifest is compared against the real constants + // rather than against a literal mirrored on this side. + const spec = smtpPortSpecifier(); + expect(spec.min, 'the mail form floors the port somewhere else than the transport') + .toBe(SMTP_PORT_MIN); + expect(spec.max, 'the mail form caps the port somewhere else than the transport') + .toBe(SMTP_PORT_MAX); + + // Control: the specifier really was read, and is the numeric field whose + // bounds this service enforces — not an undefined lookup answering `toBe`. + expect(spec.type).toBe('number'); + expect(spec.default).toBe(587); + expect(isValidSmtpPort(Number(spec.default)), 'the form default is outside the range') + .toBe(true); + }); + + it('refactors the enforcement without narrowing what it accepts', () => { + // The predicate `smtp.ts` had before this card, kept verbatim as the + // oracle. Reading the bound from the module here would assert `x === x`; + // the point is that the OLD expression and the NEW function agree. + const legacyAccepts = (port: number): boolean => + !(!Number.isFinite(port) || port < 1 || port > 65535); + + const table = [ + 1, 25, 465, 587, 2525, 65535, // inside + 0, -1, 65536, 99999, // outside + Number.NaN, Number.POSITIVE_INFINITY, Number.NEGATIVE_INFINITY, // not finite + 587.5, 0.5, 65535.5, // non-integers: accepted iff they were accepted before + ]; + for (const port of table) { + expect(isValidSmtpPort(port), `accept set changed for ${String(port)}`) + .toBe(legacyAccepts(port)); + } + + // The table is not vacuous in either direction. + expect(table.filter(legacyAccepts).length).toBeGreaterThan(0); + expect(table.filter((p) => !legacyAccepts(p)).length).toBeGreaterThan(0); + }); + + it('⛔ floors at 1, not at 0 — this range is not the CLI listen range', () => { + // The single most important fence on this card. Port 0 is meaningful only + // to a listener ("let the OS choose"); it is not a destination. A repair + // that collapsed the two contracts onto one constant would make this pass + // only by making `0` a legal SMTP port. + expect(SMTP_PORT_MIN).toBe(1); + expect(isValidSmtpPort(0), '0 became a legal SMTP port').toBe(false); + expect(() => new SmtpTransport({ host: 'smtp.example.test', port: 0 })).toThrow(/invalid port/); + expect(smtpPortSpecifier().min, 'the settings form would let 0 be saved').toBe(1); + }); + + it('never reaches for the CLI’s port contract to supply this bound', () => { + // Prose about the CLI module is expected and welcome (this package's + // contract module explains the distinction at length) — an IMPORT is not. + const importers = FILES + .filter((p) => !p.endsWith('.test.ts')) + .filter((p) => /@objectstack\/cli|utils\/port-contract\.js/.test(read(p))); + expect(importers, 'plugin-email imports a port bound from the CLI').toEqual([]); + + // Control: the same corpus DOES contain the prose, so the scan is live and + // the masking is what produced the zero. + expect(readFileSync(join(SRC, CONTRACT), 'utf8')).toContain('packages/cli/src/utils/port-contract.ts'); + }); +}); diff --git a/packages/plugins/plugin-email/src/transports/smtp-port-contract.ts b/packages/plugins/plugin-email/src/transports/smtp-port-contract.ts new file mode 100644 index 0000000000..3559d8e417 --- /dev/null +++ b/packages/plugins/plugin-email/src/transports/smtp-port-contract.ts @@ -0,0 +1,106 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The ONE SMTP port bound this package has: the range `SmtpTransport` accepts, + * the predicate that applies it, and the sentence it refuses in (#12993). + * + * ## Why this is a module and not three literals + * + * The bound was hand-written three times, across two packages: + * + * 1. the enforcement in `smtp.ts` — `port < 1 || port > 65535`; + * 2. the **message text on the very next line**, which re-spelled the same + * range as a literal: `(expected 1-65535)`; + * 3. `min: 1, max: 65535` on the `smtp_port` field of the mail manifest in + * `@objectstack/service-settings`, which that service really does enforce + * (`declaredBounds` / `validatePatch`), so it is a second door and not + * decoration. + * + * Sites 1 and 2 were ADJACENT LINES — the cheapest possible drift. Changing + * the check without changing the sentence produces a refusal that misstates + * its own rule, and nothing fails. + * + * ⭐ So the sentence is **generated**, not remembered: + * {@link formatInvalidSmtpPortNotice} renders the range from + * {@link SMTP_PORT_MIN} / {@link SMTP_PORT_MAX}, and site 2 stops existing as + * an independent spelling. A test that merely compared two literals would + * leave two literals — it would check the drift instead of deleting it. + * + * ## Why site 3 is a pinned MIRROR rather than an import + * + * MEASURED on this checkout, and it is the reason the obvious repair is the + * wrong one: `@objectstack/service-settings` does **not** depend on this + * package in any form, and this package already depends on it the other way — + * as a **devDependency, test-only, no runtime edge**. Having the manifest + * import this constant would add a runtime edge from a *service* to a + * *plugin*, invert the layering, close a cycle in the workspace graph, and + * drag `nodemailer` into the settings service's install closure — a worse + * outcome than the drift it removes. + * + * The repo has already answered this exact question for this exact pair of + * packages and this exact manifest: `mail-manifest-providers.contract.test.ts` + * holds the provider dropdown equal to `EMAIL_TRANSPORT_PROVIDERS` with a + * cross-package assertion over that same devDependency, "rather than two + * mirrored literal lists". `smtp-port-contract.test.ts` extends that mechanism + * to this bound, so the manifest's numbers cannot drift from these without a + * red test — and no new dependency edge exists in either direction. + * + * ## ⛔ This is NOT the CLI's port range, and must never be merged with it + * + * `packages/cli/src/utils/port-contract.ts` owns the range a real `listen()` + * accepts and floors at **0**, because 0 is a REQUEST — "let the OS choose" — + * and binds a kernel-assigned port. This range floors at **1**: a destination + * you connect *to* cannot be 0. The two are deliberately separate declarations + * of two different contracts that happen to share a ceiling, and collapsing + * them onto one constant would silently make `0` a legal SMTP port. + * `smtp-port-contract.test.ts` fails if this floor moves or if this package + * reaches for the CLI's module. + */ + +/** + * The lowest port an SMTP server can be reached on. + * + * ⛔ **1, not 0** — see this module's header. Port 0 is meaningful only to a + * *listener*; there is no host answering on port 0 to connect to. + */ +export const SMTP_PORT_MIN = 1; + +/** + * The highest port number that exists — the 16-bit ceiling, shared with every + * other port contract in the repo because TCP says so, not because they were + * copied from one another. + */ +export const SMTP_PORT_MAX = 65535; + +/** + * The range exactly as a refusal should state it. Derived, never re-typed: + * this is the construct that replaced the hand-written `(expected 1-65535)`. + */ +export const SMTP_PORT_RANGE_TEXT = `${SMTP_PORT_MIN}-${SMTP_PORT_MAX}`; + +/** + * Is `port` inside the range this transport will connect on? + * + * ⚠️ This is the enforcement `smtp.ts` already had, moved and NOT narrowed. + * The accept set is unchanged in both directions, deliberately: a refactor + * that quietly rejected a value which worked yesterday would be a behaviour + * change wearing a cleanup's clothes. In particular a **non-integer** inside + * the range (`587.5`) is accepted here exactly as it was before, and is + * refused later by `net.connect` under an internal name — filed separately + * rather than repaired here, because that is an accept-set defect and this + * card is about the duplication. + */ +export function isValidSmtpPort(port: number): boolean { + return Number.isFinite(port) && port >= SMTP_PORT_MIN && port <= SMTP_PORT_MAX; +} + +/** + * The refusal for a port outside the range, naming the value the caller + * actually supplied and the range from the constants above. + * + * `raw` is the caller's ORIGINAL value, not the coerced number: an operator + * who configured `"abc"` needs to see `abc`, not `NaN`. + */ +export function formatInvalidSmtpPortNotice(raw: unknown): string { + return `SmtpTransport: invalid port '${String(raw)}' (expected ${SMTP_PORT_RANGE_TEXT})`; +} diff --git a/packages/plugins/plugin-email/src/transports/smtp.ts b/packages/plugins/plugin-email/src/transports/smtp.ts index 2e56f6490f..88ae9f7dba 100644 --- a/packages/plugins/plugin-email/src/transports/smtp.ts +++ b/packages/plugins/plugin-email/src/transports/smtp.ts @@ -5,6 +5,10 @@ import type { NormalizedEmailMessage, TransportSendResult, } from '@objectstack/spec/contracts'; +import { + isValidSmtpPort, + formatInvalidSmtpPortNotice, +} from './smtp-port-contract.js'; /** * SmtpTransport — self-hosted / provider SMTP delivery via nodemailer. @@ -110,8 +114,12 @@ export class SmtpTransport implements IEmailTransport { } this.opts = opts; const port = Number(opts.port ?? DEFAULT_PORT); - if (!Number.isFinite(port) || port < 1 || port > 65535) { - throw new Error(`SmtpTransport: invalid port '${String(opts.port)}' (expected 1-65535)`); + // The range is declared ONCE, in `smtp-port-contract.ts`, and the sentence + // below is GENERATED from it. A hand-written `(expected 1-65535)` on this + // line is exactly the drift #12993 removed: it sat next to the check it + // described, so the two could disagree and nothing would fail. + if (!isValidSmtpPort(port)) { + throw new Error(formatInvalidSmtpPortNotice(opts.port)); } this.port = port; } diff --git a/packages/services/service-settings/src/manifests/mail.manifest.ts b/packages/services/service-settings/src/manifests/mail.manifest.ts index 98540f0fa7..fa6e08e1ab 100644 --- a/packages/services/service-settings/src/manifests/mail.manifest.ts +++ b/packages/services/service-settings/src/manifests/mail.manifest.ts @@ -70,6 +70,20 @@ const manifest = { { type: 'group', id: 'smtp', label: 'SMTP', required: false, visible: "${data.provider === 'smtp'}" }, { type: 'text', key: 'smtp_host', label: 'Host', required: true, description: 'Example: smtp.example.com', visible: "${data.provider === 'smtp'}" }, + // ⚠️ `min`/`max` here are the SMTP port contract, and this service ENFORCES + // them (`declaredBounds` / `validatePatch`) — so this is a real door, not a + // decoration. Its ONE declaration lives in `@objectstack/plugin-email` + // (`src/transports/smtp-port-contract.ts`), beside the refusal that actually + // rejects a bad port. This is a MIRROR, not a second source: importing it + // would add a runtime edge from a service to a plugin and invert the + // layering, so the two are held equal by an executable cross-package + // assertion instead — exactly as the provider list above is. See + // `packages/plugins/plugin-email/src/transports/smtp-port-contract.test.ts`, + // which goes red the moment these numbers stop matching the transport's + // (#12993). + // ⛔ Never "unify" this floor with the CLI's listen range, which floors at 0 + // (`0` = let the OS choose a port to LISTEN on). 0 is not a destination you + // can connect to, so it is not a legal SMTP port. { type: 'number', key: 'smtp_port', label: 'Port', required: false, default: 587, min: 1, max: 65535, visible: "${data.provider === 'smtp'}" }, { type: 'toggle', key: 'smtp_secure', label: 'Use TLS', required: false, default: true, From 75dd2a67e17ec6f40d844dfd37c86e3c5f728c1e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 29 Aug 2026 08:50:20 +0000 Subject: [PATCH 2/2] test(plugin-email): declare the smtp-port-contract test's cross-package input radius `check:cross-package-test-inputs` flagged the new pin test: it imports `maskComments` from the repo-root `js-comment-mask.mjs`, which is an escaping read, and a test whose real inputs are wider than its package is invisible to both the affected-subset filter and the turbo cache. Declares the same pair @objectstack/metadata, @objectstack/cloud-connection and @objectstack/runtime already declare for the identical import, plus the matching `@objectstack/plugin-email#test` turbo task so the cache is keyed on it. The `.d.mts` sibling is declared because it types `maskComments`, so this package's `tsc --noEmit` verdict depends on it too. Two follow-on fixes in the test itself, both found by that gate: - it named `packages/cli/.../port-contract.ts` in a string literal, and the collector takes quoted paths without parsing, which would have forced an input radius over the CLI's source for a string the test only reads out of its own file. The control now uses the bare filename, which carries no separator and is refused as too generic. - the seed moved from `import.meta.url` to `__dirname`: this package is CJS-typed, so the meta-property is a TS1470 under `module: NodeNext` and the package's tests are in front of tsc. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012WkdHQwHr2KQmaX7P1BHzi --- .../src/transports/smtp-port-contract.test.ts | 17 +++++++++++++---- scripts/cross-package-test-inputs.mjs | 19 +++++++++++++++++++ turbo.json | 14 ++++++++++++++ 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/packages/plugins/plugin-email/src/transports/smtp-port-contract.test.ts b/packages/plugins/plugin-email/src/transports/smtp-port-contract.test.ts index b22e58fb18..0794107a81 100644 --- a/packages/plugins/plugin-email/src/transports/smtp-port-contract.test.ts +++ b/packages/plugins/plugin-email/src/transports/smtp-port-contract.test.ts @@ -43,9 +43,12 @@ import { describe, it, expect } from 'vitest'; import { readdirSync, readFileSync, statSync } from 'node:fs'; import { join, resolve } from 'node:path'; -// The one code/prose separator in this repo, typed by the hand-written -// `.d.mts` beside it. This file asks "is this a DECLARATION or a sentence -// about one", which is precisely the question that module answers. +// The one code/prose separator in this repo. This file asks "is this a +// DECLARATION or a sentence about one", which is precisely the question that +// module answers. The `.mjs` specifier is deliberate, and +// `scripts/js-comment-mask.d.mts` beside it is the hand-written declaration +// that gives `maskComments` its type — so this package's `tsc --noEmit` +// verdict is a function of that file too, not just of the module. import { maskComments } from '../../../../../scripts/js-comment-mask.mjs'; import { mailSettingsManifest } from '@objectstack/service-settings'; @@ -252,6 +255,12 @@ describe('#12993 — one SMTP port range, every door states it from there', () = // Control: the same corpus DOES contain the prose, so the scan is live and // the masking is what produced the zero. - expect(readFileSync(join(SRC, CONTRACT), 'utf8')).toContain('packages/cli/src/utils/port-contract.ts'); + // ⛔ The bare filename, deliberately — NOT the repo-relative path. The + // cross-package-test-inputs collector takes quoted paths without parsing, + // so spelling the CLI's path here would force this package to declare an + // input radius over `packages/cli/src/**` for a string it only reads out + // of its own file. A literal with no separator is refused as too generic, + // which is the whole point: the control stays live, the radius stays honest. + expect(readFileSync(join(SRC, CONTRACT), 'utf8')).toContain('port-contract.ts'); }); }); diff --git a/scripts/cross-package-test-inputs.mjs b/scripts/cross-package-test-inputs.mjs index e5d5da0476..ab73ec5d90 100644 --- a/scripts/cross-package-test-inputs.mjs +++ b/scripts/cross-package-test-inputs.mjs @@ -458,6 +458,25 @@ export const CROSS_PACKAGE_TEST_INPUTS = { 'scripts/js-comment-mask.d.mts', ], }, + '@objectstack/plugin-email': { + // src/transports/smtp-port-contract.test.ts (#12993) imports `maskComments` + // from `js-comment-mask.mjs` to decide which text in this package's `src/` is + // a comment and which is a DECLARATION of the SMTP port bound — the single + // question that gate exists to answer, since the whole point of the card is + // that `smtp.ts` still explains the range in prose while declaring it + // nowhere. The coupling is real: that guard's zero, and the masking control + // that makes the zero a measurement rather than a grep that ran, are both a + // function of the module's masking behaviour, so a change to it has to + // re-run this package's suite. The `.d.mts` sibling is declared alongside it + // because it is what gives `maskComments` its type, so this package's + // typecheck verdict is a function of it too — the reason the + // `@objectstack/cli` entry above declares the pair rather than the module + // alone. + globs: [ + 'scripts/js-comment-mask.mjs', + 'scripts/js-comment-mask.d.mts', + ], + }, '@objectstack/platform-objects': { // src/managed-api-method-affordance-sweep.test.ts (#7934) imports every // `*.object.ts` in the monorepo and runs `validateManagedApiMethods` over diff --git a/turbo.json b/turbo.json index ad53c31b53..49f6fe3cdb 100644 --- a/turbo.json +++ b/turbo.json @@ -153,6 +153,20 @@ "$TURBO_ROOT$/scripts/js-comment-mask.d.mts" ] }, + "@objectstack/plugin-email#test": { + "dependsOn": [ + "^build" + ], + "outputs": [], + "inputs": [ + "$TURBO_DEFAULT$", + "!dist/**", + "!coverage/**", + "!.turbo/**", + "$TURBO_ROOT$/scripts/js-comment-mask.mjs", + "$TURBO_ROOT$/scripts/js-comment-mask.d.mts" + ] + }, "@objectstack/platform-objects#test": { "dependsOn": ["^build"], "outputs": [],