fix(spec): wait-timeout 处方改印能真正解析的 timerDuration: '60000'（#6758）

.changeset/wait-timeout-prescription-quotes-the-duration.md

@@ -0,0 +1,11 @@
+---
+'@objectstack/spec': patch
+---
+
+fix(spec): `waitEventConfig` 的两处 wait-timeout 处方改写成能真正解析的形式（#6758）
+
+`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」。
+
+两处处方现在都印引号形式 `timerDuration: '60000'`（并给出等价的 ISO 8601 写法 `'PT1M'`），并说明为什么要加引号：该键是字符串，裸数字字符串按毫秒读取。同一段的 TSDoc 一并订正——「retired in 18」改为 17（两处墓碑与转换的 `toMajor` 都是 17），以及把「`parseIsoDuration` accepts a bare number」改为「reads a bare numeric *string*」，因为作者遇到的是 schema 而不是那个 helper。
+
+**接受面逐字节不变。** 改动全部落在 `retiredKey()` 的 guidance 参数、`strictObject` 的 `guidance` 取值和 TSDoc 注释里；`retiredKey()` 返回的始终是 `z.never({ error: () => guidance }).optional()`，`guidance` 也只为一个已被拒绝的键提供文案，因此 `WaitEventConfig` 接受的输入集合完全没有变化。

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

@@ -6,6 +6,9 @@ import type { NodeExecutor } from '../engine.js';
 import { InMemorySuspendedRunStore } from '../suspended-run-store.js';
 import { registerWaitNode, parseIsoDuration, rearmSuspendedWaitTimers } from './wait-node.js';
 import type { IJobService, JobHandler, JobSchedule } from '@objectstack/spec/contracts';
+// #6758 — the wait tombstone's prescription is checked against BOTH gates an
+// author's value must clear: the spec schema and `parseIsoDuration` above.
+import { FlowNodeSchema } from '@objectstack/spec/automation';
 
 function silentLogger() {
   return { info() {}, warn() {}, error() {}, debug() {}, child() { return silentLogger(); } } as any;
@@ -74,6 +77,90 @@ describe('parseIsoDuration', () => {
   });
 });
 
+/**
+ * #6758 — the spec's own upgrade prescription, EXECUTED rather than read.
+ *
+ * `waitEventConfig.timeoutMs` is a #4158 tombstone whose message tells an
+ * upgrading author which `timerDuration` value to write instead. That value has
+ * to survive TWO gates, and each gate is blind to the other's failure:
+ *
+ *   1. **The schema.** `timerDuration` is `z.string()`, so an unquoted number is
+ *      TS2322 at the authoring site and `expected string, received number` at
+ *      the parse. This is what the message printed until #6758 — `parseIsoDuration`
+ *      below does accept a bare JS number, which is where the wrong wording came
+ *      from, but no number can REACH it through `timerDuration`.
+ *   2. **This reader.** `z.string()` takes any string at all, so schema-green
+ *      cannot tell `'60000'` from `'about a minute'` (the latter parses to
+ *      `undefined` here and silently waits on nothing).
+ *
+ * The spec package can only check gate 1 — it does not depend on this one — so
+ * the round trip is pinned here, where both halves are importable. Every value
+ * is EXTRACTED from the live message, never compared to a copy: a hard-coded
+ * `'60000'` would go green the moment someone reworded the prose, which is
+ * exactly when this needs to be checked.
+ */
+describe("the spec's `timeoutMs` → `timerDuration` prescription round-trips (#6758)", () => {
+  const waitNode = (waitEventConfig: Record<string, unknown>) => ({
+    id: 'w', type: 'wait', label: 'Wait', waitEventConfig,
+  });
+  const issueMessage = (waitEventConfig: Record<string, unknown>, code: string) => {
+    const result = FlowNodeSchema.safeParse(waitNode(waitEventConfig));
+    expect(result.success).toBe(false);
+    return result.error!.issues.find((i) => i.code === code)?.message;
+  };
+
+  it('every `timerDuration` it prints parses AND yields the wait it promises', () => {
+    // Channel 1 — the tombstone itself. Channel 2 — the `timeout` misspelling's
+    // `guidance` entry, which repeats the same advice to an author who has had
+    // no first failure to learn from.
+    const tombstone = issueMessage({ eventType: 'timer', timeoutMs: 60_000 }, 'invalid_type');
+    const guidance = issueMessage({ eventType: 'timer', timeout: 60_000 }, 'unrecognized_keys');
+
+    for (const [channel, message] of Object.entries({ tombstone, guidance })) {
+      // Anti-vacuity: gut either channel and this fails, rather than the loop
+      // below passing because it found nothing to check.
+      expect(message, `${channel}: raised no message at all`).toBeDefined();
+      const printed = [...message!.matchAll(/`timerDuration:\s*([^`]+)`/g)].map((m) => m[1]);
+      expect(printed.length, `${channel} must PRINT a \`timerDuration\` value to copy`)
+        .toBeGreaterThan(0);
+
+      for (const literal of printed) {
+        // Read the literal exactly as an author retypes it: `'60000'` is a
+        // string, a bare `60000` is a number — and that gap IS the defect.
+        const authored: unknown = JSON.parse(literal.replace(/^'(.*)'$/, '"$1"'));
+
+        // Gate 1 — the schema.
+        const parsed = FlowNodeSchema.safeParse(waitNode({ eventType: 'timer', timerDuration: authored }));
+        expect(
+          parsed.success,
+          `${channel} prints \`timerDuration: ${literal}\`, but the spec REJECTS it: `
+          + JSON.stringify(parsed.error?.issues),
+        ).toBe(true);
+
+        // Gate 2 — this reader. A wait it cannot parse is a wait on nothing.
+        const ms = parseIsoDuration(parsed.data!.waitEventConfig?.timerDuration);
+        expect(ms, `${channel} prints \`timerDuration: ${literal}\`, which this reader cannot parse`)
+          .toBeGreaterThan(0);
+      }
+    }
+
+    // The tombstone does not merely prescribe a value, it claims an EQUALITY:
+    // "`timeoutMs: N` and `timerDuration: X` the same wait". Read both sides out
+    // of the message and hold it to that claim.
+    const claimedMs = Number(/`timeoutMs:\s*(\d+)`/.exec(tombstone!)?.[1]);
+    expect(claimedMs, 'the tombstone must still name the `timeoutMs` wait it is equating')
+      .toBeGreaterThan(0);
+    for (const literal of [...tombstone!.matchAll(/`timerDuration:\s*([^`]+)`/g)].map((m) => m[1])) {
+      const authored = JSON.parse(literal.replace(/^'(.*)'$/, '"$1"')) as string;
+      expect(
+        parseIsoDuration(authored),
+        `the tombstone equates \`timeoutMs: ${claimedMs}\` with \`timerDuration: ${literal}\`, `
+        + 'but they are not the same wait',
+      ).toBe(claimedMs);
+    }
+  });
+});
+
 describe('wait node executor', () => {
   let engine: AutomationEngine;
   let ran: string[];

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

@@ -1234,6 +1234,72 @@ describe('BPMN — Wait Event Configuration', () => {
     }
   });
 
+  /**
+   * #6758 — a prescription that does not parse is WORSE than no prescription:
+   * the author lands in the exact rejection the tombstone exists to spare them,
+   * and the second one is a bare `invalid_type` with no guidance attached. Both
+   * author-facing channels on this block print a `timerDuration` value to copy,
+   * and `timerDuration` is `z.string()` — so every value they print must be
+   * QUOTED. Until this test they printed the bare number `60000`, which is
+   * TS2322 at the authoring site and `expected string, received number` at the
+   * parse (the ADR-0087 conversion has always known better: it writes
+   * `String(next.timeoutMs)`, `conversions/registry.ts` — "Moving the number
+   * unstringified would produce a block that no longer parses").
+   *
+   * The value is EXTRACTED from the message rather than compared to a copy: a
+   * hard-coded `'60000'` here would go green the moment someone reworded the
+   * prose, which is precisely when this needs to be checked.
+   */
+  it('every `timerDuration` value the wait-timeout prescriptions print actually parses (#6758)', () => {
+    const waitNode = (waitEventConfig: Record<string, unknown>) => ({
+      id: 'wait_timer', type: 'wait', label: 'Wait', waitEventConfig,
+    });
+    const messageFor = (waitEventConfig: Record<string, unknown>, code: string) => {
+      const result = FlowNodeSchema.safeParse(waitNode(waitEventConfig));
+      expect(result.success).toBe(false);
+      return result.error!.issues.find((i) => i.code === code)?.message;
+    };
+
+    const channels = {
+      // Channel 1 — `retiredKey()`'s `z.never` message, raised when an upgrading
+      // author still writes the removed key.
+      'the `timeoutMs` tombstone': messageFor({ eventType: 'timer', timeoutMs: 60_000 }, 'invalid_type'),
+      // Channel 2 — the `strictObject` `guidance` entry for the `timeout`
+      // misspelling. Worse than channel 1: there is no first failure to learn
+      // from, so a bad prescription here is the FIRST thing the schema ever says.
+      'the `timeout` misspelling guidance': messageFor({ eventType: 'timer', timeout: 60_000 }, 'unrecognized_keys'),
+    };
+
+    for (const [channel, message] of Object.entries(channels)) {
+      // Anti-vacuity. Delete the guidance entry (or gut the tombstone) and these
+      // two fail loudly, rather than the loop below passing on an empty match set.
+      expect(message, `${channel}: raised no message at all`).toBeDefined();
+      expect(message, `${channel} must still point the author at \`timerDuration\``)
+        .toContain('`timerDuration');
+
+      const printed = [...message!.matchAll(/`timerDuration:\s*([^`]+)`/g)].map((m) => m[1]);
+      expect(printed.length, `${channel} must PRINT a \`timerDuration\` value to copy`)
+        .toBeGreaterThan(0);
+
+      for (const literal of printed) {
+        // Read the printed literal exactly as an author retypes it: `'60000'` is
+        // a string, a bare `60000` is a number — and that gap IS the defect.
+        let authored: unknown;
+        try {
+          authored = JSON.parse(literal.replace(/^'(.*)'$/, '"$1"'));
+        } catch {
+          expect.fail(`${channel} prints \`timerDuration: ${literal}\`, which is not a writable literal`);
+        }
+        const result = FlowNodeSchema.safeParse(waitNode({ eventType: 'timer', timerDuration: authored }));
+        expect(
+          result.success,
+          `${channel} prints \`timerDuration: ${literal}\`, but the schema REJECTS it: `
+          + JSON.stringify(result.error?.issues),
+        ).toBe(true);
+      }
+    }
+  });
+
   it('should accept wait node with manual resume', () => {
     const result = FlowNodeSchema.safeParse({
       id: 'wait_manual',

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

@@ -353,8 +353,9 @@ function flowNodeObject() { return strictObject(
       // helper once told an author to write something that gets rejected next.
       timeout:
         '`wait` has no timeout — nothing has ever failed or resumed a wait on a deadline ' +
-        '(#4158 retired the two keys that claimed one). Use `timerDuration`: it accepts a ' +
-        'bare number as milliseconds, so `timerDuration: 60000` is a 60s wait.',
+        '(#4158 retired the two keys that claimed one). Use `timerDuration`, and QUOTE the ' +
+        'number: the key is a string, and a bare numeric string is read as milliseconds, so ' +
+        "`timerDuration: '60000'` is a 60s wait (`timerDuration: 'PT1M'` says the same in ISO 8601).",
     },
     history:
       'Until #4001 these were dropped silently — the block still parsed, so a wait node ' +
@@ -370,14 +371,18 @@ function flowNodeObject() { return strictObject(
 
     /**
      * `wait` never had a timeout. Both keys below described one and neither
-     * delivered it (#4158) — the pair is retired in 18 rather than left standing
-     * as a promise the runtime does not keep (PD #10).
+     * delivered it (#4158) — the pair is retired in 17 rather than left standing
+     * as a promise the runtime does not keep (PD #10). (Both tombstones below say
+     * 17 and the ADR-0087 conversion is `toMajor: 17`; this line said 18, the
+     * #4350 class — a tombstone naming a major that never shipped it.)
      *
      * `timeoutMs` said "maximum wait time" and its ONLY reader used it as the
      * timer *duration* when `timerDuration` was absent — so it did something, just
      * not what it said. `timerDuration` already expresses that (`parseIsoDuration`
-     * accepts a bare number as milliseconds), which is why the conversion can move
-     * it losslessly instead of dropping it.
+     * reads a bare numeric *string* as milliseconds — the number must be quoted,
+     * because `timerDuration` is `z.string()` and the schema is what the author
+     * meets), which is why the conversion can move it losslessly, stringifying on
+     * the way, instead of dropping it.
      *
      * `onTimeout` had ZERO readers anywhere. Setting it changed nothing, and the
      * showcase set it — a declared default (`'fail'`) stamped on every wait node
@@ -392,8 +397,10 @@ function flowNodeObject() { return strictObject(
       '`waitEventConfig.timeoutMs` was removed in @objectstack/spec 17 (#4158). It documented a '
       + 'timeout guard that never existed: nothing ever failed or resumed a wait on a deadline. Its '
       + 'only reader treated it as the timer DURATION when `timerDuration` was absent, so use '
-      + '`timerDuration` — it accepts a bare number as milliseconds, making `timeoutMs: 60000` and '
-      + "`timerDuration: 60000` the same wait. Stored flows are converted automatically.",
+      + '`timerDuration` — but QUOTE the number: the key is a string, and a bare numeric string is '
+      + "read as milliseconds, making `timeoutMs: 60000` and `timerDuration: '60000'` the same wait "
+      + "(`timerDuration: 'PT1M'` is the ISO 8601 spelling of that same 60s). Stored flows are "
+      + 'converted automatically — the conversion does the quoting for you.',
     ),
     onTimeout: retiredKey(
       '`waitEventConfig.onTimeout` was removed in @objectstack/spec 17 (#4158). It had no readers at '