From 645e27c8f9c568e96f5d9de4e839a2bbb1c8e473 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 15:37:21 +0000 Subject: [PATCH 1/3] feat(spec)!: converge RetryPolicy onto one declaration (#4661) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `@objectstack/spec/automation` and `@objectstack/spec/system` both exported `RetryPolicySchema` / `RetryPolicy` resolving to DIFFERENT declarations, so the shape a consumer got depended only on the import path (the #4411 trap). They were never two concepts: the `try_catch` node's `retry` region and `job.retryPolicy` both compute `delay = base * multiplier^(retry-1)`, and both executors implemented that identical formula. One declaration now lives in `shared/retry-policy.zod.ts`, re-exported by both entries, carrying the union of what the two sides could express. Because the published def key is derived from the entry namespace, both `automation/RetryPolicy` and `system/RetryPolicy` survive with an identical key set — so the convergence costs exactly ONE authorable key instead of eight. Authorable surface: `automation/RetryPolicy:retryDelayMs` is the single casualty, TOMBSTONED (`retiredKey`) rather than deleted because neither owning schema is `.strict()` — a plain removal would have Zod swallow the authored number and silently fall back to the 1000ms default. Defaults are the half no gate can see: the authorable-surface ratchet compares key sets, and a default is not a key. `job.retryPolicy` defaulted `maxRetries: 3` / `backoffMultiplier: 2` where automation defaulted 0 / 1. The merged declaration takes 0 / 1 (retry replays side effects, so it is opt-in), and the `retry-policy-converged` conversion writes the pre-17 numbers explicitly into every existing `job.retryPolicy` that omitted them — deployed stacks keep their exact behaviour; only a newly authored omission changes meaning. Filed separately as #4666 (gates are blind to default/constraint changes). `job.retryPolicy` gains `maxRetryDelayMs` and `jitter`, both now enforced in `runWithPolicy` rather than merely declared (ADR-0049). Baseline: 22 -> 20. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01M9uWvoEp9CoLzYjNExj9sL --- .changeset/retry-policy-dual-source-c8.md | 76 +++++++++ .../references/automation/control-flow.mdx | 2 +- content/docs/references/automation/job.mdx | 34 ---- content/docs/references/automation/meta.json | 2 +- .../references/automation/retry-policy.mdx | 35 ++++ content/docs/references/system/job.mdx | 19 +-- content/docs/references/system/meta.json | 3 +- .../docs/references/system/retry-policy.mdx | 35 ++++ .../2026-07-unknown-key-strictness-ledger.md | 4 +- docs/protocol-upgrade-guide.md | 8 + .../src/builtin/config-schemas.test.ts | 10 ++ .../src/builtin/try-catch-node.ts | 12 +- .../service-job/src/run-with-policy.ts | 38 ++++- packages/spec/api-surface.json | 2 + packages/spec/authorable-surface.json | 8 +- .../spec/dual-source-exports.baseline.json | 2 - packages/spec/spec-changes.json | 26 +++ .../spec/src/automation/control-flow.test.ts | 29 +++- .../spec/src/automation/control-flow.zod.ts | 26 +-- packages/spec/src/contracts/job-service.ts | 19 ++- packages/spec/src/conversions/registry.ts | 154 ++++++++++++++++++ packages/spec/src/migrations/registry.ts | 42 ++++- packages/spec/src/shared/retry-policy.test.ts | 104 ++++++++++++ packages/spec/src/shared/retry-policy.zod.ts | 110 +++++++++++++ packages/spec/src/system/job.test.ts | 39 ++++- packages/spec/src/system/job.zod.ts | 35 ++-- 26 files changed, 775 insertions(+), 99 deletions(-) create mode 100644 .changeset/retry-policy-dual-source-c8.md delete mode 100644 content/docs/references/automation/job.mdx create mode 100644 content/docs/references/automation/retry-policy.mdx create mode 100644 content/docs/references/system/retry-policy.mdx create mode 100644 packages/spec/src/shared/retry-policy.test.ts create mode 100644 packages/spec/src/shared/retry-policy.zod.ts diff --git a/.changeset/retry-policy-dual-source-c8.md b/.changeset/retry-policy-dual-source-c8.md new file mode 100644 index 0000000000..b93535a919 --- /dev/null +++ b/.changeset/retry-policy-dual-source-c8.md @@ -0,0 +1,76 @@ +--- +"@objectstack/spec": major +--- + +**Retry policy converges onto one declaration** (#4661 — the #4535 C8 dual-source cluster). + +`@objectstack/spec/automation` and `@objectstack/spec/system` both exported +`RetryPolicySchema` / `RetryPolicy`, resolving to **different declarations** — so the +shape you got depended only on which entry you imported (the #4411 trap). They were +never two concepts: the `try_catch` node's `retry` region and `job.retryPolicy` both +compute `delay = base * multiplier^(retry-1)`, and both executors implemented that +identical formula. There is now one declaration, re-exported by both entries, carrying +the union of what the two sides could express. + +## FROM → TO + +| | FROM `./automation` | FROM `./system` | TO (both entries) | +|---|---|---|---| +| base delay | `retryDelayMs`, min 0, default 1000 | `backoffMs`, positive, default 1000 | **`backoffMs`**, min 0, default 1000 | +| `maxRetries` | 0–10, default **0** | ≥0 unbounded, default **3** | 0–**10**, default **0** | +| `backoffMultiplier` | ≥**1**, default **1** | positive, default **2** | ≥**1**, default **1** | +| `maxRetryDelayMs` | default 30000 | *(absent)* | default 30000 | +| `jitter` | default false | *(absent)* | default false | +| `RetryPolicy` type | `z.input` | `z.infer` | `z.input` (+ new `RetryPolicyParsed` for `z.infer`) | + +## What you must change + +**1. Rename `retryDelayMs` → `backoffMs`** in any `try_catch` node's `retry` block. +The value (milliseconds before the first retry) is unchanged. The old spelling is +**tombstoned**, not deleted — it rejects with the rename prescription instead of being +silently swallowed, because neither owning schema is `.strict()`. Automated: + +``` +os migrate meta --from 16 +``` + +**2. Nothing for existing jobs — but read this if you author new ones.** `maxRetries` +now defaults to **0** and `backoffMultiplier` to **1**, where `job.retryPolicy` +previously defaulted to 3 and 2. Left alone that would silently stop deployed jobs from +retrying, so the `retry-policy-converged` conversion **writes the pre-17 numbers +explicitly into every existing `job.retryPolicy`** that omitted them: + +```jsonc +// before // after `os migrate meta` +{ "backoffMs": 5000 } { "backoffMs": 5000, "maxRetries": 3, "backoffMultiplier": 2 } +``` + +Deployed stacks therefore keep their exact behaviour. What changes is what a **newly +authored** omission means: declaring a retry block without `maxRetries` now means *no +retry*. Retry is opt-in because a retry replays whatever the attempt already did — a job +handler's writes and callouts, a `try` region's side effects — and an implicit replay is +the failure mode hardest to catch in tests and most expensive in production. (The same +reading is already recorded for flow-level retry in `flow-retry-max-retries-required`, +#4247.) + +> This defaults change is the part **no gate can see**: the authorable-surface ratchet +> compares key sets, and a default is not a key. It is called out here because a +> changeset is the only channel that carries it. + +**3. Two bounds now apply to jobs that did not have them** — `maxRetries` is capped at +**10** and `backoffMultiplier` floored at **1**. Both fail loudly at parse time rather +than being silently reinterpreted; neither has a lossless rewrite, so they are recorded +as the `job-retry-policy-constraints-tightened` semantic migration note. A multiplier +below 1 described a delay that *shrinks* on each attempt — retrying a failing dependency +ever faster, the opposite of backoff. + +**4. `import type { RetryPolicy } from '@objectstack/spec/system'` is now the input +shape** (every key optional) rather than the post-parse shape. Use the new +`RetryPolicyParsed` where you need defaults applied. + +## What you gain + +`job.retryPolicy` accepts **`maxRetryDelayMs`** (ceiling on a single backoff delay) and +**`jitter`** (randomize each delay into [50%, 100%]). Both are enforced by +`runWithPolicy`, not merely declared — jitter is what stops a fleet of jobs that failed +on one outage from retrying in lockstep. diff --git a/content/docs/references/automation/control-flow.mdx b/content/docs/references/automation/control-flow.mdx index 147bbdf545..5c98907410 100644 --- a/content/docs/references/automation/control-flow.mdx +++ b/content/docs/references/automation/control-flow.mdx @@ -149,7 +149,7 @@ const result = FlowRegionSchema.parse(data); | **try** | `{ nodes: { id: string; type: string; label: string; config?: Record; … }[]; edges?: { id: string; source: string; target: string; condition?: string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }; … }[] }` | ✅ | Protected region | | **catch** | `{ nodes: { id: string; type: string; label: string; config?: Record; … }[]; edges?: { id: string; source: string; target: string; condition?: string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }; … }[] }` | optional | Handler region run when the try region fails | | **errorVariable** | `string` | optional | Variable holding the caught error in the catch region | -| **retry** | `{ maxRetries?: integer; retryDelayMs?: integer; backoffMultiplier?: number; maxRetryDelayMs?: integer; … }` | optional | Optional retry policy for the try region | +| **retry** | `{ maxRetries?: integer; backoffMs?: integer; backoffMultiplier?: number; maxRetryDelayMs?: integer; … }` | optional | Optional retry policy for the try region | --- diff --git a/content/docs/references/automation/job.mdx b/content/docs/references/automation/job.mdx deleted file mode 100644 index 6acf71d321..0000000000 --- a/content/docs/references/automation/job.mdx +++ /dev/null @@ -1,34 +0,0 @@ ---- -title: Job -description: Job protocol schemas ---- - -{/* ⚠️ AUTO-GENERATED — DO NOT EDIT. Run build-docs.ts to regenerate. Hand-written docs live in the module folders under content/docs/. */} - -## TypeScript Usage - -```typescript -import { RetryPolicySchema } from '@objectstack/spec/automation'; -import type { RetryPolicy } from '@objectstack/spec/automation'; - -// Validate data -const result = RetryPolicySchema.parse(data); -``` - ---- - -## RetryPolicy - -### Properties - -| Property | Type | Required | Description | -| :--- | :--- | :--- | :--- | -| **maxRetries** | `integer` | ✅ | Retry attempts before giving up | -| **retryDelayMs** | `integer` | ✅ | Base delay between retries (ms) | -| **backoffMultiplier** | `number` | ✅ | Exponential backoff multiplier | -| **maxRetryDelayMs** | `integer` | ✅ | Maximum delay between retries (ms) | -| **jitter** | `boolean` | ✅ | Add random jitter to retry delay | - - ---- - diff --git a/content/docs/references/automation/meta.json b/content/docs/references/automation/meta.json index 1021c8f5b5..2745091a7f 100644 --- a/content/docs/references/automation/meta.json +++ b/content/docs/references/automation/meta.json @@ -17,12 +17,12 @@ "webhook", "---Approvals & Jobs---", "approval", - "job", "---More---", "builtin-node-config", "events-core", "flow-function", "io-node-config", + "retry-policy", "schemaless-node-config" ] } \ No newline at end of file diff --git a/content/docs/references/automation/retry-policy.mdx b/content/docs/references/automation/retry-policy.mdx new file mode 100644 index 0000000000..e89e986509 --- /dev/null +++ b/content/docs/references/automation/retry-policy.mdx @@ -0,0 +1,35 @@ +--- +title: Retry Policy +description: Retry Policy protocol schemas +--- + +{/* ⚠️ AUTO-GENERATED — DO NOT EDIT. Run build-docs.ts to regenerate. Hand-written docs live in the module folders under content/docs/. */} + +## TypeScript Usage + +```typescript +import { RetryPolicySchema } from '@objectstack/spec/automation'; +import type { RetryPolicy } from '@objectstack/spec/automation'; + +// Validate data +const result = RetryPolicySchema.parse(data); +``` + +--- + +## RetryPolicy + +### Properties + +| Property | Type | Required | Description | +| :--- | :--- | :--- | :--- | +| **maxRetries** | `integer` | ✅ | Retry attempts after the initial one. 0 (the default) means no retry — state a count to opt in. | +| **backoffMs** | `integer` | ✅ | Base delay before the first retry (ms); subsequent delays multiply by backoffMultiplier | +| **backoffMultiplier** | `number` | ✅ | Exponential backoff multiplier; 1 (the default) keeps the delay flat | +| **maxRetryDelayMs** | `integer` | ✅ | Ceiling for a single backoff delay (ms) | +| **jitter** | `boolean` | ✅ | Randomize each delay within [50%, 100%] of its computed value — spreads a thundering herd of simultaneous retries | +| **retryDelayMs** | `any` | optional | [REMOVED] `retryDelayMs` was removed in @objectstack/spec 17.0.0 (#4661) — the retry policy now has one spelling for its base delay across `job.retryPolicy` and a `try_catch` node's `retry`. Rename the key to `backoffMs`; the value (milliseconds before the first retry) is unchanged. `os migrate meta --from 16` rewrites it for you. | + + +--- + diff --git a/content/docs/references/system/job.mdx b/content/docs/references/system/job.mdx index 54d159f9d2..627262997c 100644 --- a/content/docs/references/system/job.mdx +++ b/content/docs/references/system/job.mdx @@ -16,8 +16,8 @@ Schedule jobs using cron expressions ## TypeScript Usage ```typescript -import { CronScheduleSchema, IntervalScheduleSchema, JobSchema, JobExecutionSchema, JobExecutionStatus, OnceScheduleSchema, RetryPolicySchema, ScheduleSchema } from '@objectstack/spec/system'; -import type { CronSchedule, IntervalSchedule, Job, JobExecution, JobExecutionStatus, OnceSchedule, RetryPolicy, Schedule } from '@objectstack/spec/system'; +import { CronScheduleSchema, IntervalScheduleSchema, JobSchema, JobExecutionSchema, JobExecutionStatus, OnceScheduleSchema, ScheduleSchema } from '@objectstack/spec/system'; +import type { CronSchedule, IntervalSchedule, Job, JobExecution, JobExecutionStatus, OnceSchedule, Schedule } from '@objectstack/spec/system'; // Validate data const result = CronScheduleSchema.parse(data); @@ -62,7 +62,7 @@ const result = CronScheduleSchema.parse(data); | **description** | `string` | optional | Job description / purpose | | **schedule** | `{ type: 'cron'; expression: string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }; timezone?: string } \| { type: 'interval'; intervalMs: integer } \| { type: 'once'; at: string }` | ✅ | Job schedule configuration | | **handler** | `string` | ✅ | Handler function name (must match a key in `defineStack({ functions })`) | -| **retryPolicy** | `{ maxRetries?: integer; backoffMs?: integer; backoffMultiplier?: number }` | optional | Retry policy: failed runs (including timeouts) are retried with exponential backoff (delay = backoffMs * backoffMultiplier^(retry-1)) up to maxRetries retries after the initial attempt (#3494). Omit for the legacy single-attempt behavior. | +| **retryPolicy** | `{ maxRetries?: integer; backoffMs?: integer; backoffMultiplier?: number; maxRetryDelayMs?: integer; … }` | optional | Retry policy: failed runs (including timeouts) are retried with exponential backoff (delay = min(backoffMs * backoffMultiplier^(retry-1), maxRetryDelayMs), optionally jittered) up to maxRetries retries after the initial attempt (#3494). Omit the block for a single attempt; declaring it without `maxRetries` also means no retry since 17.0.0 (#4661) — state a count to opt in. | | **timeout** | `integer` | optional | Per-attempt time limit in milliseconds; an over-limit run is recorded with execution status "timeout" (#3494). The in-flight handler is abandoned, not forcibly cancelled. Omit for no time limit. | | **enabled** | `boolean` | optional | Whether the job is enabled | | **_lock** | `Enum<'none' \| 'no-overlay' \| 'no-delete' \| 'full'>` | optional | Item-level lock — controls overlay & delete (ADR-0010). | @@ -114,19 +114,6 @@ const result = CronScheduleSchema.parse(data); | **at** | `string` | ✅ | ISO 8601 datetime when to execute | ---- - -## RetryPolicy - -### Properties - -| Property | Type | Required | Description | -| :--- | :--- | :--- | :--- | -| **maxRetries** | `integer` | ✅ | Maximum number of retry attempts | -| **backoffMs** | `integer` | ✅ | Initial backoff delay in milliseconds | -| **backoffMultiplier** | `number` | ✅ | Multiplier for exponential backoff | - - --- ## Schedule diff --git a/content/docs/references/system/meta.json b/content/docs/references/system/meta.json index 42f1559f7a..56a5dde8a0 100644 --- a/content/docs/references/system/meta.json +++ b/content/docs/references/system/meta.json @@ -45,6 +45,7 @@ "collaboration", "doc", "---More---", - "metadata-types" + "metadata-types", + "retry-policy" ] } \ No newline at end of file diff --git a/content/docs/references/system/retry-policy.mdx b/content/docs/references/system/retry-policy.mdx new file mode 100644 index 0000000000..b8b7822586 --- /dev/null +++ b/content/docs/references/system/retry-policy.mdx @@ -0,0 +1,35 @@ +--- +title: Retry Policy +description: Retry Policy protocol schemas +--- + +{/* ⚠️ AUTO-GENERATED — DO NOT EDIT. Run build-docs.ts to regenerate. Hand-written docs live in the module folders under content/docs/. */} + +## TypeScript Usage + +```typescript +import { RetryPolicySchema } from '@objectstack/spec/system'; +import type { RetryPolicy } from '@objectstack/spec/system'; + +// Validate data +const result = RetryPolicySchema.parse(data); +``` + +--- + +## RetryPolicy + +### Properties + +| Property | Type | Required | Description | +| :--- | :--- | :--- | :--- | +| **maxRetries** | `integer` | ✅ | Retry attempts after the initial one. 0 (the default) means no retry — state a count to opt in. | +| **backoffMs** | `integer` | ✅ | Base delay before the first retry (ms); subsequent delays multiply by backoffMultiplier | +| **backoffMultiplier** | `number` | ✅ | Exponential backoff multiplier; 1 (the default) keeps the delay flat | +| **maxRetryDelayMs** | `integer` | ✅ | Ceiling for a single backoff delay (ms) | +| **jitter** | `boolean` | ✅ | Randomize each delay within [50%, 100%] of its computed value — spreads a thundering herd of simultaneous retries | +| **retryDelayMs** | `any` | optional | [REMOVED] `retryDelayMs` was removed in @objectstack/spec 17.0.0 (#4661) — the retry policy now has one spelling for its base delay across `job.retryPolicy` and a `try_catch` node's `retry`. Rename the key to `backoffMs`; the value (milliseconds before the first retry) is unchanged. `os migrate meta --from 16` rewrites it for you. | + + +--- + diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.md b/docs/audits/2026-07-unknown-key-strictness-ledger.md index 0404cf4cc4..607ff76ac0 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.md @@ -477,7 +477,7 @@ tightening (the #4001 "sharing-rule lesson": candidates, not verdicts). | `validation.zod.ts` | 6 | authorable | **strict as of #4001 batch 3b** — a `z.lazy()` discriminated union, so the one-call conversion does not apply: each of the six variants builds its own `strictObject` from a shared `BASE_VALIDATION_SHAPE`. Closing the base alone would have rejected correctly but suggested from the SHARED keys only, so a typo of a variant's own key (`transtions` → `transitions`) would get no rename. Site count 1 → 6 because the six variants are now object sites in their own right. The ADR-0010 envelope lives in the shared shape, so all six inherit it | | `field-value.zod.ts` / `seed.zod.ts` | 1+1 | mixed (p) | `seed` is strict (registered-types batch) | -### `automation/` — 88 sites +### `automation/` — 87 sites | File | Sites | Class | Note | |---|---|---|---| @@ -485,7 +485,7 @@ tightening (the #4001 "sharing-rule lesson": candidates, not verdicts). | `sync.zod.ts` / `etl.zod.ts` | 12+10 | authorable (p) | authored pipelines — **candidates** | | `execution.zod.ts` | 13 | wire | run-state envelopes — never strict. +5 at #4354 (the run-summary family: step metrics / skip reason / per-node / per-gate / the summary itself) — engine-emitted telemetry read by the Console and by operator queries, nobody authors them, so the `wire` verdict covers them unchanged | | `state-machine.zod.ts` | 7 | authorable (p) | | -| `control-flow.zod.ts` | 6 | authorable (p) | validated structurally by `validateControlFlow` | +| `control-flow.zod.ts` | 5 | authorable (p) | validated structurally by `validateControlFlow`. **−1 at #4661**: `RetryPolicySchema` moved out to `shared/retry-policy.zod.ts` — `./automation` and `./system` published the same name for two different declarations (#4411), so the retry policy converged onto one. The site still exists and is still non-strict and authorable; it is simply no longer in a directory this ledger sections. ⚠️ That is a coverage gap worth knowing about: this audit sections `ui/` / `data/` / `automation/` / `security/` / `studio/` only, so a `shared/` shape is unaudited by construction. The tolerance is deliberate here — the `retryDelayMs` → `backoffMs` rename is tombstoned via `retiredKey()` precisely because a non-strict parent would otherwise swallow the old spelling | | `bpmn-interop.zod.ts` | 5 | wire (p) | interop import shapes | | `approval.zod.ts` | 4 | authorable | **strict as of #4001 step 3** — all four authoring schemas (node config / approver / escalation / decision-output). The published JSON schema carries `additionalProperties: false` into the Studio form AND `registerFlow()` config validation (#4027/#4040), so an unknown key in an approval node's `config` is rejected at registration too — verified: `z.toJSONSchema` on the strict lazySchema does not throw (#3746 hazard checked) | | `node-executor.zod.ts` | 4 | wire | executor contract | diff --git a/docs/protocol-upgrade-guide.md b/docs/protocol-upgrade-guide.md index 40466e50c5..321fa1e4e4 100644 --- a/docs/protocol-upgrade-guide.md +++ b/docs/protocol-upgrade-guide.md @@ -164,6 +164,10 @@ The `script` flow node converges on its one real path (#4343). It had four ways The same audit reaches the driver contract itself: `IDataDriver.findStream` is removed (#4484). It was REQUIRED — every driver and every test double had to implement it — and documented as the read "optimized for large datasets to avoid memory overflow", while two of its three implementations awaited `find()` for the whole result set and then yielded it row by row, reaching exactly the peak it promised to avoid; the third streamed for real but was the one read in that driver that skipped `buildFindOptions`, so it dropped `query.fields`. Nothing anywhere called it, which is why a contract method could carry an inverted guarantee for this long and why ~20 test doubles could satisfy it by throwing `not implemented`. Paged `find()` is the read that exists and is enforced (its total-order guarantee is checked by the shared pagination-conformance cases); a cursor-based read is worth building when a caller asks for one, which is the honest order. A TS/API surface, never stored — one semantic TODO for driver authors, no source rewrite, and no tombstone: `DriverInterfaceSchema` describes a contract that code IMPLEMENTS and nothing ever `.parse()`d a driver, so tsc is the only channel that could carry the prescription, and it carries it where it matters — at a call site. +Finally it converges the retry policy (#4661). `@objectstack/spec/automation` and `@objectstack/spec/system` each exported a `RetryPolicy`/`RetryPolicySchema` resolving to a DIFFERENT declaration, so which shape a consumer got depended only on the import path (#4411) — yet both computed `delay = base * multiplier^(retry-1)` and both executors implemented that same formula. One declaration now serves both entries with the union of their capabilities, so `job.retryPolicy` gains the `maxRetryDelayMs` ceiling and `jitter` (both enforced in `runWithPolicy`, not merely declared — jitter is what stops a fleet of jobs that failed on one outage from retrying in lockstep). The single authorable casualty is the automation spelling of the base delay: `retryDelayMs` → `backoffMs`, a pure rename that replays losslessly and is what the already-enforced retry policies (`job.retryPolicy`, `hook.retryPolicy`) call it. + +The subtle half is the defaults, and it is worth stating because no gate can see it: `job.retryPolicy` defaulted `maxRetries: 3` / `backoffMultiplier: 2` while the automation shape defaulted 0 / 1, and the authorable-surface gate compares KEY SETS — a changed default is invisible to it, to the tombstone mechanism and to `spec_changes` alike. The merged declaration takes 0 / 1 (retry replays side effects, so it is opt-in — the same reading already recorded in `flow-retry-max-retries-required`), and the conversion writes the pre-17 numbers into every existing `job.retryPolicy` that omitted them. Deployed stacks therefore keep their exact behaviour; what changes is only what a NEWLY authored omission means. + ### Mechanical (applied for you) | Conversion | Surface | Change | Load window | @@ -196,9 +200,13 @@ The same audit reaches the driver contract itself: `IDataDriver.findStream` is r | `datasource-inert-blocks-removed` | `datasource.retryPolicy / datasource.healthCheck / datasource.external.label / datasource.external.requirePermission` | datasource keys 'retryPolicy'/'healthCheck' and external 'label'/'requirePermission' removed (#4583 — nothing retried, nothing probed on a schedule, and the federation label/permission were read by nobody) | retired — `migrate meta` only | | `datasource-config-driver-key-aliases` | `datasource.config` | datasource config keys → canonical per driver: sqlite 'file'/'database' → 'filename', postgres/mysql 'connectionString' → 'url' and 'user' → 'username', mongo 'uri' → 'url' and 'user' → 'username' (#4456 — driver-factory `??` fallback graduation) | retired — `migrate meta` only | | `flow-node-script-branch-keys-removed` | `flow.node.script.config.actionType / flow.node.script.config.template / flow.node.script.config.recipients / flow.node.script.config.variables / flow.node.script.config.script` | script flow-node config keys 'actionType' (→ 'function' when it was shorthand for one; otherwise removed — 'email'/'slack' were logger-backed stubs that delivered nothing), plus 'template' / 'recipients' / 'variables' (fed those stubs) and 'script' (inline JS the runtime never executed) (#4343) | retired — `migrate meta` only | +| `retry-policy-converged` | `flow.node.config.retry.retryDelayMs / job.retryPolicy.maxRetries / job.retryPolicy.backoffMultiplier` | retry policy unified across job.retryPolicy and try_catch retry: base delay 'retryDelayMs' → 'backoffMs', and the pre-17 job defaults (maxRetries 3, backoffMultiplier 2) written out explicitly now that the merged default is 0 / 1 (#4661) | live — protocol 17 loader accepts the old shape | ### Semantic (delegated to you, with acceptance criteria) +- **`job-retry-policy-constraints-tightened`** — `job.retryPolicy.maxRetries (> 10) / job.retryPolicy.backoffMultiplier (< 1)` → maxRetries <= 10, and backoffMultiplier >= 1 + - Why not automatic: The converged RetryPolicy (#4661) keeps the automation side's bounds, which the job side never had: `maxRetries` is capped at 10 and `backoffMultiplier` floored at 1. Neither has a lossless rewrite. Clamping `maxRetries: 20` to 10 would halve a retry budget its author chose, and a `backoffMultiplier` below 1 describes a delay that SHRINKS on each attempt — retrying a failing dependency ever faster, which is the opposite of backoff and was never a shape the engine meant to offer. Both now fail at parse time with the bound named, rather than being silently reinterpreted. Choosing the replacement count (or accepting the cap) is the author's call. + - Done when: Every job declaring `retryPolicy` parses: no `maxRetries` above 10 and no `backoffMultiplier` below 1 remain, and each adjusted value was re-chosen knowing a retry re-runs the handler with its writes and callouts. No job fails to register with the retry-policy bound prescription. - **`flow-retry-max-retries-required`** — `flow.errorHandling.maxRetries (under strategy: 'retry')` → an explicit count >= 1 (e.g. maxRetries: 3), or strategy: 'fail' - Why not automatic: maxRetries had two defaults — FlowSchema `.default(0)` and the engine's `maxRetries ?? 3` — so an unstated count retried 0 times through the schema and 3 times through a hand-built definition (#4247). With the engine's copy removed the unstated count is unambiguously 0, and retrying zero times is exactly `strategy: 'fail'`, so the schema now refuses the combination instead of it silently doing nothing. There is no lossless rewrite: 0 preserves the behaviour a parsed flow got but contradicts what its author wrote, and any positive count is a NEW decision about re-running the whole flow with its side effects. That choice is the author's. - Done when: Every flow declaring `errorHandling.strategy: 'retry'` also declares `maxRetries` >= 1, and each count was chosen knowing a retry replays the flow FROM THE START (records re-created, callouts re-fired); flows that never actually wanted retries say `strategy: 'fail'`. No flow fails to register with the maxRetries prescription. diff --git a/packages/services/service-automation/src/builtin/config-schemas.test.ts b/packages/services/service-automation/src/builtin/config-schemas.test.ts index 2c70cabe59..20b9be0fc6 100644 --- a/packages/services/service-automation/src/builtin/config-schemas.test.ts +++ b/packages/services/service-automation/src/builtin/config-schemas.test.ts @@ -203,6 +203,16 @@ describe('builtin node configSchemas — designer parity (#3304)', () => { expect(retry?.maxRetries?.maximum).toBe(10); expect(retry?.backoffMultiplier?.minimum).toBe(1); expect(retry?.jitter?.type).toBe('boolean'); + + // #4661: the base delay is `backoffMs` — one spelling shared with + // `job.retryPolicy` since the retry policy converged onto a single + // declaration. Pinned here because the form/Zod ledger test compares only + // the TOP-LEVEL config keys (`try`/`catch`/`errorVariable`/`retry`), so + // nothing else would notice this nested key being renamed back or the + // Studio form drifting off the contract it is supposed to drive. + expect(retry?.backoffMs?.type).toBe('integer'); + expect(retry?.backoffMs?.minimum).toBe(0); + expect(retry?.retryDelayMs).toBeUndefined(); }); }); diff --git a/packages/services/service-automation/src/builtin/try-catch-node.ts b/packages/services/service-automation/src/builtin/try-catch-node.ts index 6faf3f5b09..bf497e402b 100644 --- a/packages/services/service-automation/src/builtin/try-catch-node.ts +++ b/packages/services/service-automation/src/builtin/try-catch-node.ts @@ -59,7 +59,9 @@ export function registerTryCatchNode(engine: AutomationEngine, ctx: PluginContex type: 'object', properties: { maxRetries: { type: 'integer', minimum: 0, maximum: 10 }, - retryDelayMs: { type: 'integer', minimum: 0 }, + // `backoffMs` (was `retryDelayMs`) since spec 17.0.0 — one retry + // policy spelling across job.retryPolicy and try_catch (#4661). + backoffMs: { type: 'integer', minimum: 0 }, backoffMultiplier: { type: 'number', minimum: 1 }, maxRetryDelayMs: { type: 'integer', minimum: 0 }, jitter: { type: 'boolean' }, @@ -72,9 +74,11 @@ export function registerTryCatchNode(engine: AutomationEngine, ctx: PluginContex async execute(node, variables, context) { // Parse against the ADR-0031 contract. Note the retry defaults now come // from the CONTRACT (RetryPolicySchema): a declared `retry` block that - // omits `retryDelayMs` gets the documented 1000ms base delay, where this + // omits `backoffMs` gets the documented 1000ms base delay, where this // executor historically filled in 0 — the declared default is the - // enforced one (#4277). + // enforced one (#4277). Since spec 17.0.0 that contract is one shared + // declaration with `job.retryPolicy`, and the base delay is spelled + // `backoffMs` (was `retryDelayMs`, tombstoned + converted — #4661). const parsed = parseNodeConfig('try_catch', node.id, TryCatchConfigSchema, node.config); if (!parsed.ok) return parsed.refusal; const cfg = parsed.config; @@ -85,7 +89,7 @@ export function registerTryCatchNode(engine: AutomationEngine, ctx: PluginContex const ctxOrEmpty = context ?? ({} as AutomationContext); const maxRetries = retry?.maxRetries ?? 0; - const baseDelay = retry?.retryDelayMs ?? 0; + const baseDelay = retry?.backoffMs ?? 0; const multiplier = retry?.backoffMultiplier ?? 1; const maxDelay = retry?.maxRetryDelayMs ?? 30000; const useJitter = retry?.jitter === true; diff --git a/packages/services/service-job/src/run-with-policy.ts b/packages/services/service-job/src/run-with-policy.ts index 07c67c5357..2dbe2fb93b 100644 --- a/packages/services/service-job/src/run-with-policy.ts +++ b/packages/services/service-job/src/run-with-policy.ts @@ -13,7 +13,25 @@ export class JobTimeoutError extends Error { } } -const RETRY_DEFAULTS = { maxRetries: 3, backoffMs: 1000, backoffMultiplier: 2 } as const; +/** + * Mirrors the declared defaults of `RetryPolicySchema` + * (`@objectstack/spec` `shared/retry-policy.zod.ts`) — the declared default IS + * the enforced one (#4277). They apply only to a caller that hand-builds + * `JobScheduleOptions` without going through Zod; an authored `job.retryPolicy` + * arrives already defaulted. + * + * `maxRetries` was 3 and `backoffMultiplier` 2 before 17.0.0 (#4661). Existing + * job documents keep those numbers — the `retry-policy-converged` + * conversion writes them in — so this change is only about what an omission + * means from now on: no retry unless asked for. + */ +const RETRY_DEFAULTS = { + maxRetries: 0, + backoffMs: 1000, + backoffMultiplier: 1, + maxRetryDelayMs: 30000, + jitter: false, +} as const; function sleep(ms: number): Promise { return new Promise((resolve) => { @@ -41,9 +59,14 @@ function withTimeout(run: () => Promise, jobId: string, timeoutMs?: number * {@link JobTimeoutError}. JavaScript cannot forcibly cancel the in-flight * handler — the attempt is abandoned, not killed. * - `retryPolicy` re-runs failed attempts (including timeouts) with - * exponential backoff: delay = backoffMs * backoffMultiplier^(retry-1), - * up to maxRetries retries after the initial attempt. The last error is - * rethrown when all attempts fail. + * exponential backoff: delay = min(backoffMs * backoffMultiplier^(retry-1), + * maxRetryDelayMs), randomized into [50%, 100%] when `jitter` is set, up to + * maxRetries retries after the initial attempt. The last error is rethrown + * when all attempts fail. + * - `maxRetryDelayMs` / `jitter` arrived with the 17.0.0 retry-policy + * convergence (#4661) and are enforced here, not merely declared: jitter is + * what stops a fleet of jobs that failed on the same outage from retrying in + * lockstep. */ export async function runWithPolicy( jobId: string, @@ -58,11 +81,16 @@ export async function runWithPolicy( const maxRetries = options.retryPolicy.maxRetries ?? RETRY_DEFAULTS.maxRetries; const backoffMs = options.retryPolicy.backoffMs ?? RETRY_DEFAULTS.backoffMs; const multiplier = options.retryPolicy.backoffMultiplier ?? RETRY_DEFAULTS.backoffMultiplier; + const maxRetryDelayMs = options.retryPolicy.maxRetryDelayMs ?? RETRY_DEFAULTS.maxRetryDelayMs; + const jitter = options.retryPolicy.jitter ?? RETRY_DEFAULTS.jitter; let lastError: unknown; for (let attempt = 0; attempt <= maxRetries; attempt++) { if (attempt > 0) { - await sleep(backoffMs * Math.pow(multiplier, attempt - 1)); + // Same formula the try_catch executor runs — one policy, one backoff. + let delay = Math.min(backoffMs * Math.pow(multiplier, attempt - 1), maxRetryDelayMs); + if (jitter) delay = delay * (0.5 + Math.random() * 0.5); + await sleep(delay); } try { await withTimeout(run, jobId, timeoutMs); diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index c92efda61b..0c18e5f85e 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -1190,6 +1190,7 @@ "ResolverDoc (interface)", "ResultDialogLike (interface)", "RetryPolicy (type)", + "RetryPolicyParsed (type)", "RetryPolicySchema (const)", "RollbackPlan (type)", "RollbackPlanSchema (const)", @@ -2253,6 +2254,7 @@ "RegionAnalysis (interface)", "ResolvedFlowNodeExpression (interface)", "RetryPolicy (type)", + "RetryPolicyParsed (type)", "RetryPolicySchema (const)", "SCHEMALESS_NODE_CONFIG_SCHEMAS (const)", "ScheduleState (type)", diff --git a/packages/spec/authorable-surface.json b/packages/spec/authorable-surface.json index a886cbf3cb..ba2d18ae90 100644 --- a/packages/spec/authorable-surface.json +++ b/packages/spec/authorable-surface.json @@ -1,5 +1,5 @@ { - "description": "Ratchet of every AUTHORABLE key in the spec \u2014 what a metadata author may write, which for this platform IS the third-party API. Auto-updated on additions (commit the change). A key that disappears without a tombstone fails gen:schema, because these schemas are not .strict() and Zod would silently strip it. \"[RETIRED]\" marks a tombstoned key that still rejects with an upgrade prescription. See #3855, ADR-0059 \u00a75.", + "description": "Ratchet of every AUTHORABLE key in the spec — what a metadata author may write, which for this platform IS the third-party API. Auto-updated on additions (commit the change). A key that disappears without a tombstone fails gen:schema, because these schemas are not .strict() and Zod would silently strip it. \"[RETIRED]\" marks a tombstoned key that still rejects with an upgrade prescription. See #3855, ADR-0059 §5.", "keys": [ "ai/AIModelConfig:maxTokens", "ai/AIModelConfig:model", @@ -2436,11 +2436,12 @@ "automation/ParallelBranch:name", "automation/ParallelBranch:nodes", "automation/ParallelConfig:branches", + "automation/RetryPolicy:backoffMs", "automation/RetryPolicy:backoffMultiplier", "automation/RetryPolicy:jitter", "automation/RetryPolicy:maxRetries", "automation/RetryPolicy:maxRetryDelayMs", - "automation/RetryPolicy:retryDelayMs", + "automation/RetryPolicy:retryDelayMs [RETIRED]", "automation/ScheduleState:consecutiveFailures", "automation/ScheduleState:createdAt", "automation/ScheduleState:createdBy", @@ -6617,7 +6618,10 @@ "system/ResolvedSettingValue:value", "system/RetryPolicy:backoffMs", "system/RetryPolicy:backoffMultiplier", + "system/RetryPolicy:jitter", "system/RetryPolicy:maxRetries", + "system/RetryPolicy:maxRetryDelayMs", + "system/RetryPolicy:retryDelayMs [RETIRED]", "system/RollbackPlan:description", "system/RollbackPlan:steps", "system/RollbackPlan:testProcedure", diff --git a/packages/spec/dual-source-exports.baseline.json b/packages/spec/dual-source-exports.baseline.json index 06bf9369b3..4a47e51746 100644 --- a/packages/spec/dual-source-exports.baseline.json +++ b/packages/spec/dual-source-exports.baseline.json @@ -19,8 +19,6 @@ "PackageDependencySchema — [./cloud (const)] ≠ [./kernel (const)]", "RateLimitConfig — [./integration (type)] ≠ [./shared (type)]", "RateLimitConfigSchema — [./integration (const)] ≠ [./shared (const)]", - "RetryPolicy — [./automation (type)] ≠ [./system (type)]", - "RetryPolicySchema — [./automation (const)] ≠ [./system (const)]", "TenantPlan — [./cloud (type)] ≠ [./system (type)]", "TenantPlanSchema — [./cloud (const)] ≠ [./system (const)]" ] diff --git a/packages/spec/spec-changes.json b/packages/spec/spec-changes.json index 35d3b15f2d..54e24f7bf2 100644 --- a/packages/spec/spec-changes.json +++ b/packages/spec/spec-changes.json @@ -235,6 +235,12 @@ "to": "script flow-node config keys 'actionType' (→ 'function' when it was shorthand for one; otherwise removed — 'email'/'slack' were logger-backed stubs that delivered nothing), plus 'template' / 'recipients' / 'variables' (fed those stubs) and 'script' (inline JS the runtime never executed) (#4343)", "conversionId": "flow-node-script-branch-keys-removed", "toMajor": 17 + }, + { + "surface": "flow.node.config.retry.retryDelayMs / job.retryPolicy.maxRetries / job.retryPolicy.backoffMultiplier", + "to": "retry policy unified across job.retryPolicy and try_catch retry: base delay 'retryDelayMs' → 'backoffMs', and the pre-17 job defaults (maxRetries 3, backoffMultiplier 2) written out explicitly now that the merged default is 0 / 1 (#4661)", + "conversionId": "retry-policy-converged", + "toMajor": 17 } ], "migrated": [ @@ -308,6 +314,13 @@ "toMajor": 16, "rationale": "The `.strict()` flip turns a previously silently-stripped unknown key into a parse error. There is no mapping target for an arbitrary unknown key — auto-deleting it would be exactly the silent data loss ADR-0078 bans — so each occurrence needs the author to decide: bind a `dataset` and select `dimensions`/`values`, move a renderer setting under `options`, or delete the dead key." }, + { + "surface": "job.retryPolicy.maxRetries (> 10) / job.retryPolicy.backoffMultiplier (< 1)", + "replacement": "maxRetries <= 10, and backoffMultiplier >= 1", + "migrationId": "job-retry-policy-constraints-tightened", + "toMajor": 17, + "rationale": "The converged RetryPolicy (#4661) keeps the automation side's bounds, which the job side never had: `maxRetries` is capped at 10 and `backoffMultiplier` floored at 1. Neither has a lossless rewrite. Clamping `maxRetries: 20` to 10 would halve a retry budget its author chose, and a `backoffMultiplier` below 1 describes a delay that SHRINKS on each attempt — retrying a failing dependency ever faster, which is the opposite of backoff and was never a shape the engine meant to offer. Both now fail at parse time with the bound named, rather than being silently reinterpreted. Choosing the replacement count (or accepting the cap) is the author's call." + }, { "surface": "flow.errorHandling.maxRetries (under strategy: 'retry')", "replacement": "an explicit count >= 1 (e.g. maxRetries: 3), or strategy: 'fail'", @@ -755,9 +768,22 @@ "to": "script flow-node config keys 'actionType' (→ 'function' when it was shorthand for one; otherwise removed — 'email'/'slack' were logger-backed stubs that delivered nothing), plus 'template' / 'recipients' / 'variables' (fed those stubs) and 'script' (inline JS the runtime never executed) (#4343)", "conversionId": "flow-node-script-branch-keys-removed", "toMajor": 17 + }, + { + "surface": "flow.node.config.retry.retryDelayMs / job.retryPolicy.maxRetries / job.retryPolicy.backoffMultiplier", + "to": "retry policy unified across job.retryPolicy and try_catch retry: base delay 'retryDelayMs' → 'backoffMs', and the pre-17 job defaults (maxRetries 3, backoffMultiplier 2) written out explicitly now that the merged default is 0 / 1 (#4661)", + "conversionId": "retry-policy-converged", + "toMajor": 17 } ], "migrated": [ + { + "surface": "job.retryPolicy.maxRetries (> 10) / job.retryPolicy.backoffMultiplier (< 1)", + "replacement": "maxRetries <= 10, and backoffMultiplier >= 1", + "migrationId": "job-retry-policy-constraints-tightened", + "toMajor": 17, + "rationale": "The converged RetryPolicy (#4661) keeps the automation side's bounds, which the job side never had: `maxRetries` is capped at 10 and `backoffMultiplier` floored at 1. Neither has a lossless rewrite. Clamping `maxRetries: 20` to 10 would halve a retry budget its author chose, and a `backoffMultiplier` below 1 describes a delay that SHRINKS on each attempt — retrying a failing dependency ever faster, which is the opposite of backoff and was never a shape the engine meant to offer. Both now fail at parse time with the bound named, rather than being silently reinterpreted. Choosing the replacement count (or accepting the cap) is the author's call." + }, { "surface": "flow.errorHandling.maxRetries (under strategy: 'retry')", "replacement": "an explicit count >= 1 (e.g. maxRetries: 3), or strategy: 'fail'", diff --git a/packages/spec/src/automation/control-flow.test.ts b/packages/spec/src/automation/control-flow.test.ts index 5485ab671e..985c0eeabe 100644 --- a/packages/spec/src/automation/control-flow.test.ts +++ b/packages/spec/src/automation/control-flow.test.ts @@ -104,10 +104,37 @@ describe('TryCatchConfigSchema', () => { const parsed = TryCatchConfigSchema.parse({ try: { nodes: [node('t')] }, catch: { nodes: [node('c')] }, - retry: { maxRetries: 3, retryDelayMs: 500 }, + retry: { maxRetries: 3, backoffMs: 500 }, }); expect(parsed.errorVariable).toBe('$error'); expect(parsed.retry?.maxRetries).toBe(3); + expect(parsed.retry?.backoffMs).toBe(500); + }); + + // #4661: `retryDelayMs` was the automation spelling of `backoffMs` before the + // retry policy converged onto one declaration. It is TOMBSTONED rather than + // deleted precisely because this schema is not `.strict()` — a plain removal + // would have Zod swallow the authored number and silently fall back to the + // 1000ms default. Assert the loud rejection, not merely its absence. + it('rejects the retired `retryDelayMs` spelling with the rename prescription', () => { + const parse = () => TryCatchConfigSchema.parse({ + try: { nodes: [node('t')] }, + retry: { maxRetries: 3, retryDelayMs: 500 }, + }); + expect(parse).toThrow(/backoffMs/); + expect(parse).toThrow(/retryDelayMs/); + }); + + it('retry is opt-in: a declared but empty retry block does not retry', () => { + const parsed = TryCatchConfigSchema.parse({ + try: { nodes: [node('t')] }, + retry: {}, + }); + expect(parsed.retry?.maxRetries).toBe(0); + expect(parsed.retry?.backoffMs).toBe(1000); + expect(parsed.retry?.backoffMultiplier).toBe(1); + expect(parsed.retry?.maxRetryDelayMs).toBe(30000); + expect(parsed.retry?.jitter).toBe(false); }); }); diff --git a/packages/spec/src/automation/control-flow.zod.ts b/packages/spec/src/automation/control-flow.zod.ts index 9a29ccfc7b..796aa43577 100644 --- a/packages/spec/src/automation/control-flow.zod.ts +++ b/packages/spec/src/automation/control-flow.zod.ts @@ -165,18 +165,22 @@ export type ParallelConfigParsed = z.infer; /** * Structured retry policy — surfaces the engine's existing exponential-backoff - * retry (`FlowSchema.errorHandling`) as a per-construct policy. Mirrors that - * shape so the engine can reuse one backoff implementation. + * retry (`FlowSchema.errorHandling`) as a per-construct policy. + * + * The declaration moved to `shared/retry-policy.zod.ts` in 17.0.0 (#4661): the + * identically-named `system/job.zod.ts` shape was the same concept under a + * different spelling, so `@objectstack/spec/automation` and + * `@objectstack/spec/system` handed out two different `RetryPolicy` types for + * one idea (the #4411 trap). One declaration now serves both entries. For THIS + * entry the visible change is the base delay: `retryDelayMs` → `backoffMs` + * (tombstoned, with a conversion), plus `maxRetries`/`backoffMultiplier` + * defaults that are unchanged here — 0 and 1 were already the automation values. + * + * Re-exported so `./automation` keeps publishing the name (and its + * `automation/RetryPolicy` JSON-Schema def, which is keyed by entry namespace). */ -export const RetryPolicySchema = lazySchema(() => z.object({ - maxRetries: z.number().int().min(0).max(10).default(0).describe('Retry attempts before giving up'), - retryDelayMs: z.number().int().min(0).default(1000).describe('Base delay between retries (ms)'), - backoffMultiplier: z.number().min(1).default(1).describe('Exponential backoff multiplier'), - maxRetryDelayMs: z.number().int().min(0).default(30000).describe('Maximum delay between retries (ms)'), - jitter: z.boolean().default(false).describe('Add random jitter to retry delay'), -})); - -export type RetryPolicy = z.input; +export { RetryPolicySchema, type RetryPolicy, type RetryPolicyParsed } from '../shared/retry-policy.zod'; +import { RetryPolicySchema } from '../shared/retry-policy.zod'; /** * `try_catch` config — structured error handling. The `try` region runs; if it diff --git a/packages/spec/src/contracts/job-service.ts b/packages/spec/src/contracts/job-service.ts index 10c4f1af87..876eb2e50d 100644 --- a/packages/spec/src/contracts/job-service.ts +++ b/packages/spec/src/contracts/job-service.ts @@ -50,16 +50,25 @@ export interface JobSchedule { export type JobHandler = (context: { jobId: string; data?: unknown }) => Promise; /** - * Retry policy for a scheduled job (mirrors the authorable - * `RetryPolicySchema` in system/job.zod.ts). + * Retry policy for a scheduled job (mirrors the authorable `RetryPolicySchema`, + * which since 17.0.0 is declared once in `shared/retry-policy.zod.ts` and + * re-exported by both `./automation` and `./system` — #4661). + * + * Defaults restated here because this is a hand-written boundary type: a caller + * that builds `JobScheduleOptions` itself never goes through Zod, so + * `runWithPolicy` applies the same values the schema declares. */ export interface JobRetryPolicy { - /** Maximum number of retry attempts after the initial run (default 3) */ + /** Retry attempts after the initial run. 0 (the default since 17.0.0, #4661) means no retry. */ maxRetries?: number; - /** Initial backoff delay in milliseconds (default 1000) */ + /** Base delay before the first retry, in milliseconds (default 1000) */ backoffMs?: number; - /** Multiplier for exponential backoff (default 2) */ + /** Multiplier for exponential backoff (default 1 since 17.0.0, #4661 — a flat delay) */ backoffMultiplier?: number; + /** Ceiling for a single backoff delay, in milliseconds (default 30000) */ + maxRetryDelayMs?: number; + /** Randomize each delay within [50%, 100%] of its computed value (default false) */ + jitter?: boolean; } /** diff --git a/packages/spec/src/conversions/registry.ts b/packages/spec/src/conversions/registry.ts index ca85bfcc00..7450e52247 100644 --- a/packages/spec/src/conversions/registry.ts +++ b/packages/spec/src/conversions/registry.ts @@ -2710,6 +2710,159 @@ const flowNodeScriptBranchKeysRemoved: MetadataConversion = { }, }; +/** + * The retry policy converges to one declaration (protocol 17, #4661 — the + * #4535 C8 dual-source cluster). + * + * `@objectstack/spec/automation` and `@objectstack/spec/system` both exported a + * `RetryPolicy` / `RetryPolicySchema`, resolving to DIFFERENT declarations — so + * which shape a consumer got depended only on the import path (#4411). They + * were never two concepts: `try_catch`'s `retry` region and `job.retryPolicy` + * both compute `delay = base * multiplier^(retry-1)`, and the two executors + * implemented that identical formula. What differed was cosmetic and + * accidental, and this conversion pays for both halves of the merge: + * + * 1. **The base delay had two spellings.** automation said `retryDelayMs`, + * system said `backoffMs`. `backoffMs` wins — it is what the *enforced* + * retry policies already spell it (`job.retryPolicy` and `hook.retryPolicy`; + * see the `datasource-inert-blocks-removed` note above, which leans on + * exactly that distinction), so the platform drops from three spellings to + * two rather than four. `retryDelayMs` is tombstoned (`retiredKey`) — NOT + * deleted — because neither owning shape is `.strict()`: a plain deletion + * would have Zod silently swallow the authored number and fall back to the + * 1000ms default, which is the quiet-failure class ADR-0049 removes. + * + * 2. **The defaults were opposite, and no gate can see a default.** Pre-17, + * `job.retryPolicy` defaulted `maxRetries: 3` / `backoffMultiplier: 2` + * while the automation shape defaulted 0 / 1. The merged declaration takes + * 0 / 1 (retry is opt-in: a retry replays whatever the attempt already did, + * and an implicit replay of side effects is the failure mode hardest to + * catch in tests — the same reading already recorded for flow-level retry + * in `flow-retry-max-retries-required`, #4247). Taken alone that would + * SILENTLY stop existing jobs from retrying, and the authorable-surface + * gate would never notice: it compares key sets, and a default is not a key. + * So this conversion writes the pre-17 numbers into every existing + * `job.retryPolicy` that omitted them. Deployed stacks keep their exact + * behaviour; only a newly authored omission means "no retry". + * + * Jobs with no `retryPolicy` block at all are left alone — absence already + * meant a single attempt on both sides of the change. + * + * `retiredFromLoadPath` is NOT set: `FlowNodeSchema.config` is an unconstrained + * record, so no schema rejection can reach `config.retry.retryDelayMs` and the + * conversion layer is the only seam that can declare and retire that spelling. + * The `RetryPolicySchema` tombstone still fires for anyone who reaches the + * policy through a parsed job. + */ +const retryPolicyConverged: MetadataConversion = { + id: 'retry-policy-converged', + toMajor: 17, + surface: 'flow.node.config.retry.retryDelayMs / job.retryPolicy.maxRetries / job.retryPolicy.backoffMultiplier', + summary: + "retry policy unified across job.retryPolicy and try_catch retry: base delay 'retryDelayMs' → 'backoffMs', " + + "and the pre-17 job defaults (maxRetries 3, backoffMultiplier 2) written out explicitly now that the merged default is 0 / 1 (#4661)", + apply(stack, emit) { + // ── 1. try_catch nodes: retry.retryDelayMs → retry.backoffMs ────── + const withFlows = mapFlowNodes(stack, (node, path) => { + if (node.type !== 'try_catch') return node; + const config = node.config; + if (!config || typeof config !== 'object' || Array.isArray(config)) return node; + const configDict = config as Record; + const retry = configDict.retry; + if (!retry || typeof retry !== 'object' || Array.isArray(retry)) return node; + const renamed = renameKey(retry as Record, 'retryDelayMs', 'backoffMs'); + if (renamed === null) return node; + emit({ from: 'retryDelayMs', to: 'backoffMs', path: `${path}.config.retry.backoffMs` }); + return { ...node, config: { ...configDict, retry: renamed } }; + }); + + // ── 2. jobs: materialize the pre-17 implicit defaults ───────────── + return mapCollection(withFlows, 'jobs', (job, path) => { + const policy = job.retryPolicy; + if (!policy || typeof policy !== 'object' || Array.isArray(policy)) return job; + const policyDict = policy as Record; + let next = policyDict; + if (next.maxRetries === undefined) { + next = { ...next, maxRetries: 3 }; + emit({ + from: 'maxRetries unset (implied 3)', + to: 'maxRetries: 3', + path: `${path}.retryPolicy.maxRetries`, + }); + } + if (next.backoffMultiplier === undefined) { + next = { ...next, backoffMultiplier: 2 }; + emit({ + from: 'backoffMultiplier unset (implied 2)', + to: 'backoffMultiplier: 2', + path: `${path}.retryPolicy.backoffMultiplier`, + }); + } + return next === policyDict ? job : { ...job, retryPolicy: next }; + }); + }, + fixture: { + before: { + flows: [{ + name: 'sync_orders', + nodes: [ + { id: 'n1', type: 'start' }, + { + id: 'n2', + type: 'try_catch', + config: { + try: { nodes: [], edges: [] }, + retry: { maxRetries: 3, retryDelayMs: 500, jitter: true }, + }, + }, + // Already canonical — left alone, contributes no notice. + { + id: 'n3', + type: 'try_catch', + config: { try: { nodes: [], edges: [] }, retry: { maxRetries: 2, backoffMs: 250 } }, + }, + ], + }], + jobs: [ + // Omits both defaults — both get written out. + { name: 'nightly_sync', schedule: { type: 'cron', expression: '0 0 * * *' }, handler: 'jobs.ts:sync', retryPolicy: { backoffMs: 5000 } }, + // States both — untouched. + { name: 'hourly_roll', schedule: { type: 'cron', expression: '0 * * * *' }, handler: 'jobs.ts:roll', retryPolicy: { maxRetries: 1, backoffMultiplier: 3 } }, + // No policy block at all — absence already meant one attempt. + { name: 'weekly_purge', schedule: { type: 'cron', expression: '0 0 * * 0' }, handler: 'jobs.ts:purge' }, + ], + }, + after: { + flows: [{ + name: 'sync_orders', + nodes: [ + { id: 'n1', type: 'start' }, + { + id: 'n2', + type: 'try_catch', + config: { + try: { nodes: [], edges: [] }, + retry: { maxRetries: 3, jitter: true, backoffMs: 500 }, + }, + }, + { + id: 'n3', + type: 'try_catch', + config: { try: { nodes: [], edges: [] }, retry: { maxRetries: 2, backoffMs: 250 } }, + }, + ], + }], + jobs: [ + { name: 'nightly_sync', schedule: { type: 'cron', expression: '0 0 * * *' }, handler: 'jobs.ts:sync', retryPolicy: { backoffMs: 5000, maxRetries: 3, backoffMultiplier: 2 } }, + { name: 'hourly_roll', schedule: { type: 'cron', expression: '0 * * * *' }, handler: 'jobs.ts:roll', retryPolicy: { maxRetries: 1, backoffMultiplier: 3 } }, + { name: 'weekly_purge', schedule: { type: 'cron', expression: '0 0 * * 0' }, handler: 'jobs.ts:purge' }, + ], + }, + // n2's rename, plus nightly_sync's two materialized defaults. + expectedNotices: 3, + }, +}; + export const CONVERSIONS_BY_MAJOR: Readonly> = { 11: [flowNodeHttpRename, pageKindJsxToHtml, flowNodeFilterAlias, objectCompactLayoutRename], 13: [stackRolesToPositions, owdLegacyReadAliases, sharingRecipientRoleToPosition], @@ -2746,6 +2899,7 @@ export const CONVERSIONS_BY_MAJOR: Readonly 10) / job.retryPolicy.backoffMultiplier (< 1)', + replacement: 'maxRetries <= 10, and backoffMultiplier >= 1', + reason: + 'The converged RetryPolicy (#4661) keeps the automation side\'s bounds, which the job ' + + 'side never had: `maxRetries` is capped at 10 and `backoffMultiplier` floored at 1. ' + + 'Neither has a lossless rewrite. Clamping `maxRetries: 20` to 10 would halve a ' + + 'retry budget its author chose, and a `backoffMultiplier` below 1 describes a delay ' + + 'that SHRINKS on each attempt — retrying a failing dependency ever faster, which is ' + + 'the opposite of backoff and was never a shape the engine meant to offer. Both now ' + + 'fail at parse time with the bound named, rather than being silently reinterpreted. ' + + 'Choosing the replacement count (or accepting the cap) is the author\'s call.', + acceptanceCriteria: + 'Every job declaring `retryPolicy` parses: no `maxRetries` above 10 and no ' + + '`backoffMultiplier` below 1 remain, and each adjusted value was re-chosen knowing a ' + + 'retry re-runs the handler with its writes and callouts. No job fails to register ' + + 'with the retry-policy bound prescription.', + }, { id: 'flow-retry-max-retries-required', surface: "flow.errorHandling.maxRetries (under strategy: 'retry')", diff --git a/packages/spec/src/shared/retry-policy.test.ts b/packages/spec/src/shared/retry-policy.test.ts new file mode 100644 index 0000000000..7318f093a3 --- /dev/null +++ b/packages/spec/src/shared/retry-policy.test.ts @@ -0,0 +1,104 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The retry policy is ONE declaration (#4661 — the #4535 C8 dual-source cluster). + * + * `@objectstack/spec/automation` and `@objectstack/spec/system` both publish + * `RetryPolicySchema` / `RetryPolicy`. Until 17 those names resolved to two + * different declarations, so the shape a consumer got depended only on which + * entry they imported (the #4411 trap) — and the two disagreed on the base + * delay's spelling (`retryDelayMs` vs `backoffMs`), on two keys only one side + * had, and on the defaults for `maxRetries` / `backoffMultiplier`. + * + * ## Why these assertions run at RUNTIME + * + * A compile-time pin cannot fail in this package (#4642): `tsconfig.json` + * excludes `**\/*.test.ts` so `pnpm typecheck` never compiles this file, and + * vitest transpiles without typechecking — a conditional-type assertion here + * would be inert. Worse, `keyof typeof import(...)` enumerates only VALUE + * exports, so a bare type name cannot be asserted that way at all. These are + * reference-identity and shape checks on the loaded module namespaces, which + * actually execute. + * + * The reference-identity check is the load-bearing one: `check:dual-source-exports` + * judges by symbol identity, so `Automation.RetryPolicySchema === + * System.RetryPolicySchema` is exactly the invariant whose loss would put the + * two baseline rows back. + */ + +import { describe, it, expect } from 'vitest'; +import * as Automation from '../automation'; +import * as System from '../system'; +import { RetryPolicySchema } from './retry-policy.zod'; + +describe('RetryPolicy is a single declaration across entries (#4661)', () => { + it('./automation and ./system export the SAME RetryPolicySchema object', () => { + expect(Automation.RetryPolicySchema).toBe(System.RetryPolicySchema); + }); + + it('both entries re-export the canonical shared declaration', () => { + expect(Automation.RetryPolicySchema).toBe(RetryPolicySchema); + expect(System.RetryPolicySchema).toBe(RetryPolicySchema); + }); + + // Identity alone would still pass if someone converged the two onto a NEW + // shape that quietly dropped a key. Pin the authorable key set too — this is + // the same list `authorable-surface.json` carries for both def keys. + it('publishes exactly the converged key set from both entries', () => { + const expected = ['maxRetries', 'backoffMs', 'backoffMultiplier', 'maxRetryDelayMs', 'jitter']; + + for (const entry of [Automation.RetryPolicySchema, System.RetryPolicySchema]) { + const parsed = entry.parse({}); + expect(Object.keys(parsed).sort()).toEqual([...expected].sort()); + } + }); + + // The defaults are the half of this convergence NO gate can see: the + // authorable-surface ratchet compares key sets, and a default is not a key. + // If they are ever flipped back, deployed jobs silently start retrying again + // (or try_catch regions silently start replaying their side effects), so pin + // the numbers explicitly. + it('pins the opt-in defaults that no gate can observe', () => { + const parsed = RetryPolicySchema.parse({}); + + expect(parsed.maxRetries).toBe(0); + expect(parsed.backoffMs).toBe(1000); + expect(parsed.backoffMultiplier).toBe(1); + expect(parsed.maxRetryDelayMs).toBe(30000); + expect(parsed.jitter).toBe(false); + }); + + it('keeps the `retryDelayMs` tombstone loud from both entries', () => { + for (const entry of [Automation.RetryPolicySchema, System.RetryPolicySchema]) { + const parse = () => entry.parse({ retryDelayMs: 500 }); + expect(parse).toThrow(/backoffMs/); + } + }); +}); + +describe('RetryPolicySchema — converged shape', () => { + it('computes a bounded exponential backoff', () => { + const policy = RetryPolicySchema.parse({ + maxRetries: 5, + backoffMs: 1000, + backoffMultiplier: 2, + maxRetryDelayMs: 5000, + }); + + // The formula both executors run: min(base * multiplier^(n-1), ceiling). + const delays = [1, 2, 3, 4, 5].map((n) => + Math.min(policy.backoffMs * Math.pow(policy.backoffMultiplier, n - 1), policy.maxRetryDelayMs), + ); + + expect(delays).toEqual([1000, 2000, 4000, 5000, 5000]); + }); + + it('rejects a shrinking backoff (multiplier below 1)', () => { + expect(() => RetryPolicySchema.parse({ backoffMultiplier: 0.5 })).toThrow(); + }); + + it('caps maxRetries at 10', () => { + expect(() => RetryPolicySchema.parse({ maxRetries: 10 })).not.toThrow(); + expect(() => RetryPolicySchema.parse({ maxRetries: 11 })).toThrow(); + }); +}); diff --git a/packages/spec/src/shared/retry-policy.zod.ts b/packages/spec/src/shared/retry-policy.zod.ts new file mode 100644 index 0000000000..dfb7cfe842 --- /dev/null +++ b/packages/spec/src/shared/retry-policy.zod.ts @@ -0,0 +1,110 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * @module shared/retry-policy + * + * The **single declaration** of the exponential-backoff retry policy (#4661, + * the #4535 C8 dual-source cluster). + * + * Until 17 this shape existed twice — `automation/control-flow.zod.ts` (the + * `try_catch` node's `retry` region) and `system/job.zod.ts` (`job.retryPolicy`) + * — under the *same exported name*, so which `RetryPolicy` a consumer got + * depended only on whether they imported `@objectstack/spec/automation` or + * `@objectstack/spec/system` (the #4411 trap). They were not two concepts: + * both drive `delay = base * multiplier^(retry-1)`, and both executors + * implemented that identical formula. What differed was the *spelling* of the + * base delay (`retryDelayMs` vs `backoffMs`), two keys only one side had + * (`maxRetryDelayMs` / `jitter`), and the defaults. + * + * ## Why this file, and why it is not in `shared/index.ts` + * + * The published JSON-Schema def key is `/`, derived from + * which entry barrel re-exports the const (`scripts/build-schemas.ts` iterates + * the namespace objects). Keeping ONE declaration re-exported from both + * `./automation` and `./system` therefore preserves *both* def keys + * (`automation/RetryPolicy` and `system/RetryPolicy`) with an identical key set + * — which is exactly what makes this convergence cost a single authorable key + * instead of eight. + * + * It is deliberately NOT added to `shared/index.ts`: exporting it from a third + * entry would publish a third `shared/RetryPolicy` def and add five more rows to + * `authorable-surface.json` for a def no author ever writes directly (#4535 §1 + * already flags that file for over-collecting). Same reason `retired-key.ts`, + * `strict-object.ts` and `connector-auth.zod.ts` sit here without a barrel line. + * + * The home is `shared/` rather than either domain because the two owning + * schemas must not depend on each other: `automation/control-flow.zod.ts` + * imports the whole `flow.zod` node/edge graph, and pulling that into + * `system/job.zod.ts` to reach a five-field policy would be a real edge in the + * package graph for no runtime need. This module depends on nothing but `zod` + * and `lazySchema`. + */ + +import { z } from 'zod'; +import { lazySchema } from './lazy-schema'; +import { retiredKey } from './retired-key'; + +/** + * Exponential-backoff retry policy — the one shape for both `job.retryPolicy` + * and a `try_catch` node's `retry` region. + * + * Delay before retry *n* is `min(backoffMs * backoffMultiplier^(n-1), + * maxRetryDelayMs)`, optionally jittered. + * + * ## Defaults are opt-in, not opt-out (17.0.0, #4661) + * + * `maxRetries` defaults to **0** — declaring a retry block does not by itself + * buy retries. The pre-17 `job.retryPolicy` defaulted to 3, so a job that wrote + * `{ backoffMs: 5000 }` and nothing else silently got three attempts; the + * `retry-policy-converged` conversion writes that `3` (and the old + * `backoffMultiplier: 2`) into existing job documents, so no deployed stack + * changes behaviour. What changes is what a NEWLY authored omission means. + * + * The reason to make absence mean "no retry" rather than "retry three times": + * a retry replays whatever the attempt already did — a job handler's writes and + * callouts, a `try` region's side effects. An implicit retry is the failure mode + * that is hardest to catch in tests and most expensive in production, and + * metadata written by an LLM is exactly where an unstated key hides. The same + * reading is already recorded for flow-level retry in the protocol-17 migration + * step (`flow-retry-max-retries-required`, #4247): an unstated count is + * unambiguously 0, and "retry zero times" is a decision the author must state. + */ +export const RetryPolicySchema = lazySchema(() => z.object({ + maxRetries: z.number().int().min(0).max(10).default(0) + .describe('Retry attempts after the initial one. 0 (the default) means no retry — state a count to opt in.'), + backoffMs: z.number().int().min(0).default(1000) + .describe('Base delay before the first retry (ms); subsequent delays multiply by backoffMultiplier'), + backoffMultiplier: z.number().min(1).default(1) + .describe('Exponential backoff multiplier; 1 (the default) keeps the delay flat'), + maxRetryDelayMs: z.number().int().min(0).default(30000) + .describe('Ceiling for a single backoff delay (ms)'), + jitter: z.boolean().default(false) + .describe('Randomize each delay within [50%, 100%] of its computed value — spreads a thundering herd of simultaneous retries'), + + // ── Tombstone (ADR-0087) ──────────────────────────────────────────── + // `retryDelayMs` was the automation-side spelling of `backoffMs`. It is the + // ONE authorable key this convergence costs, and it is tombstoned rather than + // deleted because neither owning shape is `.strict()`: a plain deletion would + // have Zod silently strip the authored value and drop the delay back to the + // 1000ms default, which is precisely the quiet-failure class ADR-0049 exists + // to remove. `retry-policy-converged` rewrites the key. + retryDelayMs: retiredKey( + '`retryDelayMs` was removed in @objectstack/spec 17.0.0 (#4661) — the retry policy now ' + + 'has one spelling for its base delay across `job.retryPolicy` and a `try_catch` node\'s ' + + '`retry`. Rename the key to `backoffMs`; the value (milliseconds before the first retry) ' + + 'is unchanged. `os migrate meta --from 16` rewrites it for you.', + ), +})); + +/** + * What an author writes — every key optional, defaults unapplied. + * + * Note for pre-17 `@objectstack/spec/system` consumers: this used to be + * `z.infer` (the post-parse shape, every key present) on that entry only. It is + * now the input shape on both, matching the house `X` / `XParsed` convention + * used by the sibling control-flow configs. + */ +export type RetryPolicy = z.input; + +/** The post-parse shape — every key present, defaults applied. */ +export type RetryPolicyParsed = z.infer; diff --git a/packages/spec/src/system/job.test.ts b/packages/spec/src/system/job.test.ts index 215a1fbd91..3c321c36b6 100644 --- a/packages/spec/src/system/job.test.ts +++ b/packages/spec/src/system/job.test.ts @@ -184,12 +184,45 @@ describe('RetryPolicySchema', () => { expect(() => RetryPolicySchema.parse(policy)).not.toThrow(); }); - it('should apply default values', () => { + // #4661: retry is opt-in since 17.0.0. `maxRetries` defaulted to 3 and + // `backoffMultiplier` to 2 while this shape was job-only; the converged + // declaration takes the automation side's 0 / 1, because a retry replays the + // handler's writes and callouts and that has to be asked for. Existing job + // documents keep the old numbers — `retry-policy-converged` writes them in — + // so what changed is only what a NEWLY authored omission means. + it('applies opt-in defaults: a declared but empty policy does not retry', () => { const policy = RetryPolicySchema.parse({}); - expect(policy.maxRetries).toBe(3); + expect(policy.maxRetries).toBe(0); expect(policy.backoffMs).toBe(1000); - expect(policy.backoffMultiplier).toBe(2); + expect(policy.backoffMultiplier).toBe(1); + expect(policy.maxRetryDelayMs).toBe(30000); + expect(policy.jitter).toBe(false); + }); + + // The two keys the convergence brought over from the automation side. They + // are declared here only because `runWithPolicy` actually honours them + // (ADR-0049: declared IS enforced). + it('accepts the maxRetryDelayMs ceiling and jitter', () => { + const policy = RetryPolicySchema.parse({ maxRetries: 5, maxRetryDelayMs: 60000, jitter: true }); + + expect(policy.maxRetryDelayMs).toBe(60000); + expect(policy.jitter).toBe(true); + }); + + it('rejects the retired `retryDelayMs` spelling with the rename prescription', () => { + const parse = () => RetryPolicySchema.parse({ retryDelayMs: 500 }); + + expect(parse).toThrow(/backoffMs/); + expect(parse).toThrow(/retryDelayMs/); + }); + + // Bounds the job side did not have before the merge. Both fail loudly rather + // than being silently reinterpreted — see the + // `job-retry-policy-constraints-tightened` semantic migration note. + it('enforces the converged bounds (maxRetries <= 10, backoffMultiplier >= 1)', () => { + expect(() => RetryPolicySchema.parse({ maxRetries: 20 })).toThrow(); + expect(() => RetryPolicySchema.parse({ backoffMultiplier: 0.5 })).toThrow(); }); it('should accept zero retries', () => { diff --git a/packages/spec/src/system/job.zod.ts b/packages/spec/src/system/job.zod.ts index 02dc744828..f1ebf20fb0 100644 --- a/packages/spec/src/system/job.zod.ts +++ b/packages/spec/src/system/job.zod.ts @@ -56,16 +56,31 @@ export type OnceSchedule = z.infer; // name: `Schedule`. /** - * Retry Policy Schema - * Configuration for job retry behavior with exponential backoff + * Retry Policy Schema — job retry behaviour with exponential backoff. + * + * The declaration moved to `shared/retry-policy.zod.ts` in 17.0.0 (#4661). + * `@objectstack/spec/automation` exported an identically-named, differently + * shaped `RetryPolicy` for the `try_catch` node's `retry` region, so which type + * a consumer got depended only on the import path (the #4411 trap) — and they + * were never two concepts: both compute + * `delay = base * multiplier^(retry-1)`. Re-exported so `./system` keeps + * publishing the name (and its `system/RetryPolicy` def, keyed by entry + * namespace). + * + * Three things change for job authors, all covered by the + * `retry-policy-converged` conversion: + * + * - `maxRetryDelayMs` and `jitter` are now available here (they were + * automation-only). Both are honoured by `runWithPolicy`. + * - `maxRetries` now defaults to **0**, not 3, and `backoffMultiplier` to 1, + * not 2 — the conversion writes the old values into existing documents, so + * no deployed job changes behaviour; only a NEWLY authored omission means + * "no retry". + * - `maxRetries` is capped at 10 and `backoffMultiplier` floored at 1 (the + * automation constraints). Both reject loudly at parse time. */ -export const RetryPolicySchema = lazySchema(() => z.object({ - maxRetries: z.number().int().min(0).default(3).describe('Maximum number of retry attempts'), - backoffMs: z.number().int().positive().default(1000).describe('Initial backoff delay in milliseconds'), - backoffMultiplier: z.number().positive().default(2).describe('Multiplier for exponential backoff'), -})); - -export type RetryPolicy = z.infer; +export { RetryPolicySchema, type RetryPolicy, type RetryPolicyParsed } from '../shared/retry-policy.zod'; +import { RetryPolicySchema } from '../shared/retry-policy.zod'; /** * Job Schema @@ -99,7 +114,7 @@ export const JobSchema = lazySchema(() => strictObject({ description: z.string().optional().describe('Job description / purpose'), schedule: ScheduleSchema.describe('Job schedule configuration'), handler: z.string().describe('Handler function name (must match a key in `defineStack({ functions })`)'), - retryPolicy: RetryPolicySchema.optional().describe('Retry policy: failed runs (including timeouts) are retried with exponential backoff (delay = backoffMs * backoffMultiplier^(retry-1)) up to maxRetries retries after the initial attempt (#3494). Omit for the legacy single-attempt behavior.'), + retryPolicy: RetryPolicySchema.optional().describe('Retry policy: failed runs (including timeouts) are retried with exponential backoff (delay = min(backoffMs * backoffMultiplier^(retry-1), maxRetryDelayMs), optionally jittered) up to maxRetries retries after the initial attempt (#3494). Omit the block for a single attempt; declaring it without `maxRetries` also means no retry since 17.0.0 (#4661) — state a count to opt in.'), timeout: z.number().int().positive().optional().describe('Per-attempt time limit in milliseconds; an over-limit run is recorded with execution status "timeout" (#3494). The in-flight handler is abandoned, not forcibly cancelled. Omit for no time limit.'), enabled: z.boolean().default(true).describe('Whether the job is enabled'), From d67691a138f1ef7b985b6c601ecd9102a281c2e4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 15:55:20 +0000 Subject: [PATCH 2/3] docs(changeset): clarify mapping.errorPolicy is unrelated to the retry convergence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #4664 retires `mapping.errorPolicy` in the same 17.0.0 window and its values included 'retry', so a reader could plausibly assume it migrates into a `retryPolicy` block. It does not — different type, inert enum, and its own prescription points at the import REQUEST's options. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01M9uWvoEp9CoLzYjNExj9sL --- .changeset/retry-policy-dual-source-c8.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.changeset/retry-policy-dual-source-c8.md b/.changeset/retry-policy-dual-source-c8.md index b93535a919..0711e442d6 100644 --- a/.changeset/retry-policy-dual-source-c8.md +++ b/.changeset/retry-policy-dual-source-c8.md @@ -68,6 +68,14 @@ ever faster, the opposite of backoff. shape** (every key optional) rather than the post-parse shape. Use the new `RetryPolicyParsed` where you need defaults applied. +## Not related to `mapping.errorPolicy` (#4509 / #4664, same release) + +17.0.0 also retires `mapping.errorPolicy`, whose values included `'retry'`. That is a +different thing on a different type: an inert enum on the stored **mapping**, whose +prescription is "error handling on the import path belongs to the import REQUEST's own +options". It does **not** migrate to a `retryPolicy` block, and nothing in this change +affects it. + ## What you gain `job.retryPolicy` accepts **`maxRetryDelayMs`** (ceiling on a single backoff delay) and From fe2bfd160ffa8f1e753932be8ca4aa10134999a0 Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Sun, 2 Aug 2026 16:30:07 +0000 Subject: [PATCH 3/3] fix(spec): regenerate spec-changes.json after the main merge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 上一个合并提交用集合合并解决 spec-changes.json 的冲突,但该文件是 对象数组而非字符串数组,集合合并退化成「取变了的那一侧」,丢掉了 main 侧 #3355 的 object-managed-by-system-to-system-data 条目, check:spec-changes 因此报 stale。 改为真的跑 gen:spec-changes(+ gen:upgrade-guide 确认无漂移)。 现三条条目齐备:retry-policy-converged、 object-managed-by-system-to-system-data、 job-retry-policy-constraints-tightened。 check:spec-changes / check:upgrade-guide / check:skill-docs 均绿。 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01M9uWvoEp9CoLzYjNExj9sL --- packages/spec/spec-changes.json | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/spec/spec-changes.json b/packages/spec/spec-changes.json index de788482f2..f1704f966d 100644 --- a/packages/spec/spec-changes.json +++ b/packages/spec/spec-changes.json @@ -247,6 +247,12 @@ "to": "retry policy unified across job.retryPolicy and try_catch retry: base delay 'retryDelayMs' → 'backoffMs', and the pre-17 job defaults (maxRetries 3, backoffMultiplier 2) written out explicitly now that the merged default is 0 / 1 (#4661)", "conversionId": "retry-policy-converged", "toMajor": 17 + }, + { + "surface": "object.managedBy", + "to": "object managedBy 'system' → 'system-data' (#3355 — ADR-0103's residual bucket named the engine-owned half v16 had already moved out to `engine-owned`; the rename leaves the name describing what the bucket actually holds: admin/user-writable platform data)", + "conversionId": "object-managed-by-system-to-system-data", + "toMajor": 17 } ], "migrated": [ @@ -786,6 +792,12 @@ "to": "retry policy unified across job.retryPolicy and try_catch retry: base delay 'retryDelayMs' → 'backoffMs', and the pre-17 job defaults (maxRetries 3, backoffMultiplier 2) written out explicitly now that the merged default is 0 / 1 (#4661)", "conversionId": "retry-policy-converged", "toMajor": 17 + }, + { + "surface": "object.managedBy", + "to": "object managedBy 'system' → 'system-data' (#3355 — ADR-0103's residual bucket named the engine-owned half v16 had already moved out to `engine-owned`; the rename leaves the name describing what the bucket actually holds: admin/user-writable platform data)", + "conversionId": "object-managed-by-system-to-system-data", + "toMajor": 17 } ], "migrated": [