Skip to content

test(chat): patch the refresh-state rules where they now live - #4566

Closed
huangruiteng wants to merge 1 commit into
mainfrom
codex/goal-channel-test-live-owner
Closed

huangruiteng wants to merge 1 commit into
mainfrom
codex/goal-channel-test-live-owner

Conversation

@huangruiteng

Copy link
Copy Markdown
Collaborator

What

Restore the nine red tests/cli_commands/test_project_lifecycle_goal_channel.py cases on main. The patch targets moved, the assertions did not.

Why they were red

#4521 moved the refresh-state parser, rules, and dispatch into loopx/cli_commands/project_lifecycle_refresh_state.py and deliberately left no re-export behind, so project_lifecycle no longer carries refresh_state_run, sync_explore_graph_after_material_refresh, sync_human_gate_after_refresh, read_heartbeat_settlement, settlement_result_payload, or resolve_runtime_root. The nine tests still patched those names on the dispatching module, so they failed with AttributeError: module 'loopx.cli_commands.project_lifecycle' has no attribute 'refresh_state_run' even though the behaviour under test was intact. tests/test_project_lifecycle_refresh_state_ownership.py — added by the same PR — already names project_lifecycle_refresh_state as the owner.

Change

A test-only change: each patch targets the owning module, and handle_project_lifecycle_command stays the dispatch entry. No assertion, fixture, or production module changed.

Contract coverage preserved:

  • the goal-channel delivery postcondition decides ok and exit status (satisfied/blocked);
  • external sink suppression reaches the gate sync end to end;
  • a disabled post-writeback hook performs zero projection calls;
  • the post-writeback sidecar dispatches once and replays once;
  • goal-channel exception detail stays redacted (no local path, no private binding id);
  • NaN/Infinity usage JSON fails as strict JSON before any refresh call.

Validation

  • python -m pytest tests/cli_commands tests/test_project_lifecycle_refresh_state_ownership.py tests/canary -q -> 123 passed
  • python -m ruff check tests loopx/canary -> All checks passed
  • python -m mypy -> Success, no issues

Risk

Test-only. No runtime, CLI, protocol, permission, or evidence boundary is touched; a failure here can only be a wrong patch target.

#4521 moved the refresh-state parser, rules and dispatch into
`loopx/cli_commands/project_lifecycle_refresh_state.py` and deliberately left no
re-export behind, so `project_lifecycle` no longer carries `refresh_state_run`,
`sync_explore_graph_after_material_refresh`, `sync_human_gate_after_refresh`,
`read_heartbeat_settlement`, `settlement_result_payload` or
`resolve_runtime_root`. The nine goal-channel tests still patched those names on
the dispatching module, so they failed with AttributeError on main even though
the behaviour under test was intact.

Each patch now targets the owning module and the dispatch entry
(`handle_project_lifecycle_command`) is unchanged, so the tests keep asserting
the same contract: the delivery postcondition decides ok/exit, external sinks
are suppressed end to end, a disabled post-writeback hook performs zero
projection calls, the sidecar dispatch replays once, exception detail stays
redacted, and NaN/Infinity usage JSON never reaches a refresh. No assertion,
fixture or production module changed.

Verified: pytest tests/cli_commands tests/test_project_lifecycle_refresh_state_ownership.py tests/canary -q -> 123 passed.

Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
@huangruiteng

Copy link
Copy Markdown
Collaborator Author

Pre-merge gate

loopx canary premerge --from-git-diff --git-diff-base origin/main (tier standard):

  • status passed, ok: true, merge_gate_passed: true, self_merge_allowed: true, manual_holds: 0
  • changed surface: python (1 file, test-only); no risk profile selected
  • direct checks: all three diff checks passed; changed_python_py_compile passed for tests/cli_commands/test_project_lifecycle_goal_channel.py
  • catalog canaries and risk-profile smokes: 0 selected (test-only diff), 0 failures

