Repository navigation
fix(plugin-email, service-sms): Send test goes to the signed-in admin when the request names no recipient - #24848
Conversation
…when the request names no recipient The served mail and SMS settings forms offer "Send test" but no recipient input, so the console's request carries no `to`. mail/test fell back to from_email (the sender address) and refused when it was empty; sms/test refused every press. With no `to`, both now send to the caller's own address / number, read from their own sys_user row; an explicit `to` still wins. sms/test no longer reports a log-only "send" as a success. Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK Co-authored-by: Claude <noreply@anthropic.com>
…t recipient Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK Co-authored-by: Claude <noreply@anthropic.com>
…ine contract declares it Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK Co-authored-by: Claude <noreply@anthropic.com>
…L's update and findOne contract The new double routes `update` through assertEngineUpdateDispatch and `findOne` through assertEngineFindOnePredicate; the pinned ledger records the two rows (`check-engine-double-contract --write`, 2 added, 0 lost). Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK Co-authored-by: Claude <noreply@anthropic.com>
…ttings-test-send-recipient
…via log" With the recipient defaulting to the admin's own number, a deployment that configured its SMS transport at boot while the namespace's provider sits at its `log` default now reaches this path from the served form: the live service delivers, and the answer said "via log". Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 2 package(s): 3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 2 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 7 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 96564d50ebfe78985c2b7f15210067b91d3b7709 && git checkout 96564d50ebfe78985c2b7f15210067b91d3b7709
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 7c2eab929c6b724672849c5b295fa5e2f35d04c0 f0128702e5b595c69da6aa2a58a954b0b98db9ca && git checkout -B drift-repro 7c2eab929c6b724672849c5b295fa5e2f35d04c0 && git merge --no-ff f0128702e5b595c69da6aa2a58a954b0b98db9ca
node scripts/docs-audit/affected-docs.mjs --json 7c2eab929c6b724672849c5b295fa5e2f35d04c0
|
席位核对 · ACCEPT ·
|
Part of #24767
Clause-②: no
This lands half A of the card, the two Send test actions. Half B, the Mail save that succeeds with its required fields empty, is left open: it is the settings save gate working as designed, and changing it is a decision about
settings-service.ts, which this claim does not hold. The section below has the measurement.Half A: Send test had no recipient
The Mail and SMS settings pages offer Send test and no recipient input. The served console posts its unsaved edits as the action body (
runSettingsAction(namespace, actionId, draft), objectuiapps/console/src/pages/settings/SettingsView.tsxat the pinned.objectui-sha12dc7b47c3), so the request carries noto:mail/test(plugin-email) readctx.body.to ?? payload.to ?? from_email. It mailed the test to the From address (the sender, often a no-reply mailbox), or refused withProvide a "to" address (or set from_email).when From was empty.sms/test(service-sms) readctx.body.to ?? payload.toand refused every press from the served form.Now: when the request names no
to, the test goes to the person who pressed the button.mail/testsends to the caller's ownsys_user.email, andsms/testsends to the caller's ownsys_user.phone_number, the column the messagingsmschannel delivers to. Atoin the body still wins.findOne('sys_user', { where: { id: ctx.userId }, fields: [ONE_COLUMN] })under the system opt-in.where.idis the caller's own id from the settings context, never a value the body names. So the one value it returns is the caller's own address or number, and the SMS answer masks the number.phoneNumberauth plugin) is enabled. On other deployments "Send test SMS" still refuses from the served form. The refusal now says why, where before it asked for a field the page does not have.Two riders on the same handler. The recipient default makes both reachable from the served form:
None (log only)provider),sms/testansweredok: true, "Sent test SMS … via log", and delivered nothing. It now answersok: falsewith severitywarningand sends nothing.mail/testalready follows this rule for itsLogTransport.Also removed:
ctx.body.to.SettingsService.runActionhands a handler aSettingsContext, which has nobody, so no route ever reached that read.MailSettingsSurfacenow types the handler context as{ userId?: string }. Two queue-delivery tests passedtothat way, and now pass it aspayload.to, the channel the route uses.Unchanged:
settings-service.ts,settings-service-plugin.ts,packages/spec/**, objectui.Half B: why it is left open
The measurement used a real
SettingsServicewith the realmailmanifest and a fresh in-memory store, on objectstack87c09ebb7a(packages/services/service-settings/distbuilt from that commit). Each console save was replayed assetMany('mail', DIRTY_KEYS):smtp_host/from_emailafterwardsfrom_name,smtp_port,smtp_secure,smtp_userorqueue_deliveryaloneprovider: 'smtp'SETTINGS_VALIDATION,smtp_hostrequiredprovider: 'resend'SETTINGS_VALIDATION,api_keyrequiredfrom_email: ''SETTINGS_VALIDATION,from_emailrequiredsmtp_host: ''SETTINGS_VALIDATION,smtp_hostrequiredsmtp_host: 'smtp.example.com'from_emailstill emptysmtp_hostandfrom_emailboth declarerequired: trueinmail.manifest.ts.SettingsService.validatePatch: "A specifier is checked only when the patch TOUCHES it — its own key is in the patch, or a key itsvisibleexpression references is". The console posts only its dirty keys. So a save that edits From name, Port, TLS, Username or the queue toggle never touches the empty required fields, and Branding's refusal is the same gate on a key the save does write.validatePatch(settings-service.ts) or in the console. Neither is this claim's, so this PR does not close the card. The choice, with its four axes, is in the dev report on the card.Pins, on the real service and the real routes
Each suite builds a real
SettingsServicewith the real manifest and mounts its real routes throughregisterSettingsRoutes. The real plugin then registers its action atkernel:ready. Each case pressesPOST /api/settings/NS/testwith the body the console sends.packages/plugins/plugin-email/src/mail-test-recipient.pin.test.ts:{}, the answer is 200,ok: true, and nodemailer is handed the admin's own address, never From. Thesys_userread is pinned to the caller's id and theemailcolumn.{ to }wins, and no account is read.success: false,error.code: SETTINGS_ACTION_FAILED,error.detailscarryingok: false), and nothing is sent: no address, no account row, a failed account read. An in-process call with no user also refuses.packages/services/service-sms/src/sms-test-recipient.pin.test.ts:ToTwilio receives is the admin's own number, and it is masked in the answer.{ to }wins.SETTINGS_ACTION_FAILEDand send nothing: no number, no account row, a failed read. The log-only provider answers severitywarning.packages/services/service-sms/src/sms-plugin.test.ts:ok: true. It now pinsok: false,warning, and that the transport was never called.updateandfindOnethroughassertEngineUpdateDispatchandassertEngineFindOnePredicate.scripts/engine-double-contract.pinned.jsonrecords the two rows (check-engine-double-contract --write: 2 added, 0 lost).Ablation: one leg per behaviour changed
Each leg was a committed tree, mutated by
scripts/ablation-replace.mjs: the anchor hit 1 → 0 and the blob changed. Each was restored withgit checkout HEAD -- PATH, and the restore was proven by blob == HEAD and an emptygit diff HEAD. The subject is imported by relative path, so no build sits between mutation and measurement.from_emailfallback put backLogSmsTransportcheck disabledviaput back to the form's providerVerification at
f0128702e5pnpm --filter @objectstack/plugin-email exec vitest run --maxWorkers=2: 35 files, 577 tests passed.pnpm --filter @objectstack/service-sms exec vitest run --maxWorkers=2: 7 files, 103 tests passed.pnpm --filter @objectstack/plugin-email run typecheckand the same for@objectstack/service-sms: exit 0, both test layers OK. The new pin files are in eachtsconfig.test.jsonprogram, counted with--listFiles.node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackderived 76 commands. All 76 ran with their exit captured before any pipe, and every one exited 0.--rananswered "76 run, 0 NOT-MEASURED (a DERIVED zero …)".pnpm --filter '@objectstack/plugin-email^...' --filter '@objectstack/service-sms^...' --filter '@objectstack/service-settings^...' build, after mergingorigin/mainat08d201a80e.Acceptance notes
action_buttonhandler's declared shape. The console ignoresSpecifierHandlerSchema'shttpbody,url,methodandconfirmText. It postsdraftto/api/settings/NS/ACTIONwhatever the handler declares. Every in-repo manifest's handler URL is that path, and none declaresbodyorconfirmText, so nothing visible diverges today. That is zero pull, so it is not filed. It is also why "a recipient carried by the existing payload contract" is not a route here:bodyinterpolates against stored namespace values and request context, and no renderer reads it.smsTestActionHandler(the service-settings fallback) still answersok: true, "Configuration looks valid … Wire @objectstack/service-sms for actual delivery". It runs only whereSmsServicePluginregisters no action: no SMS plugin, or a host-injected transport.smsis always-on, so a served boot mounts the plugin, and no public door into the injected-transport composition was measured. This is a read-only inference. Carrier: none.to. The messaging email channel delivers tosys_user.emailwith no verification check either. So a verification requirement would add no protection the gate does not already give, and it would refuse the test for an admin whose own address is unverified.Generated by Claude Code