fix(cli): point the lifecycle smoke at the module that owns refresh-state - #4534
Conversation
…tate `examples/cli-project-lifecycle-command-modularization-smoke.py` fails on `main` with `project lifecycle module missing retry it before delivery`. The marker is not missing: #4521 moved the `refresh-state` command into `loopx/cli_commands/project_lifecycle_refresh_state.py`, which is where the string now lives, while the smoke still required it in the dispatcher module. The check now requires that marker, and the `refresh-state` command name, in the module that owns the command, and keeps requiring the dispatcher markers where they still are. Nothing is weakened: the marker is still required, and a regression that drops it from the refresh-state module still fails. This red blocked the pre-merge gate for any diff that selected the smoke, which is why it is worth fixing rather than working around. Verified: examples/cli-project-lifecycle-command-modularization-smoke.py ok (it was the failing check). Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
一、变更内容
examples/cli-project-lifecycle-command-modularization-smoke.py:新增REFRESH_MODULE指向loopx/cli_commands/project_lifecycle_refresh_state.py,把属于该命令的 marker(retry it before delivery)与命令名refresh-state改为在拥有该命令的模块里断言;dispatcher 模块(project_lifecycle.py)仍断言它自己仍然拥有的 marker。- 仅测试期望修正,未改任何产品代码、权限或默认值。
二、依据与一致性
- 事实核对:
grep -rn 'retry it before delivery' loopx/显示该字符串存在于loopx/cli_commands/project_lifecycle_refresh_state.py:685,711,即 #4521 把 refresh-state 命令拆到独立模块后,字符串随之迁走,而 smoke 仍在 dispatcher 模块里找它——所以这条红是过期断言而不是产品回归。 - 没有放宽检查:该 marker 仍然被要求(只是要求它出现在正确的模块),从 refresh-state 模块里删掉它仍会让检查失败。这与仓库"不要靠放宽门禁解决 self-repair"的要求一致。
- 影响面:这条红会在任何选中该 smoke 的 diff 上硬失败,从而阻断其它 lane 的自合并;修的是测试期望,不改变任何运行时语义。
三、验证
examples/cli-project-lifecycle-command-modularization-smoke.py:ok(原失败项)。loopx canary premerge --from-git-diff:merge_gate_passed=true、self_merge_allowed=true、manual_holds=0、failures 0;1 条 advisory(已知 maintainability ratchet 基线 finding,按门禁要求记录,不阻断本 diff)。
四、风险与残余缺口
- 风险极低:单文件、断言改位置。
- 故意未处理的另一条既有红:
control-plane-maintainability-ratchet-smoke.py的两处 unreviewed finding(loopx/extensions/lark/goal_topic_runtime.py的 module metric budget、loopx/control_plane/quota/should_run_prepare.py:_prepare_quota_should_run_item的 oversized decision function)。接受或修掉它们属于拥有这些文件的 lane 的判断,因此我只登记、不越权改,也不会用"reviewed exception"条目把它掩盖过去。
五、结论
批准以 admin squash 合并(self_merge_allowed=true)。单目的、可回滚、无行为变化;它移除的是 main 上会阻断他人自合并的过期断言,并明确把另一条需要 owner 判断的红留给相应 lane。
English verdict: Approved for an admin squash merge. The lifecycle smoke was asserting a marker in a module that no longer owns the refresh-state command after #4521 moved it; the assertion now targets the owning module, the marker is still required, and the previously failing smoke plus the canary premerge gate are green. The unrelated maintainability-ratchet red is deliberately left to the lanes that own those files rather than masked with a reviewed-exception entry.
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
动机
问题是真的,我在父提交上复现了同样的失败:AssertionError: project lifecycle module missing retry it before delivery。marker 并没有丢,只是随 #4521 一起搬到了 loopx/cli_commands/project_lifecycle_refresh_state.py,而 smoke 仍在 dispatcher 模块里找它——于是任何选中这个 smoke 的 diff 都会红。
恒红的 gate 的代价前面已经说过:它要么挡住无关 diff,要么让人学会不选这个 smoke,而后者正是 gate 存在的意义被抵消。
改动思路
修法就是"把期望搬到拥有该命令的模块",并且没有顺手放宽:marker 仍然被要求,只是要求在新的归属地;同时新增了 refresh-state 这个命令名标记,把"这个模块拥有这条命令"也钉住。dispatcher 侧原有的标记列表(PROJECT_LIFECYCLE_COMMANDS、register_project_lifecycle_commands、handle_project_lifecycle_command 与四个命令名)保持不变。
具体改动
examples/cli-project-lifecycle-command-modularization-smoke.py:新增 REFRESH_MODULE 常量与一段针对该模块的 marker 循环(retry it before delivery、refresh-state),并从 dispatcher 的 marker 列表里移除已搬走的那一条。+9/-1,单文件,无产品代码。
我做的验证:本 head 运行该 smoke → cli-project-lifecycle-command-modularization-smoke: ok;父提交运行同一文件 → 正是 PR 正文引用的那条断言;git diff --check 干净。我也确认了这次改动没有削弱其它断言——禁止泄漏到 cli.py 的标记列表、四个命令的 --help 选项检查、以及对 refresh-state/read-only-map/reward/operator-gate 的真实 dry-run 调用与"run index 未被改动"的断言都原样保留;也就是说丢失 dispatch 或 dry-run 误写这类真回归仍然会被抓到。
关于"这个 smoke 是否值得保留",我做了覆盖扫描:cli-command-module-size-ownership-command-modularization-smoke.py(行预算 + 重复注册)与 tests/test_project_lifecycle_refresh_state_ownership.py(命令集归属、只注册一次、其它命令穿透)覆盖的是预算与注册面;本 smoke 覆盖的是命令族的 marker 归属 + 真实 CLI 调用 + 不写状态,没有其它工件做这三件事,因此不构成重复,也不属于一次性脚手架。
另外我扫了同一作者近期的 PR 形状:#4718「fix(smokes): restore the three public smoke contracts broken on main」是同一形状(修复被别的改动弄坏的 smoke 契约),#4531 是相邻的 fallout 修复。两者都在恢复真实守卫、不是批量生产新脚手架,所以这属于反复出现的维护形状而不是 batch farming;不需要升级为贡献限制,但要落一条具体的过程修复:任何搬动命令归属的 PR 都必须跑这些基于 marker 的 smoke(我在 #4521 的审查里已记录同样结论),若再出现第三次则值得把该警告升级。
对主干的风险
几乎为零:单文件、只有 smoke 期望被搬迁,不触任何产品路径。风险面只有"期望搬错地方导致 smoke 变绿但不再守卫"这一种,而这里 marker 依然被要求、周边断言原样保留,我用父/head 双向运行验证了它确实在守卫真问题。
更值得记住的是复发的根因:这类 smoke 用硬编码字符串钉住"哪条命令属于哪个模块",只要再搬一次命令就会再红一次。若这个列表继续变旧,更强的形式是从 PROJECT_LIFECYCLE_COMMANDS 与注册扫描推导期望集合,而不是继续维护字符串——这是有界的后续改进,不是删掉检查的理由。
我的整体评价
这是一次干净、必要的期望修复:真实 red gate、单文件九行、marker 搬家而不放宽,并且保留了真实调用与"不写状态"的断言;我用父提交与 head 两个方向复现过失败与通过。覆盖扫描显示它不与其他 smoke 重复,保留价值成立。
唯一建议是把"搬命令归属必须跑 marker 类 smoke"固化成流程项(P3 级别的过程建议,已写在审查记录里),不构成合并阻塞。
English verdict: APPROVE (exact head a78426d)
Problem
main's pre-merge gate is red for any diff that selectsexamples/cli-project-lifecycle-command-modularization-smoke.py:The marker is not missing. #4521 moved the
refresh-statecommand intoloopx/cli_commands/project_lifecycle_refresh_state.py, which is where that string now lives, whilethe smoke still required it in the dispatcher module
project_lifecycle.py.What changed
retry it before deliveryand therefresh-statecommand name in the modulethat owns the command, and keeps requiring the dispatcher markers where they still are.
refresh-state module still fails the check.
Validation
examples/cli-project-lifecycle-command-modularization-smoke.py: ok (was the failing check).loopx canary premerge --from-git-diff:merge_gate_passed=true,self_merge_allowed=true,manual_holds=0, failures 0 (one advisory: the known maintainability-ratchet baseline finding,recorded and not blocking for this diff).
Boundaries
Test-expectation fix only, no product code, no authority, no default change. Its value is that the red
stops blocking other lanes' self-merge. The other inherited gate red — the maintainability ratchet's
two unreviewed findings in
loopx/extensions/lark/goal_topic_runtime.pyandloopx/control_plane/quota/should_run_prepare.py— is deliberately not touched here: accepting orresolving those needs the judgment of the lanes that own those files, and it is tracked separately
rather than papered over with a reviewed-exception entry.