Coverage sufficiency: the nine cases assert dispatch behaviour of handle_project_lifecycle_command, which is unchanged, so the focused suite is the right evidence — pytest tests/cli_commands tests/test_project_lifecycle_refresh_state_ownership.py tests/canary -q -> 123 passed. tests/test_project_lifecycle_refresh_state_ownership.py (added by #4521) independently pins the module ownership this patch now follows, so a future re-export or re-move fails there first.

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval conclusion (author-owned PR; GitHub blocks formal self-approval)

Reviewed exact head: 10c60ed8d1ba42877975ae7b264baeaa706719f3

动机

#4521 把 refresh-state 的 parser、rules 与 dispatch 移到 loopx/cli_commands/project_lifecycle_refresh_state.py,并刻意不留 re-export,因此 project_lifecycle 上不再有 refresh_state_run、sync_explore_graph_after_material_refresh、sync_human_gate_after_refresh、read_heartbeat_settlement、settlement_result_payload、resolve_runtime_root。goal-channel 的九个用例仍把 monkeypatch.setattr 打在 dispatcher 上,于是在 main 上以 AttributeError 失败——行为其实是好的,红的只是测试接线。代价不只是这九条:它让所有 open PR 的 merge gate 变红(我在 #4561 的合并记录里就是把它记为“继承性红”并附了复现证据),因此修好它能让整个仓库的合并流程回到绿色。本 PR 把每处补丁改到 owning module,dispatch 入口不变,断言、fixture 与生产模块一行未动。

改动思路

入口保持 project_lifecycle.handle_project_lifecycle_command;权威输入是 owning module project_lifecycle_refresh_state(名字在那里的命名空间解析,模块内调用点才会被 double 拦截)。改动方向与 #4521 的所有权拆分一致:不给 dispatcher 补 re-export(那会重新制造第二处名字来源),而是让测试对准真正的 owner。正向路径:补丁装到 owning module → dispatcher 转发 → 模块内实际调用被拦截 → 原契约断言照旧执行。复用判断:沿用既有测试与 fixture,没有新 smoke、没有新模块;仅把 import 别名命名为 refresh_state_command 并加两行说明所有权,便于后来者不再打错目标。

具体改动

tests/cli_commands/test_project_lifecycle_goal_channel.py(+25/-20):增加 owning module 的 import(别名 refresh_state_command)与两行注释;19 处 monkeypatch.setattr 目标改为该模块。9 条原本 AttributeError 的用例在 head 上全部通过。

关键代码讲解

  1. import 与注释:project_lifecycle_refresh_state as refresh_state_command,并在注释里写明“refresh-state rules 住在自己的模块里,project_lifecycle 只做转发,因此补丁必须打在 owning module 上”——这是对 #4521 所有权拆分的显式说明,避免下一次同样的 stale-patch。
  2. 19 处 monkeypatch.setattr:目标统一改为 refresh_state_command;我按目标统计过 {refresh_state_command: 19},与社区版 #4565 的目标集合完全一致(只是别名不同)。
  3. 断言/契约未变:交付 postcondition 决定 ok/exit、external sink 端到端抑制、禁用 post-writeback hook 零投影调用、sidecar dispatch 只 replay 一次、异常详情脱敏、NaN/Infinity usage JSON 不进入 refresh——与 main 上的原意一致,只有补丁落点变了。

对主干的风险

最强回归场景不是测试本身,而是与 #4565 的重复:社区作者 songoow 对同一文件、同一 19 个目标提交了等价修复;两分支互 diff 只剩 import 别名与注释,谁第二个合并谁冲突。这是 P2(非阻塞):建议二者取一(任一都能让 CI 转绿),另一条以指针关闭,并在合并说明里注明社区侧独立定位了同一处破损。负向验证:test_project_lifecycle_goal_channel.py 在 main 上 9 failed、本 head 9 passed;pytest tests/cli_commands tests/test_project_lifecycle_refresh_state_ownership.py -q 在本 head 102 passed(你 body 里的 123 passed 含 tests/canary,与其后 #4567 修 ratchet 的范围相邻)。残留风险:dispatcher 未留所有权注释之外的防呆机制,建议未来以 lint(补丁目标必须存在于被 patch 的模块)彻底根治这一类漂移。

我的整体评价

APPROVE(作者自有 PR,GitHub 不允许自我正式批准,故以 COMMENT 记录同一结论)。 修复方向正确且最小:只改测试接线、不动断言与生产代码、公开入口仍被覆盖,9 条失败用例转绿,并使仓库 merge gate 恢复可判定。唯一需要处理的是与 #4565 的重复——两 PR 改同一文件同一 19 hunk,请择一落地(若落本 PR,建议在合并说明中致谢社区侧同样的定位)。

English verdict: APPROVE — at head 10c60ed the 19 monkeypatch.setattr calls in tests/cli_commands/test_project_lifecycle_goal_channel.py target project_lifecycle_refresh_state, the module that actually resolves refresh_state_run and its collaborators after #4521, so the nine goal-channel cases pass (9 passed here; 9 failed on the unmodified base) with assertions, fixtures and production code untouched, and the wider tests/cli_commands suite passes 102. One P2 to resolve: #4565 is the same repair for the same 19 targets and the two heads differ only by an import alias and a two-line comment, so land one, close the other with a pointer, and credit the community author.

@huangruiteng

Copy link
Copy Markdown
Collaborator Author

Superseded by #4565, which landed the same retarget (the monkeypatch.setattr targets moved to the module that resolves them, project_lifecycle_refresh_state, with handle_project_lifecycle_command still the entry under test) at 15:07Z. Closing so the same fix is not merged twice.

@huangruiteng
huangruiteng deleted the codex/goal-channel-test-live-owner branch September 16, 2026 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant