Skip to content

refactor(turn): derive every Turn decision from one shared owner - #4496

Merged
huangruiteng merged 1 commit into
mainfrom
codex/turn-decision-single-owner
Sep 16, 2026
Merged

huangruiteng merged 1 commit into
mainfrom
codex/turn-decision-single-owner

Conversation

@huangruiteng

Copy link
Copy Markdown
Collaborator

Summary

run-once and managed-step must agree on what the current governing Turn
decision is, but only managed-step still derived it through
loopx/cli_commands/turn_decision.py. After #4443 put the construction back
inline, turn.py rebuilt the same status read, scheduler context, scheduler
route source, capability hooks and decision arguments itself, and a later fix
only recovered part of it. Adding one decision condition could then change one
command and not the other.

This PR restores a single owner for the whole chain and makes the duplicated
inputs unavailable to call sites:

input before after
live status inline collect_status in turn.py, collect_turn_status_payload in turn_decision.py collect_turn_status_payload, also reused by the settlement current_status() re-read
scheduler context rebuilt in each owner, and again for the envelope one value on the owner, reused for the decision and its envelope
route source inline "loopx_turn_plan" literal plus the shared constant TURN_DECISION_ROUTE_SOURCE only
capability hooks operator-inbox projector, explore frontier and pending-intent hook assembled per owner assembled once in build_fresh_turn_decision_owner
advisory primary inline closure in turn.py FreshTurnDecisionOwner.resolve()

FreshTurnDecisionOwner carries the inputs beside the decision, so a Turn that
later spends a quota slot or re-checks the scheduler settles against the same
live status, scheduler context and activation-bound projector instead of
re-deriving them. _build_turn_decision is private now: there is no second way
to construct a fresh decision.

Two smaller duplications went with it: the scan_roots resolution and the
current_status() status read are the shared collector, and
build_turn_decision_builder's runtime_root_arg parameter was dead -- it was
never read, which made the resolved-root contract look wired up when only the
direct-call test exercised it.

Validation

  • pytest tests/test_loopx_turn_managed_step.py tests/test_loopx_turn_driver.py tests/test_turn_default_host_binding.py tests/test_turn_managed_executor_binding.py -q -> 136 passed
  • pytest tests/control_plane/test_cli_output_budget.py tests/control_plane/test_cli_output_differential.py tests/control_plane/test_actual_default_model_behavior_portfolio.py -q -> 115 passed
  • ruff check clean on the changed files
  • Real CLI readback parity: loopx --runtime-root ~/.codex/loopx --format json turn plan --goal-id loopx-meta --host dsh --execution-mode isolated-headless --agent-id codex-side-bypass run from this branch and from a clean origin/main worktree, back to back. The two payloads are byte-identical apart from the time-derived source_decision_hash / source_json_bytes pair, and each branch reproduced the other's value on a later run, so the difference is wall-clock content rather than a branch difference.
  • test_shared_decision_owner_binds_both_turn_owners_to_one_set_of_inputs replaces the old builder-level test and now drives both entry points: the activation check receives the resolved root for each, resolve() feeds the decision the owner's own status/scheduler/projector, and the managed-step envelope is signed with the owner's scheduler context.

Risk

Behavior-preserving refactor. The one deliberate difference: an executing Turn
now reuses the owner's operator-inbox projector for quota spend and the run-once
scheduler re-check instead of building a second one from the same resolved root,
which is the same activation binding. No CLI option, output shape, decision
argument or route source changes.

中文摘要

run-once 与 managed-step 现在都只能通过 build_fresh_turn_decision_owner
拿到当前 Turn 决策;status、scheduler context、能力钩子与决策参数都收在 owner
上,事后 spend/scheduler 复核也复用同一批输入。_build_turn_decision 变为私有,
runtime_root_arg 这个从未被读取的死参数被删除。已验证 136 项 Turn 测试、115 项
CLI 输出测试,并用真实 CLI 在分支与 origin/main 之间做了逐字节对齐(仅时间派生
字段不同)。

`run-once` rebuilt the fresh Turn decision inline after #4443, so the status
read, scheduler context, route source, capability hooks and decision arguments
were maintained twice and the later fix only recovered part of them. Both
commands now build through `build_fresh_turn_decision_owner`, which also carries
the inputs a later spend or scheduler re-check must settle against, and the
decision builder is private.

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

@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.

Self-review of the final head (39ac7210a), re-read against origin/main.

What I re-checked

  • Every remaining construction of a Turn decision: loopx/cli_commands/turn.py:916 (run-once post-settlement scheduler re-check), loopx/cli_commands/quota.py, loopx/cli_commands/quota_scheduler_followup.py. Only turn.py was a duplicate of the governing-decision chain.
  • No remaining caller of the removed build_turn_decision_builder in loopx/, docs, examples or smokes.
  • ruff check clean; the changed files are the only ones touched.

