Skip to content

Commit 41610f6

Browse files
fix(spec): wait-timeout 处方改印能真正解析的 timerDuration: '60000'#6758) (#6847)
`waitEventConfig.timeoutMs` 墓碑与 `timeout` 拼写错误的 `guidance` 条目都让作者 写 `timerDuration: 60000`,而 `timerDuration` 是 `z.string()`。照做的作者先吃 TS2322,再吃一个不带处方的裸 `invalid_type` —— 正是墓碑要替他们挡掉的两个错误。 ADR-0087 转换早就写对了(`String(next.timeoutMs)`),只有散文说反。 两处处方改印引号形式,并加自校验 pin:从消息里**提取**被规定的值再喂回 schema (spec 侧)和 `parseIsoDuration`(service-automation 侧)。 接受面逐字节不变:只改 `retiredKey()` guidance 参数、`strictObject` guidance 取值与 TSDoc。 Claude-Session: https://claude.ai/code/session_018ffcE95NaMJcL9XJ9VDYgk Co-authored-by: Claude <noreply@anthropic.com>
1 parent 56664f5 commit 41610f6

4 files changed

Lines changed: 179 additions & 8 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@objectstack/spec': patch
3+
---
4+
5+
fix(spec): `waitEventConfig` 的两处 wait-timeout 处方改写成能真正解析的形式(#6758
6+
7+
`waitEventConfig.timeoutMs` 墓碑(`retiredKey()`)与 `timeout` 拼写错误的 `guidance` 条目都让作者写 `timerDuration: 60000`,而 `timerDuration``z.string()`。照着写的作者先在编写处吃一个 TS2322,再在解析时吃一个不带任何处方的裸 `invalid_type` —— 正是墓碑本该替他们挡掉的那两个错误。ADR-0087 转换早就知道正确答案:它写的是 `String(next.timeoutMs)`,其文档注释直言「Moving the number unstringified would produce a block that no longer parses」。
8+
9+
两处处方现在都印引号形式 `timerDuration: '60000'`(并给出等价的 ISO 8601 写法 `'PT1M'`),并说明为什么要加引号:该键是字符串,裸数字字符串按毫秒读取。同一段的 TSDoc 一并订正——「retired in 18」改为 17(两处墓碑与转换的 `toMajor` 都是 17),以及把「`parseIsoDuration` accepts a bare number」改为「reads a bare numeric *string*」,因为作者遇到的是 schema 而不是那个 helper。
10+
11+
**接受面逐字节不变。** 改动全部落在 `retiredKey()` 的 guidance 参数、`strictObject``guidance` 取值和 TSDoc 注释里;`retiredKey()` 返回的始终是 `z.never({ error: () => guidance }).optional()``guidance` 也只为一个已被拒绝的键提供文案,因此 `WaitEventConfig` 接受的输入集合完全没有变化。

packages/services/service-automation/src/builtin/wait-node.test.ts

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ import type { NodeExecutor } from '../engine.js';
66
import { InMemorySuspendedRunStore } from '../suspended-run-store.js';
77
import { registerWaitNode, parseIsoDuration, rearmSuspendedWaitTimers } from './wait-node.js';
88
import type { IJobService, JobHandler, JobSchedule } from '@objectstack/spec/contracts';
9+
// #6758 — the wait tombstone's prescription is checked against BOTH gates an
10+
// author's value must clear: the spec schema and `parseIsoDuration` above.
11+
import { FlowNodeSchema } from '@objectstack/spec/automation';
912

1013
function silentLogger() {
1114
return { info() {}, warn() {}, error() {}, debug() {}, child() { return silentLogger(); } } as any;
@@ -74,6 +77,90 @@ describe('parseIsoDuration', () => {
7477
});
7578
});
7679

