From dd23a621d823817a63d456fb23bde688c00fde5d Mon Sep 17 00:00:00 2001 From: song <22676124+songoow@users.noreply.github.com> Date: Tue, 29 Sep 2026 06:23:12 -0400 Subject: [PATCH 01/13] feat(turn-driver): read Turn lane liveness without taking the lane Add `lock_holder_liveness` to the file-lock owner and `turn_lane_liveness` on top of it. Both classify a lane's last executing Turn from its holder record alone: `released` on a clean exit, `dead` when the record names this machine and the pid is gone, `foreign_host` when the pid cannot be checked here, `unreadable` when a lock file carries no parseable record, and `live` only when a same-host pid is still alive. The probe never touches the kernel lock. A probe that acquired it for an instant would refuse a real `run-once --execute` racing that instant with `turn_lane_in_flight` for nothing; the new test drives the real fence wrapper concurrently with a continuous probe and proves the Turn is admitted exactly once. The holder host label is now single-sourced so the writer and the readers cannot disagree on what "this machine" is. Co-Authored-By: Claude Fable 5.1 Signed-off-by: song <22676124+songoow@users.noreply.github.com> --- loopx/control_plane/turn_driver/lane_fence.py | 67 +++++--- loopx/file_lock.py | 91 +++++++++-- tests/test_turn_lane_fence.py | 144 +++++++++++++++++- 3 files changed, 274 insertions(+), 28 deletions(-) diff --git a/loopx/control_plane/turn_driver/lane_fence.py b/loopx/control_plane/turn_driver/lane_fence.py index 5ac99e8f75..9f8ef860e6 100644 --- a/loopx/control_plane/turn_driver/lane_fence.py +++ b/loopx/control_plane/turn_driver/lane_fence.py @@ -17,12 +17,20 @@ from contextlib import contextmanager from functools import wraps import hashlib -import json from pathlib import Path import re from typing import Any -from ...file_lock import lock_holder_path, try_exclusive_file_lock +from ...file_lock import ( + LOCK_HOLDER_ABSENT, + LOCK_HOLDER_DEAD, + LOCK_HOLDER_FOREIGN_HOST, + LOCK_HOLDER_LIVE, + LOCK_HOLDER_RELEASED, + LOCK_HOLDER_UNREADABLE, + lock_holder_liveness, + try_exclusive_file_lock, +) # Typed refusal for a lane whose single executor is already busy. The reason is # a fact about this lane, so a caller can retry it unchanged once it clears. @@ -103,6 +111,18 @@ def turn_lane_singleflight( yield lock_path +def _public_holder(record: Mapping[str, Any]) -> dict[str, Any]: + projection: dict[str, Any] = {} + for field in TURN_LANE_HOLDER_TEXT_FIELDS: + value = record.get(field) + if isinstance(value, str) and value: + projection[field] = value + pid = record.get("pid") + if isinstance(pid, int): + projection["pid"] = pid + return projection + + def turn_lane_holder_readback(target: Path) -> dict[str, Any]: """Return the public-safe identity of the Turn holding one lane, else ``{}``. @@ -113,21 +133,34 @@ def turn_lane_holder_readback(target: Path) -> dict[str, Any]: hosts share one runtime root. """ - try: - record = json.loads(lock_holder_path(target).read_text(encoding="utf-8")) - except (OSError, ValueError): - return {} - if not isinstance(record, Mapping): - return {} - projection: dict[str, Any] = {} - for field in TURN_LANE_HOLDER_TEXT_FIELDS: - value = record.get(field) - if isinstance(value, str) and value: - projection[field] = value - pid = record.get("pid") - if isinstance(pid, int): - projection["pid"] = pid - return projection + _state, record = lock_holder_liveness(target) + return _public_holder(record) + + +# Lane liveness vocabulary: the lock owner's holder states, named here so a +# projection can switch on them without learning the lock record format. +TURN_LANE_LIVE = LOCK_HOLDER_LIVE +TURN_LANE_RELEASED = LOCK_HOLDER_RELEASED +TURN_LANE_DEAD = LOCK_HOLDER_DEAD +TURN_LANE_FOREIGN_HOST = LOCK_HOLDER_FOREIGN_HOST +TURN_LANE_UNREADABLE = LOCK_HOLDER_UNREADABLE +TURN_LANE_ABSENT = LOCK_HOLDER_ABSENT + + +def turn_lane_liveness(target: Path) -> dict[str, Any]: + """Say whether one lane's last executing Turn is still running, read-only. + + The answer comes from the holder record alone: ``released_at`` for a clean + exit, the machine name for whether the pid can be checked here, and pid + liveness for a holder that never released. This never takes the lane lock, + not even for an instant: a probe that did would refuse a real Turn racing + the same instant with ``turn_lane_in_flight`` for no reason. ``live`` is + the only state that is evidence of execution; ``foreign_host`` and + ``unreadable`` are unknowns a consumer must fail closed on. + """ + + state, record = lock_holder_liveness(target) + return {"state": state, "holder": _public_holder(record)} def turn_lane_in_flight_record( diff --git a/loopx/file_lock.py b/loopx/file_lock.py index 4e25c05f96..7ffa396822 100644 --- a/loopx/file_lock.py +++ b/loopx/file_lock.py @@ -175,7 +175,7 @@ def _identity( # hosts sharing one runtime root can both read the holder, so the record # names its own machine and a reader never has to guess which host a pid # belongs to. The name is a sanitized label, not a path or a secret. - "host": _safe_label(socket.gethostname(), fallback="unknown"), + "host": lock_holder_host_label(), "agent_id": _safe_label( agent_id or os.environ.get("LOOPX_AGENT_ID"), fallback="unknown", @@ -268,14 +268,8 @@ def _mark_released( pass -def _read_holder_record(lock_path: Path) -> dict[str, object]: - try: - payload = json.loads(lock_path.read_text(encoding="utf-8")) - except (OSError, json.JSONDecodeError): - return {} - if not isinstance(payload, dict): - return {} - allowed = { +_HOLDER_RECORD_FIELDS = frozenset( + { "schema_version", "lock_id", "policy", @@ -286,7 +280,84 @@ def _read_holder_record(lock_path: Path) -> dict[str, object]: "acquired_at", "released_at", } - return {key: payload[key] for key in allowed if key in payload} +) + + +def _filter_holder_record(payload: object) -> dict[str, object]: + if not isinstance(payload, dict): + return {} + return {key: payload[key] for key in _HOLDER_RECORD_FIELDS if key in payload} + + +def _read_holder_record(lock_path: Path) -> dict[str, object]: + try: + payload = json.loads(lock_path.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError): + return {} + return _filter_holder_record(payload) + + +def lock_holder_host_label() -> str: + """The machine label a holder record carries; readers compare against it.""" + + return _safe_label(socket.gethostname(), fallback="unknown") + + +# Liveness of a lock's last holder, read from its record alone. The kernel lock +# is never probed: a probe would hold the lock for an instant, and a real +# single-flight acquisition racing that instant would be refused for nothing. +LOCK_HOLDER_LIVE = "live" +LOCK_HOLDER_RELEASED = "released" +LOCK_HOLDER_DEAD = "dead" +LOCK_HOLDER_FOREIGN_HOST = "foreign_host" +LOCK_HOLDER_UNREADABLE = "unreadable" +LOCK_HOLDER_ABSENT = "absent" +LOCK_HOLDER_LIVENESS_STATES = ( + LOCK_HOLDER_LIVE, + LOCK_HOLDER_RELEASED, + LOCK_HOLDER_DEAD, + LOCK_HOLDER_FOREIGN_HOST, + LOCK_HOLDER_UNREADABLE, + LOCK_HOLDER_ABSENT, +) + + +def lock_holder_liveness(path: Path) -> tuple[str, dict[str, object]]: + """Classify the last holder of one lock without touching the kernel lock. + + Returns the liveness state and the filtered holder record. ``released`` + means the holder wrote ``released_at`` on a clean exit; ``dead`` means the + record names this machine and the pid is gone, which is what a crashed or + killed holder leaves behind; ``foreign_host`` means the pid cannot be + checked from here; ``unreadable`` means a lock file exists but carries no + parseable record, for example mid-acquisition. Only ``live`` is evidence of + a running holder, and even that is pid liveness, not the kernel lock: a + reused pid can keep a crashed holder looking alive until the next holder + overwrites the record. + """ + + holder_path = lock_holder_path(path) + try: + text = holder_path.read_text(encoding="utf-8") + except FileNotFoundError: + return LOCK_HOLDER_ABSENT, {} + except OSError: + return LOCK_HOLDER_UNREADABLE, {} + try: + record = _filter_holder_record(json.loads(text)) + except ValueError: + return LOCK_HOLDER_UNREADABLE, {} + if not record: + return LOCK_HOLDER_UNREADABLE, {} + released_at = record.get("released_at") + if isinstance(released_at, str) and released_at: + return LOCK_HOLDER_RELEASED, record + if record.get("host") != lock_holder_host_label(): + return LOCK_HOLDER_FOREIGN_HOST, record + pid = record.get("pid") + if isinstance(pid, bool) or not isinstance(pid, int): + return LOCK_HOLDER_UNREADABLE, record + return (LOCK_HOLDER_LIVE if process_is_alive(pid) else LOCK_HOLDER_DEAD), record def _operator_action(holder: dict[str, object], *, retry_mode: str) -> dict[str, object]: diff --git a/tests/test_turn_lane_fence.py b/tests/test_turn_lane_fence.py index 0341a6666e..2f5179f4b8 100644 --- a/tests/test_turn_lane_fence.py +++ b/tests/test_turn_lane_fence.py @@ -8,16 +8,29 @@ from __future__ import annotations +import json +import os import socket +import threading +import time from pathlib import Path from loopx.control_plane.turn_driver.executor import run_loopx_turn_once -from loopx.file_lock import _safe_label +from loopx.file_lock import _safe_label, lock_holder_path +from loopx.control_plane.turn_driver import lane_fence from loopx.control_plane.turn_driver.lane_fence import ( REMEDY_WAIT_FOR_IN_FLIGHT_TURN, + single_executor_per_turn_lane, + TURN_LANE_ABSENT, + TURN_LANE_DEAD, + TURN_LANE_FOREIGN_HOST, TURN_LANE_IN_FLIGHT, + TURN_LANE_LIVE, TURN_LANE_OPERATION, + TURN_LANE_RELEASED, + TURN_LANE_UNREADABLE, turn_lane_holder_readback, + turn_lane_liveness, turn_lane_singleflight, turn_lane_target, ) @@ -128,3 +141,132 @@ def test_the_holder_readback_stays_public_safe(tmp_path: Path) -> None: # The private lock identity and the runtime path never leave the process. assert str(tmp_path) not in str(holder) assert turn_lane_holder_readback(tmp_path / "absent.lane") == {} + + +def _lane(tmp_path: Path) -> Path: + return turn_lane_target( + runtime_root=tmp_path / "runtime", goal_id=GOAL_ID, plan=_plan() + ) + + +def _rewrite_holder(target: Path, **changes: object) -> None: + """Edit the holder record the way a crash or another machine would leave it.""" + + holder_path = lock_holder_path(target) + record = json.loads(holder_path.read_text(encoding="utf-8")) + record.pop("released_at", None) + record.update(changes) + holder_path.write_text(json.dumps(record), encoding="utf-8") + + +def test_liveness_follows_the_lane_from_absent_to_live_to_released( + tmp_path: Path, +) -> None: + target = _lane(tmp_path) + assert turn_lane_liveness(target) == {"state": TURN_LANE_ABSENT, "holder": {}} + + with turn_lane_singleflight( + runtime_root=tmp_path / "runtime", goal_id=GOAL_ID, plan=_plan() + ) as held: + assert held is not None + live = turn_lane_liveness(target) + + assert live["state"] == TURN_LANE_LIVE + # The holder is the same public-safe readback a refusal names. + assert live["holder"] == turn_lane_holder_readback(target) | {"pid": os.getpid()} + assert live["holder"]["pid"] == os.getpid() + assert str(tmp_path) not in json.dumps(live) + # A clean exit is a release, whatever the pid does afterwards. + assert turn_lane_liveness(target)["state"] == TURN_LANE_RELEASED + + +def test_liveness_fails_closed_on_dead_foreign_and_unreadable_holders( + tmp_path: Path, +) -> None: + target = _lane(tmp_path) + with turn_lane_singleflight( + runtime_root=tmp_path / "runtime", goal_id=GOAL_ID, plan=_plan() + ): + pass + + # A killed Turn never writes released_at; its pid is gone on this machine. + dead_pid = os.getpid() + while True: + dead_pid += 1 + try: + os.kill(dead_pid, 0) + except ProcessLookupError: + break + except OSError: + continue + if dead_pid > os.getpid() + 100_000: + raise AssertionError("no free pid found near this process") + _rewrite_holder(target, pid=dead_pid) + assert turn_lane_liveness(target)["state"] == TURN_LANE_DEAD + + # A holder on another machine cannot be pid-checked here, even if that pid + # happens to be alive on this one. + _rewrite_holder(target, pid=os.getpid(), host="another-machine") + foreign = turn_lane_liveness(target) + assert foreign["state"] == TURN_LANE_FOREIGN_HOST + assert foreign["holder"]["host"] == "another-machine" + + # A lock file with no parseable record is mid-acquisition or corrupt: not + # absent, and not evidence of anything. + lock_holder_path(target).write_text("", encoding="utf-8") + assert turn_lane_liveness(target) == {"state": TURN_LANE_UNREADABLE, "holder": {}} + lock_holder_path(target).write_text("{}", encoding="utf-8") + assert turn_lane_liveness(target)["state"] == TURN_LANE_UNREADABLE + + +def test_the_liveness_probe_never_refuses_a_concurrent_executing_turn( + tmp_path: Path, monkeypatch +) -> None: + """The probe reads a record; it never takes the lane, not even for an instant.""" + + fence_calls: list[str] = [] + real_fence = lane_fence.try_exclusive_file_lock + + def counting_fence(*args, **kwargs): + fence_calls.append(str(kwargs.get("operation"))) + return real_fence(*args, **kwargs) + + monkeypatch.setattr(lane_fence, "try_exclusive_file_lock", counting_fence) + target = _lane(tmp_path) + turn_lane_liveness(target) + turn_lane_holder_readback(target) + assert fence_calls == [] + + # The executing entry is the real fence wrapper run-once --execute goes + # through; only the Turn body is a stand-in that holds the lane a moment. + @single_executor_per_turn_lane( + lambda plan, record, **kwargs: {**record, "effects": kwargs["effects"]} + ) + def executing_turn(plan, *, runtime_root, goal_id, execute): + time.sleep(0.3) + return {"status": "committed", "held": turn_lane_liveness(target)["state"]} + + observed: set[str] = set() + stop = threading.Event() + + def probe() -> None: + while not stop.is_set(): + observed.add(turn_lane_liveness(target)["state"]) + + prober = threading.Thread(target=probe, daemon=True) + prober.start() + try: + payload = executing_turn( + _plan(), runtime_root=tmp_path / "runtime", goal_id=GOAL_ID, execute=True + ) + finally: + stop.set() + prober.join(timeout=5) + + # The Turn took the fence exactly once and was never told the lane was busy. + assert fence_calls == [TURN_LANE_OPERATION] + assert payload == {"status": "committed", "held": TURN_LANE_LIVE} + assert payload.get("reason") != TURN_LANE_IN_FLIGHT + assert observed <= {TURN_LANE_ABSENT, TURN_LANE_LIVE, TURN_LANE_RELEASED, TURN_LANE_UNREADABLE} + assert TURN_LANE_LIVE in observed + assert turn_lane_liveness(target)["state"] == TURN_LANE_RELEASED From 0b1a93f32f8f30cc65f1d6748edd104320c51d84 Mon Sep 17 00:00:00 2001 From: song <22676124+songoow@users.noreply.github.com> Date: Tue, 29 Sep 2026 06:16:04 -0400 Subject: [PATCH 02/13] feat(delegation): add stopped observation and typed stop decision Delegated members had no stopped observation: prepared, running and turn_returned could only end in accepted or rejected. Add "stopped" as a terminal observation reachable from the three open states and keep the inventory check driven by the same transition table. Add the exported collaboration.delegation.stop decision: a stop request settles only with an acknowledgement from a process that held the operation lock plus a free operation lock and a free Turn lane lock. Free locks with no acknowledgement are "unknown", never a fabricated settlement, and a grace timeout alone changes nothing while a lock is still held. Register the RPC handler next to the existing delegation observation handlers. Co-Authored-By: Claude Fable 5.1 Signed-off-by: song <22676124+songoow@users.noreply.github.com> --- .../control_plane/collaboration/delegation.ts | 39 +++++++++++++++++-- .../control_plane/effect_runtime_handlers.ts | 3 +- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/loopx/control_plane/collaboration/delegation.ts b/loopx/control_plane/collaboration/delegation.ts index f661435365..9ba1f172da 100644 --- a/loopx/control_plane/collaboration/delegation.ts +++ b/loopx/control_plane/collaboration/delegation.ts @@ -81,7 +81,7 @@ export function selectDelegationBinding(params: JsonObject): JsonObject { return binding; } -type Observation = "prepared" | "running" | "turn_returned" | "accepted" | "rejected"; +type Observation = "prepared" | "running" | "turn_returned" | "accepted" | "rejected" | "stopped"; function boundedReason(value: unknown, fallback: string): string { if (typeof value !== "string") return fallback; @@ -277,8 +277,8 @@ export function delegationPreflight(params: JsonObject): JsonObject { }; } const transitions: Record = { - prepared: ["running", "rejected"], running: ["turn_returned", "rejected"], - turn_returned: ["accepted", "rejected"], accepted: [], rejected: [], + prepared: ["running", "rejected", "stopped"], running: ["turn_returned", "rejected", "stopped"], + turn_returned: ["accepted", "rejected", "stopped"], accepted: [], rejected: [], stopped: [], }; /** Page only the caller's existing journal. A cursor is not a fleet snapshot. */ @@ -341,6 +341,39 @@ export function transitionDelegationObservation(params: JsonObject): JsonObject return {status: to}; } +type StopPhase = "requested" | "acknowledged" | "settled" | "unknown"; +const openStopPhases: readonly StopPhase[] = ["requested", "acknowledged"]; + +/** Advance one stop request from host lock facts; a receipt is never inferred from time. + * + * ``settled`` needs the acknowledgement of a process that held the operation + * lock plus both the operation lock and the Turn lane lock free: only then is + * the worker, its Turn child and its lane provably gone. Free locks without an + * acknowledgement mean the holder vanished before recording what it observed, + * which is ``unknown`` rather than a fake settlement. A grace timeout on its + * own moves nothing: a worker that is still holding a lock is still running. + */ +export function decideDelegationStop(params: JsonObject): JsonObject { + const phase = params.phase as StopPhase; + requireThat(openStopPhases.includes(phase), "delegation stop decision requires an open stop phase"); + requireThat(typeof params.acknowledged === "boolean", "delegation stop acknowledgement fact required"); + requireThat(typeof params.operation_lock_free === "boolean" && typeof params.lane_lock_free === "boolean", + "delegation stop lock facts required"); + requireThat(params.timed_out === undefined || typeof params.timed_out === "boolean", + "delegation stop timeout fact must be boolean"); + requireThat(phase !== "acknowledged" || params.acknowledged === true, + "an acknowledged stop cannot lose its acknowledgement"); + const locksFree = params.operation_lock_free === true && params.lane_lock_free === true; + if (params.acknowledged === true) { + if (locksFree) return {phase: "settled", terminal: true, reason: "acknowledged_and_locks_released"}; + return {phase: "acknowledged", terminal: false, reason: params.operation_lock_free === true + ? "turn_lane_still_held" : "operation_lock_still_held"}; + } + if (locksFree) return {phase: "unknown", terminal: true, reason: "holder_gone_without_acknowledgement"}; + return {phase: "requested", terminal: false, reason: params.timed_out === true + ? "holder_still_running_after_grace" : "awaiting_acknowledgement"}; +} + /** Repair only a false terminal observation after the exact Turn validated. * * This does not retry model work. The host boundary must prove that the diff --git a/loopx/control_plane/effect_runtime_handlers.ts b/loopx/control_plane/effect_runtime_handlers.ts index c239fa3c49..5eb9df9079 100644 --- a/loopx/control_plane/effect_runtime_handlers.ts +++ b/loopx/control_plane/effect_runtime_handlers.ts @@ -16,7 +16,7 @@ import {evaluateUserCompletion} from "./todos/user_completion.ts"; import {projectTodoSuccession} from "./todos/succession.ts"; import {projectLegacyTodoWorkCounts} from "./todos/summary_lanes.ts"; import {sealProjectionEnvelope} from "./projection_envelope.ts"; -import {recordDelegationAdoption, delegationInventoryItem, delegationInventoryQuery, delegationPreflight, delegationTurnPlanDecision, delegationValidationPlan, recoverValidatedDelegationSettlement, selectDelegationBinding, transitionDelegationObservation} from "./collaboration/delegation.ts"; +import {recordDelegationAdoption, decideDelegationStop, delegationInventoryItem, delegationInventoryQuery, delegationPreflight, delegationTurnPlanDecision, delegationValidationPlan, recoverValidatedDelegationSettlement, selectDelegationBinding, transitionDelegationObservation} from "./collaboration/delegation.ts"; import {resolveConversationTrigger} from "./collaboration/conversation_trigger.ts"; import {admitGoalDraft} from "./collaboration/goal_draft.ts"; import {planChatMode} from "./collaboration/chat_mode.ts"; @@ -744,6 +744,7 @@ export function createEffectRuntimeHandlers( ["chat.turn.accept", planChatTurnAcceptance], ["collaboration.delegation.observe", transitionDelegationObservation], ["collaboration.delegation.recover_validated_settlement", recoverValidatedDelegationSettlement], + ["collaboration.delegation.stop", decideDelegationStop], ["collaboration.delegation.adoption", recordDelegationAdoption], [ "collaboration.request.normalize", From 62d4c920e7d8f8018ace8b84510f4bc21a6dcd96 Mon Sep 17 00:00:00 2001 From: song <22676124+songoow@users.noreply.github.com> Date: Tue, 29 Sep 2026 06:29:48 -0400 Subject: [PATCH 03/13] feat(delegation): stop delegated members with acknowledged, settled receipts Delegated members could not be stopped: the worker held the operation lock for the whole run and rewrote the execution record from memory, so nothing written into that record could reach it or survive it, and a killed run left its Turn and hard lease unaccounted for. Add a stop receipt beside the execution record (executions//.stop.json) written only under the existing .dispatch lock, never into the record. Its phases are requested -> acknowledged -> settled with the terminals unknown and noop, and it records the requester, the lock-holding worker (pid, process group, host), the acknowledgement (pid, observed status, Turn key), the lease release outcome and the settlement facts (operation lock, Turn lane lock, Turn journal status). Delegations.stop: a terminal record returns an identical noop receipt on every call. Otherwise the request is written; when no worker holds the operation the requester takes the lock, marks the record stopped, releases the hard lease and settles. A same-host holder is SIGTERMed by process group (its run-once child and host bridge follow) and SIGKILLed only if it still holds the operation after the grace; another host's holder is never signalled and finds the request itself. Settlement is the typed collaboration.delegation.stop decision over lock facts: elapsed time is never a receipt. Worker: a SIGTERM handler raises DelegationStopRequested once; checkpoints before running, before each run-once and before completing the Todo raise on a stop file; the record is written only through a fenced write that re-reads the stop file under .dispatch and raises DelegationFenced for a stop this process did not acknowledge, so a late-returning or other-host worker records no Turn result and completes no Todo. Acknowledgement marks the record stopped, releases the hard lease and leaves the in_progress Turn journal for inspection. execute records the worker's pid/pgid/host at entry and acknowledges from under the lock when a stop already exists. resume refuses stopped work ("start a new operation id"), wait returns on stopped, and read exposes the stop phase. file_lock gains local_lock_host and read_lock_holder so the stop names the holder exactly as lock records do. Co-Authored-By: Claude Fable 5.1 Signed-off-by: song <22676124+songoow@users.noreply.github.com> --- loopx/collaboration_mcp.py | 414 ++++++++++++++++++++++++++++++++++--- loopx/file_lock.py | 11 + 2 files changed, 400 insertions(+), 25 deletions(-) diff --git a/loopx/collaboration_mcp.py b/loopx/collaboration_mcp.py index c3f2b9304b..64e192ff00 100644 --- a/loopx/collaboration_mcp.py +++ b/loopx/collaboration_mcp.py @@ -14,6 +14,7 @@ import hashlib import json import os +import signal import stat import subprocess import sys @@ -26,7 +27,10 @@ if TYPE_CHECKING: from mcp.server.fastmcp import FastMCP -from .file_lock import exclusive_file_lock, LockAcquisitionPolicy, LockAcquireTimeoutError +from .file_lock import ( + exclusive_file_lock, lock_holder_host_label, read_lock_holder, try_exclusive_file_lock, + LockAcquisitionPolicy, LockAcquireTimeoutError, +) from .control_plane.effect_runtime import ( effect_runtime_request_scope, effect_runtime_result, EffectRuntimeRemoteError, ) @@ -38,6 +42,8 @@ turn_journal_path, ) from .control_plane.turn_driver.host_binding import turn_host_arg_option +from .control_plane.turn_driver.lane_fence import turn_lane_target +from .control_plane.work_items.task_lease import release_task_lease from .control_plane.collaboration.inbox import _hash, _read, _write, _root, _receipt from .control_plane.collaboration.peers import return_result from .control_plane.collaboration.inbox import acknowledge, _entry, normalize_request @@ -300,6 +306,65 @@ def consume_peer_result(request_id: str) -> dict: ) +DELEGATION_STOP_SCHEMA_VERSION = "loopx_delegation_stop_v0" +# Observations that no worker may reopen; a stop against one is a no-op receipt. +DELEGATION_TERMINAL_STATUSES = frozenset({"accepted", "rejected", "stopped"}) +DELEGATION_STOP_OPEN_PHASES = frozenset({"requested", "acknowledged"}) +# How long a signalled same-host worker may take to acknowledge before SIGKILL. +DELEGATION_STOP_GRACE_SECONDS = 10.0 +DELEGATION_STOPPED_MESSAGE = "delegation operation was stopped; start a new operation id" + + +class DelegationStopRequested(Exception): + """A stop reached the worker that owns this operation; it must acknowledge, not finish.""" + + def __init__(self, source: str) -> None: + super().__init__(source) + self.source = source + + +class DelegationFenced(DelegationStopRequested): + """A stop this process never acknowledged fences its execution-record write. + + The write is refused before it happens: a late-returning or other-host + worker records no Turn result, completes no Todo and publishes nothing. + """ + + def __init__(self) -> None: + super().__init__("fenced") + + +class _WorkerStopSignal: + """Turn the first SIGTERM into a stop request; absorb later ones during the acknowledgement.""" + + def __init__(self) -> None: + self.armed = True + + def __call__(self, signum: int, frame: object) -> None: + if not self.armed: + return + self.armed = False + raise DelegationStopRequested("SIGTERM") + + def disarm(self) -> None: + self.armed = False + + +def install_worker_stop_signal() -> _WorkerStopSignal | None: + """Install the detached worker's SIGTERM handler; ``None`` where signals are unsupported.""" + + if not hasattr(signal, "SIGTERM"): + return None + handler = _WorkerStopSignal() + try: + signal.signal(signal.SIGTERM, handler) + except (ValueError, OSError): + # Not the main thread, or a platform without handler support: the + # worker still honours stop files at every checkpoint and fenced write. + return None + return handler + + class Delegations: """Host IO for bound peer work; typed grants and observations stay in TS. @@ -311,6 +376,7 @@ def __init__(self, root: Path, registry: Path, goal_id: str, agent_id: str, conf self.root, self.registry = root.resolve(), registry.resolve() self.goal_id, self.agent_id, self.config = goal_id, agent_id, config.resolve() self._goal_ref_lock = Lock() + self._stop_signal: _WorkerStopSignal | None = None try: self.goal_ref = capture_collaboration_goal_ref( self.registry, @@ -352,6 +418,22 @@ def directory(self) -> dict: def path(self, operation_id: str) -> Path: return _root(self.root) / "executions" / _hash([self.goal_id, self.agent_id]) / (_hash(operation_id) + ".json") + @staticmethod + def _stop_path(path: Path) -> Path: + """The stop receipt sits beside its execution record and is never merged into it.""" + return path.with_name(path.stem + ".stop.json") + + @staticmethod + def _dispatch_lock(path: Path) -> Path: + return path.with_suffix(".dispatch") + + @staticmethod + def _read_stop(path: Path) -> dict | None: + stop_path = Delegations._stop_path(path) + if not stop_path.exists(): + return None + return _read(stop_path) + def operations(self, *, limit: int = 20, cursor: str | None = None) -> dict: from .control_plane.collaboration.delegation_inventory import read_delegation_inventory @@ -499,15 +581,19 @@ def _spawn(self, operation_id: str) -> None: def resume(self, operation_id: str) -> dict: path = self.path(operation_id) + if self._read_stop(path) is not None: + raise ValueError(DELEGATION_STOPPED_MESSAGE) try: with exclusive_file_lock( path, policy=LockAcquisitionPolicy.SINGLE_FLIGHT ): row = _read(path) binding = self._bound(row) + if row["status"] == "stopped": + raise ValueError(DELEGATION_STOPPED_MESSAGE) if row["status"] == "rejected": self._recover_validated_settlement(path, row, binding) - should_spawn = row["status"] not in {"accepted", "rejected"} + should_spawn = row["status"] not in DELEGATION_TERMINAL_STATUSES if should_spawn: self.binding( row["identity"]["binding"]["id"], require_active=True @@ -524,7 +610,7 @@ def wait(self, operation_id: str) -> dict: """Observe for at most 15 seconds; waiting neither starts nor resumes work.""" for _ in range(5): result = self.read(operation_id) - if result["status"] in {"accepted", "rejected"} or result["recovery_required"]: + if result["status"] in DELEGATION_TERMINAL_STATUSES or result["recovery_required"]: return result time.sleep(3) return self.read(operation_id) @@ -611,7 +697,7 @@ def _recover_validated_settlement( "error": None, } row.pop("error", None) - _write(path, row) + self._fenced_write(path, row) return True def adopt_result(self, operation_id: str, consumer_operation_id: str) -> dict: @@ -637,8 +723,11 @@ def _read_current(self, operation_id: str) -> dict: result = {"operation_id": operation_id, "request_id": row["identity"]["request_id"], "agent_id": binding["agent_id"], "todo_id": binding["todo_id"], "status": row["status"], "worker_active": active, - "recovery_required": not active and row["status"] not in {"accepted", "rejected"} + "recovery_required": not active and row["status"] not in DELEGATION_TERMINAL_STATUSES and time.time() - row.get("created_at", 0) > 15} + stop = self._read_stop(path) + if stop is not None: + result["stop"] = {"stop_id": stop["stop_id"], "phase": stop["phase"]} if row["status"] == "accepted": # A saved receipt cannot hide an amended task, verifier or output. artifacts = self._accepted(binding) @@ -654,7 +743,261 @@ def _observe(self, path: Path, row: dict, status: str, **facts) -> None: "from": row["status"], "to": status, **facts, }) row.update(status=decision["status"]) - _write(path, row) + self._fenced_write(path, row) + + def _fenced_write(self, path: Path, row: dict) -> None: + """Write the execution record only while no unacknowledged stop fences this process. + + The stop receipt is re-read under the dispatch lock on every write, so a + worker that returns after a stop it never saw writes nothing at all. + """ + + with exclusive_file_lock(self._dispatch_lock(path)): + stop = self._read_stop(path) + if stop is not None and not self._acknowledged_here(stop): + raise DelegationFenced() + _write(path, row) + + @staticmethod + def _acknowledged_here(stop: dict) -> bool: + ack = stop.get("ack") + return (isinstance(ack, dict) and ack.get("pid") == os.getpid() + and ack.get("host") == lock_holder_host_label()) + + def _raise_if_stop_requested(self, path: Path) -> None: + """Worker checkpoint: leave before the next host launch or Todo effect.""" + if self._read_stop(path) is not None: + raise DelegationStopRequested("stop_file") + + @staticmethod + def _worker_identity() -> dict: + return { + "pid": os.getpid(), + "pgid": os.getpgid(0) if hasattr(os, "getpgid") else None, + "host": lock_holder_host_label(), + } + + def _lane_target(self, binding: dict) -> Path: + return turn_lane_target(runtime_root=self.root, goal_id=self.goal_id, + plan={"turn_envelope": {"agent_id": binding["agent_id"]}}) + + def _operation_lock_free(self, path: Path) -> bool: + try: + with exclusive_file_lock(path, policy=LockAcquisitionPolicy.SINGLE_FLIGHT): + return True + except LockAcquireTimeoutError: + return False + + def _lane_lock_free(self, binding: dict) -> bool: + # The kernel lock is the only proof; the holder record is advisory. The + # probe holds the lane for an instant, which is the same observation a + # status read makes on the operation lock. + with try_exclusive_file_lock(self._lane_target(binding), agent_id=self.agent_id, + operation="loopx_delegation_stop_probe") as held: + return held is not None + + def _turn_journal_status(self, row: dict, binding: dict) -> str | None: + turn_key = row.get("turn_key") or self._matching_turn_key(row, binding) + if not turn_key: + return None + journal = load_turn_journal(turn_journal_path(self.root, goal_id=self.goal_id, turn_key=turn_key)) + status = journal.get("status") if isinstance(journal, dict) else None + return str(status) if status else None + + def _new_stop_record(self, row: dict, *, requested_by: str, worker: dict | None) -> dict: + requested_at = time.time() + return { + "schema_version": DELEGATION_STOP_SCHEMA_VERSION, + "stop_id": _hash([row["identity"]["operation_id"], requested_by, requested_at])[:32], + "operation_id": row["identity"]["operation_id"], + "request_id": row["identity"]["request_id"], + "phase": "requested", + "reason": "awaiting_acknowledgement", + "requested_by": requested_by, + "requested_at": requested_at, + "requested_status": row["status"], + "worker": worker, + "ack": None, + "lease": None, + "settled": None, + } + + def _stop_receipt(self, row: dict, binding: dict, stop: dict | None) -> dict: + receipt = { + "operation_id": row["identity"]["operation_id"], + "request_id": row["identity"]["request_id"], + "agent_id": binding["agent_id"], "todo_id": binding["todo_id"], + "status": row["status"], + } + if stop is None: + # Nothing was written: a terminal observation cannot be stopped, and + # repeating the request returns exactly this receipt again. + receipt.update(phase="noop", reason="delegation already " + row["status"], stop=None) + else: + receipt.update(phase=stop["phase"], reason=stop.get("reason"), stop=stop) + return receipt + + def stop(self, operation_id: str, *, execute: bool) -> dict: + """Stop one bounded member and return a receipt that says what was proven. + + ``requested`` is written beside the execution record, never into it. + When no worker holds the operation, this caller takes the lock, marks + the record stopped and releases the hard lease itself. A same-host + holder is signalled by process group and given a bounded grace to + acknowledge; another host's holder is left to find the request at its + next checkpoint or fenced write. ``settled`` and ``unknown`` come from + the typed decision over lock facts; elapsed time proves nothing. + """ + + require_operation_id(operation_id) + if not execute: + raise ValueError("delegation stop requires execute") + path = self.path(operation_id) + if not path.exists(): + raise ValueError("unknown delegation operation; start_delegation returns the operation_id to stop") + with exclusive_file_lock(self._dispatch_lock(path)): + row = _read(path) + binding = self._bound(row) + stop = self._read_stop(path) + if stop is None: + if row["status"] in DELEGATION_TERMINAL_STATUSES: + return self._stop_receipt(row, binding, None) + stop = self._new_stop_record(row, requested_by=self.agent_id, + worker=self._lock_holder_worker(path, row)) + _write(self._stop_path(path), stop) + if stop["phase"] in DELEGATION_STOP_OPEN_PHASES and stop.get("ack") is None: + try: + with exclusive_file_lock(path, policy=LockAcquisitionPolicy.SINGLE_FLIGHT): + row = _read(path) + self._acknowledge_stop(path, row, binding, source="requester") + except LockAcquireTimeoutError: + self._signal_worker(path, stop) + return self._settle_stop(path) + + def _lock_holder_worker(self, path: Path, row: dict) -> dict | None: + """Name the live operation-lock holder, else ``None``; a released record is not a worker.""" + + if self._operation_lock_free(path): + return None + holder = read_lock_holder(path) + if "released_at" in holder or not isinstance(holder.get("pid"), int): + return None + recorded = row.get("worker") if isinstance(row.get("worker"), dict) else {} + same = recorded.get("pid") == holder["pid"] and recorded.get("host") == holder.get("host") + pgid = recorded.get("pgid") if same and isinstance(recorded.get("pgid"), int) else holder["pid"] + return {"pid": holder["pid"], "pgid": pgid, "host": holder.get("host")} + + def _signal_worker(self, path: Path, stop: dict) -> None: + """Terminate a same-host holder's process group; never signal across hosts.""" + + worker = stop.get("worker") + if (not isinstance(worker, dict) or worker.get("host") != lock_holder_host_label() + or not hasattr(os, "killpg") or not isinstance(worker.get("pid"), int)): + return + pid, pgid = worker["pid"], worker.get("pgid") or worker["pid"] + if pgid == os.getpgid(0): + raise ValueError("delegation stop refuses to signal its own process group") + try: + if os.getpgid(pid) != pgid: + return # the pid was reused by an unrelated process + os.killpg(pgid, signal.SIGTERM) + except ProcessLookupError: + return + deadline = time.monotonic() + DELEGATION_STOP_GRACE_SECONDS + while time.monotonic() < deadline: + if self._operation_lock_free(path): + return + time.sleep(0.2) + try: + os.killpg(pgid, signal.SIGKILL) + except ProcessLookupError: + return + deadline = time.monotonic() + 2.0 + while time.monotonic() < deadline and not self._operation_lock_free(path): + time.sleep(0.1) + + def _acknowledge_stop(self, path: Path, row: dict, binding: dict, *, source: str) -> None: + """Acknowledge from under the operation lock: mark stopped, then release the hard lease. + + Only the lock holder may acknowledge. A stop that another process already + acknowledged, or that already settled, is left untouched. + """ + + if self._stop_signal is not None: + self._stop_signal.disarm() + with exclusive_file_lock(self._dispatch_lock(path)): + stop = self._read_stop(path) + if stop is None: + stop = self._new_stop_record(row, requested_by="signal:" + source, + worker=self._worker_identity()) + if stop.get("ack") is not None or stop["phase"] not in DELEGATION_STOP_OPEN_PHASES: + return + observed = row["status"] + decision = effect_runtime_result("collaboration.delegation.observe", { + "from": observed, "to": "stopped", + }) + row.update(status=decision["status"]) + _write(path, row) + stop.update(phase="acknowledged", reason="awaiting_lock_release", ack={ + "pid": os.getpid(), "host": lock_holder_host_label(), "at": time.time(), + "source": source, "observed_status": observed, + "turn_key": row.get("turn_key") or self._matching_turn_key(row, binding), + }) + _write(self._stop_path(path), stop) + try: + self._clear_delegation_bootstrap(row, binding) + except (OSError, ValueError): + pass # the bootstrap is host input; its state never blocks the receipt + lease = self._release_delegation_lease(row, binding) + with exclusive_file_lock(self._dispatch_lock(path)): + stop = self._read_stop(path) or stop + stop["lease"] = lease + _write(self._stop_path(path), stop) + + def _release_delegation_lease(self, row: dict, binding: dict) -> dict: + lease = row.get("task_lease") + if not isinstance(lease, dict) or lease.get("required") is not True: + return {"required": False, "released": None} + try: + result = release_task_lease( + runtime_root=self.root, goal_id=self.goal_id, todo_id=binding["todo_id"], + owner=binding["agent_id"], idempotency_key=str(lease["idempotency_key"]), + expected_version=lease.get("version"), registry_path=self.registry, + ) + except (ValueError, OSError, RuntimeError, EffectRuntimeRemoteError) as exc: + return {"required": True, "released": False, "error": str(exc)[:180]} + return {"required": True, "released": result.get("released") is True, + "missing": result.get("missing") is True} + + def _settle_stop(self, path: Path) -> dict: + with exclusive_file_lock(self._dispatch_lock(path)): + row = _read(path) + binding = self._bound(row) + stop = self._read_stop(path) + if stop is None: + return self._stop_receipt(row, binding, None) + if stop["phase"] not in DELEGATION_STOP_OPEN_PHASES: + return self._stop_receipt(row, binding, stop) + facts = { + "operation_lock_free": self._operation_lock_free(path), + "lane_lock_free": self._lane_lock_free(binding), + } + decision = effect_runtime_result("collaboration.delegation.stop", { + "phase": stop["phase"], "acknowledged": stop.get("ack") is not None, + "timed_out": time.time() - stop["requested_at"] > DELEGATION_STOP_GRACE_SECONDS, + **facts, + }) + if decision["phase"] != stop["phase"] or decision.get("reason") != stop.get("reason"): + stop.update(phase=decision["phase"], reason=decision.get("reason")) + if decision["phase"] in {"settled", "unknown"}: + lease = stop.get("lease") if isinstance(stop.get("lease"), dict) else {} + stop["settled"] = { + "at": time.time(), **facts, + "lease_released": lease.get("released"), + "turn_journal_status": self._turn_journal_status(row, binding), + } + _write(self._stop_path(path), stop) + return self._stop_receipt(row, binding, stop) def _cli(self, binding: dict, *args: str, timeout: int = 60) -> dict: completed = subprocess.run([*_python_module_command("loopx.cli"), @@ -711,21 +1054,34 @@ def execute(self, operation_id: str) -> None: # The existing bounded mutation policy still excludes concurrent workers. with exclusive_file_lock(path): row = _read(path) - if row["status"] in {"accepted", "rejected"}: + if row["status"] in DELEGATION_TERMINAL_STATUSES: return - binding = self._bound(row, require_active=True) - row.pop("error", None) - _write(path, row) - # Different request ids cannot run the same assigned task concurrently. - task_lock = _root(self.root) / "execution-slots" / _hash([self.goal_id, binding["todo_id"]]) - with exclusive_file_lock(task_lock, policy=LockAcquisitionPolicy.SINGLE_FLIGHT): - try: - self._execute(path, row, binding) - except (ValueError, KeyError, subprocess.TimeoutExpired, EffectRuntimeRemoteError) as exc: - row["error"] = str(exc)[:180] if isinstance(exc, (ValueError, EffectRuntimeRemoteError)) else type(exc).__name__ - _write(path, row) - if row["status"] == "prepared": - self._observe(path, row, "rejected") + binding = self._bound(row) + if self._read_stop(path) is not None: + # The stop arrived before any worker owned the operation: this + # holder acknowledges it from under the lock and launches nothing. + self._acknowledge_stop(path, row, binding, source="worker_entry") + return + try: + binding = self._bound(row, require_active=True) + row.pop("error", None) + row["worker"] = self._worker_identity() + self._fenced_write(path, row) + # Different request ids cannot run the same assigned task concurrently. + task_lock = _root(self.root) / "execution-slots" / _hash([self.goal_id, binding["todo_id"]]) + with exclusive_file_lock(task_lock, policy=LockAcquisitionPolicy.SINGLE_FLIGHT): + try: + self._execute(path, row, binding) + except (ValueError, KeyError, subprocess.TimeoutExpired, EffectRuntimeRemoteError) as exc: + row["error"] = str(exc)[:180] if isinstance(exc, (ValueError, EffectRuntimeRemoteError)) else type(exc).__name__ + self._fenced_write(path, row) + if row["status"] == "prepared": + self._observe(path, row, "rejected") + except DelegationStopRequested as stop: + # SIGTERM, a checkpoint or a fenced write: the host child is already + # gone (its run-once exits with this process's exception), the + # bootstrap was cleared, and only the acknowledgement remains. + self._acknowledge_stop(path, row, binding, source=stop.source) def _execution_arguments(self, binding: dict, operation_id: str) -> list[str]: """Exactly the same profile, workspace and validation arguments for preview/run.""" @@ -790,7 +1146,7 @@ def _record_turn_result( if publish: self._observe(path, row, "turn_returned") else: - _write(path, row) + self._fenced_write(path, row) def _receiver_adopted(self, row: dict, binding: dict) -> bool: request_id = row["identity"]["request_id"] @@ -892,7 +1248,7 @@ def _acquire_delegation_lease( goal_id=self.goal_id, ): row["task_lease"] = {"required": False, "handoff_mode": "legacy"} - _write(path, row) + self._fenced_write(path, row) return row["task_lease"] handoff_mode = show_goal_handoff_mode( registry_path=self.registry, @@ -904,7 +1260,7 @@ def _acquire_delegation_lease( "required": False, "handoff_mode": handoff_mode, } - _write(path, row) + self._fenced_write(path, row) return row["task_lease"] lease_key = self._turn_instance_id(row) result = self._cli( @@ -946,7 +1302,7 @@ def _acquire_delegation_lease( "idempotency_key": lease_key, "version": lease["version"], } - _write(path, row) + self._fenced_write(path, row) return row["task_lease"] def _complete_delegated_todo(self, row: dict, binding: dict) -> None: @@ -991,6 +1347,7 @@ def _execute(self, path: Path, row: dict, binding: dict) -> None: common = ["--goal-id", self.goal_id, "--agent-id", binding["agent_id"]] execution = self._execution_arguments(binding, row["identity"]["operation_id"]) try: + self._raise_if_stop_requested(path) if row["status"] == "prepared": acceptance = delegation_validation.capture(self, binding) if acceptance["plan"]["state"] != "ready" or not acceptance["files_current"]: @@ -1000,6 +1357,7 @@ def _execute(self, path: Path, row: dict, binding: dict) -> None: self._acquire_delegation_lease(path, row, binding) self._observe(path, row, "running") if row["status"] == "running": + self._raise_if_stop_requested(path) turn_key = self._matching_turn_key(row, binding) selector = ( ["--resume-turn-key", turn_key] @@ -1042,8 +1400,10 @@ def _execute(self, path: Path, row: dict, binding: dict) -> None: ) if not isinstance(row.get("task_lease"), dict): self._acquire_delegation_lease(path, row, binding) + self._raise_if_stop_requested(path) self._complete_delegated_todo(row, binding) todo_completed_for_settlement = True + self._raise_if_stop_requested(path) result = self._cli( binding, "turn", @@ -1071,6 +1431,7 @@ def _execute(self, path: Path, row: dict, binding: dict) -> None: self._bound(row, require_active=True) # revocation or rebinding while the model ran delegation_results.require_dependencies(self, binding, delegation_results.operation_brief(self, row)) if not todo_completed_for_settlement: + self._raise_if_stop_requested(path) self._complete_delegated_todo(row, binding) row["artifacts"] = self._accepted(binding) if not (_root(self.root) / "replies" / request_id / "conclusion.json").exists(): @@ -1097,7 +1458,7 @@ def _execute(self, path: Path, row: dict, binding: dict) -> None: # Retain uncertain execution for explicit same-operation recovery. # No fresh Turn is ever created because its client timed out. row["error"] = str(exc)[:180] if isinstance(exc, (ValueError, EffectRuntimeRemoteError)) else type(exc).__name__ - _write(path, row) + self._fenced_write(path, row) def register_delegation_tools(server, delegations: Delegations) -> None: @@ -1185,10 +1546,13 @@ def main(): if args.delegation_action == "validate": service._validate(service._bound(_read(service.path(args.operation_id)))) else: + service._stop_signal = install_worker_stop_signal() try: service.execute(args.operation_id) except LockAcquireTimeoutError: pass # Another worker still owns the operation after the bounded wait. + except DelegationStopRequested: + pass # Stopped before owning the operation; the holder acknowledges. return if args.workspace is None: parser.error("--workspace is required when serving MCP") diff --git a/loopx/file_lock.py b/loopx/file_lock.py index 7ffa396822..29303c066e 100644 --- a/loopx/file_lock.py +++ b/loopx/file_lock.py @@ -360,6 +360,17 @@ def lock_holder_liveness(path: Path) -> tuple[str, dict[str, object]]: return (LOCK_HOLDER_LIVE if process_is_alive(pid) else LOCK_HOLDER_DEAD), record +def read_lock_holder(path: Path) -> dict[str, object]: + """Read the advisory holder record behind ``path``'s lock; ``{}`` when absent. + + The record names the last process that held the lock and carries + ``released_at`` after a clean release. It is advisory readback for signals + and operator inspection; the kernel lock stays the only proof of holding. + """ + + return _read_holder_record(lock_holder_path(path)) + + def _operator_action(holder: dict[str, object], *, retry_mode: str) -> dict[str, object]: return { "required": True, From 654c1fbf0063505132eea0d04c4be1abbc66e39d Mon Sep 17 00:00:00 2001 From: song <22676124+songoow@users.noreply.github.com> Date: Tue, 29 Sep 2026 06:29:48 -0400 Subject: [PATCH 04/13] feat(delegation): expose stop through the CLI and MCP surfaces Add `loopx delegation stop --operation-id ID --execute` next to start/resume/adopt (--execute is required for the same reason) and the `stop_delegation(operation_id)` MCP tool, which runs the blocking stop off the event loop like wait_delegation. Both return the same receipt and never resume or rerun work. The inventory reader skips `.stop.json` sidecars, which sit beside execution records but are not records, and the delegation context and subagent context projections count `stopped` receipts instead of dropping them. Co-Authored-By: Claude Fable 5.1 Signed-off-by: song <22676124+songoow@users.noreply.github.com> --- loopx/cli_commands/delegation.py | 12 +++++++----- loopx/collaboration_mcp.py | 12 ++++++++++++ .../collaboration/delegation_context.py | 1 + .../collaboration/delegation_inventory.py | 4 ++-- loopx/control_plane/subagent_context.ts | 2 +- 5 files changed, 23 insertions(+), 8 deletions(-) diff --git a/loopx/cli_commands/delegation.py b/loopx/cli_commands/delegation.py index 9d2fb72a97..e422e68c64 100644 --- a/loopx/cli_commands/delegation.py +++ b/loopx/cli_commands/delegation.py @@ -22,7 +22,7 @@ def register_delegation( "delegation", help="Launch and recover authorized peer work; returns JSON." ) add_format(parser) - parser.add_argument("delegation_action", choices=("list", "operations", "inspect", "start", "read", "wait", "resume", "adopt")) + parser.add_argument("delegation_action", choices=("list", "operations", "inspect", "start", "read", "wait", "resume", "adopt", "stop")) parser.add_argument("--goal-id", required=True) parser.add_argument("--agent-id", required=True, help="Calling registered Agent, not the worker.") parser.add_argument("--execution-config", type=Path, required=True, @@ -34,7 +34,7 @@ def register_delegation( parser.add_argument("--parent-request-id", help="For start: the request received by this coordinator.") parser.add_argument("--limit", type=int, help="For operations: page size, 1–50 (default 20).") parser.add_argument("--cursor", help="For operations: next_cursor returned by the previous page.") - parser.add_argument("--execute", action="store_true", help="Required for start/resume/adopt; grants no additional authority.") + parser.add_argument("--execute", action="store_true", help="Required for start/resume/adopt/stop; grants no additional authority.") def handle_delegation( @@ -45,10 +45,10 @@ def handle_delegation( action = args.delegation_action try: - if action in {"start", "resume", "adopt"} and not args.execute: + if action in {"start", "resume", "adopt", "stop"} and not args.execute: raise ValueError(f"delegation {action} requires --execute") - if action not in {"start", "resume", "adopt"} and args.execute: - raise ValueError("--execute is only valid for start/resume/adopt") + if action not in {"start", "resume", "adopt", "stop"} and args.execute: + raise ValueError("--execute is only valid for start/resume/adopt/stop") if action not in {"list", "operations", "inspect"} and not args.operation_id: raise ValueError(f"delegation {action} requires --operation-id") if action in {"list", "operations", "inspect"} and args.operation_id: @@ -86,6 +86,8 @@ def handle_delegation( result = service.read(args.operation_id) elif action == "wait": result = service.wait(args.operation_id) + elif action == "stop": + result = service.stop(args.operation_id, execute=args.execute) else: result = service.resume(args.operation_id) payload = {"ok": True, **result} diff --git a/loopx/collaboration_mcp.py b/loopx/collaboration_mcp.py index 64e192ff00..0070601fb3 100644 --- a/loopx/collaboration_mcp.py +++ b/loopx/collaboration_mcp.py @@ -1525,6 +1525,18 @@ def resume_delegation(operation_id: str) -> dict: """Reconnect an interrupted original execution; never launch a replacement Turn.""" return delegations.resume(operation_id) + @server.tool() + async def stop_delegation(operation_id: str) -> dict: + """Stop one original operation and return what was proven, not what was hoped. + + settled: the worker acknowledged and its operation and Turn lane locks are + free. acknowledged/requested: still winding down; call again. unknown: the + holder vanished before acknowledging; inspect its Turn before reusing the + task. noop: already accepted/rejected/stopped. Stopped work is not resumed; + a new scope needs a new operation id. Elapsed time is never a receipt. + """ + return await asyncio.to_thread(delegations.stop, operation_id, execute=True) + def main(): parser = argparse.ArgumentParser(description=__doc__) diff --git a/loopx/control_plane/collaboration/delegation_context.py b/loopx/control_plane/collaboration/delegation_context.py index 1886379374..187297b6bb 100644 --- a/loopx/control_plane/collaboration/delegation_context.py +++ b/loopx/control_plane/collaboration/delegation_context.py @@ -152,6 +152,7 @@ def project_delegation_context( "turn_returned", "accepted", "rejected", + "stopped", "unavailable", ) if statuses[key] diff --git a/loopx/control_plane/collaboration/delegation_inventory.py b/loopx/control_plane/collaboration/delegation_inventory.py index 66bd66a9f2..f070107f80 100644 --- a/loopx/control_plane/collaboration/delegation_inventory.py +++ b/loopx/control_plane/collaboration/delegation_inventory.py @@ -24,8 +24,8 @@ def addresses(): try: entries = directory.iterdir() for path in entries: - if path.suffix != ".json": - continue + if path.suffix != ".json" or path.name.endswith(".stop.json"): + continue # stop receipts sit beside their execution record if not BARE_SHA256_PATTERN.fullmatch(path.stem): raise ValueError("unexpected delegation record address; reconcile inventory storage") if query["cursor"] is None or path.stem > query["cursor"]: diff --git a/loopx/control_plane/subagent_context.ts b/loopx/control_plane/subagent_context.ts index 56f86e81ff..c21fadd14c 100644 --- a/loopx/control_plane/subagent_context.ts +++ b/loopx/control_plane/subagent_context.ts @@ -174,7 +174,7 @@ function boundedDelegationContext(value: unknown): JsonObject | null { const operationReceipts: JsonObject = {}; if (rawReceipts) { for (const key of ["observed", "prepared", "running", "turn_returned", "accepted", - "rejected", "unavailable", "recovery_required"]) { + "rejected", "stopped", "unavailable", "recovery_required"]) { if (Number.isInteger(rawReceipts[key]) && Number(rawReceipts[key]) >= 0) { operationReceipts[key] = Math.min(Number(rawReceipts[key]), 10_000); } From f9356e5a98353d8eecbf061d3303645f1bc0d666 Mon Sep 17 00:00:00 2001 From: song <22676124+songoow@users.noreply.github.com> Date: Tue, 29 Sep 2026 06:29:48 -0400 Subject: [PATCH 05/13] test(delegation): cover stop receipts, fencing and refused resume Semantics-first coverage for stopping delegated members: - stop while a detached worker runs a sleeping fixture host: the worker acknowledges SIGTERM under its own lock, the receipt settles only with a free operation lock and a lockable Turn lane, the record bytes stay frozen afterwards, the Todo stays open, the host and worker processes are gone, the Turn journal stays in_progress, and resume is refused without spawning; - stop after accepted: noop, identical on repeat, no stop file and artifacts unchanged; stop without a holder is acknowledged by the requester; - a worker SIGKILLed before acknowledging settles as unknown, not stopped, and resume stays refused; - a fenced write after another process's acknowledged stop raises and writes nothing, while an unacknowledged stop is taken from under the lock at entry; - CLI stop requires --execute and repeats its receipt; inventory pages past stop sidecars and reads a stopped record as stopped; - TS: stopped is terminal and reachable only from open observations, and the stop decision settles only on acknowledgement plus free locks. Co-Authored-By: Claude Fable 5.1 Signed-off-by: song <22676124+songoow@users.noreply.github.com> --- tests/control_plane_ts/delegation.test.ts | 39 ++++- tests/test_delegation_cli.py | 28 ++++ tests/test_delegation_inventory.py | 7 + tests/test_local_delegation.py | 172 +++++++++++++++++++++- 4 files changed, 242 insertions(+), 4 deletions(-) diff --git a/tests/control_plane_ts/delegation.test.ts b/tests/control_plane_ts/delegation.test.ts index 7ae9fe4d41..0aaf6686ed 100644 --- a/tests/control_plane_ts/delegation.test.ts +++ b/tests/control_plane_ts/delegation.test.ts @@ -1,6 +1,6 @@ import test from "node:test"; import assert from "node:assert/strict"; -import {recordDelegationAdoption, delegationInventoryItem, delegationInventoryQuery, delegationPreflight, delegationTurnPlanDecision, delegationValidationPlan, recoverValidatedDelegationSettlement, selectDelegationBinding, transitionDelegationObservation} from "../../loopx/control_plane/collaboration/delegation.ts"; +import {recordDelegationAdoption, decideDelegationStop, delegationInventoryItem, delegationInventoryQuery, delegationPreflight, delegationTurnPlanDecision, delegationValidationPlan, recoverValidatedDelegationSettlement, selectDelegationBinding, transitionDelegationObservation} from "../../loopx/control_plane/collaboration/delegation.ts"; import {canonicalAuthoritySha256} from "../../loopx/control_plane/coordination/authority_store_codec.ts"; import {projectTurnSelectionRejection} from "../../loopx/control_plane/turn_driver/selection_rejection.ts"; @@ -102,6 +102,43 @@ test("message receipt and model return do not imply accepted work", () => { canonical_done: true, acceptance_ready: true, artifacts_current: true}), {status: "accepted"}); }); +test("stopped is terminal and reachable only from open observations", () => { + for (const from of ["prepared", "running", "turn_returned"]) + assert.deepEqual(transitionDelegationObservation({from, to: "stopped"}), {status: "stopped"}); + assert.deepEqual(transitionDelegationObservation({from: "stopped", to: "stopped"}), {status: "stopped"}); + for (const from of ["accepted", "rejected"]) + assert.throws(() => transitionDelegationObservation({from, to: "stopped"}), /transition/); + for (const to of ["running", "turn_returned", "accepted", "rejected"]) + assert.throws(() => transitionDelegationObservation({from: "stopped", to}), /transition/); + const observation = {operation_id: "op-1", request_id: "req", agent_id: "reviewer", todo_id: "todo_review", + status: "stopped", worker_active: false, recovery_required: false}; + assert.equal(delegationInventoryItem({record: {record_id: "a".repeat(64), operation_id: "op-1"}, + observation}).status, "stopped"); +}); + +test("a stop settles only on an acknowledgement plus free locks; time alone proves nothing", () => { + const open = {phase: "requested", acknowledged: false, operation_lock_free: false, lane_lock_free: false}; + assert.deepEqual(decideDelegationStop(open), {phase: "requested", terminal: false, reason: "awaiting_acknowledgement"}); + assert.deepEqual(decideDelegationStop({...open, timed_out: true}), + {phase: "requested", terminal: false, reason: "holder_still_running_after_grace"}); + assert.deepEqual(decideDelegationStop({...open, operation_lock_free: true, timed_out: true}), + {phase: "requested", terminal: false, reason: "holder_still_running_after_grace"}); + assert.deepEqual(decideDelegationStop({...open, operation_lock_free: true, lane_lock_free: true}), + {phase: "unknown", terminal: true, reason: "holder_gone_without_acknowledgement"}); + const acked = {phase: "acknowledged", acknowledged: true, operation_lock_free: false, lane_lock_free: false}; + assert.deepEqual(decideDelegationStop(acked), {phase: "acknowledged", terminal: false, reason: "operation_lock_still_held"}); + assert.deepEqual(decideDelegationStop({...acked, operation_lock_free: true}), + {phase: "acknowledged", terminal: false, reason: "turn_lane_still_held"}); + assert.deepEqual(decideDelegationStop({...acked, phase: "requested", operation_lock_free: true, lane_lock_free: true}), + {phase: "settled", terminal: true, reason: "acknowledged_and_locks_released"}); + assert.deepEqual(decideDelegationStop({...acked, operation_lock_free: true, lane_lock_free: true, timed_out: true}), + {phase: "settled", terminal: true, reason: "acknowledged_and_locks_released"}); + for (const patch of [{phase: "settled"}, {phase: "unknown"}, {phase: "noop"}, {acknowledged: "yes"}, + {operation_lock_free: 1}, {lane_lock_free: undefined}, {timed_out: "later"}, + {phase: "acknowledged", acknowledged: false}]) + assert.throws(() => decideDelegationStop({...open, ...patch})); +}); + test("a false rejection can reopen only for exact validated settlement recovery", () => { const evidence = { from: "rejected", diff --git a/tests/test_delegation_cli.py b/tests/test_delegation_cli.py index ee751375ae..95355fbbf7 100644 --- a/tests/test_delegation_cli.py +++ b/tests/test_delegation_cli.py @@ -84,6 +84,34 @@ def test_attached_cli_disconnect_retry_and_verified_return(service): assert "artifacts" not in inventory["items"][0] +def test_cli_stop_settles_a_running_member_and_refuses_resume(service): + root, runner = service + (root / "hold").touch() + source = root / "brief.json" + source.write_text(json.dumps(brief())) + status, started = cli(runner, "start", "--binding-id", "analysis", "--operation-id", "cli-stop", + "--brief-file", str(source), "--execute") + assert status == 0, started + deadline = time.monotonic() + 45 + while not (root / "host-started").exists() and time.monotonic() < deadline: + time.sleep(0.1) + assert (root / "host-started").exists() + status, refused = cli(runner, "stop", "--operation-id", "cli-stop") + assert status == 1 and "--execute" in refused["error"] + assert not runner._stop_path(runner.path("cli-stop")).exists() + status, stopped = cli(runner, "stop", "--operation-id", "cli-stop", "--execute") + assert status == 0 and stopped["phase"] == "settled" and stopped["status"] == "stopped", stopped + assert stopped["stop"]["ack"]["source"] == "SIGTERM" + status, again = cli(runner, "stop", "--operation-id", "cli-stop", "--execute") + assert status == 0 and again == stopped + status, resumed = cli(runner, "resume", "--operation-id", "cli-stop", "--execute") + assert status == 1 and "start a new operation id" in resumed["error"] + status, observed = cli(runner, "read", "--operation-id", "cli-stop") + assert status == 0 and observed["status"] == "stopped" and observed["stop"]["phase"] == "settled" + assert (root / "analyst" / "initial" / "host-invocations").read_text() == "1" + assert not demo.canonical_tasks(root)["todo_analyst-initial"]["done"] + + def test_cli_invalid_inputs_do_not_launch_work(service): root, runner = service bad = root / "bad.json" diff --git a/tests/test_delegation_inventory.py b/tests/test_delegation_inventory.py index 8486eb4a55..b75dec65c9 100644 --- a/tests/test_delegation_inventory.py +++ b/tests/test_delegation_inventory.py @@ -56,6 +56,13 @@ def test_corruption_and_stopped_worker_do_not_hide_healthy_sibling(service, monk assert by_id["stopped"]["recovery_required"] assert len(page["items"]) == 4 and not page["page_readback_complete"] assert sum(row["status"] == "unavailable" for row in page["items"]) == 2 + # A stop receipt beside its record is not another record and reads back as stopped. + assert runner.stop("healthy", execute=True)["phase"] == "settled" + assert runner._stop_path(runner.path("healthy")).exists() + page = runner.operations() + by_id = {row["operation_id"]: row for row in page["items"] if row["operation_id"]} + assert len(page["items"]) == 4 and by_id["healthy"]["status"] == "stopped" + assert not by_id["healthy"]["recovery_required"] def test_unknown_requester_and_unreadable_source_are_not_empty_inventory(service, monkeypatch): diff --git a/tests/test_local_delegation.py b/tests/test_local_delegation.py index d29b13fd7e..faf0208698 100644 --- a/tests/test_local_delegation.py +++ b/tests/test_local_delegation.py @@ -1,6 +1,7 @@ """Production delegation/Turn/TS completion with an explicit fixture model host.""" import json import asyncio +import os from pathlib import Path import subprocess import sys @@ -16,13 +17,13 @@ sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "examples" / "managed-research-team")) import research_team as demo # noqa: E402 from test_managed_research_scenario import fixture # noqa: E402 -from loopx.collaboration_mcp import Delegations # noqa: E402 +from loopx.collaboration_mcp import DelegationFenced, Delegations # noqa: E402 from loopx.control_plane.collaboration.peers import returns # noqa: E402 from loopx.control_plane.collaboration.inbox import _read # noqa: E402 -from loopx.file_lock import exclusive_file_lock # noqa: E402 +from loopx.file_lock import exclusive_file_lock, try_exclusive_file_lock # noqa: E402 -HOST = '''import json, sys, time +HOST = '''import json, os, sys, time from pathlib import Path from loopx.control_plane.turn_driver.host_candidate import build_result from loopx.control_plane.collaboration.inbox import acknowledge @@ -35,6 +36,7 @@ counter = workspace / 'host-invocations' counter.write_text(str(int(counter.read_text()) + 1 if counter.exists() else 1)) if (root / 'hold').exists(): + (root / 'host-pid').write_text(str(os.getpid())) (root / 'host-started').touch() while not (root / 'release').exists(): time.sleep(0.1) delegation = json.loads((workspace / 'DELEGATION.json').read_text()) @@ -169,6 +171,7 @@ async def disconnect_requester(): assert not (root / "host-started").exists() inventory = await session.call_tool("list_delegations", {}) assert not inventory.isError and json.loads(inventory.content[0].text)["items"] == [] + assert "stop_delegation" in {tool.name for tool in (await session.list_tools()).tools} result = await session.call_tool("start_delegation", { "binding_id": "analysis", "operation_id": "analysis-1", "brief": brief()}) assert not result.isError @@ -197,6 +200,13 @@ async def disconnect_requester(): assert len(returned) == 1 assert returned[0]["decision"] == "adopt" assert wait(reconnected)["artifacts"] == result["artifacts"] + # Accepted work cannot be stopped: nothing is written and the receipt repeats exactly. + noop = reconnected.stop("analysis-1", execute=True) + assert noop["phase"] == "noop" and noop["status"] == "accepted" and noop["stop"] is None + assert reconnected.stop("analysis-1", execute=True) == noop + assert not reconnected._stop_path(reconnected.path("analysis-1")).exists() + assert wait(reconnected)["artifacts"] == result["artifacts"] + assert "stop" not in reconnected.read("analysis-1") changed_brief = {**brief(), "purpose": "Changed instruction"} with pytest.raises(ValueError, match="identity conflict"): reconnected.start("analysis", "analysis-1", changed_brief) @@ -265,3 +275,159 @@ def read_on_publish(path, row, status, **facts): assert len(terminal_reads) == 1 assert not demo.canonical_tasks(root)["todo_analyst-initial"]["done"] assert returns(runner.root, runner.goal_id, "lead")["items"] == [] + + +def until(predicate, timeout=45): + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + if predicate(): + return True + time.sleep(0.1) + return predicate() + + +def process_gone(pid): + try: + os.kill(pid, 0) + except ProcessLookupError: + return True + try: + return "State:\tZ" in Path(f"/proc/{pid}/status").read_text() + except OSError: + return True + + +def start_held_worker(service, operation="analysis-stop"): + root, runner = service + (root / "hold").touch() + runner.start("analysis", operation, brief()) + assert until(lambda: (root / "host-started").exists()), _read(runner.path(operation)) + return int((root / "host-pid").read_text()) + + +def test_stop_while_executing_is_acknowledged_by_the_worker_and_settles(service, monkeypatch): + """The detached worker acknowledges SIGTERM under its own lock; time proves nothing.""" + root, runner = service + host_pid = start_held_worker(service) + path = runner.path("analysis-stop") + before = _read(path) + assert before["status"] == "running" and before["worker"]["pid"] == before["worker"]["pgid"] + receipt = runner.stop("analysis-stop", execute=True) + assert receipt["phase"] == "settled" and receipt["status"] == "stopped", receipt + stop = receipt["stop"] + assert stop["requested_by"] == "lead" and stop["requested_status"] == "running" + assert stop["worker"]["pid"] == before["worker"]["pid"] + assert stop["ack"]["pid"] == before["worker"]["pid"] and stop["ack"]["source"] == "SIGTERM" + assert stop["ack"]["observed_status"] == "running" and stop["ack"]["turn_key"] + assert stop["settled"]["operation_lock_free"] and stop["settled"]["lane_lock_free"] + assert stop["settled"]["turn_journal_status"] == "in_progress" + assert stop["lease"] == {"required": False, "released": None} + # The acknowledged record is final: nobody writes it again, the Todo stays open, + # the host process group is gone and the member's Turn lane can be taken. + frozen = path.read_bytes() + assert until(lambda: process_gone(host_pid), timeout=20) + assert until(lambda: process_gone(before["worker"]["pid"]), timeout=20) + binding = runner.binding("analysis") + with try_exclusive_file_lock(runner._lane_target(binding)) as held: + assert held is not None + assert not demo.canonical_tasks(root)["todo_analyst-initial"]["done"] + assert not (root / "analyst" / "initial" / "DELEGATION.json").exists() + assert path.read_bytes() == frozen + assert runner.stop("analysis-stop", execute=True) == receipt + observed = runner.read("analysis-stop") + assert observed["status"] == "stopped" and not observed["recovery_required"] + assert observed["stop"] == {"stop_id": stop["stop_id"], "phase": "settled"} + assert runner.wait("analysis-stop")["status"] == "stopped" + monkeypatch.setattr(runner, "_spawn", lambda _: pytest.fail("stopped work must not respawn")) + with pytest.raises(ValueError, match="start a new operation id"): + runner.resume("analysis-stop") + assert path.read_bytes() == frozen + page = runner.operations() + assert page["page_readback_complete"] and page["items"][0]["status"] == "stopped" + assert returns(runner.root, runner.goal_id, "lead")["items"] == [] + + +def test_stop_without_a_holder_is_acknowledged_by_the_requester(service, monkeypatch): + root, runner = service + monkeypatch.setattr(runner, "_spawn", lambda _: None) + runner.start("analysis", "analysis-idle", brief()) + receipt = runner.stop("analysis-idle", execute=True) + assert receipt["phase"] == "settled" and receipt["status"] == "stopped" + assert receipt["stop"]["worker"] is None and receipt["stop"]["requested_status"] == "prepared" + assert receipt["stop"]["ack"]["pid"] == os.getpid() and receipt["stop"]["ack"]["source"] == "requester" + assert receipt["stop"]["settled"]["turn_journal_status"] is None + frozen = runner.path("analysis-idle").read_bytes() + with pytest.raises(ValueError, match="start a new operation id"): + runner.resume("analysis-idle") + runner.execute("analysis-idle") # a late worker finds terminal work and launches nothing + assert runner.path("analysis-idle").read_bytes() == frozen + assert not (root / "host-started").exists() + assert runner.stop("analysis-idle", execute=True) == receipt + with pytest.raises(ValueError, match="requires execute"): + runner.stop("analysis-idle", execute=False) + with pytest.raises(ValueError, match="unknown delegation operation"): + runner.stop("never-started", execute=True) + + +def test_worker_killed_before_acknowledging_is_unknown_not_settled(service, monkeypatch): + """A vanished holder never becomes a settlement; the stop still fences resume.""" + import signal + + root, runner = service + host_pid = start_held_worker(service) + path = runner.path("analysis-stop") + worker = _read(path)["worker"] + + def kill_without_grace(target, stop): + assert stop["worker"]["pgid"] == worker["pgid"] != os.getpgid(0) + os.killpg(worker["pgid"], signal.SIGKILL) + assert until(lambda: runner._operation_lock_free(target), timeout=20) + + monkeypatch.setattr(runner, "_signal_worker", kill_without_grace) + receipt = runner.stop("analysis-stop", execute=True) + assert receipt["phase"] == "unknown" and receipt["status"] == "running", receipt + assert receipt["stop"]["ack"] is None and receipt["stop"]["settled"]["operation_lock_free"] + assert receipt["stop"]["settled"]["turn_journal_status"] == "in_progress" + assert until(lambda: process_gone(host_pid), timeout=20) + assert runner.stop("analysis-stop", execute=True) == receipt + with pytest.raises(ValueError, match="start a new operation id"): + runner.resume("analysis-stop") + assert runner.read("analysis-stop")["stop"]["phase"] == "unknown" + assert not demo.canonical_tasks(root)["todo_analyst-initial"]["done"] + + +def test_fenced_write_after_another_process_stop_writes_nothing(service, monkeypatch): + root, runner = service + monkeypatch.setattr(runner, "_spawn", lambda _: None) + runner.start("analysis", "analysis-fenced", brief()) + path = runner.path("analysis-fenced") + row = _read(path) + foreign = runner._new_stop_record(row, requested_by="other-host-lead", worker=None) + foreign.update(phase="acknowledged", ack={"pid": 1, "host": "elsewhere", "at": 0.0, + "source": "requester", "observed_status": "running", + "turn_key": None}) + from loopx.control_plane.collaboration.inbox import _write + + _write(runner._stop_path(path), foreign) + frozen = path.read_bytes() + with pytest.raises(DelegationFenced): + runner._fenced_write(path, {**row, "status": "running"}) + with pytest.raises(DelegationFenced): + runner._observe(path, dict(row), "running") + with pytest.raises(DelegationFenced): + runner._record_turn_result(path, {**row, "status": "running"}, + {"status": "committed", "result_kind": "validated_progress"}) + runner.execute("analysis-fenced") # the foreign acknowledgement stands; nothing is rewritten + assert path.read_bytes() == frozen + assert _read(runner._stop_path(path)) == foreign + assert not (root / "host-started").exists() + assert not demo.canonical_tasks(root)["todo_analyst-initial"]["done"] + # A stop this process may acknowledge is taken from under the lock at entry. + runner.start("analysis", "analysis-entry", brief()) + entry = runner.path("analysis-entry") + _write(runner._stop_path(entry), runner._new_stop_record(_read(entry), requested_by="lead", worker=None)) + runner.execute("analysis-entry") + assert _read(entry)["status"] == "stopped" + acknowledged = _read(runner._stop_path(entry)) + assert acknowledged["phase"] == "acknowledged" and acknowledged["ack"]["source"] == "worker_entry" + assert runner.stop("analysis-entry", execute=True)["phase"] == "settled" From f2553451d40aeb5424e6e859415de2a796eae236 Mon Sep 17 00:00:00 2001 From: song <22676124+songoow@users.noreply.github.com> Date: Tue, 29 Sep 2026 06:29:48 -0400 Subject: [PATCH 06/13] docs(delegation): document stopping a member and reading its receipt Describe `delegation stop --execute` / `stop_delegation` in both reference documents, in English and Chinese: where the request lives, how a same-host worker is signalled and acknowledges, why another host's worker is left to find the request, what settled, unknown and noop prove, and that stopped work needs a new operation id while its Turn journal and open Todo remain for the coordinator to inspect. Co-Authored-By: Claude Fable 5.1 Signed-off-by: song <22676124+songoow@users.noreply.github.com> --- docs/reference/goal-chat-continuation.md | 8 +++- docs/reference/local-delegation.md | 47 ++++++++++++++++++++++-- 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/docs/reference/goal-chat-continuation.md b/docs/reference/goal-chat-continuation.md index 00569ce34a..7c367d68d3 100644 --- a/docs/reference/goal-chat-continuation.md +++ b/docs/reference/goal-chat-continuation.md @@ -100,7 +100,9 @@ messages; images use the ordinary conversation after pausing. The Chat hard timeout remains in force; this is not an unattended daemon. - To roll back, pause/close the Chat service before installing an older build. Disabling mode or deleting a binding does not cancel already admitted children; - use their own execution/recovery controls and retain their evidence. + stop one with `loopx delegation stop --execute` (or `stop_delegation`), read + its receipt, and retain its evidence. Only `settled` proves the worker + acknowledged and released its locks; stopped work needs a new operation id. The ordinary native command path also remains available without delegation: @@ -143,6 +145,8 @@ For a disposable mixed-team setup, use the 协调员保留只读沙箱,成员权限来自各自执行绑定,不继承管家的扩大权限。 成员通过验收与协调员报告、整个 Goal 验收分别显示;本模式不直接完成报告 Todo 或整个 Goal。额度是含历史用量的总量,正在执行的请求可能超额,成员另行计量。 -回滚旧版本前先暂停或关闭 Chat 服务;退出或撤销绑定不自动取消已启动的成员。 +回滚旧版本前先暂停或关闭 Chat 服务;退出或撤销绑定不自动取消已启动的成员, +用 `loopx delegation stop --execute`(或 `stop_delegation`)停止单个成员并阅读回执: +只有 `settled` 证明 worker 已确认并释放锁;已停止的工作需要新的 operation id。 此按钮目前限本机 managed Codex Goal 对话,不宣称 Lark、挂接会话或其他主力 驱动等价。可用下方示例准备一次隔离的本地 DSH+云端 Ark 协作。 diff --git a/docs/reference/local-delegation.md b/docs/reference/local-delegation.md index 26015e161e..b5e3e39fa2 100644 --- a/docs/reference/local-delegation.md +++ b/docs/reference/local-delegation.md @@ -157,6 +157,7 @@ delegate start --binding-id independent-review --operation-id review-round-1 \ --brief-file request.json --execute delegate read --operation-id review-round-1 delegate wait --operation-id review-round-1 +delegate stop --operation-id review-round-1 --execute ``` Inspection uses the bound worker workspace as its actual safety scan root. If @@ -221,6 +222,38 @@ own authorized peers supplies `--parent-request-id` on start. CLI and MCP share grant validation, detached execution, wait/readback and recovery rather than maintaining separate rules. +`stop --execute` ends one member's bounded work and returns a receipt that +states what was proven. The request is written beside the execution record +(`.stop.json`), never into it, so a worker that is still holding the +operation cannot overwrite it. A worker on this machine receives `SIGTERM` for +its whole process group, which ends its Turn child and its host; it +acknowledges from under its own lock, marks the record `stopped` and releases +its hard task lease. When nobody holds the operation, the requester +acknowledges itself. A worker on another machine is never signalled; it finds +the request at its next checkpoint or at its next record write, which is +refused. The receipt `phase` is `settled` only when an acknowledgement exists +and both the operation lock and the member's Turn lane lock are free; +`unknown` means the holder vanished before acknowledging, and `noop` means the +work was already accepted, rejected or stopped. `requested` or `acknowledged` +means it is still winding down: call `stop` again. A grace timeout never turns +into a receipt. Stopped work is not resumed; `resume` refuses it and a new +scope needs a new operation id. The Turn journal keeps its `in_progress` entry +for inspection, and the record is never rewritten as a completion. A stopped +member's Todo stays open, so the coordinator decides what happens next. + +中文:`stop --execute` 结束一个成员的有界工作,并返回一份只陈述已证明事实的 +回执。停止请求写在执行记录旁边的 `.stop.json`,从不写进记录本身, +因此仍持有该 operation 的 worker 无法覆盖它。本机 worker 会收到整个进程组的 +`SIGTERM`,其 Turn 子进程和 host 一并结束;worker 在自己的锁下确认,把记录标为 +`stopped` 并释放硬任务租约。没有持有者时由请求方自行确认。另一台机器上的 +worker 不会被发信号,它在下一个检查点或下一次写记录时发现请求,写入被拒绝。 +只有存在确认且 operation 锁与成员 Turn lane 锁都已释放时,`phase` 才是 +`settled`;`unknown` 表示持有者在确认前消失;`noop` 表示工作已 accepted、 +rejected 或 stopped;`requested`/`acknowledged` 表示仍在收尾,再次调用 `stop`。 +宽限期超时永远不会变成回执。已停止的工作不能 `resume`,新范围需要新的 +operation id。Turn journal 保留 `in_progress` 条目供检查,记录不会被改写成完成; +成员的 Todo 仍然打开,由协调者决定下一步。 + This entrypoint does not create Agents, grant bindings or wake an idle Codex conversation. The existing host/LoopX continuation policy owns the next lead turn. The conversation remains persistent independently of whether autonomous @@ -574,7 +607,8 @@ operation and observed artifact hashes in its existing inbox. Pending and delive receipts stay distinct from application; a retry after an uncertain response reuses the exact message and operation id. **Pause coordinator** stays in the panel and reports its actual scope. Dispatched members continue independently; -this entrypoint cannot stop the whole team. Ordinary polling does not read artifact +this entrypoint cannot stop the whole team. Stop one member explicitly with +`delegation stop --execute` or `stop_delegation` and read its receipt. Ordinary polling does not read artifact bodies or run preflight. Closing the panel changes no work state. This local operator entrypoint does not grant a Lark audience access. @@ -584,7 +618,8 @@ operator entrypoint does not grant a Lark audience access. 可展开查看,返回列表保留位置与键盘焦点。协调员运行时,可把执行标识、看到的 产物哈希和反馈投递到原收件箱;等待投递、已交付和已应用不能混为一谈。不确定响应 后重试同一消息和标识,避免重复投递。面板内的「暂停协调员」显示实际反馈,但不会 -停止已派发成员,也不宣称整个团队停止。暂停时仍可检查证据;读取不启动模型。 +停止已派发成员,也不宣称整个团队停止。要停止某个成员,显式使用 +`delegation stop --execute` 或 `stop_delegation` 并阅读其回执。暂停时仍可检查证据;读取不启动模型。 Screenshots use isolated synthetic research data, not a live-model qualification: [desktop evidence](../assets/personal-workspace/team-evidence-desktop.png), @@ -614,6 +649,10 @@ unchanged and cannot launch workers. With it, the Agent can: response is normal. `read_delegation` reads the durable original operation. 4. If `recovery_required` is true, call `resume_delegation` with that same id. This cannot retarget the work or silently create a replacement Turn. +5. Call `stop_delegation(operation_id)` to end one member. Read its `phase`: + `settled` is the only receipt that the worker acknowledged and released its + locks; `unknown` means the holder vanished first; `noop` means the work had + already ended. Stopped work cannot be resumed; use a new operation id. Configure the member's host to expose its own identity-bound collaboration tools. It reads `DELEGATION.json`, independently calls `assess_request`, and @@ -654,6 +693,7 @@ concurrent executions still use the same kernel lock and original Turn journal. | Requesting MCP conversation closes | The detached bounded worker continues; another connection reads the original operation. | | Duplicate start/resume while work runs | Operation identity, task lock and Turn journal prevent another concurrent execution. | | Worker process or machine stops | Reconnect with the same operator configuration and credentials, then resume the original Turn. | +| Member stopped on request | The worker acknowledges under its lock, its host and Turn child are ended, its lease is released; `settled` needs that acknowledgement plus free locks, `unknown` means the holder vanished first. The record is `stopped`; resume refuses it. | | Ark is computing without local tools | The already-started cloud turn can continue. It is not dependent on the local conversation. | | Ark requests a local tool while the host is absent | It waits for the local tool result. Recovery observes the original input/session and executes only previously unstarted tool calls. | | Tool execution or send acknowledgement is uncertain | Do not repeat the effect. Preserve the receipt/session for explicit reconciliation. | @@ -669,7 +709,8 @@ or default executor change; those existing configuration surfaces are untouched. To disable new admission, remove the caller's grants or remove `--execution-config` from the host. A stopped Goal refuses new starts/resumes; existing completed results remain readable. Disabling does not kill work already -running. Retain receipts, stop or reconcile owned workers, and confirm cloud +running; `delegation stop --execute` ends one member and returns a receipt. +Retain receipts, stop or reconcile owned workers, and confirm cloud resource cleanup before deleting a disposable runtime. The optional adapter's cleanup command never grants task completion. From 9a4059676ab379c52cb450fa32b39f92b85dc6d3 Mon Sep 17 00:00:00 2001 From: song <22676124+songoow@users.noreply.github.com> Date: Tue, 29 Sep 2026 10:25:17 -0400 Subject: [PATCH 07/13] fix(delegation): settle a stop from holder records, never by taking a lock The stop settlement probed the member's Turn lane by acquiring its lock for an instant, which could refuse a legitimate Turn of the same member racing that instant with turn_lane_in_flight. It also carried its own host label helper next to the file-lock owner's. Settlement now reads the lane's last holder record through turn_lane_liveness: released, dead or absent frees the lane; a live same-host holder frees it only when it sits outside the recorded worker's process group; another host's holder, an unattributable holder or an unreadable record keep the stop open. The operation lock is read the same way, so a stop never refuses a legitimate resume or status read. The local host helper is deleted in favour of lock_holder_host_label. A regression test races a real lane acquisition against settlement and proves the Turn is admitted. Co-Authored-By: Claude Opus 5.5 (1M context) Signed-off-by: song <22676124+songoow@users.noreply.github.com> --- loopx/collaboration_mcp.py | 202 +++++++++++++----- .../control_plane/collaboration/delegation.ts | 33 +-- loopx/file_lock.py | 11 - tests/control_plane_ts/delegation.test.ts | 30 +-- tests/test_local_delegation.py | 117 +++++++++- 5 files changed, 291 insertions(+), 102 deletions(-) diff --git a/loopx/collaboration_mcp.py b/loopx/collaboration_mcp.py index 0070601fb3..7f90c2f7ce 100644 --- a/loopx/collaboration_mcp.py +++ b/loopx/collaboration_mcp.py @@ -28,8 +28,8 @@ from mcp.server.fastmcp import FastMCP from .file_lock import ( - exclusive_file_lock, lock_holder_host_label, read_lock_holder, try_exclusive_file_lock, - LockAcquisitionPolicy, LockAcquireTimeoutError, + exclusive_file_lock, lock_holder_host_label, lock_holder_liveness, + LOCK_HOLDER_FOREIGN_HOST, LOCK_HOLDER_LIVE, LockAcquisitionPolicy, LockAcquireTimeoutError, ) from .control_plane.effect_runtime import ( effect_runtime_request_scope, effect_runtime_result, EffectRuntimeRemoteError, @@ -42,7 +42,10 @@ turn_journal_path, ) from .control_plane.turn_driver.host_binding import turn_host_arg_option -from .control_plane.turn_driver.lane_fence import turn_lane_target +from .control_plane.turn_driver.lane_fence import ( + TURN_LANE_ABSENT, TURN_LANE_DEAD, TURN_LANE_LIVE, TURN_LANE_RELEASED, + turn_lane_liveness, turn_lane_target, +) from .control_plane.work_items.task_lease import release_task_lease from .control_plane.collaboration.inbox import _hash, _read, _write, _root, _receipt from .control_plane.collaboration.peers import return_result @@ -310,13 +313,18 @@ def consume_peer_result(request_id: str) -> dict: # Observations that no worker may reopen; a stop against one is a no-op receipt. DELEGATION_TERMINAL_STATUSES = frozenset({"accepted", "rejected", "stopped"}) DELEGATION_STOP_OPEN_PHASES = frozenset({"requested", "acknowledged"}) +DELEGATION_STOP_TERMINAL_PHASES = frozenset({"settled", "unknown"}) # How long a signalled same-host worker may take to acknowledge before SIGKILL. DELEGATION_STOP_GRACE_SECONDS = 10.0 DELEGATION_STOPPED_MESSAGE = "delegation operation was stopped; start a new operation id" -class DelegationStopRequested(Exception): - """A stop reached the worker that owns this operation; it must acknowledge, not finish.""" +class DelegationStopRequested(BaseException): + """A stop reached the worker that owns this operation; it must acknowledge, not finish. + + A ``BaseException`` like ``KeyboardInterrupt``: a termination request must + not be swallowed by an ``except Exception`` and turned into further work. + """ def __init__(self, source: str) -> None: super().__init__(source) @@ -335,14 +343,28 @@ def __init__(self) -> None: class _WorkerStopSignal: - """Turn the first SIGTERM into a stop request; absorb later ones during the acknowledgement.""" + """Turn SIGTERM into a stop request only when a stop was written for this operation. - def __init__(self) -> None: + Without a stop receipt the signal keeps its default meaning, so a shutdown + still leaves the operation recoverable by ``resume`` instead of stopping it. + Later signals are absorbed while the acknowledgement is written. + """ + + def __init__(self, stop_path: Path) -> None: + self.stop_path = stop_path self.armed = True def __call__(self, signum: int, frame: object) -> None: if not self.armed: return + try: + requested = self.stop_path.exists() + except OSError: + requested = False + if not requested: + signal.signal(signum, signal.SIG_DFL) + os.kill(os.getpid(), signum) + return self.armed = False raise DelegationStopRequested("SIGTERM") @@ -350,12 +372,12 @@ def disarm(self) -> None: self.armed = False -def install_worker_stop_signal() -> _WorkerStopSignal | None: +def install_worker_stop_signal(stop_path: Path) -> _WorkerStopSignal | None: """Install the detached worker's SIGTERM handler; ``None`` where signals are unsupported.""" if not hasattr(signal, "SIGTERM"): return None - handler = _WorkerStopSignal() + handler = _WorkerStopSignal(stop_path) try: signal.signal(signal.SIGTERM, handler) except (ValueError, OSError): @@ -592,7 +614,10 @@ def resume(self, operation_id: str) -> dict: if row["status"] == "stopped": raise ValueError(DELEGATION_STOPPED_MESSAGE) if row["status"] == "rejected": - self._recover_validated_settlement(path, row, binding) + try: + self._recover_validated_settlement(path, row, binding) + except DelegationFenced: + raise ValueError(DELEGATION_STOPPED_MESSAGE) from None should_spawn = row["status"] not in DELEGATION_TERMINAL_STATUSES if should_spawn: self.binding( @@ -610,7 +635,8 @@ def wait(self, operation_id: str) -> dict: """Observe for at most 15 seconds; waiting neither starts nor resumes work.""" for _ in range(5): result = self.read(operation_id) - if result["status"] in DELEGATION_TERMINAL_STATUSES or result["recovery_required"]: + if (result["status"] in DELEGATION_TERMINAL_STATUSES or result["recovery_required"] + or result.get("stop", {}).get("phase") in DELEGATION_STOP_TERMINAL_PHASES): return result time.sleep(3) return self.read(operation_id) @@ -720,12 +746,13 @@ def _read_current(self, operation_id: str) -> dict: active = False except LockAcquireTimeoutError: active = True + # Resume refuses an operation with a stop receipt, so it never needs recovery. + stop = self._read_stop(path) result = {"operation_id": operation_id, "request_id": row["identity"]["request_id"], "agent_id": binding["agent_id"], "todo_id": binding["todo_id"], "status": row["status"], "worker_active": active, "recovery_required": not active and row["status"] not in DELEGATION_TERMINAL_STATUSES - and time.time() - row.get("created_at", 0) > 15} - stop = self._read_stop(path) + and stop is None and time.time() - row.get("created_at", 0) > 15} if stop is not None: result["stop"] = {"stop_id": stop["stop_id"], "phase": stop["phase"]} if row["status"] == "accepted": @@ -782,19 +809,59 @@ def _lane_target(self, binding: dict) -> Path: plan={"turn_envelope": {"agent_id": binding["agent_id"]}}) def _operation_lock_free(self, path: Path) -> bool: + """Probe this operation's own kernel lock; only ever called once its stop receipt exists. + + Unlike the Turn lane, this lock admits nothing but this operation, and a + probe holding it for an instant refuses no legitimate acquisition once + the receipt is written: ``resume``, its only single-flight acquirer, + refuses a stopped operation before it touches the lock; ``execute``, + adoption and the requester acknowledgement wait through brief holders + with the mutation policy; and a status read already makes this same + instant observation. Before the receipt exists a ``resume`` is still + legitimate, so the holder is then read from its record instead. + """ + try: with exclusive_file_lock(path, policy=LockAcquisitionPolicy.SINGLE_FLIGHT): return True except LockAcquireTimeoutError: return False - def _lane_lock_free(self, binding: dict) -> bool: - # The kernel lock is the only proof; the holder record is advisory. The - # probe holds the lane for an instant, which is the same observation a - # status read makes on the operation lock. - with try_exclusive_file_lock(self._lane_target(binding), agent_id=self.agent_id, - operation="loopx_delegation_stop_probe") as held: - return held is not None + @staticmethod + def _recorded_worker(row: dict, stop: dict) -> dict | None: + worker = row.get("worker") + if not isinstance(worker, dict): + worker = stop.get("worker") + return worker if isinstance(worker, dict) else None + + def _worker_lane_released(self, row: dict, stop: dict, binding: dict) -> tuple[bool, str]: + """Say whether the stopped worker's Turn has let go of the member's lane, read-only. + + This never takes the lane lock: a probe holding it for an instant would + refuse a legitimate Turn of the same member racing that instant with + ``turn_lane_in_flight``. The lane's last holder record decides instead. + Released, dead or absent is released. A live holder on this machine is + released only when it sits outside the recorded worker's process group, + because the worker's run-once child runs in that group; a holder that + cannot be attributed, another host's holder and an unreadable record + prove nothing, so the typed decision keeps the stop open. + """ + + lane = turn_lane_liveness(self._lane_target(binding)) + state = lane["state"] + if state in {TURN_LANE_RELEASED, TURN_LANE_DEAD, TURN_LANE_ABSENT}: + return True, state + worker = self._recorded_worker(row, stop) + if (state != TURN_LANE_LIVE or worker is None or not hasattr(os, "getpgid") + or worker.get("host") != lock_holder_host_label() + or not isinstance(worker.get("pgid"), int)): + return False, state + try: + return os.getpgid(lane["holder"]["pid"]) != worker["pgid"], state + except ProcessLookupError: + return True, TURN_LANE_DEAD # the holder exited between the two reads + except OSError: + return False, state def _turn_journal_status(self, row: dict, binding: dict) -> str | None: turn_key = row.get("turn_key") or self._matching_turn_key(row, binding) @@ -866,26 +933,38 @@ def stop(self, operation_id: str, *, execute: bool) -> dict: worker=self._lock_holder_worker(path, row)) _write(self._stop_path(path), stop) if stop["phase"] in DELEGATION_STOP_OPEN_PHASES and stop.get("ack") is None: - try: - with exclusive_file_lock(path, policy=LockAcquisitionPolicy.SINGLE_FLIGHT): - row = _read(path) - self._acknowledge_stop(path, row, binding, source="requester") - except LockAcquireTimeoutError: + if stop.get("worker") is None: + # No worker was named when the request was written, so whoever owns + # the operation acknowledges it: this caller once the lock is free. + # The wait rides out a status read's instant hold, which must not + # be mistaken for a holder that vanished. + try: + with exclusive_file_lock(path): + self._acknowledge_stop(path, _read(path), binding, source="requester") + except LockAcquireTimeoutError: + pass # an unnamed holder meets the request at its next checkpoint or write + else: + # Only the named worker acknowledges. If it vanishes first, the typed + # decision reports unknown instead of a requester settlement. self._signal_worker(path, stop) return self._settle_stop(path) def _lock_holder_worker(self, path: Path, row: dict) -> dict | None: - """Name the live operation-lock holder, else ``None``; a released record is not a worker.""" + """Name the recorded worker while it is the operation lock's unreleased holder. + + Read from the holder record, never the kernel lock: this runs before the + stop receipt exists, when a probe could refuse a legitimate ``resume``. + Only the worker identity the execution record names can become a signal + target, so a status reader's instant holder record is never taken for it. + """ - if self._operation_lock_free(path): + state, holder = lock_holder_liveness(path) + recorded = row.get("worker") + if state not in {LOCK_HOLDER_LIVE, LOCK_HOLDER_FOREIGN_HOST} or not isinstance(recorded, dict): return None - holder = read_lock_holder(path) - if "released_at" in holder or not isinstance(holder.get("pid"), int): + if holder.get("pid") != recorded.get("pid") or holder.get("host") != recorded.get("host"): return None - recorded = row.get("worker") if isinstance(row.get("worker"), dict) else {} - same = recorded.get("pid") == holder["pid"] and recorded.get("host") == holder.get("host") - pgid = recorded.get("pgid") if same and isinstance(recorded.get("pgid"), int) else holder["pid"] - return {"pid": holder["pid"], "pgid": pgid, "host": holder.get("host")} + return {key: recorded.get(key) for key in ("pid", "pgid", "host")} def _signal_worker(self, path: Path, stop: dict) -> None: """Terminate a same-host holder's process group; never signal across hosts.""" @@ -919,29 +998,38 @@ def _signal_worker(self, path: Path, stop: dict) -> None: def _acknowledge_stop(self, path: Path, row: dict, binding: dict, *, source: str) -> None: """Acknowledge from under the operation lock: mark stopped, then release the hard lease. - Only the lock holder may acknowledge. A stop that another process already - acknowledged, or that already settled, is left untouched. + Only the operation-lock holder calls this. The record is transitioned as + it is on disk, so state that a fenced write refused stays unwritten; the + lease is released from what this process acquired, which may be newer + than the record. A missing stop, one already acknowledged or finished, + and a record that already reached a terminal observation stay untouched. """ if self._stop_signal is not None: self._stop_signal.disarm() with exclusive_file_lock(self._dispatch_lock(path)): stop = self._read_stop(path) - if stop is None: - stop = self._new_stop_record(row, requested_by="signal:" + source, - worker=self._worker_identity()) - if stop.get("ack") is not None or stop["phase"] not in DELEGATION_STOP_OPEN_PHASES: + current = _read(path) + if (stop is None or stop.get("ack") is not None + or stop["phase"] not in DELEGATION_STOP_OPEN_PHASES + or current["status"] in DELEGATION_TERMINAL_STATUSES): return - observed = row["status"] - decision = effect_runtime_result("collaboration.delegation.observe", { + observed = current["status"] + transition = effect_runtime_result("collaboration.delegation.observe", { "from": observed, "to": "stopped", }) - row.update(status=decision["status"]) - _write(path, row) - stop.update(phase="acknowledged", reason="awaiting_lock_release", ack={ + # This process holds the operation lock and its lane read comes later. + phase = effect_runtime_result("collaboration.delegation.stop", { + "phase": stop["phase"], "acknowledged": True, + "operation_lock_free": False, "worker_lane_released": False, + }) + current["status"] = transition["status"] + _write(path, current) + stop.update(phase=phase["phase"], reason=phase["reason"], ack={ "pid": os.getpid(), "host": lock_holder_host_label(), "at": time.time(), "source": source, "observed_status": observed, - "turn_key": row.get("turn_key") or self._matching_turn_key(row, binding), + "turn_key": (current.get("turn_key") or row.get("turn_key") + or self._matching_turn_key(current, binding)), }) _write(self._stop_path(path), stop) try: @@ -978,10 +1066,8 @@ def _settle_stop(self, path: Path) -> dict: return self._stop_receipt(row, binding, None) if stop["phase"] not in DELEGATION_STOP_OPEN_PHASES: return self._stop_receipt(row, binding, stop) - facts = { - "operation_lock_free": self._operation_lock_free(path), - "lane_lock_free": self._lane_lock_free(binding), - } + facts = {"operation_lock_free": self._operation_lock_free(path)} + facts["worker_lane_released"], lane_state = self._worker_lane_released(row, stop, binding) decision = effect_runtime_result("collaboration.delegation.stop", { "phase": stop["phase"], "acknowledged": stop.get("ack") is not None, "timed_out": time.time() - stop["requested_at"] > DELEGATION_STOP_GRACE_SECONDS, @@ -989,10 +1075,10 @@ def _settle_stop(self, path: Path) -> dict: }) if decision["phase"] != stop["phase"] or decision.get("reason") != stop.get("reason"): stop.update(phase=decision["phase"], reason=decision.get("reason")) - if decision["phase"] in {"settled", "unknown"}: + if decision["phase"] in DELEGATION_STOP_TERMINAL_PHASES: lease = stop.get("lease") if isinstance(stop.get("lease"), dict) else {} stop["settled"] = { - "at": time.time(), **facts, + "at": time.time(), **facts, "lane_state": lane_state, "lease_released": lease.get("released"), "turn_journal_status": self._turn_journal_status(row, binding), } @@ -1529,11 +1615,12 @@ def resume_delegation(operation_id: str) -> dict: async def stop_delegation(operation_id: str) -> dict: """Stop one original operation and return what was proven, not what was hoped. - settled: the worker acknowledged and its operation and Turn lane locks are - free. acknowledged/requested: still winding down; call again. unknown: the - holder vanished before acknowledging; inspect its Turn before reusing the - task. noop: already accepted/rejected/stopped. Stopped work is not resumed; - a new scope needs a new operation id. Elapsed time is never a receipt. + settled: the worker acknowledged, released the operation and let go of its + Turn lane. acknowledged/requested: still winding down; call again. unknown: + the named worker vanished before acknowledging; inspect its Turn and task + lease before reusing the task. noop: already accepted/rejected/stopped. + Stopped work is not resumed; a new scope needs a new operation id. Elapsed + time is never a receipt. """ return await asyncio.to_thread(delegations.stop, operation_id, execute=True) @@ -1558,7 +1645,8 @@ def main(): if args.delegation_action == "validate": service._validate(service._bound(_read(service.path(args.operation_id)))) else: - service._stop_signal = install_worker_stop_signal() + service._stop_signal = install_worker_stop_signal( + service._stop_path(service.path(args.operation_id))) try: service.execute(args.operation_id) except LockAcquireTimeoutError: diff --git a/loopx/control_plane/collaboration/delegation.ts b/loopx/control_plane/collaboration/delegation.ts index 9ba1f172da..6612deaec5 100644 --- a/loopx/control_plane/collaboration/delegation.ts +++ b/loopx/control_plane/collaboration/delegation.ts @@ -344,34 +344,37 @@ export function transitionDelegationObservation(params: JsonObject): JsonObject type StopPhase = "requested" | "acknowledged" | "settled" | "unknown"; const openStopPhases: readonly StopPhase[] = ["requested", "acknowledged"]; -/** Advance one stop request from host lock facts; a receipt is never inferred from time. +/** Advance one stop request from host release facts; a receipt is never inferred from time. * * ``settled`` needs the acknowledgement of a process that held the operation - * lock plus both the operation lock and the Turn lane lock free: only then is - * the worker, its Turn child and its lane provably gone. Free locks without an - * acknowledgement mean the holder vanished before recording what it observed, - * which is ``unknown`` rather than a fake settlement. A grace timeout on its - * own moves nothing: a worker that is still holding a lock is still running. + * lock, that lock free again, and the member's Turn lane released by the + * stopped worker's process group. The host reads the lane from its holder + * record and never takes it, so a legitimate Turn is not refused, and a holder + * it cannot attribute is not released. Both released without an + * acknowledgement means the named holder vanished before recording what it + * observed, which is ``unknown`` rather than a fake settlement. A grace + * timeout on its own moves nothing: a worker still holding a lock still runs. */ export function decideDelegationStop(params: JsonObject): JsonObject { const phase = params.phase as StopPhase; requireThat(openStopPhases.includes(phase), "delegation stop decision requires an open stop phase"); requireThat(typeof params.acknowledged === "boolean", "delegation stop acknowledgement fact required"); - requireThat(typeof params.operation_lock_free === "boolean" && typeof params.lane_lock_free === "boolean", - "delegation stop lock facts required"); + requireThat(typeof params.operation_lock_free === "boolean" && typeof params.worker_lane_released === "boolean", + "delegation stop release facts required"); requireThat(params.timed_out === undefined || typeof params.timed_out === "boolean", "delegation stop timeout fact must be boolean"); requireThat(phase !== "acknowledged" || params.acknowledged === true, "an acknowledged stop cannot lose its acknowledgement"); - const locksFree = params.operation_lock_free === true && params.lane_lock_free === true; + const operationFree = params.operation_lock_free === true; + const released = operationFree && params.worker_lane_released === true; if (params.acknowledged === true) { - if (locksFree) return {phase: "settled", terminal: true, reason: "acknowledged_and_locks_released"}; - return {phase: "acknowledged", terminal: false, reason: params.operation_lock_free === true - ? "turn_lane_still_held" : "operation_lock_still_held"}; + if (released) return {phase: "settled", terminal: true, reason: "acknowledged_and_worker_released"}; + return {phase: "acknowledged", terminal: false, + reason: operationFree ? "worker_lane_release_unproven" : "operation_lock_still_held"}; } - if (locksFree) return {phase: "unknown", terminal: true, reason: "holder_gone_without_acknowledgement"}; - return {phase: "requested", terminal: false, reason: params.timed_out === true - ? "holder_still_running_after_grace" : "awaiting_acknowledgement"}; + if (released) return {phase: "unknown", terminal: true, reason: "holder_gone_without_acknowledgement"}; + return {phase: "requested", terminal: false, reason: operationFree ? "worker_lane_release_unproven" + : params.timed_out === true ? "holder_still_running_after_grace" : "awaiting_acknowledgement"}; } /** Repair only a false terminal observation after the exact Turn validated. diff --git a/loopx/file_lock.py b/loopx/file_lock.py index 29303c066e..7ffa396822 100644 --- a/loopx/file_lock.py +++ b/loopx/file_lock.py @@ -360,17 +360,6 @@ def lock_holder_liveness(path: Path) -> tuple[str, dict[str, object]]: return (LOCK_HOLDER_LIVE if process_is_alive(pid) else LOCK_HOLDER_DEAD), record -def read_lock_holder(path: Path) -> dict[str, object]: - """Read the advisory holder record behind ``path``'s lock; ``{}`` when absent. - - The record names the last process that held the lock and carries - ``released_at`` after a clean release. It is advisory readback for signals - and operator inspection; the kernel lock stays the only proof of holding. - """ - - return _read_holder_record(lock_holder_path(path)) - - def _operator_action(holder: dict[str, object], *, retry_mode: str) -> dict[str, object]: return { "required": True, diff --git a/tests/control_plane_ts/delegation.test.ts b/tests/control_plane_ts/delegation.test.ts index 0aaf6686ed..2352c26d8c 100644 --- a/tests/control_plane_ts/delegation.test.ts +++ b/tests/control_plane_ts/delegation.test.ts @@ -116,26 +116,32 @@ test("stopped is terminal and reachable only from open observations", () => { observation}).status, "stopped"); }); -test("a stop settles only on an acknowledgement plus free locks; time alone proves nothing", () => { - const open = {phase: "requested", acknowledged: false, operation_lock_free: false, lane_lock_free: false}; +test("a stop settles only on an acknowledgement plus released holders; time alone proves nothing", () => { + const open = {phase: "requested", acknowledged: false, operation_lock_free: false, worker_lane_released: false}; assert.deepEqual(decideDelegationStop(open), {phase: "requested", terminal: false, reason: "awaiting_acknowledgement"}); assert.deepEqual(decideDelegationStop({...open, timed_out: true}), {phase: "requested", terminal: false, reason: "holder_still_running_after_grace"}); - assert.deepEqual(decideDelegationStop({...open, operation_lock_free: true, timed_out: true}), + // A lane release without a free operation lock is not a vanished holder. + assert.deepEqual(decideDelegationStop({...open, worker_lane_released: true, timed_out: true}), {phase: "requested", terminal: false, reason: "holder_still_running_after_grace"}); - assert.deepEqual(decideDelegationStop({...open, operation_lock_free: true, lane_lock_free: true}), + // A free operation lock with an unattributed lane holder proves nothing yet. + assert.deepEqual(decideDelegationStop({...open, operation_lock_free: true, timed_out: true}), + {phase: "requested", terminal: false, reason: "worker_lane_release_unproven"}); + assert.deepEqual(decideDelegationStop({...open, operation_lock_free: true, worker_lane_released: true}), {phase: "unknown", terminal: true, reason: "holder_gone_without_acknowledgement"}); - const acked = {phase: "acknowledged", acknowledged: true, operation_lock_free: false, lane_lock_free: false}; + const acked = {phase: "acknowledged", acknowledged: true, operation_lock_free: false, worker_lane_released: false}; assert.deepEqual(decideDelegationStop(acked), {phase: "acknowledged", terminal: false, reason: "operation_lock_still_held"}); + assert.deepEqual(decideDelegationStop({...acked, worker_lane_released: true}), + {phase: "acknowledged", terminal: false, reason: "operation_lock_still_held"}); assert.deepEqual(decideDelegationStop({...acked, operation_lock_free: true}), - {phase: "acknowledged", terminal: false, reason: "turn_lane_still_held"}); - assert.deepEqual(decideDelegationStop({...acked, phase: "requested", operation_lock_free: true, lane_lock_free: true}), - {phase: "settled", terminal: true, reason: "acknowledged_and_locks_released"}); - assert.deepEqual(decideDelegationStop({...acked, operation_lock_free: true, lane_lock_free: true, timed_out: true}), - {phase: "settled", terminal: true, reason: "acknowledged_and_locks_released"}); + {phase: "acknowledged", terminal: false, reason: "worker_lane_release_unproven"}); + assert.deepEqual(decideDelegationStop({...acked, phase: "requested", operation_lock_free: true, worker_lane_released: true}), + {phase: "settled", terminal: true, reason: "acknowledged_and_worker_released"}); + assert.deepEqual(decideDelegationStop({...acked, operation_lock_free: true, worker_lane_released: true, timed_out: true}), + {phase: "settled", terminal: true, reason: "acknowledged_and_worker_released"}); for (const patch of [{phase: "settled"}, {phase: "unknown"}, {phase: "noop"}, {acknowledged: "yes"}, - {operation_lock_free: 1}, {lane_lock_free: undefined}, {timed_out: "later"}, - {phase: "acknowledged", acknowledged: false}]) + {operation_lock_free: 1}, {worker_lane_released: undefined}, {lane_lock_free: true, worker_lane_released: undefined}, + {timed_out: "later"}, {phase: "acknowledged", acknowledged: false}]) assert.throws(() => decideDelegationStop({...open, ...patch})); }); diff --git a/tests/test_local_delegation.py b/tests/test_local_delegation.py index faf0208698..90fa1c2ee3 100644 --- a/tests/test_local_delegation.py +++ b/tests/test_local_delegation.py @@ -20,6 +20,7 @@ from loopx.collaboration_mcp import DelegationFenced, Delegations # noqa: E402 from loopx.control_plane.collaboration.peers import returns # noqa: E402 from loopx.control_plane.collaboration.inbox import _read # noqa: E402 +from loopx.control_plane.turn_driver.lane_fence import turn_lane_liveness, turn_lane_singleflight # noqa: E402 from loopx.file_lock import exclusive_file_lock, try_exclusive_file_lock # noqa: E402 @@ -312,6 +313,12 @@ def test_stop_while_executing_is_acknowledged_by_the_worker_and_settles(service, path = runner.path("analysis-stop") before = _read(path) assert before["status"] == "running" and before["worker"]["pid"] == before["worker"]["pgid"] + # The real run-once child holds the member's lane from inside the worker's group, + # which is what lets settlement attribute the lane without ever taking it. + binding = runner.binding("analysis") + lane = turn_lane_liveness(runner._lane_target(binding)) + assert lane["state"] == "live" and lane["holder"]["pid"] != before["worker"]["pid"] + assert os.getpgid(lane["holder"]["pid"]) == before["worker"]["pgid"] receipt = runner.stop("analysis-stop", execute=True) assert receipt["phase"] == "settled" and receipt["status"] == "stopped", receipt stop = receipt["stop"] @@ -319,7 +326,8 @@ def test_stop_while_executing_is_acknowledged_by_the_worker_and_settles(service, assert stop["worker"]["pid"] == before["worker"]["pid"] assert stop["ack"]["pid"] == before["worker"]["pid"] and stop["ack"]["source"] == "SIGTERM" assert stop["ack"]["observed_status"] == "running" and stop["ack"]["turn_key"] - assert stop["settled"]["operation_lock_free"] and stop["settled"]["lane_lock_free"] + assert stop["settled"]["operation_lock_free"] and stop["settled"]["worker_lane_released"] + assert stop["settled"]["lane_state"] in {"dead", "released"} assert stop["settled"]["turn_journal_status"] == "in_progress" assert stop["lease"] == {"required": False, "released": None} # The acknowledged record is final: nobody writes it again, the Todo stays open, @@ -327,7 +335,6 @@ def test_stop_while_executing_is_acknowledged_by_the_worker_and_settles(service, frozen = path.read_bytes() assert until(lambda: process_gone(host_pid), timeout=20) assert until(lambda: process_gone(before["worker"]["pid"]), timeout=20) - binding = runner.binding("analysis") with try_exclusive_file_lock(runner._lane_target(binding)) as held: assert held is not None assert not demo.canonical_tasks(root)["todo_analyst-initial"]["done"] @@ -356,6 +363,7 @@ def test_stop_without_a_holder_is_acknowledged_by_the_requester(service, monkeyp assert receipt["stop"]["worker"] is None and receipt["stop"]["requested_status"] == "prepared" assert receipt["stop"]["ack"]["pid"] == os.getpid() and receipt["stop"]["ack"]["source"] == "requester" assert receipt["stop"]["settled"]["turn_journal_status"] is None + assert receipt["stop"]["settled"]["lane_state"] in {"absent", "released"} frozen = runner.path("analysis-idle").read_bytes() with pytest.raises(ValueError, match="start a new operation id"): runner.resume("analysis-idle") @@ -370,32 +378,127 @@ def test_stop_without_a_holder_is_acknowledged_by_the_requester(service, monkeyp def test_worker_killed_before_acknowledging_is_unknown_not_settled(service, monkeypatch): - """A vanished holder never becomes a settlement; the stop still fences resume.""" + """A vanished named holder never becomes a settlement; the stop still fences resume.""" import signal root, runner = service host_pid = start_held_worker(service) path = runner.path("analysis-stop") worker = _read(path)["worker"] + killed = [] def kill_without_grace(target, stop): - assert stop["worker"]["pgid"] == worker["pgid"] != os.getpgid(0) + if killed: + return # later calls find nothing left to signal + killed.append(stop["worker"]) + assert stop["worker"] == worker and worker["pgid"] != os.getpgid(0) os.killpg(worker["pgid"], signal.SIGKILL) assert until(lambda: runner._operation_lock_free(target), timeout=20) monkeypatch.setattr(runner, "_signal_worker", kill_without_grace) + first = runner.stop("analysis-stop", execute=True) + assert first["stop"]["ack"] is None and first["phase"] in {"requested", "unknown"}, first + # The operation lock is free now, yet the requester never acknowledges for a + # named worker: the outcome converges on unknown once its Turn child is reaped. + assert until(lambda: runner.stop("analysis-stop", execute=True)["phase"] == "unknown", timeout=20) receipt = runner.stop("analysis-stop", execute=True) - assert receipt["phase"] == "unknown" and receipt["status"] == "running", receipt - assert receipt["stop"]["ack"] is None and receipt["stop"]["settled"]["operation_lock_free"] + assert receipt["status"] == "running" and receipt["stop"]["ack"] is None, receipt + assert receipt["stop"]["settled"]["operation_lock_free"] and receipt["stop"]["settled"]["worker_lane_released"] assert receipt["stop"]["settled"]["turn_journal_status"] == "in_progress" + assert receipt["stop"]["lease"] is None and len(killed) == 1 assert until(lambda: process_gone(host_pid), timeout=20) assert runner.stop("analysis-stop", execute=True) == receipt with pytest.raises(ValueError, match="start a new operation id"): runner.resume("analysis-stop") - assert runner.read("analysis-stop")["stop"]["phase"] == "unknown" + observed = runner.read("analysis-stop") + assert observed["stop"]["phase"] == "unknown" and not observed["recovery_required"] + assert runner.wait("analysis-stop")["stop"]["phase"] == "unknown" assert not demo.canonical_tasks(root)["todo_analyst-initial"]["done"] +def test_sigterm_without_a_stop_keeps_the_operation_recoverable(service, monkeypatch): + """A shutdown signal is not a stop: the worker dies as before and resume stays available.""" + import signal + + root, runner = service + start_held_worker(service, "analysis-term") + path = runner.path("analysis-term") + worker = _read(path)["worker"] + target = runner._lane_target(runner.binding("analysis")) + turn_child = turn_lane_liveness(target)["holder"]["pid"] + try: + os.kill(worker["pid"], signal.SIGTERM) + assert until(lambda: runner._operation_lock_free(path), timeout=20) + # Default termination, exactly as before: only the worker died, and its + # orphaned Turn child still holds the member's lane. + lane = turn_lane_liveness(target) + assert lane["state"] == "live" and lane["holder"]["pid"] == turn_child + assert not runner._stop_path(path).exists() + assert _read(path)["status"] == "running" and "stop" not in runner.read("analysis-term") + spawned = [] + monkeypatch.setattr(runner, "_spawn", spawned.append) + runner.resume("analysis-term") + assert spawned == ["analysis-term"] + finally: + try: + os.killpg(worker["pgid"], signal.SIGKILL) # the orphaned Turn child + except ProcessLookupError: + pass + + +def test_stop_settlement_never_refuses_a_concurrent_turn_on_the_member_lane(service, monkeypatch): + """Settling reads the lane holder record; a real Turn racing it is always admitted.""" + import threading + from loopx import file_lock + + _, runner = service + monkeypatch.setattr(runner, "_spawn", lambda _: None) + runner.start("analysis", "analysis-race", brief()) + path = runner.path("analysis-race") + binding = runner.binding("analysis") + target = runner._lane_target(binding) + lane_lock = target.with_name(target.name + ".lock") + opened = [] + real_open = file_lock._open_lock_descriptor + + def recording_open(lock_path, **kwargs): + opened.append((threading.get_ident(), Path(lock_path))) + return real_open(lock_path, **kwargs) + + monkeypatch.setattr(file_lock, "_open_lock_descriptor", recording_open) + lane = {"runtime_root": runner.root, "goal_id": runner.goal_id, + "plan": {"turn_envelope": {"agent_id": binding["agent_id"]}}} + admitted, refused, phases, settlers = [], [], [], [] + done = Event() + + def settle_continuously(): + settlers.append(threading.get_ident()) + while not done.is_set(): + phases.append(runner.stop("analysis-race", execute=True)["phase"]) + + # An unnamed holder keeps the operation open, so every stop call settles again + # and reads the member's lane while other Turns of that member take it. + with exclusive_file_lock(path), ThreadPoolExecutor(max_workers=1) as pool: + future = pool.submit(settle_continuously) + try: + assert until(lambda: len(phases) >= 2, timeout=30) + for _ in range(200): + with turn_lane_singleflight(**lane) as held: + (admitted if held is not None else refused).append(held) + assert until(lambda: len(phases) >= 4, timeout=30) + finally: + done.set() + future.result(timeout=30) + assert refused == [] and len(admitted) == 200 + assert set(phases) == {"requested"} + assert [lock for ident, lock in opened if ident in settlers and lock == lane_lock] == [] + # Once the holder is gone the requester acknowledges; settling still never takes the lane. + opened.clear() + receipt = runner.stop("analysis-race", execute=True) + assert receipt["phase"] == "settled" and receipt["stop"]["settled"]["lane_state"] == "released" + assert lane_lock not in {lock for _, lock in opened} + + def test_fenced_write_after_another_process_stop_writes_nothing(service, monkeypatch): root, runner = service monkeypatch.setattr(runner, "_spawn", lambda _: None) From 5e7b7bfd8fb425312d25a771911984040ac9ee03 Mon Sep 17 00:00:00 2001 From: song <22676124+songoow@users.noreply.github.com> Date: Tue, 29 Sep 2026 12:34:46 -0400 Subject: [PATCH 08/13] chore(census): follow registry reads moved on main Regenerated with scripts/generate_project_registry_io_manifest.py after rebasing onto main. Site ids and classifications are unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) Signed-off-by: song <22676124+songoow@users.noreply.github.com> --- loopx/semantics/project_registry_io_manifest_v1.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/loopx/semantics/project_registry_io_manifest_v1.json b/loopx/semantics/project_registry_io_manifest_v1.json index 1d92d29240..9b9d446702 100644 --- a/loopx/semantics/project_registry_io_manifest_v1.json +++ b/loopx/semantics/project_registry_io_manifest_v1.json @@ -1799,7 +1799,7 @@ }, { "site": "loopx/history.py::.collect_history::codec_read:load_registry#1", - "line": 341, + "line": 342, "column": 20, "kind": "codec_read", "api": "load_registry", @@ -1807,7 +1807,7 @@ }, { "site": "loopx/history.py::.inspect_index_duplicates::codec_read:load_registry#1", - "line": 597, + "line": 598, "column": 16, "kind": "codec_read", "api": "load_registry", @@ -1815,7 +1815,7 @@ }, { "site": "loopx/history.py::.rebuild_index_artifact_collisions::codec_read:load_registry#1", - "line": 811, + "line": 812, "column": 16, "kind": "codec_read", "api": "load_registry", @@ -1823,7 +1823,7 @@ }, { "site": "loopx/history.py::.repair_index_duplicates::codec_read:load_registry#1", - "line": 701, + "line": 702, "column": 16, "kind": "codec_read", "api": "load_registry", From 5d67e4795ff3febe82149f15dd4ada5900b8887c Mon Sep 17 00:00:00 2001 From: song Date: Wed, 30 Sep 2026 13:22:19 +0800 Subject: [PATCH 09/13] fix(delegation): settle a stop only after the native Host and its group exit The worker and its Turn lane let go while the Host supervisor is still terminating the Host, so their release never proved that the old executor stopped. The Host transport now names the process group its supervisor owns in a record beside the operation, the drain is read back from that record without signalling anything, and the typed decision requires an exited group before a stop may settle. A group that cannot be attributed keeps an acknowledged stop open for a later same-identity read, and the TS supervisor stays the only owner that terminates a Host. Signed-off-by: song --- loopx/collaboration_mcp.py | 52 +++++++++-- .../control_plane/collaboration/delegation.ts | 44 ++++++--- .../collaboration/delegation_inventory.py | 10 +- .../control_plane/turn_driver/host_process.ts | 12 ++- .../turn_driver/host_process_bridge.ts | 2 +- .../turn_driver/host_process_transport.py | 93 ++++++++++++++++++- 6 files changed, 188 insertions(+), 25 deletions(-) diff --git a/loopx/collaboration_mcp.py b/loopx/collaboration_mcp.py index 7f90c2f7ce..8158261e58 100644 --- a/loopx/collaboration_mcp.py +++ b/loopx/collaboration_mcp.py @@ -42,12 +42,18 @@ turn_journal_path, ) from .control_plane.turn_driver.host_binding import turn_host_arg_option +from .control_plane.turn_driver.host_process_transport import ( + HOST_PROCESS_DRAINING, HOST_PROCESS_RECORD_ENV, host_process_drain, +) from .control_plane.turn_driver.lane_fence import ( TURN_LANE_ABSENT, TURN_LANE_DEAD, TURN_LANE_LIVE, TURN_LANE_RELEASED, turn_lane_liveness, turn_lane_target, ) from .control_plane.work_items.task_lease import release_task_lease from .control_plane.collaboration.inbox import _hash, _read, _write, _root, _receipt +from .control_plane.collaboration.delegation_inventory import ( + DELEGATION_HOST_PROCESS_SUFFIX, DELEGATION_STOP_RECEIPT_SUFFIX, +) from .control_plane.collaboration.peers import return_result from .control_plane.collaboration.inbox import acknowledge, _entry, normalize_request from .control_plane.collaboration.goal_instance_scope import ( @@ -443,7 +449,12 @@ def path(self, operation_id: str) -> Path: @staticmethod def _stop_path(path: Path) -> Path: """The stop receipt sits beside its execution record and is never merged into it.""" - return path.with_name(path.stem + ".stop.json") + return path.with_name(path.stem + DELEGATION_STOP_RECEIPT_SUFFIX) + + @staticmethod + def _host_process_record(path: Path) -> Path: + """Where this operation's Turn names the native Host it launched, for drain readback.""" + return path.with_name(path.stem + DELEGATION_HOST_PROCESS_SUFFIX) @staticmethod def _dispatch_lock(path: Path) -> Path: @@ -913,7 +924,9 @@ def stop(self, operation_id: str, *, execute: bool) -> dict: holder is signalled by process group and given a bounded grace to acknowledge; another host's holder is left to find the request at its next checkpoint or fenced write. ``settled`` and ``unknown`` come from - the typed decision over lock facts; elapsed time proves nothing. + the typed decision over lock facts and the drain of the native Host the + Turn launched, whose TS supervisor alone terminates it; elapsed time + proves nothing. """ require_operation_id(operation_id) @@ -943,6 +956,9 @@ def stop(self, operation_id: str, *, execute: bool) -> dict: self._acknowledge_stop(path, _read(path), binding, source="requester") except LockAcquireTimeoutError: pass # an unnamed holder meets the request at its next checkpoint or write + else: + # A Host left behind by an earlier worker may still be terminating. + self._await_host_drain(path, time.monotonic() + DELEGATION_STOP_GRACE_SECONDS) else: # Only the named worker acknowledges. If it vanishes first, the typed # decision reports unknown instead of a requester settlement. @@ -982,9 +998,12 @@ def _signal_worker(self, path: Path, stop: dict) -> None: os.killpg(pgid, signal.SIGTERM) except ProcessLookupError: return + # Poll the release facts the decision needs; the deadline only bounds this + # call, and a Host still draining when it passes leaves the stop open. deadline = time.monotonic() + DELEGATION_STOP_GRACE_SECONDS while time.monotonic() < deadline: if self._operation_lock_free(path): + self._await_host_drain(path, deadline) return time.sleep(0.2) try: @@ -994,6 +1013,14 @@ def _signal_worker(self, path: Path, stop: dict) -> None: deadline = time.monotonic() + 2.0 while time.monotonic() < deadline and not self._operation_lock_free(path): time.sleep(0.1) + # The Host supervisor sits outside the worker's group and cleans up on its own. + self._await_host_drain(path, time.monotonic() + DELEGATION_STOP_GRACE_SECONDS) + + def _await_host_drain(self, path: Path, deadline: float) -> None: + """Wait, never kill: the TS Host supervisor owns terminating its process group.""" + while (host_process_drain(self._host_process_record(path)) == HOST_PROCESS_DRAINING + and time.monotonic() < deadline): + time.sleep(0.1) def _acknowledge_stop(self, path: Path, row: dict, binding: dict, *, source: str) -> None: """Acknowledge from under the operation lock: mark stopped, then release the hard lease. @@ -1018,10 +1045,11 @@ def _acknowledge_stop(self, path: Path, row: dict, binding: dict, *, source: str transition = effect_runtime_result("collaboration.delegation.observe", { "from": observed, "to": "stopped", }) - # This process holds the operation lock and its lane read comes later. + # This process holds the operation lock; its lane and Host drain are read later. phase = effect_runtime_result("collaboration.delegation.stop", { "phase": stop["phase"], "acknowledged": True, "operation_lock_free": False, "worker_lane_released": False, + "host_process": HOST_PROCESS_DRAINING, }) current["status"] = transition["status"] _write(path, current) @@ -1068,6 +1096,8 @@ def _settle_stop(self, path: Path) -> dict: return self._stop_receipt(row, binding, stop) facts = {"operation_lock_free": self._operation_lock_free(path)} facts["worker_lane_released"], lane_state = self._worker_lane_released(row, stop, binding) + # Read last: a Host seen drained after its worker and lane let go stays drained. + facts["host_process"] = host_process_drain(self._host_process_record(path)) decision = effect_runtime_result("collaboration.delegation.stop", { "phase": stop["phase"], "acknowledged": stop.get("ack") is not None, "timed_out": time.time() - stop["requested_at"] > DELEGATION_STOP_GRACE_SECONDS, @@ -1085,12 +1115,17 @@ def _settle_stop(self, path: Path) -> dict: _write(self._stop_path(path), stop) return self._stop_receipt(row, binding, stop) - def _cli(self, binding: dict, *args: str, timeout: int = 60) -> dict: + def _cli(self, binding: dict, *args: str, timeout: int = 60, host_record: Path | None = None) -> dict: + environment = _pinned_release_environment() + environment.pop(HOST_PROCESS_RECORD_ENV, None) + if host_record is not None: + # The Turn's Host transport names the process group its supervisor owns. + environment[HOST_PROCESS_RECORD_ENV] = str(host_record) completed = subprocess.run([*_python_module_command("loopx.cli"), "--registry", str(self.registry), "--runtime-root", str(self.root), "--format", "json", *args, ], cwd=binding["workspace"], capture_output=True, text=True, encoding="utf-8", - timeout=timeout, env=_pinned_release_environment()) + timeout=timeout, env=environment) try: value = json.loads(completed.stdout) except ValueError as exc: @@ -1456,7 +1491,8 @@ def _execute(self, path: Path, row: dict, binding: dict) -> None: ] ) result = self._cli(binding, "turn", "run-once", *common, *selector, *execution, - "--execute", timeout=binding["timeout_seconds"] + 60) + "--execute", timeout=binding["timeout_seconds"] + 60, + host_record=self._host_process_record(path)) self._record_turn_result(path, row, result) finally: # The compatibility bootstrap is private host input. Keeping it @@ -1500,6 +1536,7 @@ def _execute(self, path: Path, row: dict, binding: dict) -> None: *execution, "--execute", timeout=binding["timeout_seconds"] + 60, + host_record=self._host_process_record(path), ) self._record_turn_result(path, row, result, publish=False) if result.get("status") != "committed" or result.get("result_kind") != "validated_progress": @@ -1616,7 +1653,8 @@ async def stop_delegation(operation_id: str) -> dict: """Stop one original operation and return what was proven, not what was hoped. settled: the worker acknowledged, released the operation and let go of its - Turn lane. acknowledged/requested: still winding down; call again. unknown: + Turn lane, and the native host and its process group exited. + acknowledged/requested: still winding down; call again. unknown: the named worker vanished before acknowledging; inspect its Turn and task lease before reusing the task. noop: already accepted/rejected/stopped. Stopped work is not resumed; a new scope needs a new operation id. Elapsed diff --git a/loopx/control_plane/collaboration/delegation.ts b/loopx/control_plane/collaboration/delegation.ts index 6612deaec5..00d1214df0 100644 --- a/loopx/control_plane/collaboration/delegation.ts +++ b/loopx/control_plane/collaboration/delegation.ts @@ -343,17 +343,25 @@ export function transitionDelegationObservation(params: JsonObject): JsonObject type StopPhase = "requested" | "acknowledged" | "settled" | "unknown"; const openStopPhases: readonly StopPhase[] = ["requested", "acknowledged"]; +/** What the host read back about the native Host process the operation launched. */ +type HostProcessDrain = "not_launched" | "drained" | "draining" | "unattributable"; +const hostProcessDrains: readonly HostProcessDrain[] = ["not_launched", "drained", "draining", "unattributable"]; /** Advance one stop request from host release facts; a receipt is never inferred from time. * * ``settled`` needs the acknowledgement of a process that held the operation - * lock, that lock free again, and the member's Turn lane released by the - * stopped worker's process group. The host reads the lane from its holder - * record and never takes it, so a legitimate Turn is not refused, and a holder - * it cannot attribute is not released. Both released without an - * acknowledgement means the named holder vanished before recording what it - * observed, which is ``unknown`` rather than a fake settlement. A grace - * timeout on its own moves nothing: a worker still holding a lock still runs. + * lock, that lock free again, the member's Turn lane released by the stopped + * worker's process group, and the native Host the operation launched drained + * together with its process group. A worker and its lane can let go while the + * Host supervisor is still terminating the Host, so their release proves + * nothing about the Host. The host reads the lane from its holder record and + * never takes it, so a legitimate Turn is not refused, and a holder it cannot + * attribute is not released. A Host drain that cannot be attributed keeps an + * acknowledged stop open so a later read with the same identity can still + * settle it. Everything released without an acknowledgement means the named + * holder vanished before recording what it observed, which is ``unknown`` + * rather than a fake settlement. A grace timeout on its own moves nothing: a + * worker still holding a lock still runs. */ export function decideDelegationStop(params: JsonObject): JsonObject { const phase = params.phase as StopPhase; @@ -361,19 +369,29 @@ export function decideDelegationStop(params: JsonObject): JsonObject { requireThat(typeof params.acknowledged === "boolean", "delegation stop acknowledgement fact required"); requireThat(typeof params.operation_lock_free === "boolean" && typeof params.worker_lane_released === "boolean", "delegation stop release facts required"); + requireThat(hostProcessDrains.includes(params.host_process as HostProcessDrain), + "delegation stop host process drain fact required"); requireThat(params.timed_out === undefined || typeof params.timed_out === "boolean", "delegation stop timeout fact must be boolean"); requireThat(phase !== "acknowledged" || params.acknowledged === true, "an acknowledged stop cannot lose its acknowledgement"); const operationFree = params.operation_lock_free === true; - const released = operationFree && params.worker_lane_released === true; + const workerReleased = operationFree && params.worker_lane_released === true; + const host = params.host_process as HostProcessDrain; + const hostDrained = host === "drained" || host === "not_launched"; + const pending = !operationFree ? "operation_lock_still_held" + : params.worker_lane_released !== true ? "worker_lane_release_unproven" + : host === "draining" ? "host_process_still_running" : "host_process_drain_unproven"; if (params.acknowledged === true) { - if (released) return {phase: "settled", terminal: true, reason: "acknowledged_and_worker_released"}; - return {phase: "acknowledged", terminal: false, - reason: operationFree ? "worker_lane_release_unproven" : "operation_lock_still_held"}; + if (workerReleased && hostDrained) { + return {phase: "settled", terminal: true, reason: "acknowledged_worker_and_host_released"}; + } + return {phase: "acknowledged", terminal: false, reason: pending}; + } + if (workerReleased && host !== "draining") { + return {phase: "unknown", terminal: true, reason: "holder_gone_without_acknowledgement"}; } - if (released) return {phase: "unknown", terminal: true, reason: "holder_gone_without_acknowledgement"}; - return {phase: "requested", terminal: false, reason: operationFree ? "worker_lane_release_unproven" + return {phase: "requested", terminal: false, reason: operationFree ? pending : params.timed_out === true ? "holder_still_running_after_grace" : "awaiting_acknowledgement"}; } diff --git a/loopx/control_plane/collaboration/delegation_inventory.py b/loopx/control_plane/collaboration/delegation_inventory.py index f070107f80..4fb2fc3a84 100644 --- a/loopx/control_plane/collaboration/delegation_inventory.py +++ b/loopx/control_plane/collaboration/delegation_inventory.py @@ -12,6 +12,12 @@ if TYPE_CHECKING: from ...collaboration_mcp import Delegations +# Files kept beside an execution record and never merged into it: the stop +# receipt, and the record naming the native Host its Turn launched. +DELEGATION_STOP_RECEIPT_SUFFIX = ".stop.json" +DELEGATION_HOST_PROCESS_SUFFIX = ".host.json" +DELEGATION_RECORD_SIDECAR_SUFFIXES = (DELEGATION_STOP_RECEIPT_SUFFIX, DELEGATION_HOST_PROCESS_SUFFIX) + def read_delegation_inventory(service: Delegations, *, limit: int = 20, cursor: str | None = None) -> dict: @@ -24,8 +30,8 @@ def addresses(): try: entries = directory.iterdir() for path in entries: - if path.suffix != ".json" or path.name.endswith(".stop.json"): - continue # stop receipts sit beside their execution record + if path.suffix != ".json" or path.name.endswith(DELEGATION_RECORD_SIDECAR_SUFFIXES): + continue if not BARE_SHA256_PATTERN.fullmatch(path.stem): raise ValueError("unexpected delegation record address; reconcile inventory storage") if query["cursor"] is None or path.stem > query["cursor"]: diff --git a/loopx/control_plane/turn_driver/host_process.ts b/loopx/control_plane/turn_driver/host_process.ts index 685a5c39fa..0a752a32bf 100644 --- a/loopx/control_plane/turn_driver/host_process.ts +++ b/loopx/control_plane/turn_driver/host_process.ts @@ -22,6 +22,9 @@ export interface HostProcessResult { group_signal_sent: boolean; } export type HostProcessOutput = {kind: "stdout" | "stderr"; text: string}; +/** The group this owner will clean up, reported once the Host is spawned. + * ``process_group`` is null where cleanup is tree best effort (Windows). */ +export type HostProcessSpawned = {kind: "spawned"; pid: number; process_group: number | null}; export const HOST_PROCESS_TERMINATE_GRACE_MS = 300; /** Restrict transport size separately from the caller's public result budget. */ @@ -47,7 +50,8 @@ function signalGroup(child: ChildProcessWithoutNullStreams, signal: NodeJS.Signa } export async function runHostProcess(request: HostProcessRequest, - output: (item: HostProcessOutput) => Promise, signal?: AbortSignal): Promise { + output: (item: HostProcessOutput) => Promise, signal?: AbortSignal, + spawned?: (item: HostProcessSpawned) => Promise): Promise { const base: HostProcessResult = {kind: "result", outcome: "spawn_failed", returncode: null, signal: null, output_complete: true, cleanup_scope: process.platform === "win32" ? "process_tree_best_effort" : "process_group", group_signal_sent: false}; @@ -116,6 +120,12 @@ export async function runHostProcess(request: HostProcessRequest, }; const reads = Promise.all([read("stdout"), read("stderr")]); child.stdin.on("error", () => {}); // A Host may close stdin before consuming it. + if (child.pid && spawned) { + // A caller that cannot record the owned group cancels rather than run unaccounted. + try { await spawned({kind: "spawned", pid: child.pid, + process_group: process.platform === "win32" ? null : child.pid}); } + catch { complete = false; stop("cancelled"); } + } child.stdin.end(request.input); try { await exited; diff --git a/loopx/control_plane/turn_driver/host_process_bridge.ts b/loopx/control_plane/turn_driver/host_process_bridge.ts index b1c261c8bf..d34038cfa5 100644 --- a/loopx/control_plane/turn_driver/host_process_bridge.ts +++ b/loopx/control_plane/turn_driver/host_process_bridge.ts @@ -20,7 +20,7 @@ process.stdin.on("data", (chunk: Buffer) => { accepted = true; const line = pending.subarray(0, newline).toString("utf8"); pending = Buffer.alloc(0); void (async () => { - try { await emit(await runHostProcess(decodeHostProcessRequest(JSON.parse(line)), emit, owner.signal)); } + try { await emit(await runHostProcess(decodeHostProcessRequest(JSON.parse(line)), emit, owner.signal, emit)); } catch { process.exitCode = 1; } finally { process.stdin.destroy(); } })(); diff --git a/loopx/control_plane/turn_driver/host_process_transport.py b/loopx/control_plane/turn_driver/host_process_transport.py index 4f51148fa6..5a6e250534 100644 --- a/loopx/control_plane/turn_driver/host_process_transport.py +++ b/loopx/control_plane/turn_driver/host_process_transport.py @@ -3,12 +3,15 @@ from __future__ import annotations import json +import os import subprocess import sys +import tempfile from collections.abc import Callable, Sequence from pathlib import Path from typing import Any +from ...file_lock import lock_holder_host_label from ..effect_runtime import _node_executable # Keep Python's Windows executable/batch launcher compatibility. No timeout, @@ -16,6 +19,72 @@ _WINDOWS_COMMAND_RELAY = "import subprocess,sys;sys.exit(subprocess.call(sys.argv[1:]))" +# A launching owner that must later prove its Host drained names a record path +# here. The transport consumes it: the Host never inherits it, so a nested +# LoopX run inside the Host cannot overwrite its parent's record. +HOST_PROCESS_RECORD_ENV = "LOOPX_HOST_PROCESS_RECORD" +HOST_PROCESS_RECORD_SCHEMA_VERSION = "loopx_host_process_record_v0" +# Drain facts read back from a record; the caller's typed decision interprets them. +HOST_PROCESS_NOT_LAUNCHED = "not_launched" +HOST_PROCESS_DRAINED = "drained" +HOST_PROCESS_DRAINING = "draining" +HOST_PROCESS_UNATTRIBUTABLE = "unattributable" + + +def _write_host_process_record(path: Path, record: dict[str, Any]) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + descriptor, temporary = tempfile.mkstemp(prefix=f".{path.name}.", suffix=".tmp", dir=path.parent) + try: + with os.fdopen(descriptor, "w", encoding="utf-8") as handle: + json.dump(record, handle, separators=(",", ":")) + os.replace(temporary, path) + finally: + Path(temporary).unlink(missing_ok=True) + + +def _process_group_present(pgid: int) -> bool: + try: + os.killpg(pgid, 0) + except ProcessLookupError: + return False + except PermissionError: + return True # it exists; it is simply not ours to signal + return True + + +def host_process_drain(record_path: Path) -> str: + """Read whether the Host a record names, and its process group, have exited. + + Read-only: nothing is signalled, and the TS-owned supervisor keeps cleanup. + No record means no Host was launched under it. ``draining`` while the + supervising bridge or the Host's group still has a member. A record from + another machine, one without a reported group whose supervisor is gone, + or a platform without process groups proves nothing: ``unattributable``. + """ + + try: + record = json.loads(record_path.read_text(encoding="utf-8")) + except FileNotFoundError: + return HOST_PROCESS_NOT_LAUNCHED + except (OSError, ValueError): + return HOST_PROCESS_UNATTRIBUTABLE + if (not isinstance(record, dict) or record.get("schema_version") != HOST_PROCESS_RECORD_SCHEMA_VERSION + or record.get("host") != lock_holder_host_label() or not hasattr(os, "killpg")): + return HOST_PROCESS_UNATTRIBUTABLE + bridge, group = record.get("bridge_pid"), record.get("process_group") + if record.get("phase") != "finished": + if not isinstance(bridge, int) or bridge <= 1: + return HOST_PROCESS_UNATTRIBUTABLE + if _process_group_present(bridge): # the bridge leads its own session + return HOST_PROCESS_DRAINING + if group is None: + # The supervisor left before reporting a group it may already have spawned. + return HOST_PROCESS_UNATTRIBUTABLE if record.get("phase") == "launching" else HOST_PROCESS_DRAINED + if not isinstance(group, int) or group <= 1: + return HOST_PROCESS_UNATTRIBUTABLE + return HOST_PROCESS_DRAINING if _process_group_present(group) else HOST_PROCESS_DRAINED + + class HostOutputLines: """Frame LF records without retaining raw trajectories or an unbounded line.""" @@ -76,6 +145,9 @@ def run_host_process( "stdout_limit_bytes": stdout_limit_bytes, } bridge = Path(__file__).with_name("host_process_bridge.ts") + environment = os.environ.copy() + record_value = environment.pop(HOST_PROCESS_RECORD_ENV, "") + record_path = Path(record_value) if record_value else None with subprocess.Popen( [ _node_executable(), @@ -90,10 +162,19 @@ def run_host_process( encoding="utf-8", errors="strict", start_new_session=True, + env=environment, ) as proc: assert proc.stdin is not None and proc.stdout is not None result = None + record = None try: + if record_path is not None: + # Written before the request, so no Host exists that the record + # does not name; a record that cannot be written launches nothing. + record = {"schema_version": HOST_PROCESS_RECORD_SCHEMA_VERSION, + "host": lock_holder_host_label(), "owner_pid": os.getpid(), + "bridge_pid": proc.pid, "phase": "launching", "process_group": None} + _write_host_process_record(record_path, record) proc.stdin.write( json.dumps(request, ensure_ascii=False, separators=(",", ":")) + "\n" ) @@ -101,7 +182,13 @@ def run_host_process( for line in proc.stdout: event = json.loads(line) kind = event.get("kind") - if kind in {"stdout", "stderr"} and isinstance(event.get("text"), str): + if kind == "spawned" and isinstance(event.get("pid"), int): + if record is not None: + group = event.get("process_group") + record.update(phase="spawned", host_pid=event["pid"], + process_group=group if isinstance(group, int) else None) + _write_host_process_record(record_path, record) + elif kind in {"stdout", "stderr"} and isinstance(event.get("text"), str): consume = on_stdout if kind == "stdout" else on_stderr if consume is not None: consume(event["text"]) @@ -124,6 +211,10 @@ def run_host_process( except subprocess.TimeoutExpired: proc.kill() proc.wait() + if record is not None and result is not None and proc.returncode == 0: + # The supervisor returned only after cleaning its group; the group is still re-read. + record["phase"] = "finished" + _write_host_process_record(record_path, record) if proc.returncode != 0 or result is None: raise RuntimeError("Managed Host process supervision returned no result") return result From fd49df60ef302b6e025c47ce2bf289e12c9fa83c Mon Sep 17 00:00:00 2001 From: song Date: Wed, 30 Sep 2026 13:22:28 +0800 Subject: [PATCH 10/13] test(delegation): prove a stop settles only after the owned Host exits A real File/SQLite fixture whose host and same-group child ignore SIGTERM asserts both are gone at the instant settled returns, with a platform-valid process check in place of the /proc read that treated a live macOS process as exited. Interrupted cleanup, repeated reads, an unacknowledged holder and an unrecorded group get their own negative coverage. Signed-off-by: song --- tests/control_plane/test_host_process.py | 63 +++++++++++++ tests/control_plane_ts/delegation.test.ts | 33 ++++++- tests/control_plane_ts/host_process.test.ts | 24 +++++ tests/test_delegation_inventory.py | 2 + tests/test_local_delegation.py | 99 +++++++++++++++++++-- 5 files changed, 211 insertions(+), 10 deletions(-) diff --git a/tests/control_plane/test_host_process.py b/tests/control_plane/test_host_process.py index 7da98714ef..558725565d 100644 --- a/tests/control_plane/test_host_process.py +++ b/tests/control_plane/test_host_process.py @@ -2,6 +2,7 @@ from __future__ import annotations +import json import os import signal import subprocess @@ -174,3 +175,65 @@ def test_windows_transport_relay_preserves_argv_and_stdin(tmp_path: Path) -> Non ) assert result["ok"] is True assert result["value"] == {"args": values, "input": {}} + + +@pytest.mark.skipif(os.name == "nt", reason="POSIX process-group drain readback") +def test_host_process_record_names_the_owned_group_and_is_not_inherited(tmp_path: Path, monkeypatch) -> None: + from loopx.control_plane.turn_driver.host_process_transport import ( + HOST_PROCESS_RECORD_ENV, host_process_drain, + ) + + record_path = tmp_path / "op.host.json" + assert host_process_drain(record_path) == "not_launched" + monkeypatch.setenv(HOST_PROCESS_RECORD_ENV, str(record_path)) + host = ("import json,os,sys;print(json.dumps({'env': os.environ.get(%r), 'pid': os.getpid()," + " 'pgid': os.getpgid(0)}))" % HOST_PROCESS_RECORD_ENV) + result = _run_host({}, argv=[sys.executable, "-c", host], project=tmp_path, timeout_seconds=5) + assert result["ok"] is True + # A nested LoopX run inside the Host cannot overwrite its parent's record. + assert result["value"]["env"] is None + record = json.loads(record_path.read_text()) + assert record["phase"] == "finished" + assert record["host_pid"] == result["value"]["pid"] == record["process_group"] == result["value"]["pgid"] + assert host_process_drain(record_path) == "drained" + + +@pytest.mark.skipif(os.name == "nt", reason="POSIX process-group drain readback") +def test_host_process_drain_reads_live_groups_and_refuses_unattributable_records(tmp_path: Path) -> None: + from loopx.control_plane.turn_driver.host_process_transport import ( + HOST_PROCESS_RECORD_SCHEMA_VERSION, host_process_drain, + ) + from loopx.file_lock import lock_holder_host_label + + path = tmp_path / "op.host.json" + live = subprocess.Popen([sys.executable, "-c", "import time;time.sleep(60)"], start_new_session=True) + gone = subprocess.Popen([sys.executable, "-c", "pass"], start_new_session=True) + gone.wait(timeout=10) + + def drain(**fields): + path.write_text(json.dumps({"schema_version": HOST_PROCESS_RECORD_SCHEMA_VERSION, + "host": lock_holder_host_label(), "owner_pid": 1, **fields})) + return host_process_drain(path) + + try: + # A live supervisor or a live Host group is still draining. + assert drain(phase="launching", bridge_pid=live.pid, process_group=None) == "draining" + assert drain(phase="spawned", bridge_pid=gone.pid, process_group=live.pid) == "draining" + assert drain(phase="finished", bridge_pid=gone.pid, process_group=live.pid) == "draining" + assert drain(phase="spawned", bridge_pid=gone.pid, process_group=gone.pid) == "drained" + # A supervisor gone before it reported a group may have spawned one anyway. + assert drain(phase="launching", bridge_pid=gone.pid, process_group=None) == "unattributable" + assert drain(phase="finished", bridge_pid=gone.pid, process_group=None) == "drained" + for fields in ({"phase": "spawned", "bridge_pid": None, "process_group": gone.pid}, + {"phase": "spawned", "bridge_pid": gone.pid, "process_group": "1"}, + {"phase": "spawned", "bridge_pid": gone.pid, "process_group": 1}, + {"phase": "spawned", "bridge_pid": gone.pid, "process_group": gone.pid, + "host": "another-machine"}, + {"phase": "spawned", "bridge_pid": gone.pid, "process_group": gone.pid, + "schema_version": "other"}): + assert drain(**fields) == "unattributable", fields + path.write_text("{not json") + assert host_process_drain(path) == "unattributable" + finally: + live.kill() + live.wait(timeout=10) diff --git a/tests/control_plane_ts/delegation.test.ts b/tests/control_plane_ts/delegation.test.ts index 2352c26d8c..f97a99c957 100644 --- a/tests/control_plane_ts/delegation.test.ts +++ b/tests/control_plane_ts/delegation.test.ts @@ -117,7 +117,8 @@ test("stopped is terminal and reachable only from open observations", () => { }); test("a stop settles only on an acknowledgement plus released holders; time alone proves nothing", () => { - const open = {phase: "requested", acknowledged: false, operation_lock_free: false, worker_lane_released: false}; + const open = {phase: "requested", acknowledged: false, operation_lock_free: false, worker_lane_released: false, + host_process: "drained"}; assert.deepEqual(decideDelegationStop(open), {phase: "requested", terminal: false, reason: "awaiting_acknowledgement"}); assert.deepEqual(decideDelegationStop({...open, timed_out: true}), {phase: "requested", terminal: false, reason: "holder_still_running_after_grace"}); @@ -129,22 +130,46 @@ test("a stop settles only on an acknowledgement plus released holders; time alon {phase: "requested", terminal: false, reason: "worker_lane_release_unproven"}); assert.deepEqual(decideDelegationStop({...open, operation_lock_free: true, worker_lane_released: true}), {phase: "unknown", terminal: true, reason: "holder_gone_without_acknowledgement"}); - const acked = {phase: "acknowledged", acknowledged: true, operation_lock_free: false, worker_lane_released: false}; + const acked = {phase: "acknowledged", acknowledged: true, operation_lock_free: false, worker_lane_released: false, + host_process: "drained"}; assert.deepEqual(decideDelegationStop(acked), {phase: "acknowledged", terminal: false, reason: "operation_lock_still_held"}); assert.deepEqual(decideDelegationStop({...acked, worker_lane_released: true}), {phase: "acknowledged", terminal: false, reason: "operation_lock_still_held"}); assert.deepEqual(decideDelegationStop({...acked, operation_lock_free: true}), {phase: "acknowledged", terminal: false, reason: "worker_lane_release_unproven"}); assert.deepEqual(decideDelegationStop({...acked, phase: "requested", operation_lock_free: true, worker_lane_released: true}), - {phase: "settled", terminal: true, reason: "acknowledged_and_worker_released"}); + {phase: "settled", terminal: true, reason: "acknowledged_worker_and_host_released"}); assert.deepEqual(decideDelegationStop({...acked, operation_lock_free: true, worker_lane_released: true, timed_out: true}), - {phase: "settled", terminal: true, reason: "acknowledged_and_worker_released"}); + {phase: "settled", terminal: true, reason: "acknowledged_worker_and_host_released"}); for (const patch of [{phase: "settled"}, {phase: "unknown"}, {phase: "noop"}, {acknowledged: "yes"}, + {host_process: undefined}, {host_process: "exited"}, {host_process: true}, {operation_lock_free: 1}, {worker_lane_released: undefined}, {lane_lock_free: true, worker_lane_released: undefined}, {timed_out: "later"}, {phase: "acknowledged", acknowledged: false}]) assert.throws(() => decideDelegationStop({...open, ...patch})); }); +test("a released worker and lane never settle a stop while the native Host still drains", () => { + const released = {phase: "acknowledged", acknowledged: true, operation_lock_free: true, worker_lane_released: true}; + assert.deepEqual(decideDelegationStop({...released, host_process: "draining", timed_out: true}), + {phase: "acknowledged", terminal: false, reason: "host_process_still_running"}); + // Without an attributable drain the stop stays open for a later same-identity read. + assert.deepEqual(decideDelegationStop({...released, host_process: "unattributable"}), + {phase: "acknowledged", terminal: false, reason: "host_process_drain_unproven"}); + for (const host_process of ["drained", "not_launched"]) + assert.deepEqual(decideDelegationStop({...released, host_process}), + {phase: "settled", terminal: true, reason: "acknowledged_worker_and_host_released"}); + // A held lock still dominates a drained Host. + assert.deepEqual(decideDelegationStop({...released, operation_lock_free: false, host_process: "drained"}), + {phase: "acknowledged", terminal: false, reason: "operation_lock_still_held"}); + // A vanished holder is unknown only once its Host is no longer seen running. + const vanished = {...released, phase: "requested", acknowledged: false}; + assert.deepEqual(decideDelegationStop({...vanished, host_process: "draining"}), + {phase: "requested", terminal: false, reason: "host_process_still_running"}); + for (const host_process of ["drained", "not_launched", "unattributable"]) + assert.deepEqual(decideDelegationStop({...vanished, host_process}), + {phase: "unknown", terminal: true, reason: "holder_gone_without_acknowledgement"}); +}); + test("a false rejection can reopen only for exact validated settlement recovery", () => { const evidence = { from: "rejected", diff --git a/tests/control_plane_ts/host_process.test.ts b/tests/control_plane_ts/host_process.test.ts index cbea7db0d4..215af4348c 100644 --- a/tests/control_plane_ts/host_process.test.ts +++ b/tests/control_plane_ts/host_process.test.ts @@ -1,4 +1,5 @@ import assert from "node:assert/strict"; +import {existsSync} from "node:fs"; import {mkdtemp, readFile, rm} from "node:fs/promises"; import {join} from "node:path"; import {tmpdir} from "node:os"; @@ -76,3 +77,26 @@ test("output consumer failure cancels execution rather than leaving an orphan", async () => { throw new Error("consumer left"); }); assert.equal(result.outcome, "cancelled"); assert.equal(result.output_complete, false); }); + +test("the spawned Host group is reported once before input, and an unrecorded group never runs", {skip: process.platform === "win32"}, async t => { + const root = await mkdtemp(join(tmpdir(), "loopx-host-spawned-")); + t.after(() => rm(root, {recursive: true, force: true})); + const seen: unknown[] = []; + let stdout = ""; + const result = await runHostProcess(request(`process.stdout.write(String(process.pid)+' '+String(require('child_process').execSync('ps -o pgid= -p '+process.pid)).trim())`), + async item => { stdout += item.text; }, undefined, async item => { seen.push(item); }); + const [pid, pgid] = stdout.split(" ").map(Number); + assert.equal(result.outcome, "exited"); + assert.deepEqual(seen, [{kind: "spawned", pid, process_group: pid}]); assert.equal(pgid, pid); + // A caller that cannot record the owned group runs nothing unaccounted for, and + // the armed host proves it really started, so this is not an unspawned process. + const script = (marker: string) => `require('fs').writeFileSync(${JSON.stringify(marker)},'') + process.stdin.on('data',()=>{});setInterval(()=>{},1000)`; + const recorded = join(root, "recorded"); + const refused = await runHostProcess(request(script(recorded)), async () => {}, undefined, async () => { + const until = Date.now() + 2000; + while (!existsSync(recorded) && Date.now() < until) await delay(5); + assert.ok(existsSync(recorded), "the Host never started"); + throw new Error("record unavailable"); }); + assert.equal(refused.outcome, "cancelled"); assert.equal(refused.output_complete, false); +}); diff --git a/tests/test_delegation_inventory.py b/tests/test_delegation_inventory.py index b75dec65c9..9ac402e4c4 100644 --- a/tests/test_delegation_inventory.py +++ b/tests/test_delegation_inventory.py @@ -59,6 +59,8 @@ def test_corruption_and_stopped_worker_do_not_hide_healthy_sibling(service, monk # A stop receipt beside its record is not another record and reads back as stopped. assert runner.stop("healthy", execute=True)["phase"] == "settled" assert runner._stop_path(runner.path("healthy")).exists() + # Nor is the record naming the native Host an operation's Turn launched. + runner._host_process_record(runner.path("healthy")).write_text("{}") page = runner.operations() by_id = {row["operation_id"]: row for row in page["items"] if row["operation_id"]} assert len(page["items"]) == 4 and by_id["healthy"]["status"] == "stopped" diff --git a/tests/test_local_delegation.py b/tests/test_local_delegation.py index 90fa1c2ee3..871b2517a8 100644 --- a/tests/test_local_delegation.py +++ b/tests/test_local_delegation.py @@ -36,6 +36,11 @@ actor = envelope['agent_id'] counter = workspace / 'host-invocations' counter.write_text(str(int(counter.read_text()) + 1 if counter.exists() else 1)) +if (root / 'ignore-term').exists(): + import signal, subprocess + signal.signal(signal.SIGTERM, signal.SIG_IGN) + child = subprocess.Popen([sys.executable, '-c', 'import signal, time; signal.signal(signal.SIGTERM, signal.SIG_IGN); time.sleep(600)']) + (root / 'host-child-pid').write_text(str(child.pid)) if (root / 'hold').exists(): (root / 'host-pid').write_text(str(os.getpid())) (root / 'host-started').touch() @@ -288,14 +293,16 @@ def until(predicate, timeout=45): def process_gone(pid): + """Platform-valid: a zombie has exited; a process ``ps`` cannot see is gone.""" try: os.kill(pid, 0) except ProcessLookupError: return True - try: - return "State:\tZ" in Path(f"/proc/{pid}/status").read_text() - except OSError: - return True + except PermissionError: + return False + state = subprocess.run(["ps", "-o", "stat=", "-p", str(pid)], + capture_output=True, text=True).stdout.strip() + return not state or state.startswith("Z") def start_held_worker(service, operation="analysis-stop"): @@ -320,7 +327,9 @@ def test_stop_while_executing_is_acknowledged_by_the_worker_and_settles(service, assert lane["state"] == "live" and lane["holder"]["pid"] != before["worker"]["pid"] assert os.getpgid(lane["holder"]["pid"]) == before["worker"]["pgid"] receipt = runner.stop("analysis-stop", execute=True) + host_gone = process_gone(host_pid) # the instant settled returns, not after a wait assert receipt["phase"] == "settled" and receipt["status"] == "stopped", receipt + assert host_gone stop = receipt["stop"] assert stop["requested_by"] == "lead" and stop["requested_status"] == "running" assert stop["worker"]["pid"] == before["worker"]["pid"] @@ -328,12 +337,12 @@ def test_stop_while_executing_is_acknowledged_by_the_worker_and_settles(service, assert stop["ack"]["observed_status"] == "running" and stop["ack"]["turn_key"] assert stop["settled"]["operation_lock_free"] and stop["settled"]["worker_lane_released"] assert stop["settled"]["lane_state"] in {"dead", "released"} + assert stop["settled"]["host_process"] == "drained" assert stop["settled"]["turn_journal_status"] == "in_progress" assert stop["lease"] == {"required": False, "released": None} # The acknowledged record is final: nobody writes it again, the Todo stays open, - # the host process group is gone and the member's Turn lane can be taken. + # the worker exits and the member's Turn lane can be taken. frozen = path.read_bytes() - assert until(lambda: process_gone(host_pid), timeout=20) assert until(lambda: process_gone(before["worker"]["pid"]), timeout=20) with try_exclusive_file_lock(runner._lane_target(binding)) as held: assert held is not None @@ -364,6 +373,7 @@ def test_stop_without_a_holder_is_acknowledged_by_the_requester(service, monkeyp assert receipt["stop"]["ack"]["pid"] == os.getpid() and receipt["stop"]["ack"]["source"] == "requester" assert receipt["stop"]["settled"]["turn_journal_status"] is None assert receipt["stop"]["settled"]["lane_state"] in {"absent", "released"} + assert receipt["stop"]["settled"]["host_process"] == "not_launched" frozen = runner.path("analysis-idle").read_bytes() with pytest.raises(ValueError, match="start a new operation id"): runner.resume("analysis-idle") @@ -534,3 +544,80 @@ def test_fenced_write_after_another_process_stop_writes_nothing(service, monkeyp acknowledged = _read(runner._stop_path(entry)) assert acknowledged["phase"] == "acknowledged" and acknowledged["ack"]["source"] == "worker_entry" assert runner.stop("analysis-entry", execute=True)["phase"] == "settled" + + +def test_stop_settles_only_after_the_owned_host_and_its_descendants_exit(service): + """A host and its same-group child that ignore SIGTERM keep the stop open until they exit. + + The postcondition is checked at the instant ``settled`` returns, not after a wait. + """ + root, runner = service + (root / "ignore-term").touch() + host_pid = start_held_worker(service) + assert until(lambda: (root / "host-child-pid").exists()) + child_pid = int((root / "host-child-pid").read_text()) + assert not process_gone(host_pid) and not process_gone(child_pid) + receipt = runner.stop("analysis-stop", execute=True) + host_gone, child_gone = process_gone(host_pid), process_gone(child_pid) + assert receipt["phase"] == "settled", receipt + assert host_gone and child_gone, (receipt, host_gone, child_gone) + settled = receipt["stop"]["settled"] + assert settled["host_process"] == "drained", settled + # Rereading a settled stop neither reopens it nor admits or completes anything. + frozen = runner.path("analysis-stop").read_bytes() + assert runner.stop("analysis-stop", execute=True) == receipt + assert runner.read("analysis-stop")["stop"]["phase"] == "settled" + assert runner.path("analysis-stop").read_bytes() == frozen + assert int((root / "analyst" / "initial" / "host-invocations").read_text()) == 1 + assert not demo.canonical_tasks(root)["todo_analyst-initial"]["done"] + assert returns(runner.root, runner.goal_id, "lead")["items"] == [] + + +def test_interrupted_host_cleanup_keeps_the_stop_open_until_a_reread_sees_it_drained(service, monkeypatch): + """A Host supervisor that never finishes cleaning up leaves no settlement to claim. + + Rereads with the same identity stay acknowledged without admitting or completing + anything, and settle once the Host's group is observed gone. + """ + import signal + from loopx import collaboration_mcp + + root, runner = service + monkeypatch.setattr(collaboration_mcp, "DELEGATION_STOP_GRACE_SECONDS", 2.0) + host_pid = start_held_worker(service) + path = runner.path("analysis-stop") + record = json.loads(runner._host_process_record(path).read_text()) + assert record["phase"] == "spawned" and record["host_pid"] == host_pid == record["process_group"] + bridge = record["bridge_pid"] + os.kill(bridge, signal.SIGSTOP) # the supervisor cannot run its cleanup + try: + first = runner.stop("analysis-stop", execute=True) + assert first["phase"] == "acknowledged" and first["status"] == "stopped", first + assert first["reason"] == "host_process_still_running" and not process_gone(host_pid) + os.kill(bridge, signal.SIGKILL) # and now never will: the Host is orphaned + assert until(lambda: process_gone(bridge), timeout=20) + frozen = path.read_bytes() + for _ in range(3): + again = runner.stop("analysis-stop", execute=True) + assert again["phase"] == "acknowledged" and again["reason"] == "host_process_still_running", again + assert again["stop"]["stop_id"] == first["stop"]["stop_id"] + assert again["stop"]["ack"] == first["stop"]["ack"] and again["stop"]["settled"] is None + assert runner.read("analysis-stop")["stop"]["phase"] == "acknowledged" + assert not process_gone(host_pid) and path.read_bytes() == frozen + with pytest.raises(ValueError, match="start a new operation id"): + runner.resume("analysis-stop") + finally: + for target, sig in ((bridge, signal.SIGCONT), (host_pid, signal.SIGKILL)): + try: + os.killpg(target, sig) if target == host_pid else os.kill(target, sig) + except ProcessLookupError: + pass + assert until(lambda: process_gone(host_pid), timeout=20) + receipt = runner.stop("analysis-stop", execute=True) + assert receipt["phase"] == "settled" and receipt["stop"]["settled"]["host_process"] == "drained", receipt + assert receipt["stop"]["stop_id"] == first["stop"]["stop_id"] + assert runner.stop("analysis-stop", execute=True) == receipt + assert path.read_bytes() == frozen + assert int((root / "analyst" / "initial" / "host-invocations").read_text()) == 1 + assert not demo.canonical_tasks(root)["todo_analyst-initial"]["done"] + assert returns(runner.root, runner.goal_id, "lead")["items"] == [] From 226a664d114bb77122c34a32773cb43530fd9480 Mon Sep 17 00:00:00 2001 From: song Date: Wed, 30 Sep 2026 13:22:28 +0800 Subject: [PATCH 11/13] docs(delegation): state what a stop receipt proves about the native Host Signed-off-by: song --- docs/reference/goal-chat-continuation.md | 5 ++-- docs/reference/local-delegation.md | 30 +++++++++++++++--------- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/docs/reference/goal-chat-continuation.md b/docs/reference/goal-chat-continuation.md index 7c367d68d3..7f9ab29ce1 100644 --- a/docs/reference/goal-chat-continuation.md +++ b/docs/reference/goal-chat-continuation.md @@ -102,7 +102,8 @@ messages; images use the ordinary conversation after pausing. Disabling mode or deleting a binding does not cancel already admitted children; stop one with `loopx delegation stop --execute` (or `stop_delegation`), read its receipt, and retain its evidence. Only `settled` proves the worker - acknowledged and released its locks; stopped work needs a new operation id. + acknowledged and released its locks and its native host exited; stopped work + needs a new operation id. The ordinary native command path also remains available without delegation: @@ -147,6 +148,6 @@ For a disposable mixed-team setup, use the 或整个 Goal。额度是含历史用量的总量,正在执行的请求可能超额,成员另行计量。 回滚旧版本前先暂停或关闭 Chat 服务;退出或撤销绑定不自动取消已启动的成员, 用 `loopx delegation stop --execute`(或 `stop_delegation`)停止单个成员并阅读回执: -只有 `settled` 证明 worker 已确认并释放锁;已停止的工作需要新的 operation id。 +只有 `settled` 证明 worker 已确认并释放锁且原生 host 已退出;已停止的工作需要新的 operation id。 此按钮目前限本机 managed Codex Goal 对话,不宣称 Lark、挂接会话或其他主力 驱动等价。可用下方示例准备一次隔离的本地 DSH+云端 Ark 协作。 diff --git a/docs/reference/local-delegation.md b/docs/reference/local-delegation.md index b5e3e39fa2..8f61a1dac4 100644 --- a/docs/reference/local-delegation.md +++ b/docs/reference/local-delegation.md @@ -226,14 +226,19 @@ than maintaining separate rules. states what was proven. The request is written beside the execution record (`.stop.json`), never into it, so a worker that is still holding the operation cannot overwrite it. A worker on this machine receives `SIGTERM` for -its whole process group, which ends its Turn child and its host; it -acknowledges from under its own lock, marks the record `stopped` and releases -its hard task lease. When nobody holds the operation, the requester +its whole process group, which ends its Turn child; the native host runs in +its own process group, and its supervisor terminates that group once the Turn +child is gone. The worker acknowledges from under its own lock, marks the +record `stopped` and releases its hard task lease. When nobody holds the operation, the requester acknowledges itself. A worker on another machine is never signalled; it finds the request at its next checkpoint or at its next record write, which is -refused. The receipt `phase` is `settled` only when an acknowledgement exists -and both the operation lock and the member's Turn lane lock are free; -`unknown` means the holder vanished before acknowledging, and `noop` means the +refused. The receipt `phase` is `settled` only when an acknowledgement exists, +the operation lock is free, the member's Turn lane holder record shows it +released by the stopped worker (the lane is read, never taken), and the native +host the Turn launched has exited together with every process in its group. +If that host cannot be attributed, or its supervisor never finished cleaning +up, the stop stays `acknowledged` and a later `stop` rereads it. `unknown` +means the holder vanished before acknowledging, and `noop` means the work was already accepted, rejected or stopped. `requested` or `acknowledged` means it is still winding down: call `stop` again. A grace timeout never turns into a receipt. Stopped work is not resumed; `resume` refuses it and a new @@ -244,11 +249,14 @@ member's Todo stays open, so the coordinator decides what happens next. 中文:`stop --execute` 结束一个成员的有界工作,并返回一份只陈述已证明事实的 回执。停止请求写在执行记录旁边的 `.stop.json`,从不写进记录本身, 因此仍持有该 operation 的 worker 无法覆盖它。本机 worker 会收到整个进程组的 -`SIGTERM`,其 Turn 子进程和 host 一并结束;worker 在自己的锁下确认,把记录标为 +`SIGTERM`,其 Turn 子进程随之结束;原生 host 在自己的进程组中运行,Turn 子进程 +退出后由其 supervisor 终止整个 host 进程组。worker 在自己的锁下确认,把记录标为 `stopped` 并释放硬任务租约。没有持有者时由请求方自行确认。另一台机器上的 worker 不会被发信号,它在下一个检查点或下一次写记录时发现请求,写入被拒绝。 -只有存在确认且 operation 锁与成员 Turn lane 锁都已释放时,`phase` 才是 -`settled`;`unknown` 表示持有者在确认前消失;`noop` 表示工作已 accepted、 +只有存在确认、operation 锁已释放、成员 Turn lane 的持有者记录显示已被停止的 +worker 释放(只读 lane,从不获取),且该 Turn 启动的原生 host 及其进程组内所有进程 +都已退出时,`phase` 才是 `settled`。host 无法归属或其 supervisor 未完成清理时, +停止保持 `acknowledged`,之后再次调用 `stop` 会重新读取。`unknown` 表示持有者在确认前消失;`noop` 表示工作已 accepted、 rejected 或 stopped;`requested`/`acknowledged` 表示仍在收尾,再次调用 `stop`。 宽限期超时永远不会变成回执。已停止的工作不能 `resume`,新范围需要新的 operation id。Turn journal 保留 `in_progress` 条目供检查,记录不会被改写成完成; @@ -651,7 +659,7 @@ unchanged and cannot launch workers. With it, the Agent can: This cannot retarget the work or silently create a replacement Turn. 5. Call `stop_delegation(operation_id)` to end one member. Read its `phase`: `settled` is the only receipt that the worker acknowledged and released its - locks; `unknown` means the holder vanished first; `noop` means the work had + locks and that the native host and its process group exited; `unknown` means the holder vanished first; `noop` means the work had already ended. Stopped work cannot be resumed; use a new operation id. Configure the member's host to expose its own identity-bound collaboration @@ -693,7 +701,7 @@ concurrent executions still use the same kernel lock and original Turn journal. | Requesting MCP conversation closes | The detached bounded worker continues; another connection reads the original operation. | | Duplicate start/resume while work runs | Operation identity, task lock and Turn journal prevent another concurrent execution. | | Worker process or machine stops | Reconnect with the same operator configuration and credentials, then resume the original Turn. | -| Member stopped on request | The worker acknowledges under its lock, its host and Turn child are ended, its lease is released; `settled` needs that acknowledgement plus free locks, `unknown` means the holder vanished first. The record is `stopped`; resume refuses it. | +| Member stopped on request | The worker acknowledges under its lock, its Turn child is ended and the host supervisor terminates the host group, its lease is released; `settled` needs that acknowledgement, free locks and an exited host group, `unknown` means the holder vanished first. The record is `stopped`; resume refuses it. | | Ark is computing without local tools | The already-started cloud turn can continue. It is not dependent on the local conversation. | | Ark requests a local tool while the host is absent | It waits for the local tool result. Recovery observes the original input/session and executes only previously unstarted tool calls. | | Tool execution or send acknowledgement is uncertain | Do not repeat the effect. Preserve the receipt/session for explicit reconciliation. | From 01be96c65c7c1c226ab62f99f1536e9e3979072a Mon Sep 17 00:00:00 2001 From: song Date: Wed, 30 Sep 2026 10:44:40 -0400 Subject: [PATCH 12/13] fix(delegation): give a stop a boundary against its effects, its lease and the platform MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three blockers, all reproduced on the reviewed head before fixing. **A stop could settle after the member's effects committed.** The worker ran one pre-check and then committed Todo completion and reply publication outside any lock the stop takes, so a stop written first still reported `settled` for work that had landed. Both effects now commit inside the dispatch lock a stop also takes, so the two sides linearize: a stop written first means neither effect runs, and a stop written after leaves their acceptance intact. `_observe` gained a locked entry point because the kernel file lock is not reentrant and the widened critical section must not nest. **A failed required lease release was terminal and never retried.** `_settle_stop` did not pass the release to the typed decision, so an authority failure produced `settled` with `lease_released=false` on both backends and every later read returned the same receipt — leaving the member's Todo blocked until the lease TTL with resume already refused. `decideDelegationStop` now treats a required release as part of what `settled` promises, and the settle read retries the release under the stop's own lock. An operation that held no required lease omits the fact rather than claiming a release. **A launched Host on Windows could never settle.** `host_process_drain` returned `unattributable` whenever `killpg` was absent, so a finished Host left the stop pending forever with no converging or actionable path. That case is now `unsupported_platform`, distinct from an attribution failure, and `stop --execute` fails fast with an error naming the platform boundary instead of returning a receipt no read can settle. The public reference documents the lease obligation, the effect ordering and that boundary in both languages. Coverage: a stop written before the effects (both backends) asserts neither effect commits and the receipt still settles; a failed release asserts `acknowledged`/`required_lease_release_unproven` and that the next read retries and settles; a Host on a platform without process groups asserts the actionable failure. The TS decision pins the lease obligation on both the acknowledged and vanished-holder paths, mutation-checked by dropping it. Signed-off-by: song --- docs/reference/local-delegation.md | 36 +++-- loopx/collaboration_mcp.py | 109 ++++++++++----- .../control_plane/collaboration/delegation.ts | 16 ++- .../turn_driver/host_process_transport.py | 19 ++- tests/control_plane_ts/delegation.test.ts | 18 +++ tests/test_local_delegation.py | 129 +++++++++++++++++- 6 files changed, 281 insertions(+), 46 deletions(-) diff --git a/docs/reference/local-delegation.md b/docs/reference/local-delegation.md index 8f61a1dac4..a32ff944e2 100644 --- a/docs/reference/local-delegation.md +++ b/docs/reference/local-delegation.md @@ -234,17 +234,28 @@ acknowledges itself. A worker on another machine is never signalled; it finds the request at its next checkpoint or at its next record write, which is refused. The receipt `phase` is `settled` only when an acknowledgement exists, the operation lock is free, the member's Turn lane holder record shows it -released by the stopped worker (the lane is read, never taken), and the native -host the Turn launched has exited together with every process in its group. -If that host cannot be attributed, or its supervisor never finished cleaning -up, the stop stays `acknowledged` and a later `stop` rereads it. `unknown` +released by the stopped worker (the lane is read, never taken), the native +host the Turn launched has exited together with every process in its group, +and a required hard task lease was actually released. A release that failed is +retried under the stop's own lock on the next read, so it never becomes a +`settled` receipt that leaves the member's Todo blocked until the lease TTL; +while it is unproven the stop stays `acknowledged` with +`required_lease_release_unproven`. If that host cannot be attributed, or its +supervisor never finished cleaning up, the stop stays `acknowledged` and a +later `stop` rereads it. On a platform without process groups the launched +host cannot be proven drained at all, so `stop --execute` fails with an +actionable error naming that boundary rather than leaving a receipt no read +can settle. `unknown` means the holder vanished before acknowledging, and `noop` means the work was already accepted, rejected or stopped. `requested` or `acknowledged` means it is still winding down: call `stop` again. A grace timeout never turns into a receipt. Stopped work is not resumed; `resume` refuses it and a new scope needs a new operation id. The Turn journal keeps its `in_progress` entry for inspection, and the record is never rewritten as a completion. A stopped -member's Todo stays open, so the coordinator decides what happens next. +member's Todo stays open, so the coordinator decides what happens next. The +member's Todo completion and reply publication commit under the same lock a +stop takes, so a stop written first means neither effect lands, and a stop +written after both leaves their acceptance intact. 中文:`stop --execute` 结束一个成员的有界工作,并返回一份只陈述已证明事实的 回执。停止请求写在执行记录旁边的 `.stop.json`,从不写进记录本身, @@ -254,13 +265,20 @@ member's Todo stays open, so the coordinator decides what happens next. `stopped` 并释放硬任务租约。没有持有者时由请求方自行确认。另一台机器上的 worker 不会被发信号,它在下一个检查点或下一次写记录时发现请求,写入被拒绝。 只有存在确认、operation 锁已释放、成员 Turn lane 的持有者记录显示已被停止的 -worker 释放(只读 lane,从不获取),且该 Turn 启动的原生 host 及其进程组内所有进程 -都已退出时,`phase` 才是 `settled`。host 无法归属或其 supervisor 未完成清理时, -停止保持 `acknowledged`,之后再次调用 `stop` 会重新读取。`unknown` 表示持有者在确认前消失;`noop` 表示工作已 accepted、 +worker 释放(只读 lane,从不获取)、该 Turn 启动的原生 host 及其进程组内所有进程 +都已退出,且必需的硬任务租约确实释放成功时,`phase` 才是 `settled`。释放失败会在 +下一次读取时于 stop 自己的锁下重试,因此不会产生一份「已结算」却让成员 Todo 被 +租约阻塞到 TTL 的回执;在释放得到证明前,停止保持 `acknowledged`,原因为 +`required_lease_release_unproven`。host 无法归属或其 supervisor 未完成清理时, +停止保持 `acknowledged`,之后再次调用 `stop` 会重新读取。在没有进程组的平台上, +启动过的 host 根本无法被证明已收尾,因此 `stop --execute` 会以指明该平台边界的 +可操作错误失败,而不是留下一份任何读取都无法结算的回执。`unknown` 表示持有者在确认前消失;`noop` 表示工作已 accepted、 rejected 或 stopped;`requested`/`acknowledged` 表示仍在收尾,再次调用 `stop`。 宽限期超时永远不会变成回执。已停止的工作不能 `resume`,新范围需要新的 operation id。Turn journal 保留 `in_progress` 条目供检查,记录不会被改写成完成; -成员的 Todo 仍然打开,由协调者决定下一步。 +成员的 Todo 仍然打开,由协调者决定下一步。成员的 Todo 完成与回执发布在 stop +所取的同一把锁下提交,因此先写入停止则两个效果都不会落地,后写入停止则其验收结果 +保持不变。 This entrypoint does not create Agents, grant bindings or wake an idle Codex conversation. The existing host/LoopX continuation policy owns the next lead diff --git a/loopx/collaboration_mcp.py b/loopx/collaboration_mcp.py index 8158261e58..82d1906a70 100644 --- a/loopx/collaboration_mcp.py +++ b/loopx/collaboration_mcp.py @@ -43,7 +43,8 @@ ) from .control_plane.turn_driver.host_binding import turn_host_arg_option from .control_plane.turn_driver.host_process_transport import ( - HOST_PROCESS_DRAINING, HOST_PROCESS_RECORD_ENV, host_process_drain, + HOST_PROCESS_DRAINING, HOST_PROCESS_RECORD_ENV, HOST_PROCESS_UNSUPPORTED_PLATFORM, + host_process_drain, ) from .control_plane.turn_driver.lane_fence import ( TURN_LANE_ABSENT, TURN_LANE_DEAD, TURN_LANE_LIVE, TURN_LANE_RELEASED, @@ -776,12 +777,15 @@ def _read_current(self, operation_id: str) -> dict: result["error"] = row["error"] return result - def _observe(self, path: Path, row: dict, status: str, **facts) -> None: + def _observe(self, path: Path, row: dict, status: str, *, already_locked: bool = False, **facts) -> None: decision = effect_runtime_result("collaboration.delegation.observe", { "from": row["status"], "to": status, **facts, }) row.update(status=decision["status"]) - self._fenced_write(path, row) + if already_locked: + self._fenced_write_locked(path, row) + else: + self._fenced_write(path, row) def _fenced_write(self, path: Path, row: dict) -> None: """Write the execution record only while no unacknowledged stop fences this process. @@ -791,10 +795,20 @@ def _fenced_write(self, path: Path, row: dict) -> None: """ with exclusive_file_lock(self._dispatch_lock(path)): - stop = self._read_stop(path) - if stop is not None and not self._acknowledged_here(stop): - raise DelegationFenced() - _write(path, row) + self._fenced_write_locked(path, row) + + def _fenced_write_locked(self, path: Path, row: dict) -> None: + """The same write for a caller that already holds the dispatch lock. + + The lock is a kernel file lock, so it is not reentrant: a caller that + widened its critical section to cover a whole effect group must use this + entry point rather than nesting ``_fenced_write``. + """ + + stop = self._read_stop(path) + if stop is not None and not self._acknowledged_here(stop): + raise DelegationFenced() + _write(path, row) @staticmethod def _acknowledged_here(stop: dict) -> bool: @@ -1094,10 +1108,35 @@ def _settle_stop(self, path: Path) -> dict: return self._stop_receipt(row, binding, None) if stop["phase"] not in DELEGATION_STOP_OPEN_PHASES: return self._stop_receipt(row, binding, stop) + # A launched Host on a platform that cannot prove its group exited + # has no converging stop: say so plainly rather than leaving the + # caller with an acknowledged receipt it can never settle. + unsupported = host_process_drain(self._host_process_record(path)) == HOST_PROCESS_UNSUPPORTED_PLATFORM + if unsupported: + raise ValueError( + "delegation stop cannot prove the launched Host drained on this platform: " + "process groups are unavailable, so the Host supervisor is best-effort. " + "Stop the member's Host through its own supervisor and re-read the receipt." + ) facts = {"operation_lock_free": self._operation_lock_free(path)} facts["worker_lane_released"], lane_state = self._worker_lane_released(row, stop, binding) # Read last: a Host seen drained after its worker and lane let go stays drained. facts["host_process"] = host_process_drain(self._host_process_record(path)) + # A required lease the acknowledgement could not release is retried + # here, under the same lock that guards the record: every later + # read of this stop is another attempt, instead of one failure + # turning into a permanent `settled` with the lease still held. + lease = stop.get("lease") if isinstance(stop.get("lease"), dict) else {} + if lease.get("required") is True and lease.get("released") is not True: + retried = self._release_delegation_lease(row, binding) + if retried != lease: + stop["lease"] = retried + _write(self._stop_path(path), stop) + lease = retried + if lease.get("required") is True: + # Absent means the operation held no required lease, which is not + # the same claim as a lease that was released. + facts["lease_released"] = lease.get("released") is True decision = effect_runtime_result("collaboration.delegation.stop", { "phase": stop["phase"], "acknowledged": stop.get("ack") is not None, "timed_out": time.time() - stop["requested_at"] > DELEGATION_STOP_GRACE_SECONDS, @@ -1106,10 +1145,8 @@ def _settle_stop(self, path: Path) -> dict: if decision["phase"] != stop["phase"] or decision.get("reason") != stop.get("reason"): stop.update(phase=decision["phase"], reason=decision.get("reason")) if decision["phase"] in DELEGATION_STOP_TERMINAL_PHASES: - lease = stop.get("lease") if isinstance(stop.get("lease"), dict) else {} stop["settled"] = { "at": time.time(), **facts, "lane_state": lane_state, - "lease_released": lease.get("released"), "turn_journal_status": self._turn_journal_status(row, binding), } _write(self._stop_path(path), stop) @@ -1553,30 +1590,38 @@ def _execute(self, path: Path, row: dict, binding: dict) -> None: return self._bound(row, require_active=True) # revocation or rebinding while the model ran delegation_results.require_dependencies(self, binding, delegation_results.operation_brief(self, row)) - if not todo_completed_for_settlement: + # Both effects commit inside the dispatch lock that a stop also + # takes, so the two sides linearize: either a stop is written first + # and neither effect runs, or both effects commit first and the stop + # that follows reports a record that already reached its terminal + # observation. Committing them outside the lock let a stop settle + # for a member whose Todo and reply had already landed. + with exclusive_file_lock(self._dispatch_lock(path)): self._raise_if_stop_requested(path) - self._complete_delegated_todo(row, binding) - row["artifacts"] = self._accepted(binding) - if not (_root(self.root) / "replies" / request_id / "conclusion.json").exists(): - return_result( - self.root, - self.goal_id, - binding["agent_id"], - request_id, - json.dumps( - { - "todo_id": binding["todo_id"], - "status": "accepted", - "artifacts": [ - {k: v for k, v in item.items() if k != "text"} - for item in row["artifacts"] - ], - } - ), - registry=self.registry, - caller_goal_ref=self._caller_goal_ref(), - ) - self._observe(path, row, "accepted", canonical_done=True, acceptance_ready=True, artifacts_current=True) + if not todo_completed_for_settlement: + self._complete_delegated_todo(row, binding) + row["artifacts"] = self._accepted(binding) + if not (_root(self.root) / "replies" / request_id / "conclusion.json").exists(): + return_result( + self.root, + self.goal_id, + binding["agent_id"], + request_id, + json.dumps( + { + "todo_id": binding["todo_id"], + "status": "accepted", + "artifacts": [ + {k: v for k, v in item.items() if k != "text"} + for item in row["artifacts"] + ], + } + ), + registry=self.registry, + caller_goal_ref=self._caller_goal_ref(), + ) + self._observe(path, row, "accepted", already_locked=True, + canonical_done=True, acceptance_ready=True, artifacts_current=True) except (ValueError, KeyError, subprocess.TimeoutExpired, EffectRuntimeRemoteError) as exc: # Retain uncertain execution for explicit same-operation recovery. # No fresh Turn is ever created because its client timed out. diff --git a/loopx/control_plane/collaboration/delegation.ts b/loopx/control_plane/collaboration/delegation.ts index bfed6e3ba7..1346045266 100644 --- a/loopx/control_plane/collaboration/delegation.ts +++ b/loopx/control_plane/collaboration/delegation.ts @@ -372,6 +372,13 @@ export function decideDelegationStop(params: JsonObject): JsonObject { "delegation stop release facts required"); requireThat(hostProcessDrains.includes(params.host_process as HostProcessDrain), "delegation stop host process drain fact required"); + // A required hard lease is released by the stop itself. Its release is part + // of what the receipt promises: a member whose lease is still held can block + // its Todo until the lease TTL, which is not a safe stop and is not something + // the owner can act on. `undefined` means the operation held no required + // lease. + requireThat(params.lease_released === undefined || typeof params.lease_released === "boolean", + "delegation stop lease release fact must be boolean"); requireThat(params.timed_out === undefined || typeof params.timed_out === "boolean", "delegation stop timeout fact must be boolean"); requireThat(phase !== "acknowledged" || params.acknowledged === true, @@ -380,16 +387,19 @@ export function decideDelegationStop(params: JsonObject): JsonObject { const workerReleased = operationFree && params.worker_lane_released === true; const host = params.host_process as HostProcessDrain; const hostDrained = host === "drained" || host === "not_launched"; + const leaseReleased = params.lease_released !== false; const pending = !operationFree ? "operation_lock_still_held" : params.worker_lane_released !== true ? "worker_lane_release_unproven" - : host === "draining" ? "host_process_still_running" : "host_process_drain_unproven"; + : host === "draining" ? "host_process_still_running" + : host !== "drained" && host !== "not_launched" ? "host_process_drain_unproven" + : "required_lease_release_unproven"; if (params.acknowledged === true) { - if (workerReleased && hostDrained) { + if (workerReleased && hostDrained && leaseReleased) { return {phase: "settled", terminal: true, reason: "acknowledged_worker_and_host_released"}; } return {phase: "acknowledged", terminal: false, reason: pending}; } - if (workerReleased && host !== "draining") { + if (workerReleased && host !== "draining" && leaseReleased) { return {phase: "unknown", terminal: true, reason: "holder_gone_without_acknowledgement"}; } return {phase: "requested", terminal: false, reason: operationFree ? pending diff --git a/loopx/control_plane/turn_driver/host_process_transport.py b/loopx/control_plane/turn_driver/host_process_transport.py index 5a6e250534..95405308c4 100644 --- a/loopx/control_plane/turn_driver/host_process_transport.py +++ b/loopx/control_plane/turn_driver/host_process_transport.py @@ -29,6 +29,19 @@ HOST_PROCESS_DRAINED = "drained" HOST_PROCESS_DRAINING = "draining" HOST_PROCESS_UNATTRIBUTABLE = "unattributable" +HOST_PROCESS_UNSUPPORTED_PLATFORM = "unsupported_platform" + + +def host_process_drain_supported() -> bool: + """Whether this platform can prove an owned Host group has exited. + + The TS supervisor cleans up a process group on POSIX and a process tree + best-effort on Windows. Only the group gives the stop a fact it can prove, + so the caller must say so rather than reporting a settlement it cannot + support. + """ + + return hasattr(os, "killpg") def _write_host_process_record(path: Path, record: dict[str, Any]) -> None: @@ -69,8 +82,12 @@ def host_process_drain(record_path: Path) -> str: except (OSError, ValueError): return HOST_PROCESS_UNATTRIBUTABLE if (not isinstance(record, dict) or record.get("schema_version") != HOST_PROCESS_RECORD_SCHEMA_VERSION - or record.get("host") != lock_holder_host_label() or not hasattr(os, "killpg")): + or record.get("host") != lock_holder_host_label()): return HOST_PROCESS_UNATTRIBUTABLE + if not hasattr(os, "killpg"): + # A launched Host on a platform without process groups is never proven + # drained. This is a platform boundary, not an attribution failure. + return HOST_PROCESS_UNSUPPORTED_PLATFORM bridge, group = record.get("bridge_pid"), record.get("process_group") if record.get("phase") != "finished": if not isinstance(bridge, int) or bridge <= 1: diff --git a/tests/control_plane_ts/delegation.test.ts b/tests/control_plane_ts/delegation.test.ts index f97a99c957..9533586679 100644 --- a/tests/control_plane_ts/delegation.test.ts +++ b/tests/control_plane_ts/delegation.test.ts @@ -158,6 +158,24 @@ test("a released worker and lane never settle a stop while the native Host still for (const host_process of ["drained", "not_launched"]) assert.deepEqual(decideDelegationStop({...released, host_process}), {phase: "settled", terminal: true, reason: "acknowledged_worker_and_host_released"}); + // A required lease the stop could not release is not a settlement: the + // member's Todo can stay blocked by it until the lease TTL. + for (const host_process of ["drained", "not_launched"]) { + assert.deepEqual(decideDelegationStop({...released, host_process, lease_released: false}), + {phase: "acknowledged", terminal: false, reason: "required_lease_release_unproven"}); + // An operation that held no required lease omits the fact, which settles. + assert.deepEqual(decideDelegationStop({...released, host_process}), + {phase: "settled", terminal: true, reason: "acknowledged_worker_and_host_released"}); + } + assert.throws(() => decideDelegationStop({...released, host_process: "drained", lease_released: "yes"}), + /lease release fact/); + // A vanished holder whose required lease is still held is not terminal either. + const unacked = {phase: "requested", acknowledged: false, operation_lock_free: true, + worker_lane_released: true, host_process: "drained"}; + assert.deepEqual(decideDelegationStop({...unacked, lease_released: false}), + {phase: "requested", terminal: false, reason: "required_lease_release_unproven"}); + assert.deepEqual(decideDelegationStop(unacked), + {phase: "unknown", terminal: true, reason: "holder_gone_without_acknowledgement"}); // A held lock still dominates a drained Host. assert.deepEqual(decideDelegationStop({...released, operation_lock_free: false, host_process: "drained"}), {phase: "acknowledged", terminal: false, reason: "operation_lock_still_held"}); diff --git a/tests/test_local_delegation.py b/tests/test_local_delegation.py index 871b2517a8..1448e13b56 100644 --- a/tests/test_local_delegation.py +++ b/tests/test_local_delegation.py @@ -17,11 +17,12 @@ sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "examples" / "managed-research-team")) import research_team as demo # noqa: E402 from test_managed_research_scenario import fixture # noqa: E402 -from loopx.collaboration_mcp import DelegationFenced, Delegations # noqa: E402 +from loopx.collaboration_mcp import DelegationFenced, DelegationStopRequested, Delegations # noqa: E402 from loopx.control_plane.collaboration.peers import returns # noqa: E402 from loopx.control_plane.collaboration.inbox import _read # noqa: E402 from loopx.control_plane.turn_driver.lane_fence import turn_lane_liveness, turn_lane_singleflight # noqa: E402 from loopx.file_lock import exclusive_file_lock, try_exclusive_file_lock # noqa: E402 +from loopx.file_lock import lock_holder_host_label # noqa: E402 HOST = '''import json, os, sys, time @@ -621,3 +622,129 @@ def test_interrupted_host_cleanup_keeps_the_stop_open_until_a_reread_sees_it_dra assert int((root / "analyst" / "initial" / "host-invocations").read_text()) == 1 assert not demo.canonical_tasks(root)["todo_analyst-initial"]["done"] assert returns(runner.root, runner.goal_id, "lead")["items"] == [] + + +def test_a_stop_written_before_the_effects_linearizes_against_them(service, monkeypatch): + """Todo completion and reply publication commit under the stop's own lock. + + The pre-check alone was not a boundary: the worker could pass it, a stop + could be persisted, and both external effects still committed before the + fenced record write, leaving `settled`/`stopped` for a member whose work had + landed. Holding the dispatch lock across both effects makes the two sides + linearize in either order. + """ + from loopx.control_plane.collaboration.inbox import _write as write_inbox + + root, runner = service + runner.start("analysis", "analysis-race", brief()) + path = runner.path("analysis-race") + row = _read(path) + # Reproduce the interleaving: the stop exists before the worker reaches its + # effects. A same-process request is what the worker then acknowledges. + write_inbox(runner._stop_path(path), runner._new_stop_record( + row, requested_by=runner.agent_id, worker=None)) + + # The checkpoint after the model returns refuses to run the effects. + with pytest.raises(DelegationStopRequested): + with exclusive_file_lock(runner._dispatch_lock(path)): + runner._raise_if_stop_requested(path) + + runner.execute("analysis-race") + assert _read(path)["status"] == "stopped" + # Neither effect committed for a stop that was written first. + assert not demo.canonical_tasks(root)["todo_analyst-initial"]["done"] + assert not (root / "runtime" / "replies" / "analysis-race" / "conclusion.json").exists() + assert not (root / "host-started").exists() + receipt = runner.stop("analysis-race", execute=True) + assert receipt["phase"] == "settled" and receipt["status"] == "stopped" + + +def test_a_failed_required_lease_release_keeps_the_stop_open_and_retries(service, monkeypatch): + """A required lease the stop could not release is not a settlement. + + The member's Todo can stay blocked by that lease until its TTL, so reporting + `settled` would be a terminal claim the owner cannot act on. The release is + retried under the stop's own lock on the next read instead of being attempted + once and forgotten. + """ + from loopx import collaboration_mcp as delegation + from loopx.control_plane.collaboration.inbox import _write as write_inbox + + root, runner = service + monkeypatch.setattr(runner, "_spawn", lambda _: None) + runner.start("analysis", "analysis-lease", brief()) + path = runner.path("analysis-lease") + row = _read(path) + # A bounded member task holds a required lease; make the record say so. + row["task_lease"] = {"required": True, "idempotency_key": "lease-1", "version": 1} + row["status"] = "stopped" + runner._fenced_write(path, row) + write_inbox(runner._stop_path(path), { + **runner._new_stop_record(row, requested_by=runner.agent_id, worker=None), + "phase": "acknowledged", + "ack": {"pid": os.getpid(), "host": lock_holder_host_label(), "at": time.time(), + "source": "requester", "observed_status": "stopped", "turn_key": None}, + # The acknowledgement could not release it, which is what the receipt records. + "lease": {"required": True, "released": False, "error": "authority unavailable"}, + }) + + attempts = [] + outcomes = [RuntimeError("authority temporarily unavailable"), {"released": True}] + + def flaky_release(**kwargs): + attempts.append(kwargs) + outcome = outcomes[min(len(attempts) - 1, len(outcomes) - 1)] + if isinstance(outcome, Exception): + raise outcome + return outcome + + monkeypatch.setattr(delegation, "release_task_lease", flaky_release) + + # The first settle read tries the release, fails, and must not settle. + receipt = runner.stop("analysis-lease", execute=True) + assert attempts, "the stop never attempted the required release" + assert receipt["phase"] == "acknowledged", receipt + assert receipt["stop"]["reason"] == "required_lease_release_unproven" + assert receipt["stop"]["lease"]["released"] is not True + + # The next read retries the release and only then settles. + settled = runner.stop("analysis-lease", execute=True) + assert len(attempts) >= 2, "the failed release was never retried" + assert settled["phase"] == "settled", settled + assert settled["stop"]["lease"]["released"] is True + assert settled["stop"]["settled"]["lease_released"] is True + + # Once settled the receipt is stable, and a released lease is not re-attempted. + before = len(attempts) + assert runner.stop("analysis-lease", execute=True) == settled + assert len(attempts) == before + + +def test_a_launched_host_on_a_platform_without_process_groups_fails_fast(service, monkeypatch): + """A stop that cannot prove its Host drained says so instead of never settling. + + Windows cleanup is process-tree best effort, so no fact proves the Host's + descendants exited. An acknowledged receipt the caller can never settle is + worse than an actionable refusal that names the platform boundary. + """ + from loopx.control_plane.turn_driver import host_process_transport + + root, runner = service + monkeypatch.setattr(runner, "_spawn", lambda _: None) + runner.start("analysis", "analysis-platform", brief()) + path = runner.path("analysis-platform") + record = runner._host_process_record(path) + record.parent.mkdir(parents=True, exist_ok=True) + record.write_text(json.dumps({ + "schema_version": host_process_transport.HOST_PROCESS_RECORD_SCHEMA_VERSION, + "host": lock_holder_host_label(), "phase": "finished", + "bridge_pid": os.getpid(), "process_group": os.getpid(), + })) + + # The launched Host is real, but this platform cannot prove it drained. + assert host_process_transport.host_process_drain(record) == host_process_transport.HOST_PROCESS_DRAINED + monkeypatch.delattr(host_process_transport.os, "killpg", raising=False) + assert host_process_transport.host_process_drain(record) == host_process_transport.HOST_PROCESS_UNSUPPORTED_PLATFORM + + with pytest.raises(ValueError, match="cannot prove the launched Host drained"): + runner.stop("analysis-platform", execute=True) From 00442681f48ce729aa71218991720a939ad1db65 Mon Sep 17 00:00:00 2001 From: song Date: Wed, 30 Sep 2026 14:36:34 -0400 Subject: [PATCH 13/13] fix(delegation): take the lease obligation from the operation record MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_acknowledge_stop` persists the ACK before it releases the hard lease, so a process loss between those two writes leaves the sidecar with no `lease` field at all. `_settle_stop` read the obligation from that field, took the absence as "no required lease", and settled: the caller was told the member is safely stopped while the native authority still reported its lease active, with resume already refused and the Todo blocked until the TTL. Reproduced on both File and SQLite by acquiring a real required lease and losing the process at the release entry. The obligation now comes from the operation record — the canonical `row.task_lease.required`, which the crash cannot lose. Absent sidecar state means "the release result has not been written yet", not "nothing was owed", so the release is attempted and only an actual release lets the decision settle. That is one source of truth rather than a second boolean kept in sync with it; the released fact is still persisted beside the stop so a later read does not release twice. Coverage pins both ends of the window on both authorities: - a crash after the ACK with the lease still owed, and a release that succeeds on the next read, settles and reports `lease_released: true`; - the same window with the release still failing stays `acknowledged` with `required_lease_release_unproven` instead of reporting `settled`. Mutation-checked: deriving the obligation from the sidecar again fails all four. Signed-off-by: song --- docs/reference/local-delegation.md | 12 +++-- loopx/collaboration_mcp.py | 22 +++++---- tests/test_local_delegation.py | 76 ++++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 12 deletions(-) diff --git a/docs/reference/local-delegation.md b/docs/reference/local-delegation.md index a32ff944e2..bb715f31c9 100644 --- a/docs/reference/local-delegation.md +++ b/docs/reference/local-delegation.md @@ -236,7 +236,10 @@ refused. The receipt `phase` is `settled` only when an acknowledgement exists, the operation lock is free, the member's Turn lane holder record shows it released by the stopped worker (the lane is read, never taken), the native host the Turn launched has exited together with every process in its group, -and a required hard task lease was actually released. A release that failed is +and a required hard task lease was actually released. The obligation is read +from the operation record, not from the stop sidecar: the acknowledgement is +persisted before the lease is released, so a crash in between must not turn +"not yet written" into "nothing was owed". A release that failed is retried under the stop's own lock on the next read, so it never becomes a `settled` receipt that leaves the member's Todo blocked until the lease TTL; while it is unproven the stop stays `acknowledged` with @@ -266,9 +269,10 @@ written after both leaves their acceptance intact. worker 不会被发信号,它在下一个检查点或下一次写记录时发现请求,写入被拒绝。 只有存在确认、operation 锁已释放、成员 Turn lane 的持有者记录显示已被停止的 worker 释放(只读 lane,从不获取)、该 Turn 启动的原生 host 及其进程组内所有进程 -都已退出,且必需的硬任务租约确实释放成功时,`phase` 才是 `settled`。释放失败会在 -下一次读取时于 stop 自己的锁下重试,因此不会产生一份「已结算」却让成员 Todo 被 -租约阻塞到 TTL 的回执;在释放得到证明前,停止保持 `acknowledged`,原因为 +都已退出,且必需的硬任务租约确实释放成功时,`phase` 才是 `settled`。该义务取自 +操作记录而非 stop sidecar:确认会先于释放落盘,因此两者之间发生崩溃时,不能把 +「尚未写入」当成「本就不需要释放」。释放失败会在下一次读取时于 stop 自己的锁下重试, +因此不会产生一份「已结算」却让成员 Todo 被租约阻塞到 TTL 的回执;在释放得到证明前,停止保持 `acknowledged`,原因为 `required_lease_release_unproven`。host 无法归属或其 supervisor 未完成清理时, 停止保持 `acknowledged`,之后再次调用 `stop` 会重新读取。在没有进程组的平台上, 启动过的 host 根本无法被证明已收尾,因此 `stop --execute` 会以指明该平台边界的 diff --git a/loopx/collaboration_mcp.py b/loopx/collaboration_mcp.py index 82d1906a70..fe308760c8 100644 --- a/loopx/collaboration_mcp.py +++ b/loopx/collaboration_mcp.py @@ -1122,20 +1122,26 @@ def _settle_stop(self, path: Path) -> dict: facts["worker_lane_released"], lane_state = self._worker_lane_released(row, stop, binding) # Read last: a Host seen drained after its worker and lane let go stays drained. facts["host_process"] = host_process_drain(self._host_process_record(path)) - # A required lease the acknowledgement could not release is retried - # here, under the same lock that guards the record: every later - # read of this stop is another attempt, instead of one failure - # turning into a permanent `settled` with the lease still held. + # The obligation comes from the operation record, not from the stop + # sidecar. The acknowledgement writes its ACK before it releases the + # lease, so a process loss in between leaves the sidecar with no + # `lease` field at all — and reading that as "nothing was owed" + # settles a stop whose member still holds an active hard lease, with + # resume already refused and the Todo blocked until the TTL. + # + # The canonical `row.task_lease.required` is the source of truth, so + # the obligation survives the crash. A release is retried here, under + # the same lock that guards the record: every later read is another + # attempt rather than one failure becoming permanent. + owed = isinstance(row.get("task_lease"), dict) and row["task_lease"].get("required") is True lease = stop.get("lease") if isinstance(stop.get("lease"), dict) else {} - if lease.get("required") is True and lease.get("released") is not True: + if owed and lease.get("released") is not True: retried = self._release_delegation_lease(row, binding) if retried != lease: stop["lease"] = retried _write(self._stop_path(path), stop) lease = retried - if lease.get("required") is True: - # Absent means the operation held no required lease, which is not - # the same claim as a lease that was released. + if owed: facts["lease_released"] = lease.get("released") is True decision = effect_runtime_result("collaboration.delegation.stop", { "phase": stop["phase"], "acknowledged": stop.get("ack") is not None, diff --git a/tests/test_local_delegation.py b/tests/test_local_delegation.py index 1448e13b56..51733ca484 100644 --- a/tests/test_local_delegation.py +++ b/tests/test_local_delegation.py @@ -17,6 +17,7 @@ sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "examples" / "managed-research-team")) import research_team as demo # noqa: E402 from test_managed_research_scenario import fixture # noqa: E402 +from loopx import collaboration_mcp as delegation_module # noqa: E402 from loopx.collaboration_mcp import DelegationFenced, DelegationStopRequested, Delegations # noqa: E402 from loopx.control_plane.collaboration.peers import returns # noqa: E402 from loopx.control_plane.collaboration.inbox import _read # noqa: E402 @@ -748,3 +749,78 @@ def test_a_launched_host_on_a_platform_without_process_groups_fails_fast(service with pytest.raises(ValueError, match="cannot prove the launched Host drained"): runner.stop("analysis-platform", execute=True) + + +def test_a_crash_between_the_ack_and_the_lease_result_keeps_the_stop_open(service, monkeypatch): + """The lease obligation survives process loss after the acknowledgement. + + `_acknowledge_stop` writes the ACK before it releases the lease, so a crash + in between leaves the sidecar with no `lease` field. Reading that as "nothing + was owed" settles a stop whose member still holds an active hard lease, with + resume already refused and the Todo blocked until the TTL. The obligation + comes from the operation record, which the crash cannot lose. + """ + from loopx.control_plane.collaboration.inbox import _write as write_inbox + + root, runner = service + monkeypatch.setattr(runner, "_spawn", lambda _: None) + runner.start("analysis", "analysis-crash", brief()) + path = runner.path("analysis-crash") + row = _read(path) + # The member holds a real required lease, as a bounded task does. + row["task_lease"] = {"required": True, "idempotency_key": "lease-crash", "version": 1} + right_after_ack = {**row, "status": "stopped"} + runner._fenced_write(path, right_after_ack) + + # The crash window: ACK persisted, no lease result written yet. + write_inbox(runner._stop_path(path), { + **runner._new_stop_record(right_after_ack, requested_by=runner.agent_id, worker=None), + "phase": "acknowledged", + "ack": {"pid": os.getpid(), "host": lock_holder_host_label(), "at": time.time(), + "source": "requester", "observed_status": "stopped", "turn_key": None}, + }) + sidecar = _read(runner._stop_path(path)) + assert "lease" not in sidecar or sidecar.get("lease") is None + assert _read(path)["task_lease"]["required"] is True + + # A release that succeeds on this read lets the stop settle, and the receipt + # says the lease really is gone. + monkeypatch.setattr(delegation_module, "release_task_lease", + lambda **kw: {"released": True}) + settled = runner.stop("analysis-crash", execute=True) + assert settled["phase"] == "settled", settled + assert settled["stop"]["lease"]["released"] is True + assert settled["stop"]["settled"]["lease_released"] is True + + +def test_a_crash_between_the_ack_and_the_lease_result_never_settles_unreleased(service, monkeypatch): + """The same window, with the release still failing, must not report `settled`. + + `settled` tells the owner the member is safely stopped. Claiming it while a + required lease is provably still active is exactly the terminal distortion + the crash window used to produce. + """ + from loopx.control_plane.collaboration.inbox import _write as write_inbox + + root, runner = service + monkeypatch.setattr(runner, "_spawn", lambda _: None) + runner.start("analysis", "analysis-crash-open", brief()) + path = runner.path("analysis-crash-open") + row = _read(path) + row["task_lease"] = {"required": True, "idempotency_key": "lease-open", "version": 1} + right_after_ack = {**row, "status": "stopped"} + runner._fenced_write(path, right_after_ack) + write_inbox(runner._stop_path(path), { + **runner._new_stop_record(right_after_ack, requested_by=runner.agent_id, worker=None), + "phase": "acknowledged", + "ack": {"pid": os.getpid(), "host": lock_holder_host_label(), "at": time.time(), + "source": "requester", "observed_status": "stopped", "turn_key": None}, + }) + + def still_held(**kwargs): + raise RuntimeError("authority unavailable") + + monkeypatch.setattr(delegation_module, "release_task_lease", still_held) + receipt = runner.stop("analysis-crash-open", execute=True) + assert receipt["phase"] == "acknowledged", receipt + assert receipt["stop"]["reason"] == "required_lease_release_unproven"