Skip to content

fix(service-sms): a configured SMS provider delivers through its env or Settings credentials, without OS_SMS_PROVIDER - #22815

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-22789-sms-configured-provider
Oct 11, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-22789-sms-configured-provider

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #22789
Clause-②: no

A configuration that names an SMS provider (provider: 'twilio' or 'aliyun') and keeps that provider's keys in the environment, but sets no OS_SMS_PROVIDER, now delivers. Before this change it booted on LogSmsTransport with isConfigured() === false, through os serve and bootStack alike. The change follows triage 6106456354: mirror the mail resolver, not a second rule. Dispatch: seat domain:services#1, claim 6107123356, session session_01CBAfsWMSfM3EToQGVStEcp.

What changed (packages/services/service-sms only)

  • resolveSmsCapabilityArg (src/capability-arg.ts) merges the selected provider's credentials into providerOptions, env over config.sms.providerOptions, key by key. That is the mail resolver's rule (OS_EMAIL_SMTP_* over config.email.options). It reads the variables SettingsService env-locks the sms namespace with (envKeyOf('sms', key)):
    • twilio: OS_SMS_TWILIO_ACCOUNT_SID to accountSid, OS_SMS_TWILIO_AUTH_TOKEN to authToken, OS_SMS_TWILIO_FROM_NUMBER to from, OS_SMS_TWILIO_MESSAGING_SERVICE_SID to messagingServiceSid;
    • aliyun: OS_SMS_ALIYUN_ACCESS_KEY_ID to accessKeyId, OS_SMS_ALIYUN_ACCESS_KEY_SECRET to accessKeySecret, OS_SMS_ALIYUN_SIGN_NAME to signName, OS_SMS_ALIYUN_TEMPLATE_CODE to defaultTemplateCode.
    • The resolver still demands no credential. It refuses only a provider tag no transport can deliver through, as before.
  • applySmsSettings (src/sms-plugin.ts, a private method of SmsServicePlugin) honours the constructor's provider when the namespace names none. "Names none" is provider at source: 'default', the criterion the log branch already used.
    • It reads the namespace for that provider's credentials and overlays them on the constructor's providerOptions key by key, so a constructor-only option such as baseUrl survives the rebuild.
    • A provider the namespace names (a save, or an OS_SMS_PROVIDER env lock) still wins.
    • When the namespace holds no complete set, a transport the constructor's credentials built at init stays, without a warning. When there is none, the existing "transport NOT rebuilt" warning fires.
  • One table, SMS_PROVIDER_SETTING_KEYS (src/transports/index.ts), maps each setting key to its transport option. providerFromSettings and the resolver both read it, so the mapping is written once. It is not exported from the package root, so the published surface is unchanged.
  • A @objectstack/service-sms patch changeset.

Not in this PR: admitting sms.provider on the stack definition (packages/spec). That is the follow-on of #22748, which the seat tells. plugin-email is untouched too.

Measured through bootStack, before and after

A scratch test (not committed) in packages/verify booted bootStack({ ...defineStack(...), sms: ... }) with the environment below and read the sms service.

  • Before: origin/main 5fc57b382, with the verify dependency closure built from that tree.
  • After: this branch, with @objectstack/service-sms rebuilt at 429b114ef. Its source has not changed since.
case before after
A. sms.provider: 'twilio', Twilio keys in env, no OS_SMS_PROVIDER isConfigured false, LogSmsTransport true, TwilioSmsTransport
B. A plus OS_SMS_PROVIDER=twilio (control) true, TwilioSmsTransport true, TwilioSmsTransport
C. sms.provider: 'aliyun', Aliyun keys in env, no OS_SMS_PROVIDER false, LogSmsTransport true, AliyunSmsTransport
D. sms.provider: 'twilio', no credentials anywhere (control) boots, false, LogSmsTransport, WARN at init boots, false, LogSmsTransport, WARN at init and at kernel:ready
E. sms.provider: 'twilio' and Twilio keys, plus OS_SMS_PROVIDER=aliyun and Aliyun keys (control) true, AliyunSmsTransport true, AliyunSmsTransport

Case D is not a refusal with a code: today a configured provider with no credentials boots onto the log transport, with a warn-level line and isConfigured() === false. The controls assert exactly that kind: the log transport, isConfigured() false, and a warn line naming the provider.

