Skip to content

Commit 42da73d

Browse files
os-zhuangclaude
andauthored
fix(spec): close notify.severity to its declared info|warning|critical vocabulary (#7086) (#7192)
NotifyConfigSchema.severity was a bare z.string() whose .describe() read 'info | warning | critical', so the enumeration existed only in the sentence: 'urgent', 'INFO' and '' all parsed green, were forwarded raw by the notify executor, and were blind-cast by the messaging dispatcher into a union that declares those values impossible. Every other surface already declared the set closed (the describe, the Notification['severity'] type, the sys_inbox_message.severity select field), so this closes the last open one. Safe because the executor reads severity RAW -- it is one of three keys (channels, topic, severity) that never pass through interpolate() -- so a {token} template there never resolved. The module JSDoc claimed "every string-ish value except channels" is interpolated; that was stale for topic and severity and is corrected here, since the tightening's safety rests on it. Blast radius is an execute-time refusal, not a load failure: FlowNodeSchema .config is an untyped record, so stored flows still load and rehydrate. Also closes the Studio form descriptor to the same set, and extends the IO-node form/Zod ledger test to reconcile closed value vocabularies rather than key sets alone. Claude-Session: https://claude.ai/code/session_01PiRUoQkTSBBmpyXBY3cVn2 Co-authored-by: Claude <noreply@anthropic.com>
1 parent dc61def commit 42da73d

6 files changed

Lines changed: 205 additions & 7 deletions

File tree

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/service-automation": minor
4+
---
5+
6+
fix(spec): `notify.severity` closes its declared `info | warning | critical` vocabulary at the gate, not only in its describe (#7086)
7+
8+
<!-- adr-0087: not-required (no-migration-prescription) A stored flow is unaffected at LOAD: `FlowNodeSchema.config` is `z.record(z.string(), z.unknown()).optional()`, so `NotifyConfigSchema` runs only at EXECUTE time via `parseNodeConfig` — nothing fails to load or rehydrate, which is the population a D2 conversion exists to protect. And no automatic rewrite is correct here: mapping a stored `'urgent'` to `'info'` would silently pick a severity on the author's behalf, which is precisely the blind-cast defect this change removes. The refusal names the three legal values, so the author reconciles it once and keeps their intent. Re-measured across the monorepo: zero out-of-vocabulary spellings in any flow, example, fixture or seed. -->
9+
10+
`NotifyConfigSchema.severity` was a bare `z.string()` whose `.describe()` read
11+
`'info | warning | critical'` — no "e.g.", no qualifier. In this codebase that
12+
spelling is how a genuine closed vocabulary is documented, so the enumeration
13+
existed only in the sentence. Measured on `origin/main` before the change:
14+
15+
```
16+
severity "info" -> ACCEPTED severity "urgent" -> ACCEPTED
17+
severity "warning" -> ACCEPTED severity "INFO" -> ACCEPTED
18+
severity "critical" -> ACCEPTED severity "" -> ACCEPTED
19+
```
20+
21+
**Every other surface already declared the set closed**, which is what made the
22+
open gate a defect rather than a design choice: the `notify` executor forwards
23+
the value raw, the messaging dispatcher blind-casts it into the closed union
24+
(`severity: (p.severity as Notification['severity']) ?? 'info'`), and
25+
`sys_inbox_message.severity` is a select field offering exactly these three. So
26+
`severity: 'urgent'` parsed green, published green, and landed in inbox rows
27+
under a TypeScript type that says the value cannot exist — falling through every
28+
downstream `switch` on the three names. An author (very often an AI) who wrote
29+
`Critical` or `urgent` got no diagnostic anywhere on the path.
30+
31+
The gate is now `z.enum(['info', 'warning', 'critical']).optional()`, and the
32+
describe is a sentence about the field, because the vocabulary is carried by the
33+
type — the generated reference renders it as an enum column instead of a bare
34+
`string`. The refusal is self-prescribing:
35+
36+
```
37+
Invalid option: expected one of "info"|"warning"|"critical"
38+
```
39+
40+
**Why closing this gate takes no working authoring shape with it.** The executor
41+
reads `severity` **raw** — it is one of the three keys (`channels`, `topic`,
42+
`severity`) that never pass through `interpolate()` — so a `{record.x}` template
43+
there was forwarded verbatim and never resolved. The schema's module JSDoc
44+
claimed "every string-ish value except `channels`" is interpolated; that was
45+
stale for `topic` and `severity`, and it is corrected here, since it is the
46+
statement the safety of this tightening rests on.
47+
48+
**Blast radius is an execute-time refusal, not a load failure.** `FlowNodeSchema.config`
49+
is an untyped record, so a stored flow carrying `severity: 'urgent'` still loads
50+
and rehydrates exactly as before; the `notify` step refuses when it runs, naming
51+
the three legal values. `''` previously degraded to `info` two layers down and is
52+
now refused at the gate.
53+
54+
The `notify` descriptor's Studio form is closed in the same change
55+
(`enum: ['info', 'warning', 'critical']`). Closing only the Zod would have left
56+
the mirror-image drift the IO-node ledger test exists to prevent — a form
57+
inviting a value the gate refuses at execute time — and the `screen` node's
58+
`mode` is the in-repo precedent for enum-on-both-sides. That ledger test compared
59+
key SETS only, which is the gap this field sat in; it now also reconciles closed
60+
value vocabularies, so the two descriptions cannot drift apart again.

content/docs/references/automation/io-node-config.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ const result = HttpConfigSchema.parse(data);
100100
| **message** | `string` | optional | Notification body |
101101
| **channels** | `string \| string[]` | optional | Channels to fan out to (default: inbox) |
102102
| **topic** | `string` | optional | Event topic (default: "notify") |
103-
| **severity** | `string` | optional | info \| warning \| critical |
103+
| **severity** | `Enum<'info' \| 'warning' \| 'critical'>` | optional | Severity forwarded to the messaging service |
104104
| **sourceObject** | `string` | optional | Object name of the record the notification links to (writes sys_notification.source_object). Only takes effect together with sourceId — a half-specified click-through target is dropped at execute time, so the inbox never renders a dead link. |
105105
| **sourceId** | `string` | optional | Record id the notification links to (writes sys_notification.source_id). Only takes effect together with sourceObject — a half-specified click-through target is dropped at execute time, so the inbox never renders a dead link. |
106106
| **actorId** | `string` | optional | User id that caused the event (writes sys_notification.actor_id) |

packages/services/service-automation/src/builtin/io-node-form-zod-ledger.test.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,44 @@ describe('IO-node form ↔ Zod reconciliation (#4045)', () => {
9494
).toEqual([]);
9595
});
9696

97+
// #7086 — the reconciliation above compares KEY SETS, which is why a key
98+
// could agree on both sides while the two descriptions disagreed about the
99+
// VALUES it accepts. `notify.severity` sat in exactly that gap: the form
100+
// offered a free-text box and the Zod was a bare `z.string()`, while the
101+
// describe on both sides spelled out a closed `info | warning | critical`
102+
// that nothing enforced. Closing only the Zod would have produced the
103+
// mirror-image drift — a Studio field inviting a value the gate refuses at
104+
// execute time — so the closed set is pinned as ONE contract here.
105+
it.each(NODES)('$nodeType: a closed vocabulary is closed on BOTH sides, with the same values', ({ nodeType, zod }) => {
106+
const props = (engine.getActionDescriptor(nodeType)?.configSchema as
107+
| { properties?: Record<string, { enum?: unknown }> }
108+
| undefined)?.properties ?? {};
109+
const shape = (zod as { shape?: Record<string, unknown> }).shape ?? {};
110+
111+
/** The declared value set, or `undefined` for an open field. */
112+
const closedSet = (v: unknown): readonly string[] | undefined => {
113+
// `.options` also exists on a ZodUnion, where it holds member SCHEMAS
114+
// (`recipients`, `channels`) — a string-only array is what distinguishes
115+
// a real value vocabulary from that.
116+
const opts = (v as { options?: unknown })?.options;
117+
return Array.isArray(opts) && opts.every((o) => typeof o === 'string')
118+
? (opts as readonly string[])
119+
: undefined;
120+
};
121+
122+
for (const key of Object.keys(shape)) {
123+
const node = shape[key] as { unwrap?: () => unknown };
124+
// Unwrap the `.optional()` wrapper before asking for the vocabulary.
125+
const zodSet = closedSet(node) ?? closedSet(node?.unwrap?.());
126+
const formSet = closedSet(props[key]) ?? (Array.isArray(props[key]?.enum) ? props[key]!.enum as string[] : undefined);
127+
128+
expect(
129+
formSet,
130+
`${nodeType}.${key}: the two descriptions disagree on whether the value set is closed`,
131+
).toEqual(zodSet);
132+
}
133+
});
134+
97135
describe('connector_action: the contract is the connectorConfig sibling, not config', () => {
98136
it('publishes no configSchema (deliberately schemaless)', () => {
99137
// A published configSchema roots the schema-driven Studio form at

packages/services/service-automation/src/builtin/notify-node.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,15 @@ export function registerNotifyNode(engine: AutomationEngine, ctx: PluginContext)
159159
description: 'Channels to fan out to (default: inbox)',
160160
},
161161
topic: { type: 'string', description: 'Event topic (default: "notify")' },
162-
severity: { type: 'string', description: 'info | warning | critical' },
162+
// Closed vocabulary, declared as one so the Studio form
163+
// offers a choice instead of a free-text box the Zod gate
164+
// then refuses at execute time (#7086). Mirrors
165+
// `NotifyConfigSchema.severity`; the `screen` node's `mode`
166+
// is the in-repo precedent for enum-on-both-sides.
167+
severity: {
168+
type: 'string', enum: ['info', 'warning', 'critical'],
169+
description: 'Severity forwarded to the messaging service',
170+
},
163171
// ── Click-through target (#2675) ─────────────────────────
164172
sourceObject: {
165173
type: 'string',

packages/spec/src/automation/io-node-config.test.ts

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,76 @@ describe('NotifyConfigSchema — strict as of #4001 批 9', () => {
136136
expect(NotifyConfigSchema.safeParse({ recipients: 'u1', title: 't', sourceObject: 'showcase_task' }).success).toBe(true);
137137
expect(NotifyConfigSchema.safeParse({ recipients: 'u1', title: 't', sourceId: 'r1' }).success).toBe(true);
138138
});
139+
140+
// ── #7086 — severity is a CLOSED vocabulary, at the gate and not only in prose ──
141+
//
142+
// Until this change `severity` was a bare `z.string()` whose `.describe()`
143+
// read `'info | warning | critical'`. The enumeration lived only in the
144+
// sentence, so `'urgent'` parsed green here, was forwarded raw by the
145+
// executor, and was blind-cast by the dispatcher
146+
// (`(p.severity as Notification['severity']) ?? 'info'`) into a union that
147+
// declares those values impossible — silently falling through every
148+
// downstream `switch`. The three surfaces that already agreed on the closed
149+
// set: this describe, `Notification['severity']`, and the
150+
// `sys_inbox_message.severity` select field.
151+
describe('severity (#7086)', () => {
152+
/** The `severity` issues of a failed parse, or `[]` when it was accepted. */
153+
function severityIssues(value: unknown): ReadonlyArray<{ code: string; message: string }> {
154+
const result = NotifyConfigSchema.safeParse({ recipients: 'u1', title: 't', severity: value });
155+
if (result.success) return [];
156+
return result.error.issues.filter((i) => i.path.length === 1 && i.path[0] === 'severity');
157+
}
158+
159+
// Green BOTH before and after this change — pre-fix everything parsed, so
160+
// these prove nothing about the tightening. Stated plainly because the
161+
// template presumes before-green/after-red: their real job is the opposite
162+
// direction, that closing the gate did not OVERSHOOT and take a legal
163+
// spelling with it.
164+
it.each(['info', 'warning', 'critical'])('accepts the declared value %s', (value) => {
165+
expect(NotifyConfigSchema.safeParse({ recipients: 'u1', title: 't', severity: value }).success).toBe(true);
166+
});
167+
168+
// The pins that carry the change. Measured RED on `origin/main` before the
169+
// fix — all three parsed green there (probe on 3e8e669c0).
170+
//
171+
// `code` + `path`, never a bare `success === false`: a strictObject rejects
172+
// for several reasons, so an assertion that only asks "did it fail" would
173+
// stay green if the refusal ever came from an unknown key instead of the
174+
// vocabulary — the two defects this file has to keep apart.
175+
it.each([
176+
['urgent', 'an out-of-vocabulary spelling'],
177+
['INFO', 'a casing variant — the vocabulary is lower-case'],
178+
['', 'the empty string, which used to degrade to `info` two layers down'],
179+
])('rejects %s (%s)', (value) => {
180+
const issues = severityIssues(value);
181+
expect(issues.map((i) => i.code)).toEqual(['invalid_value']);
182+
// The prescription is behaviour (this file's stated load-bearing half):
183+
// the refusal has to tell the author what IS legal, or an AI author who
184+
// guessed `urgent` has nothing to correct towards (ADR-0033).
185+
for (const legal of ['info', 'warning', 'critical']) {
186+
expect(issues[0]!.message).toContain(legal);
187+
}
188+
});
189+
190+
it('declares the vocabulary in the TYPE, not only in the sentence', () => {
191+
const shape = (NotifyConfigSchema as unknown as {
192+
shape: Record<string, { description?: string; unwrap(): { options?: readonly string[] } }>;
193+
}).shape;
194+
195+
// The gate itself carries the closed set — this is what `'urgent'`
196+
// now collides with, and what the generated reference renders as the
197+
// `Enum<...>` type column instead of a free-text `string`.
198+
expect(shape.severity!.unwrap().options).toEqual(['info', 'warning', 'critical']);
199+
200+
// …and the describe is now a sentence about the field rather than a
201+
// bare value list standing in for a gate that did not exist. Non-empty
202+
// arm first, so the negative arm below cannot pass vacuously (#6918).
203+
const doc = shape.severity!.description ?? '';
204+
expect(doc.length, 'severity .describe() must not be empty').toBeGreaterThan(0);
205+
expect(doc).toMatch(/messaging service/i);
206+
expect(doc, 'the vocabulary belongs in the enum, not smuggled back into prose').not.toMatch(/\|/);
207+
});
208+
});
139209
});
140210

141211
describe('HttpConfigSchema — strict as of #4001 批 9', () => {

packages/spec/src/automation/io-node-config.zod.ts

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -127,9 +127,14 @@ const NOTIFY_KEY_GUIDANCE: Readonly<Record<string, string>> = {
127127
* without them). The descriptor's form deliberately publishes no `required`
128128
* array — see the comment on the `configSchema` literal — so requiredness
129129
* lives here and in the execute-time guard, not in the form.
130-
* - Every string-ish value except `channels` passes through `interpolate()`,
131-
* so `{record.x}` templates are legal anywhere they are; `channels` is read
132-
* raw (channel ids are static routing, not per-record data).
130+
* - `recipients`, `title`, `message`, `actionUrl` and `payload` pass through
131+
* `interpolate()`, so `{record.x}` templates are legal in them. `channels`,
132+
* `topic` and `severity` are read RAW — a `{token}` in those three is
133+
* forwarded verbatim, never resolved (channel ids are static routing and
134+
* `severity` is a closed vocabulary, not per-record data). Re-measured
135+
* against `notify-node.ts` for #7086: the previous wording ("every
136+
* string-ish value except `channels`") was stale for `topic` and `severity`,
137+
* and it is what makes closing the `severity` gate below safe.
133138
* - `sourceObject`/`sourceId` only take effect as a PAIR — a half-specified
134139
* click-through target is dropped so the inbox never renders a dead link.
135140
* The schema keeps both optional rather than refining, because the executor
@@ -156,8 +161,25 @@ export const NotifyConfigSchema = lazySchema(() => strictObject({
156161
.describe('Channels to fan out to (default: inbox)'),
157162
/** Event topic handed to the messaging service (default: "notify"). */
158163
topic: z.string().optional().describe('Event topic (default: "notify")'),
159-
/** Severity forwarded to the messaging service. */
160-
severity: z.string().optional().describe('info | warning | critical'),
164+
/**
165+
* Severity forwarded to the messaging service — a CLOSED vocabulary (#7086).
166+
*
167+
* Was a bare `z.string()` whose `.describe()` read `'info | warning | critical'`,
168+
* so the enumeration existed only in the sentence: `'urgent'`, `'INFO'` and `''`
169+
* all parsed green, then rode the dispatcher's blind cast
170+
* (`severity: (p.severity as Notification['severity']) ?? 'info'`) into
171+
* `sys_inbox_message.severity` under a TypeScript union that says those values
172+
* cannot exist — every downstream `switch` on the three names fell through.
173+
* The gate is the last surface that was open: the describe, the
174+
* `Notification['severity']` type, and the `sys_inbox_message.severity` select
175+
* field all already declared exactly these three.
176+
*
177+
* Safe to close because the executor reads this key RAW — see the
178+
* interpolation note above — so a `{token}` template here never resolved and
179+
* a rejection removes no working authoring shape.
180+
*/
181+
severity: z.enum(['info', 'warning', 'critical']).optional()
182+
.describe('Severity forwarded to the messaging service'),
161183
/** Click-through target object — only effective together with `sourceId` (#2675). */
162184
sourceObject: z.string().optional()
163185
.describe('Object name of the record the notification links to (writes sys_notification.source_object). Only takes effect together with sourceId — a half-specified click-through target is dropped at execute time, so the inbox never renders a dead link.'),

0 commit comments

Comments
 (0)