Findings

[P3, intended behavior fix] managed-step now hands the resolved runtime root to the Lark drain activation check. The dead runtime_root_arg parameter made the previous wiring look correct while the only test exercised the builder directly; the managed-step entry point passed the raw argument, which is None when a registry declares common_runtime_root and the command omits --runtime-root. That lane's activation check therefore read the operator's global extension state instead of the registry's own. Behavior is unchanged when --runtime-root is passed or no registry default exists. The replacement test drives both entry points instead of one builder call.

[P3, deliberately not part of this owner] run-once keeps calling build_live_quota_should_run_decision directly for its post-settlement scheduler re-check with route source loopx_turn_run_once and a fresh status read. That answers "what does the scheduler say after this settlement", not "what is the current governing decision", so folding it into the owner would make two different questions share one signature. Same for quota.py and quota_scheduler_followup.py, which belong to their own commands.

Future-facing refactor pass

Applied. The shared owner now owns the whole input set, not just the closure, and the decision builder is private, so a future decision condition cannot be added to one command's copy. The current_status() settlement re-read and the scan_roots resolution were folded into the shared collector for the same reason. Nothing larger was needed here; the neighbouring turn_managed_step.py module now consumes the owner as-is.

Validation

  • pytest tests/test_loopx_turn_managed_step.py tests/test_loopx_turn_driver.py tests/test_turn_default_host_binding.py tests/test_turn_managed_executor_binding.py -q -> 136 passed
  • pytest tests/control_plane/test_cli_output_budget.py tests/control_plane/test_cli_output_differential.py -q -> included in a 115-passed run with the portfolio file, which also passes 115/115 on a clean origin/main baseline
  • Real CLI parity: turn plan --goal-id loopx-meta --host dsh --execution-mode isolated-headless from this branch and from origin/main, back to back -> byte-identical payloads apart from the time-derived source_decision_hash / source_json_bytes pair, and each branch reproduced the other's value on a later run
  • ruff check clean

No blockers found. Merge note: the only behavior delta is the resolved-root activation read described above, which is a fix for a registry-scoped runtime root rather than a behavior regression.

@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)

Exact head reviewed: 39ac7210a9e77462e11ccdccdf5fd03036501e67

动机

#4443 把 run-once 的 Turn 决策改回内联构造后,同一条链在仓库里存在两份:turn.py 自己算 scan roots、读 live status、构造 scheduler context、构造 operator-inbox projector,再闭包调用 build_live_quota_should_run_decision;turn_decision.py 里 build_turn_decision_builder 保留第二份。两份并不只是格式重复,而是同一套 status、scheduler context、capability hook 与 route source。后果是以后加一个决策条件只改到一边,两个命令就会对"当前该做什么"给出不同结论;而且 build_turn_decision_builder 的 runtime_root_arg 参数从未被读取,让"已接上共享根路径契约"看起来成立、实际只有直接调用它的测试在跑。影响面是所有走 run-once / managed-step 的 LoopX 目标。更小的修法(只删死参数)会留下这条重复链,也就是被报告的真实成本,所以本 PR 直接删掉重复链而不是新增抽象。

改动思路

入口仍是两个 CLI 子命令,权威输入是 collect_status 的 live status、scheduler_execution_context_for_turn 的调度上下文,以及 activation-bound 的 Lark operator-inbox projector;决策所有者是 turn_decision.py:163 build_fresh_turn_decision_owner,它返回 :142 FreshTurnDecisionOwner,resolve() 内部再调 :66 _build_turn_decision 走到同一个 build_live_quota_should_run_decision(route source 仍是 loopx_turn_plan)。这个模块不执行、不写盘、不 spend,envelope 的执行与重试仍由 Turn driver 与 loop controller 拥有。复用判断:仓库里本来就有共享模块,PR 做的是删除重复而不是新增抽象——构建器转为私有、旧参数删除、__all__ 同步,没有新模块、新 flag、新字段。状态模型上这次没有引入或新强制任何持久状态:FreshTurnDecisionOwner 只是携带已经派生出的输入,没有持久化,也没有新的投影字段。规则归属上,"哪些输入喂给一次新鲜 Turn 决策"从两份实现收敛为一份;managed-step 作为文档化的第二个入口被保留,但改为共用同一所有者,而不是继续自己拼。

具体改动

