feat(turn): add pure Turn Loop Controller transition contract#2433
feat(turn): add pure Turn Loop Controller transition contract#2433Claisenn wants to merge 3 commits into
Conversation
|
Thanks for keeping this P0 slice pure. The overall boundary is right: this PR should define the transition contract, while Before merge, I would like two things closed. 1. Qualify the capability in one real long-horizon runThe 22-row decision table proves the local mapping, but it does not yet show that this transition contract is useful and stable across a real multi-Turn trajectory. Please run one explicit opt-in, non-toy workload on this exact branch, placing This is not a request to add the P1 scheduler/host adapter to this PR. A small external or manually driven harness is sufficient and may remain uncommitted. The workload should be long enough to exercise multiple consecutive Please check these invariants during the run:
Please report a compact qualification note with: reviewed head SHA, workload class, Turn count, disposition sequence/counts, the replan delta and resume behavior, final terminal condition, and any surprises that caused you to tune the table. Public-safe evidence can be posted here. If the useful trace contains private task material, raw prompts/transcripts, credentials, or host-local paths, send the detailed evidence through the private Lark channel and leave a redacted conclusion on GitHub. If this run exposes a decision-table issue, please refine the pure contract and focused tests. Please do not grow this PR into a second scheduler, resident loop, or broad integration framework. 2. Resolve one precedence contradiction, narrowlyAt That contradicts the documented row Please make the precedence explicit and add one cross-product regression for this case. If the intended invariant is the one currently documented, validated completion should win before decision-only user action is returned. If user action is intentionally stronger, update the contract/decision table and explain why a completed loop still remains non-terminal. This should be a small ordering/test change; no generic policy engine or extra abstraction is needed. Validation I ran
Decision: hold pending the real-run qualification note and the precedence fix/clarification. The implementation scope should otherwise remain as small as it is now. |
|
Both items closed at 1. Precedence fix (validated_completion outranks decision user action)The documented row is the intended invariant: once the terminal postcondition is independently proven, the loop is 2. Real-run qualification note
Validation re-run at |
huangruiteng
left a comment
There was a problem hiding this comment.
Exact-head re-review of 8de8660. Requesting changes for three fail-closed contract gaps.
- [P0] Validated completion bypasses receipt validation and freshness checks.
loop_controller.py:144-154 returns terminal from any mapping whose result_kind string is validated_completion, before the receipt is parsed and before lineage is compared at lines 184-208. On this exact head, both a bare {"result_kind": "validated_completion"} mapping and a completion receipt with agent-2 against an agent-1 envelope return terminal.
Completion should outrank a decision-only user action only after the receipt is proven valid and fresh. Please make the controller consume or require the existing loopx_turn_receipt_validation_v0 result, reject invalid or stale lineage first, and add cross-product tests for stale completion and an unvalidated completion-shaped mapping.
- [P0] The controller accepts a forged or truncated TurnEnvelope as executable.
_envelope_route only checks schema_version and action_signature.matches. A marker-only action_signature with no source_hash, envelope_hash, or compaction contract returns run_now. The existing Turn driver validator requires non-empty equal hashes and compaction.within_budget=true. Please reuse one shared envelope-validation/route contract instead of introducing a weaker second parser, and cover missing hashes plus over-budget envelopes.
- [P1] Two owner-contract decisions are still encoded as permissive fallbacks.
Malformed/unknown inputs return disposition=wait with a contract_error reason, although the projected #2432 maintainer contract places contract_error outside the six legal dispositions. Also, validated progress with bounded_turn_budget omitted returns run_now; the optional mapping has no schema, cursor, lineage, or freshness proof. Please either obtain the explicit owner decision before merge or implement the projected contract: a distinct contract-error result and a provider-neutral validated cursor/budget context that fails closed when continuation cannot prove a bound.
Independent validation:
- 87 focused controller/Turn tests passed
- autonomous-replan-obligation smoke passed
- public boundary and git diff checks passed
- minimal exact-head reproductions confirmed stale completion -> terminal, bare completion -> terminal, marker-only signature -> run_now, missing budget progress -> run_now, and malformed envelope -> wait(contract_error)
The submitted real-run note is useful evidence for the ordinary progress/replan path, but it does not close these negative cross-product boundaries. Please keep the fix inside the pure contract and focused tests; no scheduler or resident-loop expansion is needed.
|
All three fail-closed gaps closed at 1. Completion no longer bypasses receipt validation and freshness. The pre-parse 2. Envelope validation now reuses the shared typed route. 3. Contract errors are a typed disposition, and progress requires a proven budget. Added Cross-product negative coverage added: stale completion -> Validation at
Scope unchanged: pure transition contract + focused tests only; no scheduler, resident loop, or adapter added. |
huangruiteng
left a comment
There was a problem hiding this comment.
Exact-head re-review of e64de042. The envelope hardening and typed contract_error changes are good, but the receipt and bounded-context blockers are not closed yet.
Findings
-
[P0] The controller accepts a synthetic completion that the existing receipt validator never proved, while rejecting the real validator output.
loop_controller.py:152-201reads onlyresult_kindplus caller-supplied lineage. It does not requireschema_version=loopx_turn_receipt_validation_v0,ok=true, an acceptable validation/commit status, or the validatedturn_key. Therefore this mapping returnsterminal:{ "result_kind": "validated_completion", "lineage": {"goal_id": "goal-1", "agent_id": "agent-1", "todo_id": "todo-1"}, }Conversely,
validate_loopx_turn_receipt(...)andrun-onceemitloopx_turn_receipt_validation_v0without a lineage field, so an actualok=true,status=committedprogress receipt is rejected ascontract_error: ... missing goal/agent/todo lineage. The hand-built test helper attests/test_loop_turn_loop_controller.py:53-63hides this producer/consumer mismatch.Please make the controller consume the existing validated receipt contract (or add one explicit adapter that preserves its validation proof and exact lineage), then add an integration-shaped test using real
validate_loopx_turn_receiptoutput. A caller-authoredresult_kind + lineagemapping must not be enough to prove completion. -
[P0] Todo freshness is collected but never compared.
loop_controller.py:182-187compares onlygoal_idandagent_id. Avalidated_completionreceipt fortodo-oldagainst a fresh envelope selectingtodo-newreturnsterminal. That can terminate a newly selected todo with a stale predecessor receipt, despite the contract claiming full(goal_id, agent_id, todo_id)freshness.Include
todo_idin the exact lineage comparison and add the stale-todo completion/progress cross-product tests. -
[P1] Two integers are still treated as a “proven bounded turn budget.”
loop_controller.py:203-218usesisinstance(value, int), so Python booleans pass (max_turns=True,completed_turns=Falseyieldsrun_now) and negative bounds pass (max_turns=-1,completed_turns=0yieldsterminal). The mapping still has no schema or identity tying the cursor to the current goal/todo/turn.Please reject booleans and invalid ranges at minimum (
max_turns > 0,0 <= completed_turns <= max_turns) and make the bounded context carry enough identity/freshness proof to prevent a cursor from another loop being reused. Cover the invalid-domain and stale-context cases.
Validation
93 passed: controller plus Turn driver/executor/transaction tests- Exact-head public-safe reproductions confirmed:
- real committed validator receipt ->
contract_error - synthetic matching-lineage completion ->
terminal - stale-todo completion ->
terminal - negative budget ->
terminal - boolean budget ->
run_now
- real committed validator receipt ->
- GitHub
pytestis green
Decision: request changes. Keep the fix in the pure contract, its producer/adapter boundary, and focused semantic tests; no scheduler or resident-loop expansion is needed.
Implements the P0 pure transition contract from the Turn Loop Controller plan (closes #2432).
Summary
Add
loopx/control_plane/turn_driver/loop_controller.pyexposing one pure function:Given one Turn receipt (
result_kind+ optional lineage) and a freshloopx_turn_envelope_v0decision, it returns exactly one typed disposition —run_now/wait/user_action_required/repair/replan/terminal— with a reason, lineage, andspends_quota=false,launches_host=false,writes_state=falsemarkers.Boundary held deliberately narrow per the plan: the function never invokes a model, sleeps, mutates a host scheduler, writes state, or spends quota. Scheduler process management, host wake adapters, and operator presentation are untouched (P1/P2 slices).
run-onceremains the only delivery transaction.Decision semantics
validated_completion->terminal;validated_progress+ budget remaining + delivery decision ->run_now; budget exhausted ->terminal; no delivery decision ->wait.replanwith areplan_continuationpayload:requires_bounded_delta=true,fresh_envelope_required=true,stale_todo_rerun_allowed=false— a resumable host session never reruns a stale todo without a bounded todo/vision delta.host_failure,validation_failed,writeback_failed,quota_spend_failed) ->repair.user_action_required; quiet/cadence-only decision ->wait(no spend).waitwith an explicitstale_receipt/contract_errorreason. Nothing is guessed or fabricated. (This answers the open question from GH-C72: P0 pure Turn Loop Controller transition contract #2432 by keeping six dispositions with typed reasons; easy to promote to a seventh disposition if preferred.)Also adds the contract doc
docs/reference/protocols/turn-loop-controller-v0.md(decision table + replan continuation boundary) and links it from the protocols index.Validation
tests/test_loop_turn_loop_controller.py: 22-row decision table covering every disposition plus boundary rows (progress at budget limit, replan without delta, stale lineage, malformed inputs, all four failure kinds)pytest -q tests/test_loop_turn_loop_controller.py tests/test_loopx_turn_driver.py tests/test_loopx_turn_executor.py tests/test_loopx_turn_transaction.py(86 passed)python examples/autonomous-replan-obligation-smoke.pyloopx check --scan-path docs/reference/protocols/turn-loop-controller-v0.md --scan-path docs/reference/protocols/loopx-turn-v0.md --scan-path CONTRIBUTOR_TASKS.md --scan-path CONTRIBUTING.md(public boundary clean)git diff --check