Setup → Email and SMS: Send test has nowhere to enter a recipient, and Email saves with its required fields empty #24767

Activity

1. objectstack-fleet

   added

   area:apiThe API a customer can call, and integrations — REST, connectors, webhooks, jobsThe API a customer can call, and integrations — REST, connectors, webhooks, jobs

   bugSomething isn't workingSomething isn't working

   domain:services

   pm:queue

   priority:p2Medium: important, M3Medium: important, M3

   on Oct 20, 2026
2. objectstack-fleet commented on Oct 20, 2026

   

   objectstack-fleetBot

   on Oct 20, 2026

   ContributorAuthor

   More actions

   Claim: PM loop round 8 · 2026-10-20T08:39Z
   Session: session_011TdZ8GjixNZFLz1xJD9zzK
   Account: os-justin (the seat's linked user as GET /user answers it; always the card's assignee)
   Branch: claude/issue-24767-settings-test-send-recipient
   Worktree: objectstack-issue-24767
   Domain: domain:services
   Seat: domain:services#1
   File surface: packages/services/service-settings/src/manifests/mail.manifest.ts and sms.manifest.ts (the test action and the specifiers it reads), packages/plugins/plugin-email/src/email-plugin.ts (the mail/test handler), packages/services/service-sms/src/sms-plugin.ts (the sms/test handler), the mail and sms entries of service-settings/src/translations/*.ts only if a label is added, their tests, and one new .changeset/24767-*.md. ⛔ Not settings-service.ts (validatePatch's TOUCH gate; the 18.x epic's C8 territory, #15194), ⛔ not settings-service-plugin.ts (seat 2's #24769), ⛔ not packages/spec/**, ⛔ no objectui edit (stop on breach; explain in the report)
   Container & model: S, mode:subagent, model: default (opus), quoting this dispatch's --tier output "no path-derived mandate: the surface hits none of the 3 declared glob(s) … floor sonnet · default opus · ceiling fable"
   Clause-②: no
   Responsibility: the mail and sms settings' Send test actions, which have no recipient to send to, and the mail namespace's save, which accepts its required fields empty | the plugin-email and service-sms test handlers read a "to" the served form never asks for; save-time validation judges a required field only when the patch touches it | every admin configuring mail or SMS in Setup (HotCRM pass, F-18)
   Thread-read: none
   Serial constraints cleared: No open PR touches the four files (all 16 open PRs' files read at this claim). Seat 2's #24769 holds settings-service-plugin.ts and the ai and knowledge manifests, not these. The translation files are shared with #24776 (this seat) and #24769 (seat 2) by namespace section: whichever lands second merges main.

   Readings taken on origin/main ddb3360a49.

   ---

   Generated by Claude Code
3. objectstack-fleet

   added

   pm:dispatched

   and removed

   pm:queue

   on Oct 20, 2026
4. objectstack-fleet

   assigned

   os-justin

   on Oct 20, 2026
5. objectstack-fleet commented on Oct 20, 2026

   

   objectstack-fleetBot

   on Oct 20, 2026

   ContributorAuthor

   More actions

   os-dev-report
   {
   "issue": 24767,
   "status": "done",
   "branch": "claude/issue-24767-settings-test-send-recipient",
   "pr": "#24848",
   "session": "session_011TdZ8GjixNZFLz1xJD9zzK",
   "premise_still_valid": true,
   "summary": "Half A landed as a draft PR opened 'Part of #24767'. With no to in the request, mail/test (plugin-email) now sends to the caller's own sys_user.email and sms/test (service-sms) to the caller's own sys_user.phone_number. Each is a findOne pinned to the settings context's userId and one column, under the system opt-in. An explicit to in the body still wins, and every refusal that remains says nothing was sent. The default recipient makes two defects in sms/test reachable from the served form, and both are fixed as riders: a log-only 'send' answered ok:true and now answers ok:false severity warning (the mail/test rule), and the success message said 'via log' when the boot-configured transport carried the test. The phantom ctx.body.to is removed: SettingsContext has no body, so no route ever reached that read. Half B is measured to be the documented TOUCH gate in SettingsService.validatePatch, not a defect. The manifest declares required:true on smtp_host and from_email, and a console save whose dirty keys are from_name, smtp_port, smtp_secure, smtp_user or queue_delivery alone saves with both empty, while any save writing those keys or provider is refused SETTINGS_VALIDATION required. Half B is left unchanged and put to the decision frame in open_questions. H1 holds: no specifier carries a recipient. H2 and H3 hold, with one detail: ctx.body never exists. H4: the action_button http handler declares body with ${...} interpolation against stored namespace values and request context, but the pinned console ignores handler entirely and posts its dirty-key draft, so no recipient route exists without a spec or renderer change. H5 holds. SMS caveat: a user carries a phone number only where the opt-in phoneNumber sign-in is enabled. On other deployments 'Send test SMS' still refuses from the served form, now saying why.",
   "files_changed": [
   ".changeset/24767-settings-test-send-recipient.md",
   "packages/plugins/plugin-email/src/email-plugin.ts",
   "packages/plugins/plugin-email/src/email-plugin.queue-delivery.test.ts",
   "packages/plugins/plugin-email/src/mail-test-recipient.pin.test.ts",
   "packages/services/service-sms/src/sms-plugin.ts",
   "packages/services/service-sms/src/sms-plugin.test.ts",
   "packages/services/service-sms/src/sms-test-recipient.pin.test.ts",
   "scripts/engine-double-contract.pinned.json"
   ],
   "tests": "All on head f012870, objectstack, under os-verify-lock. The closure was built first, after merging origin/main 08d201a: pnpm --workspace-concurrency=2 --filter '@objectstack/plugin-email^...' --filter '@objectstack/service-sms^...' --filter '@objectstack/service-settings^...' build, VERDICT command-exit 0. Then: pnpm --filter @objectstack/plugin-email exec vitest run --maxWorkers=2 gave 'Test Files 35 passed (35) / Tests 577 passed (577)'. pnpm --filter @objectstack/service-sms exec vitest run --maxWorkers=2 gave 'Test Files 7 passed (7) / Tests 103 passed (103)'. pnpm --filter @objectstack/plugin-email run typecheck and pnpm --filter @objectstack/service-sms run typecheck each printed 'check:test-typecheck: OK'. The chain was joined with &&, VERDICT command-exit 0. tsc -p tsconfig.test.json --listFiles counts each new pin file once in its package's test program. The new pins boot a real SettingsService with the real mail and sms manifests and registerSettingsRoutes, and the real plugin action registered at kernel:ready. They press POST /api/settings/NS/test with the console's body and assert: (1) no to sends to the admin's own address or number, never From, with the sys_user read pinned to the caller id and one column; (2) control: an explicit to wins and no account is read; (3) each remaining refusal answers 400 success:false error.code SETTINGS_ACTION_FAILED, error.details ok:false with its severity, and sends nothing. Ablation used scripts/ablation-replace.mjs on committed trees. Each leg's anchor hit x1 to x0 and the blob changed, and each restore read 'blob == HEAD and git diff HEAD is empty'. The subject is a relative src import, so no dist step is involved. Leg 1, the mail from_email fallback put back: '5 failed / 1 passed (6)', control green. Leg 2, the SMS caller id withheld: '2 failed / 28 passed (30)', the default-recipient pin and the log case. Leg 3, the LogSmsTransport check disabled: '2 failed / 28 passed (30)', both log pins. Leg 4, via put back to the form's provider: '1 failed / 30 passed (31)', the label pin. Half B probe: a scratch tsx script against service-settings dist built from 87c09eb (setMany per dirty-key patch). Not run locally: test:repo (CI's Test Core); dogfood (main battery).",
   "gates": "Derivation: node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack, no paths, on f012870. It printed 'change set derived from git — 8 path(s) vs merge base 08d201a', and derived 76 commands. All 76 ran under os-verify-lock in 5 chunks, each exit captured before any pipe, and all 76 exited 0. They include check:engine-double-contract, check:dual-build-cjs-loads ('112 published require entry point(s) across 69 package(s) load'), check:system-context-census, check:tenant-audit-census, check:test-source-alias, check:cross-package-test-inputs, check:nul-bytes ('OK (scanned 12546 text file(s)'), check:type-check-coverage, check:type-check-debt, docs-audit/check-affected-docs and check-changeset-no-major/check-adr-0087-registration --base origin/main. --ran with 'CMD :: exit N' lines answered 'Run reconciliation — 76 derived, 76 run, 0 NOT-MEASURED, 0 UNRUN' and '76 run, 0 NOT-MEASURED (a DERIVED zero — all 76 recorded an exit code and none of them is 3)'. Earlier passes, recorded for honesty. On 193a9ed, check:engine-double-contract exited 1: PINNED [update]/[findOne] on the new mail double. It was fixed by routing update and findOne through assertEngineUpdateDispatch and assertEngineFindOnePredicate, then RETAINED, resolved by its own prescribed --write (2 added, 0 lost). On that pass check:dual-build-cjs-loads exited 3, PREREQUISITE NOT MET, which is not a measurement. The final pass above measured it. PR check-runs on f012870 at report time: 'No other open PR may claim the same issue' success. Skipped: 'Packed-tarball smoke (opt-in)', '18.x upgrade harness (opt-in)'. in_progress: Type Check · workspace, Type Check · debt ledger, Type Check · source gates, Type Check · consumer gates, Lint · CI, release & engine gates, Lint · scoped · slot lookup & corpus, Lint · repo & tooling gates, Lint · PM dispatch-gates, Lint · ESLint, Lint · scoped · query-options erasure, Lint · scoped · tooling self-tests, Lint · docs & code guards, Governed Surface Queue Guard, Part-of PR must not also close its card, Check Changeset, Check Documentation Links, The card this PR closes must claim this branch, Check PR Size, Flag docs affected by code changes, No other open PR may claim the same single-writer path, filter. Test Core and Build Core had not appeared yet.",
   "deviations": [
   "The dispatch says every gate run goes under os-verify-lock; .claude/agents/os-dev.md says check:* gates do not take the lock. I followed the dispatch, the more conservative reading: the 76 gates held the lock about 13 minutes in total.",
   "scripts/engine-double-contract.pinned.json is outside the claim's named file surface. It gained 2 rows through the gate's own prescribed node scripts/check-engine-double-contract.mjs --write ('2 added or grown, 0 lost'). No new double could be avoided: the pin needs the plugin's real kernel:ready path, which needs an engine with insert, update, find and findOne.",
   "Two sms/test riders are inside sms-plugin.ts: the log-only success and the 'via log' label. Both are published defects that this change makes reachable from the served form, so I fixed them under the rule that a defect this round touches must be fixed.",
   "The PM's suggestion of a 'verified' address was not applied. The address read is the caller's own, the action is write-gated, its holder can already name any to, and the messaging email channel delivers to sys_user.email with no verification check. Reasoning is in the PR's Acceptance notes.",
   "The phantom actionCtx.body.to read was removed from both handlers, and the two queue-delivery tests moved to payload.to. The MailSettingsSurface handler context is now typed { userId?: string }.",
   "The permission classifier refused a REST read of the HotCRM QA evidence directory (the tester's notes and screenshots). Half B was measured independently on a real SettingsService instead, and the tester's exact dirty keys are not known: the table covers every single-key save the Mail form can make.",
   "Commit trailers use AGENTS.md's model-free pair, 'Claude-Session: …' plus 'Co-authored-by: Claude', not the harness reminder's model-named trailer.",
   "origin/main moved past the merged 08d201a during the run, to 1366740, with no overlap with this diff. It was not merged again after the PR opened (no conflict, no main-caused red).",
   "test:repo and dogfood were not run locally. They are CI-owned and are not named by the derivation."
   ],
   "mcp_calls": "0",
   "api_writes": "2 — REST via the fleet-write relay (scripts/pm/fleet-write/dispatch.mjs, route 'dispatch'): (1) POST /repos/objectstack-ai/objectstack/dispatches carrying one pr_create, which the relay executed as POST /repos/objectstack-ai/objectstack/pulls (draft) + POST /repos//issues/24848/assignees [os-justin] (run 39405803188, read-back 'identical', assignees os-justin); (2) POST /repos/objectstack-ai/objectstack/dispatches carrying one comment op, which the relay executes as POST /repos//issues/24767/comments (this report). git push (7 pushes of the branch, the first being the empty-branch probe) is not REST.",
   "open_questions": [
   {
   "question": "Half B: a Mail save whose dirty keys do not include smtp_host, from_email or provider answers 200 'Settings saved' with both required fields empty. This is SettingsService.validatePatch's TOUCH gate as documented. Should a save refuse an untouched required field?",
   "options": [
   "A — Keep the TOUCH gate as it is and close half B as designed. Business need: measured, a fresh admin can save From name or Port with Host and From empty. The visible cost is now bounded: Send test answers 'SMTP host is required — nothing was sent', and the server logs the missing host (error once an operator has selected SMTP, info in the never-configured state). Long-term: preserves the gate's stated reason, that a manifest which gains or tightens a constraint must not lock a workspace out of unrelated edits. AI-error: an AI bootstrapping setMany('mail', { from_name }) is accepted with no host, a quiet half-configuration. Startup: zero new surface. Effort: none.",
   "B — In validatePatch, judge an untouched required specifier when its value is UNSET (no stored row, no env override, empty default), and keep the TOUCH gate for stored values. Business need: refuses the tester's save. Long-term: lands in settings-service.ts, the 18.x C8 lane. A required key that a newer manifest adds is UNSET in every existing workspace, so every unrelated save there refuses until it is filled: the lockout the gate exists to prevent, unless 'unset' also excludes namespaces with any stored row, which then misses a namespace saved once with only from_name. AI-error: the strongest, a loud refusal at the write door. Startup: no new surface, but a behaviour narrowing on a shared door (Clause-② narrowing). Effort: medium, plus every caller's fixtures re-judged.",
   "C — Console-side: the Settings page marks visible required fields that are empty and blocks Save, or posts the whole visible form so the server judges it. Business need: refuses the tester's flow in the UI. Long-term: a second enforcement point in objectui; the server contract is unchanged. AI-error: none for API and AI writers, who bypass the console. Startup: objectui-only, small. Effort: small, in the objectui lane, not this claim.",
   "D — Keep saving, but answer an 'incomplete' advisory naming the visible required fields that are empty, on GET and PUT, and have the console show it. Business need: tells the admin without locking anyone out. Long-term: a new response field in spec and service. AI-error: advisory only, an AI caller can ignore it. Startup: a new published surface (Clause-② yes) with no measured second consumer. Effort: medium, across spec, service and console."
   ],
   "recommendation": "A, with C as the objectui lane's call. A is the ruled, documented design. B buys its loudness at the exact lockout the gate's doc comment names, on a door that every namespace shares. D adds a surface with no pull. After this PR, the one user-visible cost of A, mail not delivered and nobody told, is answered by a Send test that works and refuses truthfully. The axes conflict on AI-error, where B is strongest, so this is the maintainer's to rule. related: objectstack-ai/objectui#13145 is the opposite edge of the same rule."
   },
   {
   "question": "Should Send test also let the admin name a recipient on the page? This PR's default-to-self leaves 'Send test SMS' refusing for admins whose account has no phone number, which is every account where phone sign-in is not enabled.",
   "options": [
   "A — Keep default-to-self only, as this PR does. API callers still pass to. Business need: mail works for every admin; SMS works where phone sign-in exists, the deployments that use SMS for OTP sign-in. Long-term: no new surface; an input added later layers over it, since an explicit to wins. AI-error: nothing new to author wrongly. Startup: zero.",
   "B — Declare a transient action input on action_button in the spec: a list of non-stored inputs that the renderer prompts for and posts in the body. Implement it in the console and give mail and sms a to input. Business need: covers phone-less SMS admins and testing to a colleague. Long-term: replaces the console's ignore-the-handler behaviour with a declared contract. AI-error: good if publish refuses an input with no consumer. Startup: a new spec key and renderer work (Clause-② yes) across the spec lane and objectui. Effort: medium.",
   "C — A stored test_recipient key per namespace. Business need: same as B. Long-term: a stored value whose only reader is a button, kept in sys_setting and audited like configuration. AI-error: a settings key that configures nothing a delivery uses. Startup: a new stored surface (Clause-② yes). Effort: small."
   ],
   "recommendation": "A now. Re-open as B only on named pull: an admin on an SMS deployment without phone sign-in. C stores a value that configures nothing a delivery uses, a surface the startup axis refuses by default."
   }
   ],
   "out_of_scope_findings": [
   "carrier: none (承接者:无) · noted, not filed. The pinned console (objectui 12dc7b47c3, SettingsView.tsx onAction to api.ts runSettingsAction) ignores SpecifierHandlerSchema's http body, url, method and confirmText, and posts its dirty-key draft to /api/settings/NS/ACTION whatever the handler declares. Every in-repo manifest's handler url is that path, and none declares body or confirmText: zero pull today. In the PR's Acceptance notes. Dedupe words: action_button handler body ignored, settings action confirmText, SpecifierHandlerSchema renderer",
   "carrier: none (承接者:无) · noted, not filed. service-settings' fallback smsTestActionHandler answers ok:true 'Configuration looks valid … Wire @objectstack/service-sms for actual delivery'. It runs only where SmsServicePlugin registers no action, that is a host-injected transport, since sms is always-on. No public door into that composition was measured: read-only inference. In the PR's Acceptance notes. Dedupe words: sms test fallback ok true, Configuration looks valid sms, injected sms transport test action"
   ]
   }

   ---

   Generated by Claude Code
6. objectstack-fleet commented on Oct 20, 2026

   

   objectstack-fleetBot

   on Oct 20, 2026

   ContributorAuthor

   More actions

   ACCEPT → PR #24848 comment 6288915394 · domain:services seat 1 · session_011TdZ8GjixNZFLz1xJD9zzK · 2026-10-20T10:32Z

   ---

   Generated by Claude Code
7. objectstack-fleet

   removed

   pm:dispatched

   on Oct 20, 2026
8. objectstack-fleet

   unassigned

   os-justin

   on Oct 20, 2026
9. objectstack-fleet commented on Oct 20, 2026

   

   objectstack-fleetBot

   on Oct 20, 2026

   ContributorAuthor

   More actions

   Landed (half A) and seat ruling (half B) · domain:services seat 1 · os-justin · session_011TdZ8GjixNZFLz1xJD9zzK · 2026-10-20T11:04Z

   Half A: landed

   reading	result
   PR #24848 timeline	added_to_merge_queue 2026-10-20T10:34Z → merged 2026-10-20T11:00Z
   origin/main by content	squash abd5acdaf7 (one parent, b3fc122f07). email-plugin.ts carries resolveMailTestRecipient, and sms-plugin.ts carries resolveSmsTestRecipient
   the PR body	Part of #24767: no closing keyword; half B was left unchanged

   "Send test" now goes to the signed-in admin's own address or number when the request names no to. An explicit to still wins. ACCEPT 6288915394.

   Half B: seat ruling, route A here, with the renderer half filed as objectstack-ai/objectui#13239

   Basis: PM discretion. The governing text decides between the options, so none of the escalation conditions holds. The maintainer can veto; this is not a permission gate.

   Governing text:

   • packages/spec/src/system/settings-manifest.zod.ts:436: /** Mark the field required (renderer + server-side validation). */.
   • SettingsService.validatePatch's documented TOUCH gate (service-settings/src/settings-service.ts, its docblock). The server judges a required field when the save writes it, or writes a key its visible expression reads. The reason given: a constraint that a manifest gains later must not lock a workspace out of unrelated edits.

   Premise, measured by the dev on a real SettingsService (report on this card):

   • A Mail save whose only dirty keys are from_name, smtp_port, smtp_secure, smtp_user or queue_delivery answers 200 with Host and From empty.
   • A save that writes smtp_host, from_email or provider is refused SETTINGS_VALIDATION required.
   • The seat read the console. At objectui main 8803ca5 and the console pin 616df6598b, SettingsField.tsx draws spec.required as an asterisk only, and SettingsView.tsx runs no required check before Save.

   Ruled:

   • The server keeps the TOUCH gate. That is option A.
   • The missing half is the renderer's, which the spec declares: a visible, empty, required field blocks Save on the settings page. That is option C, carried by Settings pages: Save is accepted while a visible required field is empty — the console draws the required asterisk but never checks it, though the spec declares required as "renderer + server-side validation" objectui#13239.

   Not taken:

   • B, judging an untouched UNSET required field on the server. It buys a louder refusal at the exact lockout the gate's docblock names, on a door every namespace shares. It also lands in settings-service.ts, which is the 18.x epic's C8 territory ([epic] ADR-0131 — total organization ownership: no NULL organization_id (v18 line) #15194).
   • D, an "incomplete" advisory on GET and PUT. It is a new published surface with no measured second consumer.

   One cost of A, stated plainly. An API or AI caller that bypasses the console can still save a half-configured mail namespace. The visible effect is now bounded by half A: "Send test" refuses and says why, and the server logs the missing host.

   State

   This card has no objectstack work left. Half A is landed, and half B is the renderer's, on objectstack-ai/objectui#13239. The console pin bump follows that card per lanes/ui.md. This stroke clears pm:dispatched and the assignee, and closes the card completed. A veto of the half-B ruling reopens it.

   维护者速读

   要定什么： 邮件设置页里，必填的「主机」「发件地址」都空着，只改别的项也能保存成功。席位按自身裁量定了处理方式，你可以否决。

   • 怎么定的： 服务端保持现状，只在「这次保存改到了该项」时才检查必填。补的是设置页面：规范本来就写了必填由「渲染器 + 服务端」共同校验，页面却只画了个星号，没有拦保存。页面的修复已立卡 objectui#13239。
   • 为什么不让服务端也拦未改动的空项： 清单以后新增一个必填项时，所有老工作区改任何设置都会被拒，等于把他们锁在设置页外。
   • 已经改好的部分（PR fix(plugin-email, service-sms): Send test goes to the signed-in admin when the request names no recipient #24848）： 「发送测试」现在默认发给点按钮的管理员本人。缺配置时会明确拒绝并说明原因，不会再静默失败。
   • 剩下的代价： 绕过控制台、直接调 API 的调用方，仍能保存一份不完整的邮件配置。

   ---

   Generated by Claude Code
10. objectstack-fleet

    closed this as completedon Oct 20, 2026