Repository navigation
Conversation
Child provider retries now admit only through durable continuation. The admission-mode selection in retryChildAgentWithExecution collapses into a single fail-closed guard that rejects an incomplete continuation composition before any claim or provider dispatch, and the legacy planner and runner paths are deleted. Generated-by: ZCode (GLM)
📝 WalkthroughSummaryThis PR removes the Child retries now use durable continuation admission only. The runtime rejects incomplete continuation composition before claims, Run creation, or provider dispatch. This prevents retries when continuation authority or safety inspection is unavailable. The PR extends the existing durable continuation source of truth. It does not create a parallel retry path. The legacy planner, runner, fallback logic, runtime-event replay, and admission-mode selector were removed. This is the smallest coherent solution for fail-closed behavior. The deleted legacy path and its dedicated tests can remain removed because existing coverage preserves provider rate-limit retries, continuation recovery, linked-child behavior, retry limits, backoff policy, root-turn semantics, and provider transports. The new regression test covers incomplete continuation configurations. Validation
Review-relevant risksThe public Directly constructed No other protected-area effect was identified in the current diff. The person performing the merge must review the final diff, and a maintainer makes the final determination. WalkthroughHosted child provider retries now use only durable continuation admission. Missing continuation authority or safety inspection causes failure before claim, run creation, or provider dispatch. The legacy retry planner, runner, and admission selector were removed. ChangesChild retry admission
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The PR removes the unsupported legacy retry fallback and fails closed when continuation dependencies are incomplete; the supplied checks pass, and only minor cleanup comments remain. No actionable merge-blocking risk remains. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/runtime/src/runtime-kernel.ts (2)
1285-1369: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared continuation claim and start-commit body.
Lines 1293-1369 now duplicate
resumeContinuationClaimedlines 785-893 almost exactly: samecontinuationTargetRunHeaderForExecutioncall, samecontinuationClaimForExecution, same conflict error shape, and an identicalcommitContinuationStartevent payload. With the legacy branch removed, there is no longer a reason for two copies. A future fix to the admission event payload would have to land twice, and a missed copy would weaken one lane silently.Extract one private helper that takes the continuation, session header, user input, orchestration, tool mode, and tool-boundary protocol, and returns
{ claim, claimedRunHeader, commitContinuationStart }. The explicitdurableAdmissiontype annotation then becomes unnecessary because the helper declares the shape.
1704-1712: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDelete the unreachable residue of the removed legacy branch in
runAgentContinuation. Both sites still defend against states that the durable-only path can no longer produce, because the legacybeginOperationlane is gone.
packages/runtime/src/runtime-kernel.ts#L1704-L1712: makerevalidateSafetya required parameter and delete the'Durable continuation omitted final safety revalidation'guard; both call sites (line 925 and lines 1441-1445) already pass it.packages/runtime/src/runtime-kernel.ts#L1774-L1789: replace the'continuationStartAdmission' in begintest and the throwing IIFE withbegin.continuationStartAdmission;beginContinuationalways returns that property.docs/architecture/runtime-resume-extraction-ledger.zh-CN.md (1)
353-366: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value删除重复陈述。
第 353-356 行已经完整说明:
legacy_provider_retrylane 已移除,child provider retry 只走 durable continuation 准入,组合不完整即 fail closed。第 363-366 行再次重复同一结论与同一移除理由。第 362 行的首句(关于 hosted auto-resume 尚不可默认启用)是独立信息,应保留;其后的重复段落可以删除,避免两处描述日后各自漂移。♻️ 建议修改
-- 在上述 composition/owner 测试完成前,不能把当前切片描述为 hosted auto-resume 已默认可用。 - `legacy_provider_retry` 兼容 lane(仅维持升级前的 provider 429 重试能力,不具有 durable - continuation、跨进程 exactly-once 或 crash-resume 承诺)已在 host authority lifecycle - integration 完成 typed authority composition 后移除;child provider retry 现在只走 - durable continuation 准入,组合不完整即 fail closed。 +- 在上述 composition/owner 测试完成前,不能把当前切片描述为 hosted auto-resume 已默认可用。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 368b926e-0a5e-4826-97a0-c337f7bae5c4
📒 Files selected for processing (6)
docs/architecture/runtime-resume-extraction-ledger.zh-CN.mddocs/architecture/runtime-resume-phase3-phase4-workspace-checkpoint-design.zh-CN.mdpackages/runtime/src/__tests__/session-manager.test.tspackages/runtime/src/runtime-kernel.tspackages/runtime/src/runtime-runner.tspackages/runtime/src/session-manager.ts
💤 Files with no reviewable changes (1)
- packages/runtime/src/runtime-runner.ts
Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.
|
Thanks for the cleanup — this is a clean, well-scoped deletion. I reviewed it end-to-end (references, tests, and the production composition claim). Conclusion: PASS from review. No P0/P1/P2. A few optional nits. Verified:
Optional nits:
AI-assisted review disclosure: this review was produced with AI assistance (pi review subagent on 中文摘要(AI 辅助审查)结论:PASS,无 P0/P1/P2,可合并。删除彻底(legacy 相关符号全仓 0 残留),生产组合确实总是双全(SQLite continuation authority + safety inspector),无死测试;新回归测试在 main 上必红,是真回归测试。仅 4 个 P3 可选建议:更新 session-manager.ts:4110 的过时错误文案、简化 runtime-kernel.ts 中恒真的 |
Summary
Removes the legacy
legacy_provider_retryadmission lane for child provider retries so durable continuation is the only path, per #3035. Production Runtime Hosts always provide both the SQLite continuation authority and the safety inspector, so host behavior is unchanged.ChildAgentRetryInputno longer carries anadmissionMode;retryChildAgentWithExecutionreplaces the mismatch/fallback selection with a single fail-closed guard that rejects an incomplete continuation composition (authority or safety inspector missing) before any claim, Run creation, or provider dispatch.planLegacyChildProviderRetry()(session-manager) andrunLegacyProviderRetry()+LegacyProviderRetryRunOptions(runtime-runner); the durable claim + continuation-start block instartChildRetryClaimedandrunAgentContinuationis now unconditional.Behavior change: a directly constructed
SessionManagerwithout a complete continuation composition no longer retries child provider failures — it fails closed. This is not a supported product surface (per #3035).Fixes #3035
Verification
npm run format:check— clean (8 formatting nits auto-fixed and folded in)npm run lint— 2304 files, no issuesnpm run typechecknpm --workspace @maka/runtime run build— cleannpm --workspace @maka/runtime run test:dist— 2983 tests, 2971 pass, 12 skipAI use
Select exactly one:
Tool(s) and scope: ZCode (GLM) — analyzed the retry admission paths and issue #3035, implemented the removal across runtime/runtime-runner/session-manager and the regression test, ran the verification suites. Human contributor of record reviewed the change and is responsible for the submission.
Checklist
Does this PR entail a change in behavior?