80+
/**
81+
* #6758 — the spec's own upgrade prescription, EXECUTED rather than read.
82+
*
83+
* `waitEventConfig.timeoutMs` is a #4158 tombstone whose message tells an
84+
* upgrading author which `timerDuration` value to write instead. That value has
85+
* to survive TWO gates, and each gate is blind to the other's failure:
86+
*
87+
* 1. **The schema.** `timerDuration` is `z.string()`, so an unquoted number is
88+
* TS2322 at the authoring site and `expected string, received number` at
89+
* the parse. This is what the message printed until #6758 — `parseIsoDuration`
90+
* below does accept a bare JS number, which is where the wrong wording came
91+
* from, but no number can REACH it through `timerDuration`.
92+
* 2. **This reader.** `z.string()` takes any string at all, so schema-green
93+
* cannot tell `'60000'` from `'about a minute'` (the latter parses to
94+
* `undefined` here and silently waits on nothing).
95+
*
96+
* The spec package can only check gate 1 — it does not depend on this one — so
97+
* the round trip is pinned here, where both halves are importable. Every value
98+
* is EXTRACTED from the live message, never compared to a copy: a hard-coded
99+
* `'60000'` would go green the moment someone reworded the prose, which is
100+
* exactly when this needs to be checked.
101+
*/
102+
describe("the spec's `timeoutMs` → `timerDuration` prescription round-trips (#6758)", () => {
103+
const waitNode = (waitEventConfig: Record<string, unknown>) => ({
104+
id: 'w', type: 'wait', label: 'Wait', waitEventConfig,
105+
});
106+
const issueMessage = (waitEventConfig: Record<string, unknown>, code: string) => {
107+
const result = FlowNodeSchema.safeParse(waitNode(waitEventConfig));
108+
expect(result.success).toBe(false);
109+
return result.error!.issues.find((i) => i.code === code)?.message;
110+
};
111+
112+
it('every `timerDuration` it prints parses AND yields the wait it promises', () => {
113+
// Channel 1 — the tombstone itself. Channel 2 — the `timeout` misspelling's
114+
// `guidance` entry, which repeats the same advice to an author who has had
115+
// no first failure to learn from.
116+
const tombstone = issueMessage({ eventType: 'timer', timeoutMs: 60_000 }, 'invalid_type');
117+
const guidance = issueMessage({ eventType: 'timer', timeout: 60_000 }, 'unrecognized_keys');
118+
119+
for (const [channel, message] of Object.entries({ tombstone, guidance })) {
120+
// Anti-vacuity: gut either channel and this fails, rather than the loop
121+
// below passing because it found nothing to check.
122+
expect(message, `${channel}: raised no message at all`).toBeDefined();
123+
const printed = [...message!.matchAll(/`timerDuration:\s*([^`]+)`/g)].map((m) => m[1]);
124+
expect(printed.length, `${channel} must PRINT a \`timerDuration\` value to copy`)
125+
.toBeGreaterThan(0);
126+
127+
for (const literal of printed) {
128+
// Read the literal exactly as an author retypes it: `'60000'` is a
129+
// string, a bare `60000` is a number — and that gap IS the defect.
130+
const authored: unknown = JSON.parse(literal.replace(/^'(.*)'$/, '"$1"'));
131+
132+
// Gate 1 — the schema.
133+
const parsed = FlowNodeSchema.safeParse(waitNode({ eventType: 'timer', timerDuration: authored }));
134+
expect(
135+
parsed.success,
136+
`${channel} prints \`timerDuration: ${literal}\`, but the spec REJECTS it: `
137+
+ JSON.stringify(parsed.error?.issues),
138+
).toBe(true);
139+
140+
// Gate 2 — this reader. A wait it cannot parse is a wait on nothing.
141+
const ms = parseIsoDuration(parsed.data!.waitEventConfig?.timerDuration);
142+
expect(ms, `${channel} prints \`timerDuration: ${literal}\`, which this reader cannot parse`)
143+
.toBeGreaterThan(0);
144+
}
145+
}
146+
147+
// The tombstone does not merely prescribe a value, it claims an EQUALITY:
148+
// "`timeoutMs: N` and `timerDuration: X` the same wait". Read both sides out
149+
// of the message and hold it to that claim.
150+
const claimedMs = Number(/`timeoutMs:\s*(\d+)`/.exec(tombstone!)?.[1]);
151+
expect(claimedMs, 'the tombstone must still name the `timeoutMs` wait it is equating')
152+
.toBeGreaterThan(0);
153+
for (const literal of [...tombstone!.matchAll(/`timerDuration:\s*([^`]+)`/g)].map((m) => m[1])) {
154+
const authored = JSON.parse(literal.replace(/^'(.*)'$/, '"$1"')) as string;
155+
expect(
156+
parseIsoDuration(authored),
157+
`the tombstone equates \`timeoutMs: ${claimedMs}\` with \`timerDuration: ${literal}\`, `
158+
+ 'but they are not the same wait',
159+
).toBe(claimedMs);
160+
}
161+
});
162+
});
163+
77164
describe('wait node executor', () => {
78165
let engine: AutomationEngine;
79166
let ran: string[];

packages/spec/src/automation/flow.test.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1234,6 +1234,72 @@ describe('BPMN — Wait Event Configuration', () => {
12341234
}
12351235
});
12361236

1237+
/**
1238+
* #6758 — a prescription that does not parse is WORSE than no prescription:
1239+
* the author lands in the exact rejection the tombstone exists to spare them,
1240+
* and the second one is a bare `invalid_type` with no guidance attached. Both
1241+
* author-facing channels on this block print a `timerDuration` value to copy,
1242+
* and `timerDuration` is `z.string()` — so every value they print must be
1243+
* QUOTED. Until this test they printed the bare number `60000`, which is
1244+
* TS2322 at the authoring site and `expected string, received number` at the
1245+
* parse (the ADR-0087 conversion has always known better: it writes
1246+
* `String(next.timeoutMs)`, `conversions/registry.ts` — "Moving the number
1247+
* unstringified would produce a block that no longer parses").
1248+
*
1249+
* The value is EXTRACTED from the message rather than compared to a copy: a
1250+
* hard-coded `'60000'` here would go green the moment someone reworded the
1251+
* prose, which is precisely when this needs to be checked.
1252+
*/
1253+
it('every `timerDuration` value the wait-timeout prescriptions print actually parses (#6758)', () => {
1254+
const waitNode = (waitEventConfig: Record<string, unknown>) => ({
1255+
id: 'wait_timer', type: 'wait', label: 'Wait', waitEventConfig,
1256+
});
1257+
const messageFor = (waitEventConfig: Record<string, unknown>, code: string) => {
1258+
const result = FlowNodeSchema.safeParse(waitNode(waitEventConfig));
1259+
expect(result.success).toBe(false);
1260+
return result.error!.issues.find((i) => i.code === code)?.message;
1261+
};
1262+
1263+
const channels = {
1264+
// Channel 1 — `retiredKey()`'s `z.never` message, raised when an upgrading
1265+
// author still writes the removed key.
1266+
'the `timeoutMs` tombstone': messageFor({ eventType: 'timer', timeoutMs: 60_000 }, 'invalid_type'),
1267+
// Channel 2 — the `strictObject` `guidance` entry for the `timeout`
1268+
// misspelling. Worse than channel 1: there is no first failure to learn
1269+
// from, so a bad prescription here is the FIRST thing the schema ever says.
1270+
'the `timeout` misspelling guidance': messageFor({ eventType: 'timer', timeout: 60_000 }, 'unrecognized_keys'),
1271+
};
1272+
1273+
for (const [channel, message] of Object.entries(channels)) {
1274+
// Anti-vacuity. Delete the guidance entry (or gut the tombstone) and these
1275+
// two fail loudly, rather than the loop below passing on an empty match set.
1276+
expect(message, `${channel}: raised no message at all`).toBeDefined();
1277+
expect(message, `${channel} must still point the author at \`timerDuration\``)
1278+
.toContain('`timerDuration');
1279+
1280+
const printed = [...message!.matchAll(/`timerDuration:\s*([^`]+)`/g)].map((m) => m[1]);
1281+
expect(printed.length, `${channel} must PRINT a \`timerDuration\` value to copy`)
1282+
.toBeGreaterThan(0);
1283+
1284+
for (const literal of printed) {
1285+
// Read the printed literal exactly as an author retypes it: `'60000'` is
1286+
// a string, a bare `60000` is a number — and that gap IS the defect.
1287+
let authored: unknown;
1288+
try {
1289+
authored = JSON.parse(literal.replace(/^'(.*)'$/, '"$1"'));
1290+
} catch {
1291+
expect.fail(`${channel} prints \`timerDuration: ${literal}\`, which is not a writable literal`);
1292+
}
1293+
const result = FlowNodeSchema.safeParse(waitNode({ eventType: 'timer', timerDuration: authored }));
1294+
expect(
1295+
result.success,
1296+
`${channel} prints \`timerDuration: ${literal}\`, but the schema REJECTS it: `
1297+
+ JSON.stringify(result.error?.issues),
1298+
).toBe(true);
1299+
}
1300+
}
1301+
});
1302+
12371303
it('should accept wait node with manual resume', () => {
12381304
const result = FlowNodeSchema.safeParse({
12391305
id: 'wait_manual',

packages/spec/src/automation/flow.zod.ts

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -353,8 +353,9 @@ function flowNodeObject() { return strictObject(
353353
// helper once told an author to write something that gets rejected next.
354354
timeout:
355355
'`wait` has no timeout — nothing has ever failed or resumed a wait on a deadline ' +
356-
'(#4158 retired the two keys that claimed one). Use `timerDuration`: it accepts a ' +
357-
'bare number as milliseconds, so `timerDuration: 60000` is a 60s wait.',
356+
'(#4158 retired the two keys that claimed one). Use `timerDuration`, and QUOTE the ' +
357+
'number: the key is a string, and a bare numeric string is read as milliseconds, so ' +
358+
"`timerDuration: '60000'` is a 60s wait (`timerDuration: 'PT1M'` says the same in ISO 8601).",
358359
},
359360
history:
360361
'Until #4001 these were dropped silently — the block still parsed, so a wait node ' +
@@ -370,14 +371,18 @@ function flowNodeObject() { return strictObject(
370371

371372
/**
372373
* `wait` never had a timeout. Both keys below described one and neither
373-
* delivered it (#4158) — the pair is retired in 18 rather than left standing
374-
* as a promise the runtime does not keep (PD #10).
374+
* delivered it (#4158) — the pair is retired in 17 rather than left standing
375+
* as a promise the runtime does not keep (PD #10). (Both tombstones below say
376+
* 17 and the ADR-0087 conversion is `toMajor: 17`; this line said 18, the
377+
* #4350 class — a tombstone naming a major that never shipped it.)
375378
*
376379
* `timeoutMs` said "maximum wait time" and its ONLY reader used it as the
377380
* timer *duration* when `timerDuration` was absent — so it did something, just
378381
* not what it said. `timerDuration` already expresses that (`parseIsoDuration`
379-
* accepts a bare number as milliseconds), which is why the conversion can move
380-
* it losslessly instead of dropping it.
382+
* reads a bare numeric *string* as milliseconds — the number must be quoted,
383+
* because `timerDuration` is `z.string()` and the schema is what the author
384+
* meets), which is why the conversion can move it losslessly, stringifying on
385+
* the way, instead of dropping it.
381386
*
382387
* `onTimeout` had ZERO readers anywhere. Setting it changed nothing, and the
383388
* showcase set it — a declared default (`'fail'`) stamped on every wait node
@@ -392,8 +397,10 @@ function flowNodeObject() { return strictObject(
392397
'`waitEventConfig.timeoutMs` was removed in @objectstack/spec 17 (#4158). It documented a '
393398
+ 'timeout guard that never existed: nothing ever failed or resumed a wait on a deadline. Its '
394399
+ 'only reader treated it as the timer DURATION when `timerDuration` was absent, so use '
395-
+ '`timerDuration` — it accepts a bare number as milliseconds, making `timeoutMs: 60000` and '
396-
+ "`timerDuration: 60000` the same wait. Stored flows are converted automatically.",
400+
+ '`timerDuration` — but QUOTE the number: the key is a string, and a bare numeric string is '
401+
+ "read as milliseconds, making `timeoutMs: 60000` and `timerDuration: '60000'` the same wait "
402+
+ "(`timerDuration: 'PT1M'` is the ISO 8601 spelling of that same 60s). Stored flows are "
403+
+ 'converted automatically — the conversion does the quoting for you.',
397404
),
398405
onTimeout: retiredKey(
399406
'`waitEventConfig.onTimeout` was removed in @objectstack/spec 17 (#4158). It had no readers at '

0 commit comments

Comments
 (0)