test(turn-driver): pin the host-failure retry ceiling and unmapped policies - #4951
huangruiteng merged 2 commits into
Conversation
…pt range The suite reached at most attempt=3 over three of the five retry policies, so the 300-second ceiling never executed, executor_timeout (the only two-attempt policy) and transport_lost were absent entirely, and the attempt-type guard and rebuilt-record isolation had no assertion. Pin the ladder, the cap, both missing policies, and the two fail-closed boundaries. Signed-off-by: Yue Dai <54579099+yuedai-pbc@users.noreply.github.com>
Signed-off-by: Yue Dai <54579099+yuedai-pbc@users.noreply.github.com>
|
Follow-up: the full local suite on this revision (
None of the 92 is in the file this PR touches, or in any module it reads. Composition and the two independent facts I checked instead of asserting a parity number: 90 of 92 —
Each raises 1 of 92 — Reproduces when that file is run on its own, i.e. with this PR's test file absent from the selection ( What I did not measure, stated plainly: I did not run the same 92-failure set against an unmodified checkout of this base revision, so I am not quoting a "identical failure set before and after" number — the two controls above (selection independence, and an explicit missing-dependency error) are what I actually have. CI runs the suite on the merge ref, which has the Node dependencies installed. Everything else passed: 11497 tests, including the 20 in |
huangruiteng
left a comment
There was a problem hiding this comment.
动机
既有 host_failure.py 已发布带上限的重试提示,但原测试最多走到第 3 次尝试,只覆盖 5 种可重试策略中的 3 种,也没有把非法 attempt、伪造 retryability 与标准化后的对象隔离固定下来。本 PR 是补现有行为的回归测试,不改生产策略。
改动思路
沿用现有 tests/test_loopx_turn_host_failure.py,直接断言 ceiling、executor_timeout 两次上限、transport_lost 梯度、executor journal 的 producer、空投影、返回对象重建和两条拒绝路径。测试位置贴近策略所有者,没有另造 smoke 或协议层。
具体改动
唯一改动是测试文件新增 95 行;没有生产、配置或文档更改。exact head 187df5a6cf2394913935338da1f22f4c61cfce4a 的单文件测试 20/20、相邻 Turn/host/controller 七模块 438/438、Ruff 和风险分层 premerge 均通过;基线单文件是 8 个测试。现有 executor/controller 测试覆盖首次提示,但没有重复断言本次补充的上限和拒绝分支。作者同期的 #4955 检查另一项 public-safety 契约、#4954 处理仓库页面卫生,未见同形态重复拆 PR。
对主干的风险
没有发现阻断项。第 4/10 次 rate_limited 的 300 秒断言保护的是超出自动重试上限时仍可被记录/回读的有界提示,不代表控制器会安排第 4 次自动重试;测试同时明确断言 retry_available=false。新增测试直接调用策略和 journal producer,没有声称验证真实 host 的唤醒时序或 TypeScript journal 镜像;那两处本 PR 没有修改。未跑远端 CI:Goal 的当前评审策略为 wait_for_ci=false,本地所需验证已通过。
我的整体评价
这是对已有、正在使用的重试契约的聚焦持久化测试,覆盖空白明确、相邻路径通过,规模与收益相称。我未找到需要本 PR 修复的问题,批准当前 head。对相邻 owner 做未来导向检查后,保持测试留在现有模块比抽象新 helper 更合适;批准不代表合并授权。
English verdict: APPROVE - exact head 187df5a; focused retry-policy coverage is durable and non-duplicative. 20 focused and 438 adjacent tests, Ruff, and premerge validation passed; no production behavior changed.
Goal And Delivered Outcome
build_host_failure_record()publishes the bounded retry hint thatthe outer Turn controller reads to decide when to wake the same Turn again. The suite
reached at most
attempt=3, and only for three of the five retryable kinds, so themin(..., 300)ceiling inloopx/control_plane/turn_driver/host_failure.py:84-87neverexecuted — the largest value any existing case could produce is
60 * 2**2 = 240.executor_timeout(the only two-attempt policy) andtransport_lostwere absent fromcoverage entirely, and the
attempttype guard, the rebuilt-record boundary and bothretryability-mismatch exits had no assertion anywhere in
tests/.min()call, widening the ceiling to3000, turningexecutor_timeoutinto(3, 5),removing the
isinstance(attempt, bool)exclusion, returning the caller's mapping fromnormalize_host_failure_record(), shifting the exponent, or deleting eitherretryability exit all passed the suite. After, each of those eight edits fails at least
one test — see the
regression_parityrow.regression are self-contained. Base is current
main(eb16c5443). No production fileis modified.
Scope And Continuation
both previously unmapped policies, the producer path the executor journal actually uses
(
record_host_failure()atloopx/control_plane/turn_driver/executor.py:793), the emptyprojection, the attempt-type guard and the two retryability-mismatch exits. Not claimed
here: the mirrored TypeScript journal builder and the outer controller's wake timing —
both consume these hints rather than compute them, so their contracts are unchanged by
this PR and were left alone deliberately.
new unpinned caller-visible behavior rather than a larger version of this file.
Validation
187df5a6cunitpassedpython -m pytest -q tests/test_loopx_turn_host_failure.py→ 20 passed: the 7 pre-existing cases plus 6 new functions (one parametrized over 5 rejectedattemptshapes). Covers the ceiling, both missing policies, the journal producer, the empty projection and the three validation guards.unitpassedpython -m pytest -q tests/test_loopx_turn_host_failure.py tests/test_loopx_turn_executor.py tests/test_loop_turn_loop_controller.py tests/test_loop_turn_controller_contract.py tests/test_loopx_turn_journal_inspection.py tests/test_dsh_goal_mode.py tests/capabilities/test_codex_subagent_host_capacity.py→ 437 passed / 0 failed. These are every Python module that classifies a host failure into one of these kinds or reads the retry hint back.staticpassedpython -m ruff checkclean;ruff format --checkclean on the touched file;python -m mypy(same no-argument invocation as CI) → success in 22 source files;loopx canary premerge --from-git-diff→ public boundaryok: true, 0 failures;python -m pytest -q tests/canary→ 25 passed with the reviewed module findings unchanged.regression_paritypassedhost_failure.py, one edit at a time, on this revision: dropmin(...)→ 2 failed; ceiling300→3000→ 2 failed;executor_timeout (2, 5)→(3, 5)→ 1 failed; exponentattempt-1→attempt→ 6 failed; remove theboolguard → 1 failed;normalizereturns the caller's mapping → 1 failed; delete the "non-retryable must not declare retry policy" raise → 1 failed; delete the retryability/kind consistency raise → 1 failed. Reverting the module → 20 passed. Failing-before holds by construction:grep -n "300|executor_timeout|transport_lost|record_host_failure" tests/test_loopx_turn_host_failure.pyreturned 0 matches on the base revision.real_entrypointnot_runloopx turn.integrationrunningpython -m pytest -q tests/on this revision is running locally; the aggregate will be posted as a follow-up comment on this PR rather than claimed here.every reader of the retry hint it pins. Not covered and not changed: scheduler wake
timing and the TypeScript journal mirror (they consume the number). No known-failing
local baseline is attributed to this PR; if the full run surfaces one, it will be posted
with its file and name rather than smoothed over.
Frontend / Visual Evidence
Type of Change
LoopX Area
Technical Direction
(coverage of an already-shipped bounded-retry contract). No roadmap id claimed.
Shared-authority RFC fixture impact
promotion, runtime-routing or compatibility projection.
Boundary Checklist
.loopx/,.codex/goals/, and liveACTIVE_GOAL_STATE.md).none.Signed-off-bytrailer (git commit -s).