Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions .changeset/retry-policy-etl-tombstone-enumeration.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
---
"@objectstack/spec": patch
---

fix(spec): the `retryDelayMs` tombstone no longer points an upgrading author at the retired ETL surface (#6630)

#6414 retired the whole L2 ETL layer (`automation/etl.zod.ts`: `ETLPipeline`,
`ETLPipelineRun`, their source/destination/transformation vocabulary, four enums
and the `ETL` factory — 9 defs, 27 exported names). `shared/retry-policy.zod.ts`
was updated in exactly one stanza and left every other mention of
`ETLPipeline.retry` in the present tense — including the one string that is not
documentation.

**The author-visible half.** `retryDelayMs`'s `retiredKey()` guidance is the
upgrade channel `shared/retired-key.ts` describes in its own words ("an agent
bumping `@objectstack/spec` sees THIS string, not our docs site"). It enumerated
four surfaces on which the converged spelling applies, and the fourth — "an ETL
pipeline's `retry`" — had been deleted in the same major. An author or agent
migrating `retryDelayMs` → `backoffMs` was therefore told, by the platform's own
upgrade prescription, that a surface exists where `tsc` now reports TS2724/TS2305.
The message now names exactly the three surfaces that still carry the policy:

```
… the retry policy now has ONE spelling for its base delay across every surface
that carries it: `job.retryPolicy`, a `try_catch` node's `retry` and
`flow.errorHandling`. Rename the key to `backoffMs`; …
```

The prescription itself is unchanged — same rename, same value semantics, same
`os migrate meta --from 16` pointer — and a new pin in
`shared/retry-policy.test.ts` now asserts the enumeration in both directions
(every live surface named, no retired one named) so the string cannot drift wide
again unobserved.

**No acceptance change.** `retryPolicyShape()` and `RetryPolicySchema` keep their
exact key sets, bounds and defaults; `retryDelayMs` is still rejected, still with
a prescription. Everything else in this change is comment text: the module TSDoc
in `shared/retry-policy.zod.ts` (the four-surfaces arithmetic, the strict-surface
counts, and the paragraph that claimed a `maxAttempts` tombstone carries the ETL
migration — it does not; the tombstone went with the shape that carried it, and
`RETIRED_DEFS_BY_MAJOR` plus the D3 `etl-pipeline-layer-retired` entry are the
declaration) and the matching stanza in `conversions/registry.ts`.
2 changes: 1 addition & 1 deletion content/docs/references/automation/control-flow.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -149,7 +149,7 @@ const result = FlowRegionSchema.parse(data);
| **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** | `never` | optional | [REMOVED] `retryDelayMs` was removed in @objectstack/spec 17.0.0 (#4661, #4964) — the retry policy now has ONE spelling for its base delay across every surface that carries it: `job.retryPolicy`, a `try_catch` node's `retry`, `flow.errorHandling` and an ETL pipeline'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. |
| **retryDelayMs** | `never` | optional | [REMOVED] `retryDelayMs` was removed in @objectstack/spec 17.0.0 (#4661, #4964) — the retry policy now has ONE spelling for its base delay across every surface that carries it: `job.retryPolicy`, a `try_catch` node's `retry` and `flow.errorHandling`. Rename the key to `backoffMs`; the value (milliseconds before the first retry) is unchanged. `os migrate meta --from 16` rewrites it for you. |


---
Expand Down
2 changes: 1 addition & 1 deletion content/docs/references/system/job.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -125,7 +125,7 @@ const result = CronScheduleSchema.parse(data);
| **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** | `never` | optional | [REMOVED] `retryDelayMs` was removed in @objectstack/spec 17.0.0 (#4661, #4964) — the retry policy now has ONE spelling for its base delay across every surface that carries it: `job.retryPolicy`, a `try_catch` node's `retry`, `flow.errorHandling` and an ETL pipeline'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. |
| **retryDelayMs** | `never` | optional | [REMOVED] `retryDelayMs` was removed in @objectstack/spec 17.0.0 (#4661, #4964) — the retry policy now has ONE spelling for its base delay across every surface that carries it: `job.retryPolicy`, a `try_catch` node's `retry` and `flow.errorHandling`. Rename the key to `backoffMs`; the value (milliseconds before the first retry) is unchanged. `os migrate meta --from 16` rewrites it for you. |


---
Expand Down
39 changes: 22 additions & 17 deletions packages/spec/src/conversions/registry.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -3712,7 +3712,7 @@ const objectIndexTypePartialRemoved: MetadataConversion = {
* 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 two of the four owning shapes are not `.strict()`: a
* deleted — because two of the three owning shapes are not `.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.
Expand All@@ -3733,32 +3733,37 @@ const objectIndexTypePartialRemoved: MetadataConversion = {
* Jobs with no `retryPolicy` block at all are left alone — absence already
* meant a single attempt on both sides of the change.
*
* ## The two surfaces this entry grew to cover (#4964 / #4962)
* ## The ONE further surface this entry grew to cover (#4964 / #4962)
*
* The convergence above was driven by the dual-source instrument, whose
* question is "how many declarations share one exported NAME?". Two further
* encodings of the identical policy were invisible to it because they are
* encodings of the identical policy were invisible to it because they were
* anonymous inline `z.object`s with no exported name at all — and after a
* convergence lands, a surviving dialect reads as reviewed-and-kept rather
* than missed:
* than missed. One of the two is still a surface; the other went with its
* layer:
*
* - **`flow.errorHandling`** (#4964) spelled the base delay `retryDelayMs`;
* every other key, bound and default already matched. Step 0 below renames
* it, so the ONE authorable casualty of the whole convergence is still just
* that word — now retired everywhere it was ever legal rather than on two
* surfaces out of four.
* that word — now retired everywhere it was ever legal rather than only on
* the two shapes #4661's instrument could see.
* - **`ETLPipeline.retry`** (#4962) spelled the count `maxAttempts` and
* defaulted it to 3. It gets **no step here, deliberately.** An ETL pipeline
* is not a `defineStack` collection and `etl.zod.ts` has no parse site in
* objectstack / objectui / cloud (批 12's measurement), so there is no
* stored or authored document a walker could reach: a branch for it would be
* dead code claiming migration coverage that does not exist, which is the
* ADR-0049 failure this registry is supposed to prevent, not commit. Its
* `maxAttempts` tombstone carries the rename AND the default change, and the
* tombstone reaches the only doors that exist (`tsc` at the authoring site,
* and the parse). That is also why the ETL default flip 3 → 0 needs no
* materialization step while the job one did: nothing is deployed under the
* old reading.
* defaulted it to 3. It got **no step here, deliberately** — and never will.
* An ETL pipeline was not a `defineStack` collection and `etl.zod.ts` had no
* parse site in objectstack / objectui / cloud (批 12's measurement), so
* there was no stored or authored document a walker could reach: a branch
* for it would have been dead code claiming migration coverage that does not
* exist, which is the ADR-0049 failure this registry is supposed to prevent,
* not commit. #6414 then retired the whole L2 layer, and #4962's own entry
* (`etl-retry-converged-onto-retry-policy`) was ABSORBED into
* `etl-pipeline-layer-retired` inside the same unreleased major. There is no
* `maxAttempts` tombstone left to carry the rename or the default change: it
* went with the shape that carried it, which is strictly stronger, because
* no `retry` block survives to author the key into. `tsc` (TS2724/TS2305 on
* the removed ETL names) is the whole channel. That is also why the ETL
* default flip 3 → 0 never needed a materialization step while the job one
* did: nothing was ever deployed under the old reading.
*
* `retiredFromLoadPath` is NOT set: `FlowNodeSchema.config` is an unconstrained
* record, so no schema rejection can reach `config.retry.retryDelayMs` and the
Expand Down
41 changes: 41 additions & 0 deletions packages/spec/src/shared/retry-policy.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -76,6 +76,47 @@ describe('RetryPolicy is a single declaration across entries (#4661)', () => {
expect(parse).toThrow(/backoffMs/);
}
});

// The tombstone STRING is not documentation. `shared/retired-key.ts` says so
// in its own words — "an agent bumping `@objectstack/spec` sees THIS string,
// not our docs site" — which makes its enumeration of carrying surfaces
// contract, not prose. It went stale exactly once and in the worst direction:
// it kept listing "an ETL pipeline's `retry`" after #6414 deleted the whole L2
// layer, so the one message guaranteed to reach an upgrading author named a
// fourth place to write `backoffMs` on which `tsc` now reports TS2724/TS2305
// (#6630). Nothing could see that: every pre-existing assertion on this string
// matched `/backoffMs/` or `/retryDelayMs/`, i.e. the prescription, never the
// enumeration. Pin the enumeration in BOTH directions.
//
// Note on the assertion shape: there is no ADR-0112 `code`/`status` envelope
// to assert here. `retiredKey()` is a Zod `never` whose issue carries the
// guidance as its `message`, so for this rejection class the wording IS the
// whole contract (#5240) and the message is the only thing worth asserting.
it('the tombstone enumerates exactly the surfaces that still carry the policy (#6630)', () => {
const result = RetryPolicySchema.safeParse({ retryDelayMs: 500 });
expect(result.success).toBe(false);
const message = result.error!.issues.map((issue) => issue.message).join('\n');

// Every surface that builds from `retryPolicyShape()` today, spelled the way
// an author writes it. A prescription naming a scope NARROWER than the truth
// is the #4964 defect (a real surface left out reads as "not converged").
for (const live of ['`job.retryPolicy`', "`try_catch` node's `retry`", '`flow.errorHandling`']) {
expect(message, `the prescription must still name ${live}`).toContain(live);
}

// ...and nothing retired. Wider than the truth is the #6630 defect: the
// export absence is pinned in `automation/sync-retirement.test.ts`, so a
// surface named here that cannot be imported there is a signpost to a
// compile error.
expect(
message,
'the prescription must not point at a surface #6414 retired',
).not.toMatch(/\bETL\b/i);

// None of the above may be bought by weakening the prescription itself.
expect(message).toContain('Rename the key to `backoffMs`');
expect(message).toContain('os migrate meta --from 16');
});
});

describe('RetryPolicySchema — converged shape', () => {
Expand Down
63 changes: 36 additions & 27 deletions packages/spec/src/shared/retry-policy.zod.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -39,11 +39,12 @@
* the question simply was not the one whose answer everybody read off it. After
* a convergence completes, a surviving dialect reads as *reviewed and kept*.
*
* Both now build from {@link retryPolicyShape}, so the four surfaces share one
* declaration of the key set, the bounds and the defaults. The two of them that
* are `.strict()` keep their own `strictObject` curation and their own extra
* keys (`Flow.errorHandling.strategy`) — what is shared is the *contract*, not
* the surface's framing of it.
* The surviving one of those two builds from {@link retryPolicyShape} like the
* named surfaces do, so the three remaining surfaces share one declaration of
* the key set, the bounds and the defaults. The only one of them that is
* `.strict()` — `Flow.errorHandling` — keeps its own `strictObject` curation and
* its own extra keys (`strategy`); what is shared is the *contract*, not the
* surface's framing of it.
*
* ## Why this file, and why it is not in `shared/index.ts`
*
Expand DownExpand Up@@ -77,13 +78,14 @@ import { retiredKey } from './retired-key';
* The retry policy's raw Zod shape — key set, bounds, defaults and prose, in
* ONE place.
*
* Two of the four surfaces that carry this policy cannot simply reference
* {@link RetryPolicySchema}: `Flow.errorHandling` and `ETLPipeline.retry` are
* `.strict()` (`strictObject`, the #4001 campaign standard) and the flow one
* also carries `strategy` plus its own `superRefine`. Handing them the *shape*
* rather than the *schema* is what lets them stay strict, keep their curated
* unknown-key tables, and still have exactly one declaration of what a retry
* policy IS — the alternative (a fifth hand-copied key list) is the debt #4964
* One of the three surfaces that carry this policy cannot simply reference
* {@link RetryPolicySchema}: `Flow.errorHandling` is `.strict()`
* (`strictObject`, the #4001 campaign standard) and also carries `strategy` plus
* its own `superRefine`. (`ETLPipeline.retry` was the second such surface until
* #6414 retired the whole L2 ETL layer.) Handing it the *shape* rather than the
* *schema* is what lets it stay strict, keep its curated unknown-key table, and
* still have exactly one declaration of what a retry policy IS — the
* alternative, one hand-copied key list per strict surface, is the debt #4964
* and #4962 exist to remove.
*
* It is a function rather than a const for the same reason every schema here is
Expand All@@ -110,17 +112,17 @@ export function retryPolicyShape() {
// ── Tombstone (ADR-0087) ────────────────────────────────────────────
// `retryDelayMs` was the automation-side spelling of `backoffMs`, on BOTH
// `try_catch`'s `retry` (#4661) and `Flow.errorHandling` (#4964). It is
// tombstoned rather than deleted because two of the four owning shapes are
// tombstoned rather than deleted because two of the three owning shapes are
// not `.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. On the two
// strict surfaces the tombstone is still the better channel — it carries
// the rename, where a bare unknown-key rejection would only carry the key.
// precisely the quiet-failure class ADR-0049 exists to remove. On the strict
// surface the tombstone is still the better channel — it carries the
// rename, where a bare unknown-key rejection would only carry the key.
// `retry-policy-converged` rewrites it on the load path.
retryDelayMs: retiredKey(
'`retryDelayMs` was removed in @objectstack/spec 17.0.0 (#4661, #4964) — the retry policy now ' +
'has ONE spelling for its base delay across every surface that carries it: `job.retryPolicy`, ' +
"a `try_catch` node's `retry`, `flow.errorHandling` and an ETL pipeline's `retry`. " +
"a `try_catch` node's `retry` and `flow.errorHandling`. " +
'Rename the key to `backoffMs`; the value (milliseconds before the first retry) ' +
'is unchanged. `os migrate meta --from 16` rewrites it for you.',
),
Expand All@@ -129,9 +131,10 @@ export function retryPolicyShape() {

/**
* Exponential-backoff retry policy — the named schema for `job.retryPolicy` and
* a `try_catch` node's `retry` region. `Flow.errorHandling` and
* `ETLPipeline.retry` carry the same contract via {@link retryPolicyShape},
* which they must, being `.strict()` (see that function's note).
* a `try_catch` node's `retry` region. `Flow.errorHandling` carries the same
* contract via {@link retryPolicyShape}, which it must, being `.strict()` (see
* that function's note). `ETLPipeline.retry` did the same until #6414 retired
* the L2 ETL layer.
*
* Delay before retry *n* is `min(backoffMs * backoffMultiplier^(n-1),
* maxRetryDelayMs)`, optionally jittered.
Expand All@@ -146,13 +149,19 @@ export function retryPolicyShape() {
* `backoffMultiplier: 2`) into existing job documents, so no deployed stack
* changes behaviour. What changes is what a NEWLY authored omission means.
*
* The ETL half needed no conversion branch and deliberately has none: an ETL
* pipeline is not a `defineStack` collection and `etl.zod.ts` has no parse site
* anywhere in objectstack / objectui / cloud (批 12's measurement), so there is
* no stored document for a D2 walker to reach. Writing one anyway would be a
* conversion advertising coverage it does not have. The `maxAttempts` tombstone
* on that block is the whole migration channel, and it reaches the only door
* that exists — `tsc` at the authoring site, and the parse.
* The ETL half never got a conversion branch — deliberately, and now
* permanently: an ETL pipeline was not a `defineStack` collection and
* `etl.zod.ts` had no parse site anywhere in objectstack / objectui / cloud
* (批 12's measurement), so there was no stored document for a D2 walker to
* reach and writing one anyway would have been a conversion advertising
* coverage it did not have. #6414 then retired the whole L2 layer, which took
* `ETLPipeline.retry` with it. There is therefore NO `maxAttempts` tombstone:
* a tombstone keeps a key unwritable on a shape that survives, and no `retry`
* block survives to author the key into — an absence strictly stronger than a
* tombstone. What declares the removal is `RETIRED_DEFS_BY_MAJOR` plus the D3
* `etl-pipeline-layer-retired` semantic migration (which ABSORBED #4962's own
* conversion entry for exactly this reason), and the door an upgrading author
* actually hits is `tsc` — TS2724/TS2305 on every removed ETL name.
*
* 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
Expand Down
Loading