Skip to content

Commit f606ba2

Browse files
feat(spec)!: a job retryPolicy with no attempts is refused at maxRetries; retry-policy-converged leaves the load path (F2 of #23543) (#23810)
Part of #23543 Clause-②: yes (narrowing) Regen-provenance: since the reviewed head 0bc4f42, dce2266 rewords the D3 entry's `reason` (prose only, one file, at the seat's request) and 2741849 merges origin/main at f846cc2 through os-regen-merge.sh with check:generated clean; every other file's change equals the reviewed head's. This PR lands family **F2** of ruling `6182001882` (item 2, the loud form) only. F4 and F5-http stay on #23543, one PR each, in the ruled order; #23543 remains open. ## What Ruling F2 A: "a `job.retryPolicy` block whose `maxRetries` is omitted or 0 is refused, in the shape `flow.errorHandling` already uses for `strategy: 'retry'` … A job with no block stays a single attempt; `backoffMultiplier` keeps its declared default and is not refused. `retry-policy-converged` then sets `retiredFromLoadPath`; the 17 materialization lives on in the D3 chain." - **The refusal lives on the job, not on the shared schema.** `RetryPolicySchema` (`shared/retry-policy.zod.ts`) is also a `try_catch` node's `retry` region, which the ruling does not cover. So `JobSchema` gains one root-level refine, `job.retryPolicy === undefined || job.retryPolicy.maxRetries >= 1`, reporting a `custom` issue at `retryPolicy.maxRetries` with the ruling's prescription as its first sentence: "A retry block with no attempts is no retry block: state `maxRetries` ≥ 1, for example 3, or delete the block." It then names why (the loader used to write `maxRetries: 3`; a retry re-runs whatever the failed attempt did) and ends with the house `os migrate meta --from 16` sentence. At `defineStack` this is `STACK_SCHEMA_INVALID` / 422 at `jobs.N.retryPolicy.maxRetries`. - **Omitted and 0 are one case by construction.** The refine reads the parsed policy, where the default has already filled an omitted count with 0. No pre-default hook is needed. - **No new dropped-refinement site.** The refine sits on the job's root, a site `dropped-refinements.baseline.json` already records for `system/Job` (the `pull` exclusivity refine). A field-level refine would have added a ledger site to `system/Job` and to every schema that embeds a job. - **The `retryPolicy` describe** no longer says "declaring it without `maxRetries` also means no retry"; it says a block must state `maxRetries` >= 1. `content/docs/references/system/job.mdx` is regenerated from it. - **The D2 entry.** `retry-policy-converged` gets `retiredFromLoadPath: true` and `retiredAfter: '17.7.0'`, the stamp F1, F5-jsx and F3 used. Its "recorded verdict at 18" paragraph is rewritten as the retirement record. The fixture is unchanged: every `includeRetired` seam still replays the whole entry. - **The D3 entry.** `18.job-retry-policy-without-attempts-refused` (semantic, step 18) with `conversionIds: ['retry-policy-converged']`, already a step-17 member. It states the decision no conversion makes for the author: keep the written count, or delete the block. It also covers the one silent change the ruling accepts: a block that states `maxRetries` and omits `backoffMultiplier` now gets 1 where the loader wrote 2. - **The verdict-list pin** in `conversions.test.ts` moves the entry to the retired list. The kept list now holds three entries: `flow-node-http-callout-rename`, `page-component-visibility-to-visibleWhen` and `view-visibleOn-to-visibleWhen`. The removal-version pin's `JOIN` gains one `retry-policy-converged` row: the job refusal, which names `@objectstack/spec 18` and `--from 16`. ## Mechanism assumptions, measured 1. **Who else parses `RetryPolicySchema`.** It holds. Its parsers are `job.retryPolicy` (`system/job.zod.ts`) and a `try_catch` node's `retry` (`automation/control-flow.zod.ts`). `flow.errorHandling` builds from `retryPolicyShape()`, and `worker.zod.ts` has its own `TaskRetryPolicySchema`. So the refusal is on `JobSchema`. A control pins that `RetryPolicySchema.safeParse({ backoffMs: 5000 })` still succeeds. 2. **Omitted versus 0.** It holds: judging the parsed value is enough (above). Both spellings are pinned at the key. 3. **The entry and the pins.** It holds as described. The kept list is 3. The `JOIN` row is owed, because `retiredAfter` 17.7.0 is the newest published stable release. 4. **The `retryDelayMs` half.** It needs nothing beyond the retirement. Both surfaces carry the `retryDelayMs` tombstone, and a new case in `conversions.test.ts` pins an authored flow-level `retryDelayMs` refused at `defineStack` naming `backoffMs`. Two existing tests relied on the load converting it, and both were re-pointed: - `flow-builtin-node-config-keys.test.ts` "CONTROL: a pre-17 retryDelayMs alone is converted first where the door converts" now reads the replaying seam (`applyConversionsToFlow(…, { includeRetired: true })`, which parses) and the authoring door (`defineStack`, refused at `config.retry.retryDelayMs` naming `backoffMs`). - The cli `retry-policy-key-validate-door.test.ts` "declared" fixture now writes `backoffMs`. 5. **The four load-path suites** (F3's record `6205630082`). The measurement that decides them: `os migrate meta` replays from `MIGRATION_SUPPORT_FLOOR` 16, so `--from 16` replays steps 17 and 18, and after F2 every entry of both is retired. The only entries still live on the load path are toMajor 11 and 15 (F4's two, F5-http's one), and the chain never replays those. So no live entry can be both converted by the load and listed by the chain. Per suite: - `packages/spec/src/stack-authored-input.test.ts` gets a **synthetic probe**. Its subject is the load pass itself, not an entry, so a `vi.mock` of `./conversions/registry` appends a test-owned live conversion (`stack-authored-input-probe`, the old `retryDelayMs` rename). Every assertion is unchanged and now reads the probe's id. No later family has to re-point this suite again. - `packages/cli/test/migrate-meta-load-conversions.test.ts` is **partly retired and partly re-pointed**. A probe cannot reach it: the temp project loads the built `@objectstack/spec` through a symlinked `node_modules`, outside vitest's module graph. - Retired: "lists a conversion the load already applied …" (the defect's chain reading), which has no subject on this tree. Its write / re-run / clean-reload half is pinned at the same layer by the two-refused-spellings case, as the file header records. - Re-pointed: "the authored-source load reads the argument off the default export" and "defined twice starts from the innermost argument". They now use a discriminator that needs no conversion: a strict `defineStack` fills the declared defaults (`backoffMultiplier: 1`, `maxRetryDelayMs: 30000`, `jitter: false`), and the authored argument does not have them. I measured this on the built dist before writing it. - The old defined-twice chain case moved to the refused arm, where `retryDelayMs` now lives. The mixed case was retitled "two refused spellings the chain converts". - `packages/cli/test/migrate-meta-composed-load-conversions.test.ts` gets the same treatment, for the package body. - `packages/cli/test/migrate-meta-composed.test.ts` (its load-path row) is **kept and renamed**. Its assertions still hold, but its name was false. It is now "a refused stack carrying a second refused spelling". 6. **Authored sources.** I swept `examples/**`, `packages/**`, `content/docs/**` and `skills/**` for every object-literal `retryPolicy` block, single- and multi-line, plus non-literal values. No job block omits `maxRetries` or writes 0. The hits are `hook.retryPolicy` (a separate declaration, outside the ruling) and this PR's own fixtures. One test fixture wrote `maxRetries: 0` on a job (`job.test.ts`, "one-time scheduled job"). It means a single attempt, so the block was deleted. `skills/**` has nothing to report. 7. **Serial neighbours.** PR #23781 and PR #23784 had not landed. `origin/main` moved 5 commits (none in this PR's files) and was merged through `scripts/pm/os-regen-merge.sh` with no conflict. `check:generated` then read "All 14 generated artifacts are up to date". 8. **The metadata save door** (`protocol.ts`) is untouched. For a non-flow type it judges the body as stored, with no conversion replay, so a job body meets this refusal there. `job` has not been runtime-creatable since 17, though, so no door claim is made for it. The flow half (`retryDelayMs`) meets the door's flow fallback exactly as before, because that fallback already replayed this entry while it was live. The seat's separate save-door decision covers it with the other retired flow entries. ## Readers updated (prose made false by the retirement) - `content/docs/automation/jobs.mdx`: "declaring `retryPolicy` without stating a count still means *no retry*" is now the refusal rule. - `packages/services/service-job/src/run-with-policy.ts` (comment only): the default is reached only by a hand-built caller. - `shared/retry-policy.zod.ts` (two comments), `liveness/flow.json` (one note) and the `job-timeout-to-timeout-ms` docblock in `registry.ts` no longer call the entry a load-path rewrite. ## Changeset `.changeset/23543-f2-job-retry-policy-without-attempts-refused.md`: - **Bumps:** `@objectstack/spec` minor; `@objectstack/service-job` patch (comment-only). - It is BREAKING and carries `Clause-②: yes (narrowing)`. It gives FROM → TO: `retryPolicy: {}` → `retryPolicy: { maxRetries: 3 }` or delete the block, and a block with `maxRetries: 0` → delete it. It has a "What an author who still writes it sees" paragraph, the `backoffMultiplier` 2 → 1 note, the `os migrate meta --from 16` path, and how stored rows keep loading. - **ADR-0087 disposition:** `registered job-retry-policy-without-attempts-refused`. ## Verification All readings are on head `0bc4f428` (the merge of `origin/main` `46ca793e`) unless noted. - **`@objectstack/spec`:** full `test` 683 files / 20888 passed (1 todo), exit 0. `typecheck` exit 0 (pre-merge `9e09e0dd`; the merge brought no change to this PR's files). `build` + `check:generated`: "All 14 generated artifacts are up to date". - **`@objectstack/cli`:** the full `unit` tier passed, 297 files. The first run had 295 passing, plus the 2 build-dependent published-subpath pins, which failed only with "packages/cli is not built" and passed 2 / 29 after `pnpm --filter @objectstack/cli build`. 4348 tests passed, 29 skipped. Integration tier: the one edited file, `retry-policy-key-validate-door.test.ts`, 2 / 2. `typecheck` exit 0, after building the closure with declarations. - **`@objectstack/service-job`:** `test` 11 files / 117 passed. `typecheck` exit 0. - **Gates.** `dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 114 commands on `0bc4f428`. Each ran one by one, with its exit code captured before any pipe. `--ran` reports "114 derived famil(ies) accounted for — 111 run, 3 NOT-MEASURED (3 DERIVED from a recorded exit 3)". The three are the prerequisite refusals F3 also recorded: `check-plugin-teardown-shape --self-test` (shallow clone), `check:skill-examples` (client dist absent) and `check:dual-build-cjs-loads` (full build absent). Notable greens: `check-adr-0087-registration --base origin/main`, `check-changeset-no-major`, `check-empty-changeset`, `check:dts-closure`, `check:nul-bytes`, `check:liveness`, `check:spec-changes`, `check:upgrade-guide`. - **Ablation, the refusal** (`scripts/ablation-replace.mjs`, on `9e09e0dd`). `job.retryPolicy.maxRetries >= 1` was replaced by `>= 0`. - On-disk proof: anchor 1 → 0, blob `28e227b31f4e` → `c6c4d7684caa`. - Result: 7 tests went red across `job.test.ts` (3), `conversions.test.ts` (3, including the stored-row case's "the row as written is refused" leg) and `retired-key-removal-version.pin.test.ts` (the `retry-policy-converged` `JOIN` row). Direction: red. - Restore proven: blob == HEAD and `git diff HEAD` empty. Spec tests import `src`, so there is no dist leg. ## Acceptance notes - Labels: this dispatch's write budget names no `label-write`, so this PR carries none from me. - The refusal is a root-level refine, so zod reports it only once the job's other fields pass. A job with another error and an empty retry block shows that error first and this one after it is fixed. A field-level refine would report both at once, at the cost of new dropped-refinement ledger sites in every schema that embeds a job. - `tsc` does not see the omission: `maxRetries` stays optional in `RetryPolicy`, because the shared input type also serves `try_catch`. The parse-time prescription is the channel for this family. --- _Generated by [Claude Code](https://claude.ai/code/session_01MT2aGLQQhH8ZsPrgqHMrwz)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent af3873b commit f606ba2

18 files changed

Lines changed: 519 additions & 202 deletions
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
---
2+
'@objectstack/spec': minor
3+
'@objectstack/service-job': patch
4+
---
5+
6+
feat(spec)!: a job `retryPolicy` block whose `maxRetries` is omitted or 0 is refused, and `retry-policy-converged` leaves the load path
7+
8+
Clause-②: yes (narrowing)
9+
10+
**BREAKING (authoring):** `maxRetries` has defaulted to 0 since 17, so a `job.retryPolicy` block that omits it, or writes 0, buys no retry: the job runs once. Through 17 the loader hid that by writing the pre-17 `maxRetries: 3` and `backoffMultiplier: 2` into such a block with a notice. At 18 the ADR-0087 D2 conversion `retry-policy-converged` is retired from the load path (`retiredAfter: '17.7.0'`), and the job parse refuses the block at `retryPolicy.maxRetries`, at every door that parses an authored job (`defineStack`, `os validate`, `os build`, and `os lint` when the config calls it): "A retry block with no attempts is no retry block: state `maxRetries` ≥ 1, for example 3, or delete the block." The rule is the job's, the same one `flow.errorHandling` applies to `strategy: 'retry'`; the shared `RetryPolicySchema` is unchanged, so a `try_catch` node's `retry` region is judged as before.
11+
12+
Migration:
13+
14+
- `retryPolicy: {}` or `retryPolicy: { backoffMs: 5000 }` → `retryPolicy: { maxRetries: 3, backoffMs: 5000 }` (3 is the count such a block ran with before 17), or delete the block if the job should run once.
15+
- `retryPolicy: { maxRetries: 0, … }` → delete the block: a job with no `retryPolicy` is a single attempt.
16+
17+
What an author who still writes it sees: `defineStack` throws `STACK_SCHEMA_INVALID` (422) with one issue at `jobs.N.retryPolicy.maxRetries` carrying the prescription above, and `os validate` prints the same issue. Nothing is rewritten silently any more.
18+
19+
Not refused: a block that states `maxRetries` and omits `backoffMultiplier`. It keeps the declared default of 1, a flat delay, where the loader used to write 2. State `backoffMultiplier: 2` to keep the old exponential curve.
20+
21+
Run `os migrate meta --from 16` to list the mechanical edits for existing sources; `--write` applies the ones it can prove, and you apply the rest by hand. For such a block the edit writes both pre-17 numbers (`maxRetries: 3`, `backoffMultiplier: 2`). The D3 entry `job-retry-policy-without-attempts-refused` states the decision left to you: keep the written count, or delete the block.
22+
23+
The flow half of the conversion (`retryDelayMs` → `backoffMs` on `flow.errorHandling` and a `try_catch` node's `retry`) needed nothing more: both surfaces already refuse `retryDelayMs` with the rename to `backoffMs`.
24+
25+
Stored metadata needs no edit. Stored-row rehydration, the automation engine's flow rehydration, the metadata save door's flow fallback and the artifact-ingestion door inside its window still replay the conversion, so a job row stored without a count keeps loading with the three retries it ran with.
26+
27+
`@objectstack/service-job`: a comment-only change in `runWithPolicy`, whose `maxRetries` default is now reached only by a caller that builds the options by hand.
28+
29+
<!-- adr-0087: registered job-retry-policy-without-attempts-refused -->

‎content/docs/automation/jobs.mdx‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -385,13 +385,15 @@ Delay before retry *n* is `min(backoffMs * backoffMultiplier^(n-1), maxRetryDela
385385
optionally jittered. `maxRetries` counts retries **after** the initial attempt and
386386
is capped at 10.
387387

388-
Two defaults worth knowing before you rely on the block:
388+
Two rules worth knowing before you rely on the block:
389389

390-
- **`maxRetries` defaults to `0`** — declaring `retryPolicy` without stating a
391-
count still means *no retry*. State a count to opt in.
390+
- **A block must state `maxRetries` ≥ 1.** A `retryPolicy` that omits it or
391+
writes `0` would buy no retry, so it is refused at `objectstack validate` with
392+
"a retry block with no attempts is no retry block". State a count, or delete
393+
the block: a job with no `retryPolicy` runs once.
392394
- **`backoffMultiplier` defaults to `1`** — a flat delay, not exponential.
393395

394-
Those defaults are why the block is closed: a key it does not declare — `maxRetry`
396+
The count rule and that default are why the block is closed: a key it does not declare — `maxRetry`
395397
for `maxRetries`, a `maxDelayMs` borrowed from another retry vocabulary — is
396398
refused at `objectstack validate` with the declared key it was near, rather than
397399
dropped and replaced by a default that never retries.

‎content/docs/references/system/job.mdx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ const result = CronScheduleSchema.parse(data);
6161
| **body** | `{ language: 'js'; source: string; capabilities?: Enum<'api.read' \| 'api.write' \| 'api.transaction' \| 'crypto.uuid' \| 'log'>[]; timeoutMs?: integer; … }` | optional | Job body — a sandboxed JS (L2) body, the same shape hooks and actions use; an expression (L1) body is refused, because a job runs for its effects and an expression has none. Preferred over `handler`: when both are present `body` wins. It runs in the QuickJS sandbox with no module scope (no imports, no helpers or constants from the surrounding file): it reaches data only through `ctx.api` under its declared `capabilities` (`api.read` / `api.write` / `api.transaction`) and logs through `ctx.log` (`log`); the in-process handler context (`ql`, `logger`, `bundle`) does not exist there. Its time limit is the job's `timeoutMs` (see there): long-running work declares a `timeoutMs` that covers it, or splits into bounded runs that each finish within it. Every door that brings an artifact in schedules a job's `body` — the boot, and `os package install` on install and on every restart — while a `handler` is code that travels only in the artifact's runtime module and runs only on a boot that loads it (a config, or `os start --artifact`); `os package install` therefore refuses an enabled job with no `body`. A `pull` is data too, so an enabled `pull` job is judged by its `pull` instead: it installs when the `pull` binds (it names a mapping the package declares, with a `connectorSource`) and is refused when it does not, as is a job whose `body` the declaration refuses. Refused beside `pull`. |
6262
| **pull** | `{ mapping: string }` | optional | Pull run form: on each run the platform pulls the named mapping's `connectorSource` (one action call, one response) and writes the rows through the import runner — no code. A refused pull records the run `failed` (retried per `retryPolicy`); a pull whose rows the import runner refused records it `degraded`. Data like `body`, so every door schedules it. Refused beside `body` or `handler`. |
6363
| **organization** | `string` | optional | Organization id (sys_organization.id) this job runs as — its `body`'s `ctx.api`, the execution context its `handler` is handed, and its `pull`'s reads and writes alike, as a system run carrying that organization. A scheduled run has no session to inherit one from. Judged at bind by the posture rule scheduled flows use: required under the isolated tenancy posture (a job that declares none is not scheduled); optional under group (undeclared, the run carries no organization and a tenant-scoped write it makes is refused); not required under single (the install's one organization is resolved beneath each write). Where declared, it is the organization the run acts as on every posture. |
64-
| **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. Omit the block for a single attempt; declaring it without `maxRetries` also means no retry since 17.0.0 — state a count to opt in. |
64+
| **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. Omit the block for a single attempt; a block must state `maxRetries` >= 1 — one that omits it or writes 0 is refused, since it would buy no retry. |
6565
| **timeoutMs** | `integer` | optional | Per-attempt time limit in milliseconds; an over-limit run is recorded with execution status "timeout". A `handler` run is abandoned, not forcibly cancelled. For a job with a `body` this is the ONE time limit: one attempt is one sandbox invocation, the runtime bounds that invocation by this value, and the body shape's own `timeoutMs` (capped at 30000 for hooks and actions) is refused on a job — so this key, which has no such cap, is where long-running work states how long it needs. Omit for no per-attempt limit; a `body` run is then still bounded by the sandbox's own default invocation limits. |
6666
| **timeout** | `never` | optional | [REMOVED] `job.timeout` was removed in @objectstack/spec 17 — its unit (milliseconds) lived only in the description while the sibling `retryPolicy.backoffMs` spells its own, so the same number read as two conventions on one surface. Rename the key to `timeoutMs`; the value (milliseconds) is unchanged. Run `os migrate meta --from 17` to list the mechanical edits for existing sources; `--write` applies the ones it can prove, and you apply the rest by hand. |
6767
| **enabled** | `boolean` | optional (default: `true`) | Whether the job is enabled |

0 commit comments

Comments
 (0)