Tests

  • capability-arg.config-parity.contract.test.ts (new; the shape of the mail file of the same name). It holds three things, using SettingsService, envKeyOf and smsSettingsManifest from @objectstack/service-settings (an existing devDependency):
    1. SMS_PROVIDER_SETTING_KEYS maps exactly the keys the sms manifest declares per provider (grouped by each specifier's own visible expression), with the measured key set pinned.
    2. The resolver reads each key under its envKeyOf name, env over config per credential, for the selected provider only. A configured provider with no credentials is still not refused (control).
    3. One environment builds one transport through either face. The card's door (configured provider, env credentials, no OS_SMS_PROVIDER, a settings service mounted) is isConfigured() === true from init on, and the constructor face and the settings face build equal credentials.
  • sms-plugin.test.ts, a new block:
    • the constructor provider is built from namespace credentials (twilio, aliyun);
    • a constructor-only baseUrl survives the overlay;
    • a transport built from constructor credentials is kept with no warning;
    • CONTROL: a provider the namespace names wins;
    • CONTROL: with no credentials anywhere, the log transport, isConfigured() false and a warn line naming the provider.
  • At a59a982f8: pnpm --filter @objectstack/service-sms test gave 6 files and 94 tests passed. pnpm --filter @objectstack/service-sms typecheck exited 0. The test program (tsconfig.test.json, --listFiles) holds 6 of the 6 test files.

Ablation (one-shot, at 1718ed1aa)

Each leg went through scripts/ablation-replace.mjs (WRAP mode). The anchor hit once and was proven on disk by its count and blob change. The restore was proven by blob hash equal to HEAD with an empty git diff HEAD. The subject is imported by relative path from source, so no dist/ is involved.

  • Leg 1: drop the resolver's env merge. : smsProviderOptionsFrom(provider, became : {} || smsProviderOptionsFrom(provider, . Result: 8 failed, 28 passed. Red: the four resolver reads, the two "delivers, from init on" pins, and the two "both faces build the same credentials" pins. Green, as designed: the Settings-named-provider control, both missing-credentials controls, and the resolver's not-refused control.
  • Leg 2: drop the constructor provider at kernel:ready. const configuredProvider = !named && became const configuredProvider = false && !named &&. Result: 3 failed, 33 passed. Red: the twilio and aliyun namespace-credential pins and the baseUrl overlay pin. Green: every control.
  • Restored tree: 36 of 36 passed.

The bootStack door is repaired by either leg alone at kernel:ready when a settings service is mounted. Only the init-time reading measures the resolver leg, which is why the "from init on" pins read isConfigured() at init.

Gates (at a59a982f8, after merging origin/main 098481744)

  • node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 63 commands. 62 exited 0. One is NOT MEASURED: pnpm check:dual-build-cjs-loads exited 3, PREREQUISITE NOT MET, because it reads every package's dist/ and this worktree built only the verify closure. As a supplementary reading, not the gate: require of @objectstack/service-sms's CJS dist/index.js loads and answers the resolver.
  • --ran reconciliation: "63 derived famil(ies) accounted for — 62 run, 1 NOT-MEASURED", 0 UNRUN.
  • ESLint, narrowed to the 5 changed TS files: 5 files linted, 0 errors, 0 warnings. That is a measurement and not a skip, for three reasons:
    • the population is the 5 files the diff touches, and --format json reports 5;
    • --print-config on src/sms-plugin.ts shows no parserOptions.project and no projectService, so type-aware linting is off;
    • the rules on these files are single-file AST rules, so the diff cannot move the verdict on an untouched file.

Acceptance notes


Generated by Claude Code

… its settings credentials

resolveSmsCapabilityArg merges the selected provider's credentials into
providerOptions, env over config.sms.providerOptions per key, from the
OS_SMS_* variables the sms settings namespace is env-locked with.
applySmsSettings honours the constructor's provider when the namespace
names none; a provider the namespace names still wins. One table,
SMS_PROVIDER_SETTING_KEYS, maps setting keys to transport options for
both faces.

Claude-Session: https://claude.ai/code/session_01CBAfsWMSfM3EToQGVStEcp
Co-authored-by: Claude <noreply@anthropic.com>
… and add the changeset

capability-arg.config-parity.contract.test.ts holds SMS_PROVIDER_SETTING_KEYS
equal to the sms manifest's per-provider keys, the resolver's env reads equal
to SettingsService's envKeyOf names, and one environment equal to one
transport through either face. sms-plugin.test.ts pins the constructor
provider at kernel:ready, with the Settings-named-provider and
missing-credentials controls.

Claude-Session: https://claude.ai/code/session_01CBAfsWMSfM3EToQGVStEcp
Co-authored-by: Claude <noreply@anthropic.com>
…s of the change

It asserts the log transport, isConfigured() false and a warn-level line
naming the provider, wherever in the boot it was logged, so it reads the
same before and after the kernel:ready leg.

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

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-sms, touching 22 documentable anchor(s).

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

  • content/docs/data-modeling/drivers.mdx (via authToken (literal, a string literal in twilio))
  • content/docs/kernel/runtime-services/sms-service.mdx (via aliyun_access_key_id (literal, a string literal in providerFromSettings), aliyun_access_key_secret (literal, a string literal in providerFromSettings), aliyun_sign_name (literal, a string literal in providerFromSettings), aliyun_template_code (literal, a string literal in providerFromSettings), twilio_account_sid (literal, a string literal in providerFromSettings), twilio_auth_token (literal, a string literal in providerFromSettings), twilio_from_number (literal, a string literal in providerFromSettings), twilio_messaging_service_sid (literal, a string literal in providerFromSettings))
  • content/docs/plugins/packages.mdx (via authToken (literal, a string literal in twilio))

⛔ 1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17/17-1.mdx (via authToken (literal, a string literal in twilio))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • 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 — 3 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 23419bafb742fc13fa3c22b446263bbd6041c78e → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 23419bafb742fc13fa3c22b446263bbd6041c78e

⚠️ 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 23419bafb742fc13fa3c22b446263bbd6041c78e → pass the list as
args.docs, on the commit named under Which tree this was computed on.

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/l tests tooling

Projects

None yet

2 participants