loopx/cli_commands/turn.py 从 1100 行降到 1058 行:删除 scan roots、status、scheduler context、projector 与本地 build_turn_decision 闭包,改为 :147 调用 build_fresh_turn_decision_owner(args, registry_path=…, runtime_root=…, runtime_root_arg=…, turn_start_hook_dispatch=…),随后只读 decision_owner.operator_inbox_urgency_projector、decision_owner.scheduler_execution_context 与 decision_owner.resolve()。terminal closeout 的 current_status()(:648)也改为走 collect_turn_status_payload,原来在 handle_turn_command 顶部算好的 scan_roots 与 limit 表达式在 helper 里原样保留。loopx/cli_commands/turn_decision.py 把 build_turn_decision_builder 改为私有 _build_turn_decision(:66),删除未被读取的 runtime_root_arg,把 status/scheduler/projector 三件输入显式传参,并新增 FreshTurnDecisionOwner(:142)与 build_fresh_turn_decision_owner(:163);build_fresh_envelope_for_managed_step(:242)改为同一 owner,scheduler context 复用 owner.scheduler_execution_context 而不是重算。

关键代码讲解

  • build_fresh_turn_decision_owner(turn_decision.py:163):唯一入口。关键不变量是 activation 检查使用已解析的 runtime_root 而不是 CLI 原始 --runtime-root——当 registry 声明 common_runtime_root 且命令省略该参数时,原始值是 None,否则会读到操作者全局扩展状态,而不是本 registry 自己的。
  • FreshTurnDecisionOwner(turn_decision.py:142):frozen dataclass,携带 status payload、scheduler context、projector 与决策构建器;resolve() 是唯一应用 controller advisory primary 的地方,避免每个调用点各包一层。
  • _build_turn_decision(turn_decision.py:66):私有化后仍保留 turn_start_hook_dispatch 作为调用方业务——执行中的 Turn 可以先发 Go/No-Go hook,只读的 managed step 不能。
  • current_status(turn.py:648):结算重读与决策输入使用同一状态契约,避免"结算用了一份决策没用过的输入"。
  • tests/test_loopx_turn_managed_step.py:测试改为驱动两个入口,断言它们绑定同一组输入。

对主干的风险

最强的回归场景是两条命令漂移,使 managed-step 依据决策并未使用的 status 或 scheduler context 结算。触发状态是 registry 声明 common_runtime_root 而命令不传 --runtime-root,再叠加后续只改一边的输入编辑;旧代码允许这种路径(turn.py 的重复闭包 + 共享构建器里的死参数),现在两者都走同一所有者,路径被移除。爆炸半径是所有使用任一 Turn 命令的目标的 envelope 与结算;可观测点是 loopx turn plan 载荷与 envelope 里的 scheduler context 字段;回退是三个文件的单提交 revert。验证方面我在 head 上重跑了 pytest tests/test_loopx_turn_managed_step.py = 21 passed,并在同一未变 head 上以真实 CLI turn plan 与干净的 origin/main worktree 做过对照:除时间派生的 source_decision_hash / source_json_bytes 外逐字节一致,且两边互换重跑都能复现对方的值。这不是只有 mock 的证据。未覆盖维度:没有跑 hosted automation 的端到端唤醒,parity 只到 CLI 层。重复规则方面,两条可达路径(run-once、managed-step)都映射到同一 owner;被删除的规则或退出条件是 turn.py 的内联链与死参数。guidance 与机器强制义务没有新增或混淆。可改进项只有一个非阻断的:见下方 P3。

我的整体评价

结论:APPROVE(author-owned PR,GitHub 不允许作者正式自审,故以 COMMENTED 形式记录同一结论)。 改动与已证明的问题同域、单目标、三步可回退,且实测等价;repository_reuse = separation_justified(删重复、不新增抽象),change_proportionality = proportionate(一个 dataclass、一个函数、一次私有化),observable_semantics = equivalent(真实 CLI 对照),authority_semantics = aligned(模块仍不执行/不写/不 spend),default_off_isolation = not_applicable(无开关)。

残留风险:parity 证据止于本仓库 CLI 层,且未来新调用点仍可能在 owner 之外重新派生某个输入。唯一非阻断发现(P3):FreshTurnDecisionOwner.status_payload(turn_decision.py:152)被保存但当前没有任何调用点读取它——terminal closeout 仍自己调 collect_turn_status_payload。要么让结算重读使用已在手的 owner.status_payload,要么在该字段出现真实消费者之前先不携带它("不要把设计可能性变成未使用的生产结构")。这不影响本次结论,复审只需要在该字段被使用或被删除时确认一次。

English verdict: APPROVE (recorded as a COMMENTED review because GitHub blocks formal self-approval on author-owned PRs). Exact head 39ac7210a9e77462e11ccdccdf5fd03036501e67. Both Turn entry points now derive one decision from one status payload, one scheduler context and one activation-bound projector; the duplicate turn.py chain and the never-read runtime_root_arg parameter are deleted. Verified at the head: pytest tests/test_loopx_turn_managed_step.py 21 passed, and real-CLI turn plan parity against a clean origin/main worktree byte-identical apart from time-derived hash fields. No blocking finding. One P3: FreshTurnDecisionOwner.status_payload is carried but unread by any call site, because terminal closeout still re-reads through collect_turn_status_payload. Residual risk: parity is CLI-level only; a future caller could still re-derive an input outside the owner.

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