Repository navigation
Commit ac8f2c5
fix(spec)!: close the shared retry policy, and judge try_catch config keys at the build doors (#22380)
Fixes #22343
Clause-②: no (narrowing: an undeclared key on the shared retry policy is
refused at parse wherever it is written, a job's retryPolicy and a
try_catch node's retry, and an undeclared try_catch config key at the
build doors and the save door, where each passed)
The claim (`6069618073`) declared `yes`. A pure narrowing is `no`
(`references/execution-duties.md:105`), so the seat corrected the
spelling after the contract review `6072339018`; the parenthesis is
unchanged. It is worded to what the census decided: no writer relies on
the strip, so the shared `RetryPolicySchema` is closed and every one of
its parsers narrows, not the `try_catch` slot alone. Per triage
`6068085630`, the importers named that way are `job.retryPolicy` and a
`try_catch` node's `retry`.
## What changes
- **Spec, `shared/retry-policy.zod.ts`.** `RetryPolicySchema` is a
`strictObject` (it was a plain `z.object`). A key it does not declare is
refused at parse, naming the key, with a curated near-miss table:
`maxRetry`, `retries` and `attempts` → `maxRetries`; `initialDelayMs`
and `baseDelayMs` → `backoffMs`; `maxDelayMs` → `maxRetryDelayMs`. A
`maxAttempts` gets a prescription instead of an alias, because it counts
the first attempt. The `retryDelayMs` tombstone stays, with its rename.
The docblock records the census.
- **Spec, `automation/flow-node-config-refusals.ts`.**
`BUILTIN_KEYS_JUDGED_AT_REGISTRATION` is deleted. `try_catch` was its
only member, and an empty exception set would be dead code.
`builtinNodeConfigKeysJudged(type)` is now
`getBuiltinNodeConfigContracts().has(type)`, so the key arm judges all
13 builtins. The key arm also refuses a tombstoned key the author wrote,
at its own path and in the tombstone's words. `script` is excluded
(`RETIRED_KEYS_JUDGED_ELSEWHERE`), because the lint names its retired
keys and no door before the run ever judged them. See "Route change"
below.
- **`service-automation` `engine.ts`: comments only.**
`validateNodeConfigKeys` already asks `builtinNodeConfigKeysJudged`, so
it now stands aside for `try_catch` and judges plugin node types only.
Its docblocks are corrected.
- **Ledger.** A step-18 D3 entry,
`try-catch-and-retry-policy-undeclared-keys-refused`, plus a
`STEP18_RATIONALE` fragment with order 90; the highest order on `main`
was 89. `migrations/registry.ts` was regenerated with
`gen:migration-registry`. The step-18 entry
`flow-builtin-node-config-undeclared-keys-refused` and its fragment said
`try_catch` "stays registration's", which this change makes false in the
same unreleased step, so both now point at the new entry.
- **Changeset.**
`.changeset/22343-retry-policy-try-catch-undeclared-keys-refused.md`
bumps `@objectstack/spec` `minor` (BREAKING, ADR-0087 `registered`)
under the same pre-mode convention as `2f70c2222`
(`.changeset/pre.json`: mode `pre`, tag `next`). It carries the Clause-②
line above. It has no `@objectstack/service-automation` entry: that
package's diff is comments plus one test, and no gate asked for one
(`check-changeset-no-major`, `check-adr-0087-registration` and
`check-empty-changeset` are green).
- **Docs.** `content/docs/automation/flows.mdx`: the `config` row and
the strictness callout now list `try_catch` among the judged builtins,
and one sentence says `retry` is closed. `jobs.mdx` gets one sentence
saying `retryPolicy` is closed. The strictness ledger's audit row
(`docs/audits/2026-07-unknown-key-strictness-ledger.md`) said the policy
"is still non-strict", which is no longer true, so it is corrected.
- **In-place fix, `automation/flow.zod.ts` (seat order `6071818310`).**
`Flow.errorHandling`'s refusal of `maxAttempts` said a bare rename to
`maxRetries` would run "one attempt fewer" than asked for. It runs one
more: `maxAttempts: 3` is 3 runs, and `maxRetries: 3` is 1 + 3 = 4. The
clause now says "one attempt more". The prescription `maxRetries:
<maxAttempts - 1>` is unchanged. The `flow.test.ts` case is now the pin,
and its comment, which carried the same inversion, is corrected. The
`RetryPolicySchema` pin in `retry-policy.test.ts` gains the same two
assertions, so both surfaces assert the same four things: the
prescription, "one attempt more", never "fewer", and no bare rename. The
changeset gains one `Also corrected:` line.
## Census (H1), at `origin/main` `b7fdd5085`
Parsers of `RetryPolicySchema`:
| site | parses | authored by |
|:--|:--|:--|
| `system/job.zod.ts:366` `JobSchema.retryPolicy` | yes | stack `jobs` |
| `automation/control-flow.zod.ts:330` `TryCatchConfigSchema.retry` |
yes (the executor, and the key/value arms) | flow `try_catch` nodes |
| `automation/flow.zod.ts:1224` `Flow.errorHandling` | spreads
`retryPolicyShape()` into its own `strictObject`, already closed |
unaffected |
| `integration/connector-fetch-policy.ts:49` | comment only | no parse |
| `contracts/job-service.ts:131` `JobRetryPolicy` | TS interface mirror
| no parse |
| `service-job` `run-with-policy.ts:17` | mirrors the defaults, reads an
already-parsed policy | no parse |
Writers checked for a key outside the five (`maxRetries`, `backoffMs`,
`backoffMultiplier`, `maxRetryDelayMs`, `jitter`):
- **In this repo:** `examples/app-showcase` jobs (`:22`) and flows
(`:1212`, `:1425`), `examples/app-todo`,
`content/docs/automation/flows.mdx` (`:798`, `:1856`) and `jobs.mdx`
(`:25`, `:243`), the `service-automation` and `service-job` READMEs,
every test fixture that writes `retry:` under a `try_catch` or a job
`retryPolicy:` (lint, service-automation, service-job, spec), and the
`retry-policy-converged` conversion's output and fixtures. **0
undeclared keys.**
- **objectui at the pinned `a58626c88d`:** `preview-samples.ts:331`
(`retryPolicy: { maxRetries: 3 }`); the flow inspector's fallback
`try_catch` form, which writes `errorVariable` only; and the
descriptor-driven form, which reads the descriptor that closes `retry`
to the five keys. **0 undeclared keys.**
- **Out of reach:** no connector manifest, seed or `sys_metadata` writer
of this shape was found. hotcrm and deployed metadata were not measured.
**Verdict:** no writer relies on the strip, so the shared schema is
closed. No writer sits outside `packages/spec`, `service-automation` or
`service-job`, so the stop condition does not apply.
## Hypotheses, measured
- **H1, confirmed.** See the census above.
- **H2, confirmed, with one addition (see "Route change").** With
`retry` strict, the key arm judges `try_catch` like the other 12.
Docblocks that named the exception were corrected in
`flow-node-config-refusals.ts` (module header, the value-arm carve-out,
the predicate, the main arm's bullet, the inline comment), in
`flow.zod.ts` (two comments), in `engine.ts` (three comments), and in
the two spec test headers.
- **H3, confirmed.** A probe over `installBuiltinNodes` lists the 17
builtin descriptors with their `configSchema` and judged state. Before:
the walk still reached `["try_catch"]`. After: `[]`. The remaining
builtins publish no `configSchema` (`decision`, `wait`,
`connector_action`) or are exempt (`assignment`). "Refused once" is
pinned in `config-unknown-keys.test.ts`: one parse issue, and no
`undeclared config key(s)` text from the walk.
- **H4, falsified as stated: the did-you-mean needed a curated row.**
The distance fallback cannot reach `maxRetry`. It is 3 edits from
`maxRetries` (y→i, +e, +s) against a budget of 2 for an 8-letter key:
`findClosestMatches('maxRetry', …, 2)` returns `[]`. The measured door
first printed the refusal with no did-you-mean. It now answers ``
`maxRetry` → `maxRetries` `` through an `aliases` row, the mechanism
`Flow.errorHandling` already uses for the same vocabulary. The refusal
is located (`nodes.N.config.retry.maxRetry`, and `jobs.N.retryPolicy`
for a job) and uses the `node-config-refused-by-contract` shape from
#21982.
## Route change: the `retryDelayMs` tombstone (registration would
otherwise have widened)
The key arm judged `unrecognized_keys` only, and both arms skip a
tombstone (`invalid_type` expecting `never`). The
`retry-policy-converged` conversion renames `retryDelayMs` to
`backoffMs`, but it keeps both spellings when the values differ, and it
leaves a `null`. The walk refused what survived, as an undeclared key.
With the walk standing aside and the arms unchanged, `registerFlow`
would have accepted those two variants and the run would have refused
them. So the key arm now refuses a tombstoned key the author wrote on
every judged type. `script` is the exception, so this change narrows
nothing on any other builtin: only `script` and `try_catch` carry
`retiredKey()` in their contracts, measured by grep over the five
contract modules. The ruling's intent ("one judge", and `registerFlow`
widens nowhere) is what decided this. It is named here for the contract
review.
## Registration verdicts, before and after (10-variant probe on the
built packages)
| `try_catch` config | before: `registerFlow` | after: `registerFlow` |
|:--|:--|:--|
| declared `retry` | registers | registers |
| `retry.bogusKey` | refused, walk | refused, parse at
`nodes.1.config.retry.bogusKey` |
| `retry.maxRetry` | refused, walk | refused, parse at
`nodes.1.config.retry.maxRetry` |
| `retryDelayMs` alone | registers (converted) | registers (converted) |
| `retryDelayMs` equal to `backoffMs` | registers (twin dropped) |
registers (twin dropped) |
| `retryDelayMs` differing from `backoffMs` | refused, walk | refused,
parse at `…retry.retryDelayMs` |
| `retryDelayMs: null` | refused, walk | refused, parse at
`…retry.retryDelayMs` |
| top-level `bogusKey` | refused, walk | refused, parse at
`nodes.1.config.bogusKey` |
| `try.bogusKey` (region) | refused, region check | refused, region
check |
| `retry: 5` | refused, parse | refused, parse |
Every variant registers or is refused exactly as before; only the judge
moved. A direct `FlowSchema.parse` or `defineFlow()` (no conversion) now
meets the tombstone for a pre-17 `retryDelayMs`, like every other
retired spelling.
## Measured at the CLI door
Fixture projects (`defineStack(…, { strict: false })`), at `9f71db481`,
with the built CLI closure:
| fixture | `os validate --json` |
|:--|:--|
| `try_catch` `retry: { maxRetry: 2 }` | exit 1, `custom` at
`flows.0.nodes.1.config.retry.maxRetry` |
| job `retryPolicy` with `maxRetry: 3` | exit 1, `unrecognized_keys` at
`jobs.0.retryPolicy` |
| every declared key | exit 0 |
| pre-17 `retryDelayMs` alone | exit 0 (`retry-policy-converged`
converted it) |
`examples/app-showcase` validates clean (exit 0, `valid: true`; 2
`try_catch` with `retry`, 1 job with `retryPolicy`), and so does
`examples/app-todo` (exit 0).
## Ablation
At HEAD `9f71db481`, `RetryPolicySchema` was mutated back to a plain
`z.object` through `scripts/ablation-replace.mjs`: anchor 1 → 0, blob
`c06e5bd1a6` → `dbd76790f2`. The spec was rebuilt, and
`ablation-dist-preflight` found the marker present in 20 built files.
Results:
- validate-door pin
(`packages/cli/test/retry-policy-key-validate-door.test.ts`): **1
failed, 1 passed**. The refusal went red (exit 0 where exit 1 was
expected) and the control held.
- spec pins: 6 failed, 49 passed.
- registration pins: 2 failed, 14 passed. These include "registerFlow
widens nowhere": with the strict policy reverted, the arm sees no
unknown key and the walk stands aside, so the mutation is exactly the
widening the card names.
Restore: the blob is back to `c06e5bd1a6` == HEAD and `git diff HEAD` is
empty. After a rebuild, the preflight with `--absent` found the marker
in none of 232 built files and the tree clean, and the door pin was
green again (2 passed).
## Tests (real readings)
At `b7fdd5085` (merged with `origin/main` `6a53564b9`):
- `@objectstack/spec`: 626 files, 18751 passed, 1 todo.
- `@objectstack/service-automation`: 178 files, 2179 passed.
- `@objectstack/lint`: 128 files, 5853 passed.
- CLI door pin, `--project integration`: 2 passed.
- typecheck for `spec`, `service-automation` and `cli`: exit 0. The
test-layer debt is unchanged (spec 52 files / 246 errors held, cli 3 /
28).
At `c915191dc` (the in-place fix round, no merge of `main`):
`@objectstack/spec` `src/automation` plus `retry-policy.test.ts`, 34
files, 1134 passed, and the spec typecheck exits 0 (debt held at 52 /
246). Ablation: putting back "fewer" with `scripts/ablation-replace.mjs`
turned the `flow.test.ts` pin red (1 failed). The blob was restored to
`c4c6c4453d` == HEAD.
At `a77cb02b1`: `@objectstack/service-job` 11 files, 117 passed. The
`@objectstack/cli` unit tier, 270 files, 3968 passed; it includes the
tier-partition pin, and the new door pin lands in `integration` by
behaviour. It runs per PR because it is not named `.e2e`.
Narrowed eslint (`--no-inline-config`) over the 12 changed `.ts` files:
12 files, 0 errors, 0 warnings, none ignored. This config enables no
type-aware linting (`eslint.config.mjs:327`), so the diff cannot move
any untouched file's verdict. The repo-wide `pnpm lint` is CI's.
## Gates
`node scripts/pm/dispatch-gates.mjs --commands` at `b7fdd5085` derived
115 commands, the same list as at `a77cb02b1`, and all 115 ran with exit
0. The `--ran` verdict: `115 derived famil(ies) accounted for — 115 run,
0 NOT-MEASURED (a DERIVED zero — all 115 recorded an exit code and none
of them is 3)`.
At `c915191dc`, the round's 4 paths derived 85 commands, every one of
them already among the 115. The full derivation is unchanged at 115. All
85 exited 0 (`--ran`: 85 run, 0 NOT-MEASURED). Ten dist-reading gates
first exited 3 on a fresh worktree, then exited 0 after a build.
Two gates needed a second run:
- `check:type-check-debt` hit the per-command 300 s cap. Re-run alone,
it exited 0 in 123 s.
- At `a77cb02b1`, five spec gates exited 3 (spec `dist` predated a test
edit) and `check:dual-build-cjs-loads` exited 3 (8 packages had no
`dist`). After building them, all six exited 0.
The roster gates whose roster sits under a touched directory also exited
0: `check:meta-url-spelling`, `check:authz-resolver`,
`check:error-code-casing` and `check:filter-alias-parity`.
## Acceptance notes
- `Flow.errorHandling` gets no did-you-mean for `maxRetry`, the same
distance gap closed here on the shared policy. Its own alias table has
no such row. It is a sibling surface outside this card's scope, and the
key is still refused loudly. Carrier: none.
- A `script` node carrying a retired dispatch key (`actionType`, …) is
refused at error, with exit 1, by `os validate` and `os lint` (rule
`expression-invalid`, measured at `6fbc005a6`). The save door answers
200 and stores the body with the key removed: the retired conversion
`flow-node-script-branch-keys-removed` replays on the save path and logs
`OS_METADATA_CONVERTED`. So nothing reaches the run from either door.
`defineStack` and a direct `registerFlow` were not measured.
`RETIRED_KEYS_JUDGED_ELSEWHERE` keeps the key arm out of `script`
deliberately. Carrier: none (finding 9 of the contract review
`6072339018`, measured in round 3 `6073081304`).
- Two runtime test comments still say the registration walk judges "only
those (and `try_catch`)":
`packages/runtime/src/domains/automation-put-post-error-parity.test.ts:50`
and `automation-register-error-class.test.ts:79`. After this PR, the
walk judges plugin node types only. They are comments in test files that
do not ship. Carrier: the next edit of either file.
## Line budget
17 files, +846 / −143 against the merge base (the in-place fix round: 4
files, +13 / −4; round 3: the changeset's `Clause-②:` value, +1 / −1). 0
governed paths.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01DhTqaEHqPVSVnAkjG3jywn)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent fdfdd7e commit ac8f2c5
17 files changed
Lines changed: 846 additions & 143 deletions
File tree
- .changeset
- content/docs/automation
- docs/audits
- packages
- cli/test
- services/service-automation/src
- builtin
- spec/src
- automation
- migrations
- entries/semantic
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
138 | 138 | | |
139 | 139 | | |
140 | 140 | | |
141 | | - | |
| 141 | + | |
142 | 142 | | |
143 | 143 | | |
144 | 144 | | |
| |||
151 | 151 | | |
152 | 152 | | |
153 | 153 | | |
154 | | - | |
155 | | - | |
156 | | - | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
157 | 157 | | |
158 | 158 | | |
159 | 159 | | |
160 | 160 | | |
161 | | - | |
162 | | - | |
163 | | - | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
164 | 164 | | |
165 | 165 | | |
166 | 166 | | |
| |||
800 | 800 | | |
801 | 801 | | |
802 | 802 | | |
| 803 | + | |
| 804 | + | |
| 805 | + | |
| 806 | + | |
| 807 | + | |
| 808 | + | |
803 | 809 | | |
804 | 810 | | |
805 | 811 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
391 | 391 | | |
392 | 392 | | |
393 | 393 | | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
394 | 399 | | |
395 | 400 | | |
396 | 401 | | |
| |||
0 commit comments