Skip to content

Commit 2b7c331

Browse files
fix(service-sms, runtime): the Send test SMS refusals and the flow clone notice name admin controls, not REST recipes (#25421)
<!-- claude-projects-attribution: {"github_login":"os-sales"} --> _Requested by **Sales** · [project thread](https://claude.ai/code/project/chan_01D5bWFY3rFzgJcytD5gD4Uu?thread=cmsg_01D5bWFY3rFzgJcytD5gD4UuACdPWFJTLDcTrByFCHLzqb)_ Fixes #25345 Clause-②: no Metadata-protocol: none Two pieces of admin copy that the console shows told the admin to call the REST API. Each one now names the control the admin uses instead. The REST detail stays in developer copy. ## What changed **Send test SMS** (`packages/services/service-sms/src/sms-plugin.ts`, `resolveSmsTestRecipient`). Four refusals said `pass "to" in the request body`: no signed-in user, no data engine, a failed account read, and an account with no phone number. The settings page has no recipient field, so the admin could not follow that advice from the page. Each refusal still says that nothing was sent. It now says what the admin can do there: press Send test SMS while signed in as an admin whose account has a phone number, or press it again once the account can be read. A `to` in the request still wins. That override is documented in the function's TSDoc, for API callers, and not in the refusal. **Flow clone notice** (`packages/runtime/src/flow-clone.ts`, `FLOW_CLONE_NOTICE`). The old notice said to send the clone's complete definition with `status: 'obsolete'` to the automation PUT route, and it named the toggle route. The new notice keeps its first two facts unchanged: references are not re-pointed, and `draft` is not an off-switch. The remedy now names the console's two switches: > If that is not what you want, switch one of them off. A packaged original is switched off on the Packaged automation page, with its Switch "…" off action; that switch covers packaged flows only and does not reach the clone. To switch the clone itself off, open it in Studio, set its Enabled switch to Disabled, and publish. The REST recipe stays in the client SDK's `automation.toggle` TSDoc (`packages/client/src/index.ts`), which this PR does not change. ## Landing, and one premise corrected - **Landing:** the claim's file surface. Both producers are where the card named them. No other package is touched. - **The card's SMS control does not exist.** The card says to name "the SMS settings' test recipient field (#24767)". On `origin/main` `830a63a680`, `smsSettingsManifest` (`packages/services/service-settings/src/manifests/sms.manifest.ts`) declares no recipient field. The fix for #24767 landed as #24848, and that PR did not add a field: it made the test go to the signed-in admin's own `sys_user.phone_number`. The handler's own TSDoc says "The served form has no recipient input". So the refusals name the control that does exist, which is the Send test SMS button and the admin's own account number. Adding a recipient field is a `service-settings` manifest change, outside this claim. - **The clone controls were read from objectui `main` `2c4e5f5afd`.** `packages/app-shell/src/views/setup/PackagedAutomationPage.tsx` renders the server notice verbatim, and beside it a `Switch "{{label}}" off` button. That button turns off the packaged SOURCE through the packaged-flow toggle, not the clone. Studio's flow switch (`engine.studio.auto.enabled` / `disabled`, "Disable this automation (publish to apply)") writes the clone's `status: 'obsolete'`. The notice names each control for the flow it really switches. objectui's own `CLONE_NOTICE` in `PackagedAutomationPage.test.tsx` is fixture text, and the page renders whatever string arrives. Nothing there parses the notice. ## Pins - `packages/runtime/src/domains/automation-flow-clone.test.ts`: the existing notice pin asserted `'PUT /api/v1/automation/'`. It now asserts the new remedy (`Switch "…" off`, `does not reach the clone`, the Studio sentence). A new case asserts that the served notice equals `FLOW_CLONE_NOTICE` and contains no `/api/`, no "request body" and no `PUT`/`POST`. - `packages/services/service-sms/src/sms-test-recipient.pin.test.ts`: a new block drives the served `POST /api/settings/sms/test` route for no phone number, a failed read and no data engine. A new `noEngine` harness option covers that last case. It also drives the in-process call with no user. Each refusal is checked for its opening sentence and its remedy, for "nothing was sent" and "Send test SMS", and for the absence of `/api/`, "request body" and `pass "to"`. - The grep for pins and docs that quote the old strings found the runtime pin above. It found nothing else: `packages/qa/dogfood/test/automation-flow-clone-door.dogfood.test.ts` compares against the imported constant, and no doc quotes either string. ## Verification (head `f57343bda5`) - Build: `pnpm --workspace-concurrency=2 --filter '@objectstack/service-sms^...' --filter '@objectstack/runtime^...' build`, under the verify lock: `VERDICT command-exit 0`. - `pnpm --filter @objectstack/service-sms test`: Test Files 7 passed (7), Tests 107 passed (107). `pnpm --filter @objectstack/service-sms typecheck`: exit 0. - `pnpm --filter @objectstack/runtime typecheck`: exit 0, test layer OK. `pnpm --filter @objectstack/runtime test`: Test Files 438 passed (438), Tests 7023 passed, 19 skipped (7042), `VERDICT command-exit 0`. - Derived gate families (`dispatch-gates.mjs --commands`): 69 derived. 68 ran with exit 0. `check:dual-build-cjs-loads` is NOT MEASURED: it exited 3, PREREQUISITE NOT MET, because it needs a full-repo `dist`. It is left to CI. `--ran` reconciliation: 69 accounted, 68 run, 1 NOT-MEASURED. - No ablation: the diff changes message text and its pins, not a tested behaviour. ## Acceptance notes - **Same pattern, out of scope:** `plugin-email`'s `mail/test` refusals (`packages/plugins/plugin-email/src/email-plugin.ts`, around lines 196, 209 and 216) still say `pass "to" in the request body` to the admin on Setup → Email. They are not changed here, and they are listed in the report for the seat. - The invalid-number refusal `Provide a valid "to" phone number (E.164 recommended).` is unchanged. It is reachable only when the caller names a `to`, which the served form never sends. - Breaking for anyone comparing either string verbatim: the route, the status, the code and the response shape are all unchanged. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_016g7nYP9wdNx1GDa5XDr7Uy Co-authored-by: Claude <noreply@anthropic.com>
1 parent 70ff998 commit 2b7c331

5 files changed

Lines changed: 115 additions & 21 deletions

File tree

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
'@objectstack/service-sms': patch
3+
'@objectstack/runtime': patch
4+
---
5+
6+
fix(service-sms, runtime): the Send test SMS refusals and the flow clone notice name the admin's controls instead of a REST recipe (#25345)
7+
8+
Clause-②: no
9+
Metadata-protocol: none
10+
11+
Two pieces of copy the console shows an admin told them to call the REST API.
12+
13+
- **Send test SMS** (`sms/test`): when the test could not go to the signed-in admin's own number, the refusal said to pass "to" in the request body. The settings page has no recipient field, so that advice could not be followed from it. Each refusal now says that nothing was sent and what the admin can do on the page: press Send test SMS signed in as an admin whose account has a phone number, or retry once the account can be read. A `to` in the request still wins, as before. It is now documented only in the handler's TSDoc, for API callers.
14+
- **Flow clone notice** (`FLOW_CLONE_NOTICE`, returned by `POST /api/v1/automation/:name/clone`): the notice told the admin to `PUT` the clone's complete definition with `status: 'obsolete'`, and named the toggle endpoint. It now names the two switches the console has. A packaged original is switched off with its Switch "…" off action on the Packaged automation page, and that switch does not reach the clone. The clone is switched off in Studio, with its Enabled switch and a publish. The REST recipe stays in the client SDK's `automation.toggle` TSDoc.
15+
16+
No route, response shape, status or code changes: only the message text does. A caller that compared either string verbatim must update its copy.

‎packages/runtime/src/domains/automation-flow-clone.test.ts‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ import {
4949
FLOW_CLONE_DROPPED_KEYS,
5050
FLOW_CLONE_MUTATED_FIELDS,
5151
FLOW_CLONE_NAME_TAKEN_STATUS,
52+
FLOW_CLONE_NOTICE,
5253
FLOW_CLONE_STATUS,
5354
} from '../flow-clone.js';
5455

@@ -456,11 +457,27 @@ describe('#12156 — the source, the notice, and the gate', () => {
456457
// `obsolete`/`invalid`, so a cloned record-change flow runs beside the
457458
// one it was copied from. Saying so is cheaper than the surprise.
458459
expect(notice).toMatch(/off-switch/i);
459-
// [#20726] And the off-switch it names is one the clone has: its own
460-
// `status`, through its update door. A clone carries no package
461-
// envelope, and the activation toggle switches packaged flows only.
462-
expect(notice).toContain("status: 'obsolete'");
463-
expect(notice).toContain('PUT /api/v1/automation/');
460+
// [#20726] And it names the switches an admin has. A clone carries no
461+
// package envelope, so the packaged-flow switch does not reach it; the
462+
// clone is switched off in Studio, the original from its own row.
463+
expect(notice).toContain('Switch "…" off');
464+
expect(notice).toContain('does not reach the clone');
465+
expect(notice).toContain('open it in Studio, set its Enabled switch to Disabled, and publish');
466+
});
467+
468+
it('is admin copy: it names controls, never an endpoint or a request body', async () => {
469+
// The console shows this notice verbatim on the clone result. The REST
470+
// recipe for the same switch is developer copy, in the client SDK.
471+
const source = packagedExemplar();
472+
const { dispatcher } = makeDispatcher([source]);
473+
474+
const result = await dispatcher.handleAutomation(clonePath(source.name as string), 'POST', NEW, CTX);
475+
const notice: string = ((result.response?.body?.data ?? result.response?.body) as any)?.notice ?? '';
476+
477+
expect(notice).toBe(FLOW_CLONE_NOTICE);
478+
expect(notice).not.toContain('/api/');
479+
expect(notice).not.toMatch(/request body/i);
480+
expect(notice).not.toMatch(/\b(PUT|POST)\b/);
464481
});
465482

466483
it('is an authoring write — a caller without `manage_metadata` is refused 403, nothing registered', async () => {

‎packages/runtime/src/flow-clone.ts‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -166,23 +166,29 @@ export const FLOW_CLONE_DROPPED_KEYS: readonly string[] = Object.freeze([
166166
* and the only thing standing between them and that surprise is this
167167
* sentence.
168168
*
169-
* [#20726, ADR-0126 §7.2] The off-switch it names is the CLONE's own: its
170-
* `status`, published through `PUT /:name`. A clone carries no package
169+
* [#20726, ADR-0126 §7.2] The clone's own off-switch is its `status`
170+
* (`obsolete`), published through `PUT /:name`. A clone carries no package
171171
* envelope (see {@link FLOW_CLONE_DROPPED_KEYS}), so it is a flow authored in
172172
* this deployment, and the activation toggle — which switches packaged flows
173173
* only — refuses it. That holds whatever the clone was copied from: the clone
174-
* door takes any registered flow as its source, packaged or not, so the
175-
* notice makes no claim about the source's provenance.
174+
* door takes any registered flow as its source, packaged or not.
175+
*
176+
* The notice is admin copy — the console shows it verbatim on the clone
177+
* result — so it names the controls the admin uses, never an endpoint or a
178+
* request body: the Packaged automation page's `Switch "…" off` action for a
179+
* packaged original, and Studio's Enabled switch (publish to apply) for the
180+
* clone. The REST recipe is developer copy and lives in the client SDK's
181+
* `automation.toggle` TSDoc.
176182
*/
177183
export const FLOW_CLONE_NOTICE =
178184
'References are not re-pointed: this clone calls exactly what the original called '
179185
+ '(subflows, actions and objects are unchanged). It is created with status `draft`, '
180186
+ 'which is a lifecycle label and NOT an off-switch — a cloned record-change or schedule '
181187
+ 'flow is bound to its trigger and will run alongside the flow it was copied from. '
182-
+ 'If that is not what you want, switch the clone off through its own status: send its '
183-
+ 'complete definition with `status: \'obsolete\'` to `PUT /api/v1/automation/<name>`. '
184-
+ 'The activation toggle (`POST /api/v1/automation/<name>/toggle`) switches packaged flows '
185-
+ 'only and refuses the clone.';
188+
+ 'If that is not what you want, switch one of them off. A packaged original is switched '
189+
+ 'off on the Packaged automation page, with its Switch "…" off action; that switch covers '
190+
+ 'packaged flows only and does not reach the clone. To switch the clone itself off, open it '
191+
+ 'in Studio, set its Enabled switch to Disabled, and publish.';
186192

187193
/** ADR-0112 envelope for the same-name refusal: a status AND a code. */
188194
export const FLOW_CLONE_NAME_TAKEN_STATUS = 409;

‎packages/services/service-sms/src/sms-plugin.ts‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,11 @@ const SYSTEM_CTX = { isSystem: true } as const;
7878
* sign-in is enabled, so the refusal for a caller without one says exactly
7979
* that, and that nothing was sent. A read that fails is reported as a failed
8080
* read, never as "you have no number".
81+
*
82+
* Every refusal is admin copy the settings page shows: it names what the admin
83+
* does (press Send test SMS signed in as an admin whose account has a number),
84+
* never a request field or an endpoint. The `to` override is for API callers
85+
* and is documented here, not in the refusal.
8186
*/
8287
async function resolveSmsTestRecipient(
8388
payload: unknown,
@@ -91,15 +96,17 @@ async function resolveSmsTestRecipient(
9196
}
9297
if (typeof userId !== 'string' || userId === '') {
9398
return {
94-
refusal: 'No recipient — nothing was sent. This call names no "to" number and carries no signed-in '
95-
+ 'user whose own number the test could go to; pass "to" in the request body.',
99+
refusal: 'No recipient — nothing was sent. The test SMS goes to the signed-in admin\'s own phone number, '
100+
+ 'and no one is signed in on this call. Sign in as an admin whose account has a phone number, then '
101+
+ 'press Send test SMS again.',
96102
};
97103
}
98104
const engine = getEngine();
99105
if (!engine) {
100106
return {
101107
refusal: 'No data engine is available to read your account\'s phone number — nothing was sent. '
102-
+ 'Pass "to" in the request body to name a recipient.',
108+
+ 'The test SMS goes to the signed-in admin\'s own number, so it can be sent only once that number '
109+
+ 'can be read; press Send test SMS again when the data service is up.',
103110
};
104111
}
105112
let row: Record<string, unknown> | null;
@@ -112,16 +119,17 @@ async function resolveSmsTestRecipient(
112119
} catch (err: any) {
113120
return {
114121
refusal: `Could not read your account's phone number to send the test SMS to (${err?.message ?? String(err)}) `
115-
+ '— nothing was sent. Pass "to" in the request body to name a recipient.',
122+
+ '— nothing was sent. The test SMS goes to the signed-in admin\'s own number; press Send test SMS '
123+
+ 'again once your account can be read.',
116124
};
117125
}
118126
const raw = row?.phone_number;
119127
const to = typeof raw === 'string' ? normalizeSmsRecipient(raw) : undefined;
120128
if (!to) {
121129
return {
122130
refusal: 'Your account has no phone number, and the test SMS is sent to the signed-in admin\'s own number '
123-
+ '— nothing was sent. A number is on an account only where phone sign-in is enabled; otherwise pass '
124-
+ '"to" in the request body.',
131+
+ '— nothing was sent. A number is on an account only where phone sign-in is enabled: press Send test SMS '
132+
+ 'while signed in as an admin whose account has a phone number.',
125133
};
126134
}
127135
return { to };

‎packages/services/service-sms/src/sms-test-recipient.pin.test.ts‎

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,12 @@ function fakeEngine(users: UserRow[], opts: { userReadFails?: boolean } = {}) {
5555
};
5656
}
5757

58-
async function serve(users: UserRow[], opts: { userReadFails?: boolean } = {}) {
58+
async function serve(users: UserRow[], opts: { userReadFails?: boolean; noEngine?: boolean } = {}) {
5959
const settings = new SettingsService({ env: {} });
6060
settings.registerManifest(smsSettingsManifest);
6161
const engine = fakeEngine(users, opts);
62-
const services = new Map<string, unknown>([['settings', settings], ['objectql', engine]]);
62+
const services = new Map<string, unknown>([['settings', settings]]);
63+
if (!opts.noEngine) services.set('objectql', engine);
6364
const readyHooks: Array<() => Promise<void> | void> = [];
6465
const ctx = {
6566
logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() },
@@ -179,3 +180,49 @@ describe('served sms/test — the recipient', () => {
179180
expect(twilio).not.toHaveBeenCalled();
180181
});
181182
});
183+
184+
// Every recipient refusal is admin copy: the settings page shows it beside the
185+
// Send test SMS button. It names what the admin does there, never a request
186+
// field or an endpoint — the `to` override is API-caller detail.
187+
describe('served sms/test — the recipient refusals are admin copy', () => {
188+
const adminCopy = (message: unknown) => {
189+
expect(typeof message).toBe('string');
190+
const text = message as string;
191+
expect(text).toContain('nothing was sent');
192+
expect(text).toContain('Send test SMS');
193+
expect(text).not.toContain('/api/');
194+
expect(text).not.toMatch(/request body/i);
195+
expect(text).not.toMatch(/pass "to"/i);
196+
};
197+
198+
it.each([
199+
['an account with no phone number', [{ id: ADMIN_ID, phone_number: null }], {},
200+
'Your account has no phone number', 'signed in as an admin whose account has a phone number'],
201+
['a failed account read', [{ id: ADMIN_ID, phone_number: ADMIN_PHONE }], { userReadFails: true },
202+
'Could not read your account\'s phone number', 'once your account can be read'],
203+
['no data engine', [{ id: ADMIN_ID, phone_number: ADMIN_PHONE }], { noEngine: true },
204+
'No data engine is available', 'when the data service is up'],
205+
] as const)('%s', async (_name, users, opts, opening, remedy) => {
206+
const { twilio, pressSendTest } = await serve([...users], opts);
207+
208+
const answer = await pressSendTest({ ...TWILIO_DRAFT });
209+
210+
expect(answer.status).toBe(400);
211+
expect(answer.body?.error?.code).toBe('SETTINGS_ACTION_FAILED');
212+
const message = answer.body?.error?.details?.message;
213+
expect(message).toMatch(new RegExp(`^${opening}`));
214+
expect(message).toContain(remedy);
215+
adminCopy(message);
216+
expect(twilio).not.toHaveBeenCalled();
217+
});
218+
219+
it('no signed-in user (in-process call)', async () => {
220+
const { settings } = await serve([{ id: ADMIN_ID, phone_number: ADMIN_PHONE }]);
221+
222+
const result = await settings.runAction('sms', 'test', { ...TWILIO_DRAFT }, {});
223+
224+
expect(result.message).toMatch(/^No recipient — nothing was sent\./);
225+
expect(result.message).toContain('Sign in as an admin whose account has a phone number');
226+
adminCopy(result.message);
227+
});
228+
});

0 commit comments

Comments
 (0)