fix(spec,service-automation)!: errorHandling.maxRetries has one default, and strategy: 'retry' states its count (#4247) - #4266
Merged
Conversation
…ault, and `strategy: 'retry'` states its count (#4247) `flow.errorHandling.maxRetries` was declared twice with different values: `FlowSchema` said `.default(0)`, while the engine's `retryExecution` read `errorHandling.maxRetries ?? 3`. `??` fires only on `undefined`, so the winner was decided by the ROUTE a flow took into the engine, not by what its author wrote — 0 retries for a flow parsed by the schema, 3 for a definition built by hand and handed to the engine. The neighbouring `retryDelayMs ?? 1000` / `backoffMultiplier ?? 1` agreed with their `.default()`s; only `maxRetries` disagreed. The engine now keeps no defaults of its own: `retryExecution` takes the parsed `NonNullable<FlowParsed['errorHandling']>` and destructures all five knobs, no `??`. That is safe because `AutomationEngine.flows` only ever holds `FlowSchema.parse` output (`registerFlow` parses; the version-history rollback re-seats an already-parsed snapshot), and the parsed parameter type is what keeps a second set of defaults from growing back — a knob the spec stops defaulting becomes a compile error rather than a silent engine-side guess (Prime Directive #12). BREAKING: `strategy: 'retry'` now requires `maxRetries` >= 1. With the engine's copy gone an unstated count is unambiguously 0, and 'retry' with 0 attempts runs the flow once and stops — `strategy: 'fail'` under another label, a declared capability the runtime does not deliver. Rather than pick 0 or 3 for the author, the schema refuses the combination in both spellings (omitted → defaulted 0, and an explicit 0) with the prescription in the message; a retry re-runs the WHOLE flow, side effects included, so the count is the author's to state. `maxRetries: 0` stays legal under 'fail'/'continue', which never read it. The migration chain surfaces this as the semantic TODO `flow-retry-max-retries-required` — there is no lossless rewrite, so `os migrate meta` delegates the choice instead of guessing it. Closes #4247 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SNpSdWvbikA9PiwXzkAH4i
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 106 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4247.
先说一个把结论改了的发现
issue 把两条路径列成并列的两种可能。实际读了调用图之后，其中一条不存在：
retryExecution只有一个调用点（execute()里flow.errorHandling?.strategy === 'retry'），flow来自this.flows；而this.flows只有两处写入 ——registerFlow（FlowSchema.parse之后）和版本回滚（重新放回一份已经 parse 过的快照）。没有绕过 schema 喂给引擎的路，所以?? 3在今天的调用图里根本够不到。这不是"少了一件事要修"，而是把问题换了一个：真正在跑的行为是唯一那条 ——
strategy: 'retry'不写次数 ⇒maxRetries被.default(0)填成 0 ⇒ 一次都不重试。也就是 issue 末尾那个"顺带"其实是主症状：声明了retry，运行时不 retry，正是 Prime Directive #10 推论说的"不要展示运行时并不交付的能力"。（
?? 3仍然要删 —— 它是第二份契约，随时可能因为新增一条载入路径而复活成真 bug。）改了什么
1. 引擎不再自带默认值（PD #12）。
retryExecution的参数从"五个 optional 字段 + 五个??"改成 parse 后的NonNullable<FlowParsed['errorHandling']>，直接解构。这样安全，是因为上面那条不变量（this.flows只装 parse 输出）；而用 parsed 类型做参数，也是防止第二套默认值长回来的机制 —— 哪天 spec 不再给某个旋钮默认值，这里是编译错误，不是又一次"引擎悄悄猜一个"。2.
strategy: 'retry'必须写maxRetries >= 1（BREAKING）。 引擎那份删掉之后，"不写"就明确是 0 了 —— 而重试 0 次就是strategy: 'fail'换个名字。这里没有替 owner 在 0 和 3 之间选，而是让"不写"这个状态不再合法：schema 用superRefine拒绝两种拼法（省略→默认 0，和显式写 0），错误信息带处方。为什么是"拒绝"而不是"默认 3"：flow 级重试是整条流程从头再跑一遍，已经成功的 CRUD 节点会再执行一次、外呼会再发一次。给只写了
strategy: 'retry'的存量流程静默改成跑 3 遍，是一次无声的行为变更；拒绝则是一行就能修的响亮失败，且作者拿回的正是他本来以为自己声明的东西。maxRetries: 0在'fail'/'continue'下仍然合法（它们根本不读）。3. 迁移链把它作为 semantic TODO 暴露。 加了
flow-retry-max-retries-required：这一项没有无损重写 —— 填 0 保住了 parsed 路径的实际行为却违背作者写下的字面意思，填任何正数都是一个新决定。所以os migrate meta报给你，而不是替你改。（这条是被packages/cli/test/migrate-meta.e2e.test.ts逼出来的：它断言迁移产物在当前 schema 下可解析，而 fixture 的{ strategy: 'retry', fallbackNodeId: 'n9' }去掉fallbackNodeId之后正好撞上新约束 —— 一个很好的证据，说明这类元数据确实存在于升级路径上。）改动清单
packages/spec/src/automation/flow.zod.tssuperRefine拒绝'retry'+maxRetries < 1；.describe与注释写明"这里是唯一的默认值来源"packages/services/service-automation/src/engine.tsretryExecution的五个??，参数改为 parsed 类型packages/spec/src/migrations/registry.tspackages/spec/liveness/flow.jsonmaxRetries条目补 note/verifiedAt —— 双默认值这件事记进账本packages/cli/test/migrate-meta.e2e.test.tsmaxRetries: 2（它测的是fallbackNodeId移除，不是重试次数）content/docs/automation/flows.mdxstrategy: 'retry'得说重试几次"小节；讲清maxRetries数的是重跑次数不是总次数content/docs/releases/v17.mdxdocs/protocol-upgrade-guide.md,packages/spec/spec-changes.jsongen:upgrade-guide/gen:spec-changes重跑测试
新增
packages/services/service-automation/src/flow-retry-attempt-count.test.ts（6 例）—— 钉住的是从 schema 看不出来的那部分：次数本身。maxRetries: 2⇒ 总共跑 3 次（既不是旧 parsed 路径的 1 次，也不是旧?? 3的 4 次）；两种非法拼法在registerFlow被拒；'fail'下写满旋钮仍然只跑一次；以及"引擎拿到的 block 五个字段都是实值"这条让删??得以成立的不变量。spec 侧
flow.test.ts新增#4247describe（5 例），并修正一处原本用{ strategy: 'retry' }当载体去测 backoff 默认值的用例。@objectstack/spec：276 files / 7154 tests ✅@objectstack/service-automation：44 files / 482 tests ✅migrate-meta.e2e：6/6 ✅check:liveness、check:docs、check:authorable-surface、check:spec-changes、check:upgrade-guide、check:strictness-ledger、check:skill-examples、check:doc-authoring、check:nul-bytes、check:release-notes全绿；改动文件 eslint 干净两条与本 PR 无关的环境噪音
pnpm --filter @objectstack/spec check:generated在main上就是红的（stash 掉全部改动复验过）：check:strictness-ledger是 feat(spec): 让 #4001 严格性账本接受机器校验（首次运行抓到 11 处漂移） #4232 新加的脚本，没有在check-generated.ts的GATED/NO_GENERATOR里分类。这个 PR 的 CI 不会因此变红 ——Check Generated Artifactsjob 跑的是check:skill-docs/check:spec-changes/check:upgrade-guide/check:authorable-surface，check:generated本身没有任何 workflow 调用。已由 check:generated 的双向对账从不在 CI 上执行——#4203 关闭次日即被 #4232 原样复现，main 的本地 wrapper 又红了 #4255（正是"从不在 CI 上执行"这一点）、check:generatedfails on main:check:strictness-ledger(#4232) was never classified in its ledger #4265、[P3] spec:check:generatedfails its own ledger reconciliation —check:strictness-ledgeris unclassified #4267 覆盖，不在本 PR 范围内。@objectstack/cli有 8 个 e2e 用例失败（serve-no-artifact、serve-boot-diagnostics、emit-json-pipe、sqlite-occupancy）：只在我这个容器里，干净树上一模一样地失败 —— 工作区没有全量构建导致的，CI 上不复现。