fix(turn): restore the dropped managed-step CLI surface - #4451
Merged
Merged
Conversation
2e96a57 (#4443) removed both halves of the `loopx turn managed-step` surface while rewiring host selection: the `managed-step` subparser in turn_registration.py and the `handle_turn_managed_step` dispatch in turn.py. loopx/cli_commands/turn_managed_step.py and its unit test survived, so the module still passed its own tests while the subcommand no longer existed. Evidence: - `turn managed-step` now exits 2 with `invalid choice: 'managed-step'`. - examples/loopx-turn-managed-step-self-heal-smoke.py fails on main and passes at 503991d, the commit before that change. Restore the subparser with the same flags and choices as before, and dispatch it beside the journal-inspection owner. The managed step selects from the shipped run-once hosts and always plans isolated-headless, but its default host follows resolve_default_turn_host() so an explicit LOOPX_TURN_HOST selection is honored instead of being pinned back to one host. To keep loopx/cli_commands/turn.py inside its frozen 1114-line baseline while adding the dispatch back, move the resume-binding resolution into turn_selection.py. The helper returns `(requested, binding)` because those are two different facts: a run-once Codex CLI Turn derives a binding from its own envelope, and that derived binding must not be read back as an explicit resume request or `--resume-turn-key` would refuse a legitimate resume. Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
huangruiteng
commented
Sep 15, 2026
huangruiteng
left a comment
huangruiteng
left a comment
Collaborator
Author
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval).
Exact head reviewed: 95e73f34e
Verdict: APPROVE. Three files under loopx/cli_commands/, +114/-29, restoring a surface that main had lost.
This is a real regression, and I caused it. 2e96a570d (#4443) removed both halves of turn managed-step — the subparser and the dispatch — while rewiring host selection. I verified it end to end rather than by reading the diff:
| revision | examples/loopx-turn-managed-step-self-heal-smoke.py |
|---|---|
503991dd2 (pre-#4443) |
passes — managed-step self-heal: provider_capacity -> wait(30s, 1/3) -> same-Turn validated progress, exactly one quota slot |
main before this PR |
exits 2 at argparse: invalid choice: 'managed-step' |
| this head | passes |
Why CI did not catch it in #4443. tests/test_loopx_turn_managed_step.py survived and calls handle_turn_managed_step directly, so the handler stayed covered while the CLI surface vanished. Only the public smoke exercises the parser. That is a useful lesson for the surface: a split-out command owner needs a CLI-level check, not only a handler-level one.
What I verified at this head
examples/loopx-turn-managed-step-self-heal-smoke.pypasses — provider_capacity → wait(30s, 1/3) → same-Turn validated progress with exactly one quota slot, and the journal is untouched by the managed step.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 tests/test_loopx_turn_executor.py -q→ 183 passed.pytest tests/test_loopx_turn_codex_cli.py tests/test_loopx_turn_host_failure.py tests/test_loopx_turn_transaction.py tests/test_loopx_turn_settlement_parity.py tests/test_loopx_turn_journal_inspection.py tests/test_turn_envelope.py tests/test_turn_loop_disposition.py tests/test_loop_turn_loop_controller.py tests/test_chat_turn_wait.py -q→ 204 passed.- Ratchet smoke ok,
turn.py1097 ≤ 1114;ruffclean;loopx canary premerge --from-git-diff→merge_gate_passed: true, 4 catalog canaries passed.
Two judgment calls worth naming
- The restored
managed-stepdefault host followsresolve_default_turn_host()instead of being pinned back todsh. Pinning it would have quietly ignored the explicitLOOPX_TURN_HOSTselection rule that #4443 shipped, on a surface #4443 touched. Choices and mode stay as before (run-once hosts,isolated-headlessonly), and the smoke passes--host dsh --execution-mode isolated-headlessexplicitly. - The resume-binding extraction returns
(requested, binding)rather than a bare binding. My first draft returned only the binding andtest_turn_run_once_cli_resumes_session_from_recoverable_failed_turncaught the consequence: arun-onceCodex CLI Turn derives a binding from its envelope, and reading that back as an explicit resume made--resume-turn-keyreject a legitimate resume. The two facts are now separate again, with one source of truth.
Boundary: no new surface; plan and run-once behavior is unchanged by the extraction; no budget number was raised or lowered.
Residual risk: examples/cli-help-manpage-smoke.py still fails on the pre-existing unclassified: ['goal-actions'], which is top-level command classification and unrelated to adding a turn subcommand. It is still red on main and still needs an owner.
This was referenced Sep 15, 2026
huangruiteng
added a commit
that referenced
this pull request
Sep 16, 2026
`run-once` and `managed-step` must resolve the same governing decision. #4443 (2e96a57) replaced the shared call in `loopx/cli_commands/turn.py` with a private copy of the whole chain - live status, scheduler context, capability hook projection, decision parameters, advisory-primary rebinding and the signed envelope - and a1213ed (#4451) restored only the advisory-primary half. The copy stayed, so a decision input added to the shared owner would have moved one Turn subcommand and not the other. Route `run-once` through the shared owner. `build_fresh_turn_decision` owns the whole chain and returns the decision together with the envelope signed from that same decision, so an owner that also needs the raw decision (reward recall) cannot re-derive a second copy. `build_fresh_envelope_for_managed_step` stays the read-only projection: it exposes no hook parameter, so the managed step cannot start publishing Go/No-Go hooks by accident. Evidence (`/private/tmp/loopx-turn-dedup`, origin/main 292b85e): - `turn plan` on the same fixture emits a byte-identical `turn_envelope` before and after (`diff` of the JSON dumps: no difference). - `examples/loopx-turn-managed-step-self-heal-smoke.py` prints identical output before and after. - `tests/test_loopx_turn_{managed_step,driver,executor,codex_cli}.py`, `tests/test_turn_{envelope,managed_executor_binding,default_host_binding, loop_disposition}.py`, `tests/test_loop_{turn_loop_controller}.py`, `tests/test_loopx_turn_{settlement_parity,host_failure,journal_inspection, transaction}.py`: 386 passed. - `tests/architecture` plus the control-plane portfolio/CLI-budget/capability memory/shadow-e2e suites: 140 passed. The test seam for the adaptive orchestration contract moves with the code: the envelope is signed by the shared owner, so `tests/test_loopx_turn_driver.py` injects `build_turn_envelope` where that owner resolves it, and `tests/test_loopx_turn_journal_inspection.py` guards the live reads of the shared owner instead of the removed `turn.py` re-import. A new plan-mode test fails if `turn.py` ever reads its live status outside the shared owner again. The post-settlement `loopx_turn_run_once` scheduler re-evaluation still builds its own decision: it deliberately re-reads status after the commit, has no advisory primary and carries a different route source, so folding it into the plan owner would change behavior. Boundary considered, left as is. Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
loopx turn managed-stepdoes not exist onmain:Commit
2e96a570d(#4443, "select the managed Turn host explicitly") removed both halves of the surface while rewiring host selection:managed-stepsubparser inloopx/cli_commands/turn_registration.py(59 lines of registration);handle_turn_managed_stepdispatch inloopx/cli_commands/turn.py.loopx/cli_commands/turn_managed_step.py,turn_decision.build_fresh_envelope_for_managed_step,turn_rendering.render_loopx_turn_managed_step_markdown, andtests/test_loopx_turn_managed_step.pyall survived. The unit test calls the handler directly, so it kept passing while the subcommand was gone — which is why the PR's own checks were green. The public smoke is the check that caught it:full-public-smokes (3, 330)is red onmainbecause of this.Evidence the surface regressed
examples/loopx-turn-managed-step-self-heal-smoke.py503991dd2(before2e96a570d)managed-step self-heal: provider_capacity -> wait(30s, 1/3) -> same-Turn validated progress, exactly one quota slotmainbefore this PRChange
--turn-key,--observed-attempt,--observed-max-attempts,--scan-root,--scan-path,--limit) and choices as before the regression: the shipped run-once hosts,isolated-headlessonly.The default host follows
resolve_default_turn_host()rather than being pinned back todsh, so the explicit-selection rule shipped by feat(turn): select the managed Turn host explicitly #4443 (LOOPX_TURN_HOST) is honored instead of silently ignored on this surface.handle_turn_command.turn_selection.pysoturn.pystill fits its frozen 1114-line baseline. The helper returns(requested, binding), keeping two facts apart that the inline version also kept apart: arun-onceCodex CLI Turn derives a binding from its own envelope, and that derived binding must not be read as an explicit resume request — otherwise--resume-turn-keyrefuses a legitimate resume. An earlier draft of this extraction conflated them andtest_turn_run_once_cli_resumes_session_from_recoverable_failed_turncaught it.Validation
examples/loopx-turn-managed-step-self-heal-smoke.py→managed-step self-heal: provider_capacity -> wait(30s, 1/3) -> same-Turn validated progress, exactly one quota slot(this is the red check).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 tests/test_loopx_turn_executor.py -q→ 183 passed.pytest tests/test_loopx_turn_codex_cli.py tests/test_loopx_turn_host_failure.py tests/test_loopx_turn_transaction.py tests/test_loopx_turn_settlement_parity.py tests/test_loopx_turn_journal_inspection.py tests/test_turn_envelope.py tests/test_turn_loop_disposition.py tests/test_loop_turn_loop_controller.py tests/test_chat_turn_wait.py -q→ 204 passed.examples/cli-command-module-size-ownership-command-modularization-smoke.py→ ok (turn.py1097 ≤ 1114).ruff checkon all three changed files → clean.loopx canary premerge --from-git-diff→merge_gate_passed: true, 4 catalog canaries passed.examples/cli-help-manpage-smoke.pystill fails only on the pre-existingunclassified: ['goal-actions'], which this PR does not touch (top-level command classification, unrelated to aturnsubcommand).Boundary
Three files under
loopx/cli_commands/, +114/-29. No new surface, no behavior change forrun-onceorplanbeyond the extracted helper being equivalent, and no budget number changed.