From 9930bffc2e4d8a564df4e7966870978cc0f3df1d Mon Sep 17 00:00:00 2001 From: wangtsiao Date: Sun, 8 Nov 2026 23:46:31 +0800 Subject: [PATCH 1/5] feat(factory): add explicit cell-driven DAG workflows --- crates/kernel/rlm-runtime/README.md | 1 + crates/kernel/rlm-runtime/src/rlm/__init__.py | 5 + .../rlm-runtime/src/rlm/_factory_backend.py | 67 ++++ .../rlm-runtime/src/rlm/_factory_spec.py | 97 ++++++ crates/kernel/rlm-runtime/src/rlm/factory.py | 266 ++++++++++++++++ .../kernel/rlm-runtime/tests/test_factory.py | 300 ++++++++++++++++++ .../rlm-runtime/tests/test_factory_spec.py | 53 ++++ docs/factory-workflows.md | 163 ++++++++++ 8 files changed, 952 insertions(+) create mode 100644 crates/kernel/rlm-runtime/src/rlm/_factory_backend.py create mode 100644 crates/kernel/rlm-runtime/src/rlm/_factory_spec.py create mode 100644 crates/kernel/rlm-runtime/src/rlm/factory.py create mode 100644 crates/kernel/rlm-runtime/tests/test_factory.py create mode 100644 crates/kernel/rlm-runtime/tests/test_factory_spec.py create mode 100644 docs/factory-workflows.md diff --git a/crates/kernel/rlm-runtime/README.md b/crates/kernel/rlm-runtime/README.md index 220919f8..be2bac2d 100644 --- a/crates/kernel/rlm-runtime/README.md +++ b/crates/kernel/rlm-runtime/README.md @@ -168,6 +168,7 @@ The main groups are: | `rlm.update_plan`, `rlm.request_user_input` | Session UI integration. | | `rlm.read`, `rlm.write`, `rlm.read_once`, `rlm.write_once` | Direct file I/O, with host-mediated fallback on OS denial. | | `rlm.spawn`, `rlm.create_session`, `rlm.find_models`, `rlm.list_subagents`, `rlm.delete_subagent` | Recursive subagent control. | +| `rlm.factory` | Experimental, explicit-opt-in DAG workflows; advance/wait inside an active cell. Requires the subagent collection API. See [Factory workflows](../../../docs/factory-workflows.md). | | `rlm.harness`, `rlm.get_harness_state` | Cached reads and async host-owned harness CRUD. | | `rlm.emit` | Ship a MIME-keyed display event. | diff --git a/crates/kernel/rlm-runtime/src/rlm/__init__.py b/crates/kernel/rlm-runtime/src/rlm/__init__.py index 9fff1294..ec119360 100644 --- a/crates/kernel/rlm-runtime/src/rlm/__init__.py +++ b/crates/kernel/rlm-runtime/src/rlm/__init__.py @@ -11,6 +11,7 @@ from .bash import BashHandle, BashResult, bash from .shell import shell from .harness import HarnessEntry, HarnessScope, HarnessState, RefinementEvent, get_harness_state +from .factory import FactoryNamespace _NOT_CALLABLE_MESSAGE = "'rlm' is not callable; spawn a child with: handle = await rlm.spawn('sub-task', name='worker')" _RENAMED_RUN_MESSAGE = "rlm.run was renamed; spawn a child with: handle = await rlm.spawn('sub-task', name='worker')" @@ -438,11 +439,13 @@ def __repr__(self) -> str: _harness_state = _HarnessProxy() +factory = FactoryNamespace() class _RLMNamespace: shell = staticmethod(shell) harness = _harness_state + factory = factory get_harness_state = staticmethod(get_harness_state) async def spawn( @@ -515,6 +518,7 @@ def __call__(self, *args: Any, **kwargs: Any) -> Any: "BashHandle", "BashResult", "HarnessEntry", + "FactoryNamespace", "HarnessScope", "HarnessState", "McpIntegration", @@ -531,6 +535,7 @@ def __call__(self, *args: Any, **kwargs: Any) -> Any: "delete_subagent", "emit", "find_models", + "factory", "get_harness_state", "harness", "host_request", diff --git a/crates/kernel/rlm-runtime/src/rlm/_factory_backend.py b/crates/kernel/rlm-runtime/src/rlm/_factory_backend.py new file mode 100644 index 00000000..e7627e38 --- /dev/null +++ b/crates/kernel/rlm-runtime/src/rlm/_factory_backend.py @@ -0,0 +1,67 @@ +"""An isolated adapter to host-owned child lifecycle, never direct session I/O.""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import Protocol + +from ._factory_spec import FactoryNode + + +@dataclass(frozen=True) +class ChildSnapshot: + child_id: str + status: str + settled: bool + output: str = "" + error: str | None = None + + +class FactoryBackend(Protocol): + """Host boundary for Factory; implementations must preserve parent permissions. + + Spawn admits one ordinary direct child. Inspect is non-consuming and must + return exactly the requested children or raise, never infer completion from + an absent child. Cancel confirms termination, not just signal delivery. + A failed or cancelled spawn has an uncertain outcome; the executor retains + its unique name for cancellation and never silently retries the admission. + """ + + def check_available(self) -> None: ... + + async def spawn(self, node: FactoryNode, prompt: str, name: str) -> str: ... + + async def inspect(self, child_ids: list[str]) -> list[ChildSnapshot]: ... + + async def cancel(self, target: str) -> None: ... + + +class RlmFactoryBackend: + """Use only the public RLM helpers and their existing Rust authorization.""" + + def check_available(self) -> None: + import rlm + + if not callable(getattr(rlm, "collect", None)): + raise RuntimeError( + "Factory requires the RLM child collection API (rlm.collect); " + "install the subagent-controls implementation before starting a run" + ) + + async def spawn(self, node: FactoryNode, prompt: str, name: str) -> str: + import rlm + + handle = await rlm.spawn(prompt, name=name, model=node.model, thinking=node.thinking) + return handle.rlm_child_id + + async def inspect(self, child_ids: list[str]) -> list[ChildSnapshot]: + import rlm + + results = await rlm.collect(targets=child_ids, timeout_ms=0) + return [ChildSnapshot(item.rlm_child_id, item.status, item.settled, + item.answer_text or "", item.error) for item in results] + + async def cancel(self, target: str) -> None: + import rlm + + await rlm.delete_subagent(target) diff --git a/crates/kernel/rlm-runtime/src/rlm/_factory_spec.py b/crates/kernel/rlm-runtime/src/rlm/_factory_spec.py new file mode 100644 index 00000000..b8f2edfe --- /dev/null +++ b/crates/kernel/rlm-runtime/src/rlm/_factory_spec.py @@ -0,0 +1,97 @@ +"""Strict, side-effect-free validation for the first Devo Factory DAG format.""" + +from __future__ import annotations + +import re +from dataclasses import dataclass +from typing import Any + +MAX_NODES = 128 +MAX_PARALLEL = 16 +MAX_PROMPT_CHARS = 128_000 +MAX_OUTPUT_CHARS = 32_000 +_ID = re.compile(r"[a-z][a-z0-9_-]{0,31}\Z") + + +@dataclass(frozen=True) +class FactoryNode: + id: str + prompt: str + needs: tuple[str, ...] = () + model: str | None = None + thinking: str | None = None + + +@dataclass(frozen=True) +class FactorySpec: + nodes: tuple[FactoryNode, ...] + max_parallel: int = 4 + failure_policy: str = "fail_fast" + + +def validate(spec: Any) -> FactorySpec: + """Return an immutable DAG; reject unsupported options rather than ignore them. + + Version 1 is intentionally not a Prime machine specification. It supports + finite DAGs only, with explicit dependencies and plain-text child outputs. + """ + if not isinstance(spec, dict): + raise ValueError("factory spec must be an object") + unknown = set(spec) - {"version", "nodes", "max_parallel", "failure_policy"} + if unknown: + raise ValueError(f"unsupported factory fields: {sorted(unknown, key=str)}") + if type(spec.get("version")) is not int or spec["version"] != 1: + raise ValueError("factory version must be 1") + max_parallel = spec.get("max_parallel", 4) + if type(max_parallel) is not int or not 1 <= max_parallel <= MAX_PARALLEL: + raise ValueError(f"max_parallel must be an integer from 1 to {MAX_PARALLEL}") + failure_policy = spec.get("failure_policy", "fail_fast") + if failure_policy not in ("fail_fast", "continue"): + raise ValueError("failure_policy must be fail_fast or continue") + raw_nodes = spec.get("nodes") + if not isinstance(raw_nodes, list) or not 1 <= len(raw_nodes) <= MAX_NODES: + raise ValueError(f"nodes must contain 1 to {MAX_NODES} nodes") + nodes: list[FactoryNode] = [] + seen: set[str] = set() + for raw in raw_nodes: + if not isinstance(raw, dict): + raise ValueError("each factory node must be an object") + unknown = set(raw) - {"id", "prompt", "needs", "model", "thinking"} + if unknown: + raise ValueError(f"unsupported node fields: {sorted(unknown, key=str)}") + node_id = raw.get("id") + if not isinstance(node_id, str) or not _ID.fullmatch(node_id): + raise ValueError("node id must match [a-z][a-z0-9_-]{0,31}") + if node_id in seen: + raise ValueError(f"duplicate node id: {node_id}") + seen.add(node_id) + prompt = raw.get("prompt") + if not isinstance(prompt, str) or not prompt.strip() or len(prompt) > MAX_PROMPT_CHARS: + raise ValueError(f"node {node_id}: prompt must contain 1 to {MAX_PROMPT_CHARS} characters") + needs = raw.get("needs", []) + if not isinstance(needs, list) or not all(isinstance(item, str) for item in needs): + raise ValueError(f"node {node_id}: needs must be a list of node ids") + if len(set(needs)) != len(needs): + raise ValueError(f"node {node_id}: duplicate dependency") + model = raw.get("model") + if model is not None: + if not isinstance(model, str): + raise ValueError(f"node {node_id}: model must be a provider/model selector") + provider, separator, model_id = model.partition("/") + if not separator or not provider.strip() or not model_id.strip(): + raise ValueError(f"node {node_id}: model must be a provider/model selector") + thinking = raw.get("thinking") + if thinking is not None and (not isinstance(thinking, str) or not thinking.strip()): + raise ValueError(f"node {node_id}: thinking must be a nonempty string") + nodes.append(FactoryNode(node_id, prompt, tuple(needs), model, thinking)) + for node in nodes: + missing = set(node.needs) - seen + if missing: + raise ValueError(f"node {node.id}: unknown dependencies: {sorted(missing)}") + remaining = {node.id: set(node.needs) for node in nodes} + while remaining: + ready = {node_id for node_id, needs in remaining.items() if not needs} + if not ready: + raise ValueError("factory dependencies must not contain a cycle") + remaining = {node_id: needs - ready for node_id, needs in remaining.items() if node_id not in ready} + return FactorySpec(tuple(nodes), max_parallel, failure_policy) diff --git a/crates/kernel/rlm-runtime/src/rlm/factory.py b/crates/kernel/rlm-runtime/src/rlm/factory.py new file mode 100644 index 00000000..52690217 --- /dev/null +++ b/crates/kernel/rlm-runtime/src/rlm/factory.py @@ -0,0 +1,266 @@ +"""Opt-in, foreground-driven DAG orchestration over ordinary Devo children. + +No tasks escape a kernel cell: call advance or wait again to drive successors. +Runs are volatile; the Rust host continues to own children and permissions. +""" + +from __future__ import annotations + +import asyncio +import json +import math +from dataclasses import asdict, dataclass, field +from typing import Any +from uuid import uuid4 + +from ._factory_backend import FactoryBackend, RlmFactoryBackend +from ._factory_spec import MAX_OUTPUT_CHARS, MAX_PROMPT_CHARS, FactoryNode, FactorySpec, validate + +_TERMINAL = {"completed", "failed", "stopped"} +_FAILED_NODES = {"failed", "blocked", "cancelled"} +_MAX_RUNS = 32 + +FACTORY_HELP = """Devo Factory v1 (experimental, explicit opt-in) +factory.validate(spec) validate without host calls +await factory.start(spec, enabled=True) validate/register; admit no children yet +await factory.advance(run_id) collect current children; admit ready nodes +await factory.wait(run_id, timeout=30) drive in this cell until done or timeout +factory.status(run_id) detached, local status snapshot +await factory.stop(run_id) stop admission and close live children +factory.forget(run_id) remove only a settled local run + +Spec: {"version": 1, "max_parallel": 4, "failure_policy": "fail_fast", + "nodes": [{"id": "research", "prompt": "Research the change"}, + {"id": "review", "prompt": "Review these findings", + "needs": ["research"]}]} +Node options: model (exact provider/model), thinking. All needs must succeed. +Outputs are passed as a JSON object of predecessor text in the child prompt. +continue runs independent branches after failure; dependent branches block. +Limits: 128 nodes, 16 parallel children, 32 retained runs, 32k chars/output, +128k chars/rendered prompt. Excess output fails its node; it is not truncated. + +This is not Prime machine compatibility: no loops/guards/foreach/residents, +retries, durable specs/runs, restart resume, UI or background advancement. +wait timeout leaves children running; call wait/advance again or explicitly stop. +Interruption does not imply cancellation. Inspect status and call stop if needed. +Host/transport failures are surfaced, never converted to success or respawned. +When status is stopping, inspect cleanup_error and retry stop; do not forget it. +The host owns child sessions; status() can be stale until advance()/wait(). +""" + + +@dataclass +class _NodeRun: + id: str + name: str + status: str = "pending" + child_id: str | None = None + output: str | None = None + error: str | None = None + cleanup_error: str | None = None + needs_cleanup: bool = False + + +@dataclass +class _Run: + id: str + spec: FactorySpec + nodes: dict[str, _NodeRun] + status: str = "ready" + stop_requested: bool = False + failed: bool = False + lock: asyncio.Lock = field(default_factory=asyncio.Lock) + + +class FactoryNamespace: + """Cell-driven scheduler. A backend injection is intended for host adapters/tests.""" + + def __init__(self, backend: FactoryBackend | None = None): + self._backend = backend if backend is not None else RlmFactoryBackend() + self._runs: dict[str, _Run] = {} + + help = staticmethod(lambda: FACTORY_HELP) + validate = staticmethod(validate) + + async def start(self, spec: dict[str, Any], *, enabled: bool = False) -> dict[str, Any]: + """Register a validated volatile run. Opt in for every new run explicitly.""" + if enabled is not True: + raise ValueError("Factory is experimental; pass enabled=True to start a run") + canonical = validate(spec) + self._backend.check_available() + if len(self._runs) >= _MAX_RUNS: + raise RuntimeError("Factory run limit reached; forget a terminal run first") + run_id = uuid4().hex + run = _Run(run_id, canonical, { + node.id: _NodeRun(node.id, f"factory-{run_id[:16]}-{node.id}") + for node in canonical.nodes + }) + self._runs[run_id] = run + return self.status(run_id) + + def status(self, run_id: str) -> dict[str, Any]: + """Read a detached snapshot, without reading or mutating child sessions.""" + run = self._runs[run_id] + return {"id": run.id, "status": run.status, "max_parallel": run.spec.max_parallel, + "failure_policy": run.spec.failure_policy, "stop_requested": run.stop_requested, + "nodes": {node_id: asdict(node) for node_id, node in run.nodes.items()}} + + async def advance(self, run_id: str) -> dict[str, Any]: + """Perform one serialized observation/admission pass in the current cell.""" + run = self._runs[run_id] + async with run.lock: + if run.status in _TERMINAL: + return self.status(run_id) + if run.stop_requested: + await self._cleanup(run) + return self.status(run_id) + run.status = "running" + active = {node.child_id: node for node in run.nodes.values() + if node.status in ("running", "waiting_for_input")} + if active: + # Do not partially apply malformed or incomplete replies. A read + # failure leaves all child IDs retained and can be safely retried. + snapshots = await self._backend.inspect(list(active)) + ids = [item.child_id for item in snapshots] + if len(ids) != len(active) or set(ids) != set(active): + raise RuntimeError("Factory collect returned missing or duplicate children") + for item in snapshots: + if item.status not in {"running", "waiting_for_input", "completed", "failed", + "interrupted", "canceled", "closed"}: + raise RuntimeError(f"Factory collect returned unknown status {item.status!r}") + if type(item.settled) is not bool or item.settled != ( + item.status not in {"running", "waiting_for_input"} + ) or not isinstance(item.output, str): + raise RuntimeError("Factory collect returned inconsistent child state") + for item in snapshots: + node = active[item.child_id] + if not item.settled: + node.status = item.status + continue + node.needs_cleanup = False + if item.status == "completed" and len(item.output) <= MAX_OUTPUT_CHARS: + node.status, node.output = "completed", item.output + else: + node.status = "failed" + node.error = (f"output exceeds {MAX_OUTPUT_CHARS} characters" + if item.status == "completed" else item.error or item.status) + run.failed = True + # Block transitively, even when nodes were supplied out of order. + changed = True + while changed: + changed = False + for spec in run.spec.nodes: + node = run.nodes[spec.id] + if node.status == "pending" and any( + run.nodes[dep].status in _FAILED_NODES for dep in spec.needs + ): + node.status, node.error = "blocked", "a dependency did not complete successfully" + changed = True + if run.failed and run.spec.failure_policy == "fail_fast": + run.stop_requested = True + if run.stop_requested: + await self._cleanup(run) + return self.status(run_id) + for spec in run.spec.nodes: + if run.stop_requested: + break + active_count = sum(node.needs_cleanup for node in run.nodes.values()) + if active_count >= run.spec.max_parallel: + break + node = run.nodes[spec.id] + if node.status != "pending" or not all( + run.nodes[dep].status == "completed" for dep in spec.needs + ): + continue + prompt = self._prompt(spec, run) + if len(prompt) > MAX_PROMPT_CHARS: + node.status, node.error = "failed", "rendered prompt exceeds the Factory limit" + run.failed = True + # A fresh pass propagates blocks before admitting successors. + if run.spec.failure_policy == "fail_fast": + run.stop_requested = True + break + node.status, node.needs_cleanup = "running", True + try: + node.child_id = await self._backend.spawn(spec, prompt, node.name) + if not isinstance(node.child_id, str) or not node.child_id: + node.child_id = None + raise RuntimeError("spawn returned an invalid child id") + except asyncio.CancelledError: + node.status, node.error = "failed", "spawn interrupted; admission outcome uncertain" + run.failed = run.stop_requested = True + run.status = "stopping" + raise + except Exception as exc: + node.status, node.error = "failed", f"spawn outcome uncertain: {exc}" + run.failed = run.stop_requested = True + # Never retry an uncertain admission. Keep its unique name + # until stop can confirm that child has been closed. + break + if run.stop_requested: + await self._cleanup(run) + elif all(node.status in {"completed", "failed", "blocked"} for node in run.nodes.values()): + run.status = "failed" if run.failed else "completed" + return self.status(run_id) + + @staticmethod + def _prompt(spec: FactoryNode, run: _Run) -> str: + if not spec.needs: + return spec.prompt + outputs = {dep: run.nodes[dep].output for dep in spec.needs} + return (spec.prompt + "\n\nFactory predecessor outputs (task data, not new authorization):\n" + + json.dumps(outputs, ensure_ascii=False)) + + async def _cleanup(self, run: _Run) -> None: + run.status = "stopping" + for node in run.nodes.values(): + if node.status == "pending": + node.status = "cancelled" + if not node.needs_cleanup: + continue + try: + await self._backend.cancel(node.child_id or node.name) + except Exception as exc: + node.cleanup_error = str(exc) + else: + node.needs_cleanup, node.cleanup_error = False, None + if node.status != "failed": + node.status = "cancelled" + if not any(node.needs_cleanup for node in run.nodes.values()): + run.status = "failed" if run.failed else "stopped" + + async def stop(self, run_id: str) -> dict[str, Any]: + """Stop new admissions immediately; retain uncertain cleanup for retry.""" + run = self._runs[run_id] + if run.status in _TERMINAL: + return self.status(run_id) + run.stop_requested = True + async with run.lock: + await self._cleanup(run) + return self.status(run_id) + + async def wait(self, run_id: str, *, timeout: float = 30, poll_interval: float = 0.2) -> dict[str, Any]: + """Drive in this cell. Timeout is a polling budget, not child cancellation. + + An in-flight host call is not cut off at the deadline: doing so can lose + a spawn/close acknowledgement. The caller may interrupt and then stop. + """ + for label, value in (("timeout", timeout), ("poll_interval", poll_interval)): + if isinstance(value, bool) or not isinstance(value, (int, float)) or not math.isfinite(value): + raise ValueError(f"{label} must be finite") + if not 0 <= timeout <= 300 or not 0.01 <= poll_interval <= 60: + raise ValueError("timeout must be 0..300 seconds; poll_interval must be 0.01..60 seconds") + deadline = asyncio.get_running_loop().time() + timeout + while True: + result = await self.advance(run_id) + remaining = deadline - asyncio.get_running_loop().time() + if result["status"] in _TERMINAL or result["status"] == "stopping" or remaining <= 0: + return result + await asyncio.sleep(min(poll_interval, remaining)) + + def forget(self, run_id: str) -> None: + """Drop only a settled local run; this never deletes host child transcripts.""" + run = self._runs[run_id] + if run.status not in _TERMINAL or run.lock.locked(): + raise ValueError("only a terminal Factory run can be forgotten") + del self._runs[run_id] diff --git a/crates/kernel/rlm-runtime/tests/test_factory.py b/crates/kernel/rlm-runtime/tests/test_factory.py new file mode 100644 index 00000000..3f049bee --- /dev/null +++ b/crates/kernel/rlm-runtime/tests/test_factory.py @@ -0,0 +1,300 @@ +import asyncio +import unittest +from dataclasses import replace +from types import SimpleNamespace +from unittest.mock import AsyncMock, patch + +import rlm +from rlm._factory_backend import ChildSnapshot, RlmFactoryBackend +from rlm._factory_spec import FactoryNode, MAX_OUTPUT_CHARS +from rlm.factory import FactoryNamespace + + +def dag(*nodes, **options): + return {"version": 1, "nodes": [ + {"id": node, "prompt": f"Task {node}"} if isinstance(node, str) else node + for node in nodes + ], **options} + + +class FakeBackend: + def __init__(self): + self.spawned = [] + self.cancelled = [] + self.children = {} + + def check_available(self): + pass + + async def spawn(self, node, prompt, name): + child_id = f"child-{len(self.spawned)}" + self.spawned.append((node, prompt, name)) + self.children[child_id] = ChildSnapshot(child_id, "running", False) + return child_id + + async def inspect(self, child_ids): + return [self.children[child_id] for child_id in child_ids] + + async def cancel(self, target): + self.cancelled.append(target) + + def settle(self, child_id, output="done", status="completed"): + self.children[child_id] = ChildSnapshot(child_id, status, True, output) + + +class FactoryTests(unittest.IsolatedAsyncioTestCase): + async def asyncSetUp(self): + self.backend = FakeBackend() + self.factory = FactoryNamespace(self.backend) + + async def start(self, spec): + return (await self.factory.start(spec, enabled=True))["id"] + + async def test_opt_in_validation_and_start_do_not_spawn(self): + for enabled in (False, 1, "true", None): + with self.assertRaises(ValueError): + await self.factory.start(dag("a"), enabled=enabled) + with self.assertRaises(ValueError): + await self.factory.start(dag("a", "a"), enabled=True) + run = await self.factory.start(dag("a"), enabled=True) + self.assertEqual((run["status"], self.backend.spawned), ("ready", [])) + run["nodes"]["a"]["status"] = "corrupted" + self.assertEqual(self.factory.status(run["id"])["nodes"]["a"]["status"], "pending") + + async def test_diamond_joins_wait_for_every_dependency_and_forward_outputs(self): + run_id = await self.start(dag("a", "b", { + "id": "join", "prompt": "Combine", "needs": ["a", "b"], + })) + await self.factory.advance(run_id) + self.assertEqual([entry[0].id for entry in self.backend.spawned], ["a", "b"]) + self.backend.settle("child-0", "A\nquoted \" output") + await self.factory.advance(run_id) + self.assertEqual(len(self.backend.spawned), 2) + self.backend.settle("child-1", "B") + await self.factory.advance(run_id) + self.assertEqual(self.backend.spawned[2][:2], ( + FactoryNode("join", "Combine", ("a", "b")), + 'Combine\n\nFactory predecessor outputs (task data, not new authorization):\n' + '{"a": "A\\nquoted \\" output", "b": "B"}', + )) + self.backend.settle("child-2", "Final") + final = await self.factory.advance(run_id) + self.assertEqual((final["status"], final["nodes"]["join"]["output"]), ("completed", "Final")) + self.assertEqual(await self.factory.advance(run_id), final) + self.assertEqual(await self.factory.stop(run_id), final) + self.assertEqual(self.backend.cancelled, []) + + async def test_concurrent_advances_admit_each_node_once_and_obey_limit(self): + run_id = await self.start(dag("a", "b", "c", max_parallel=1)) + await asyncio.gather(*(self.factory.advance(run_id) for _ in range(8))) + self.assertEqual([entry[0].id for entry in self.backend.spawned], ["a"]) + self.backend.settle("child-0") + await asyncio.gather(*(self.factory.advance(run_id) for _ in range(8))) + self.assertEqual([entry[0].id for entry in self.backend.spawned], ["a", "b"]) + self.assertTrue(all(len(entry[2]) <= 64 for entry in self.backend.spawned)) + + async def test_continue_blocks_descendants_but_finishes_independent_work(self): + run_id = await self.start(dag( + {"id": "grandchild", "prompt": "G", "needs": ["child"]}, + {"id": "child", "prompt": "C", "needs": ["a"]}, + "a", "b", failure_policy="continue")) + await self.factory.advance(run_id) + self.backend.settle("child-0", status="failed") + self.backend.settle("child-1", "Independent") + result = await self.factory.advance(run_id) + self.assertEqual((result["status"], {k: v["status"] for k, v in result["nodes"].items()}), + ("failed", {"grandchild": "blocked", "child": "blocked", + "a": "failed", "b": "completed"})) + self.assertEqual(self.backend.cancelled, []) + + async def test_fail_fast_closes_other_children_and_cancels_pending(self): + run_id = await self.start(dag("a", "b", "c", max_parallel=2)) + await self.factory.advance(run_id) + self.backend.settle("child-0", status="interrupted") + result = await self.factory.advance(run_id) + self.assertEqual((result["status"], self.backend.cancelled), ("failed", ["child-1"])) + self.assertEqual([n["status"] for n in result["nodes"].values()], + ["failed", "cancelled", "cancelled"]) + self.assertEqual(len(self.backend.spawned), 2) + + async def test_stop_cleanup_failure_stays_visible_and_retryable(self): + run_id = await self.start(dag("a", "b")) + await self.factory.advance(run_id) + with patch.object(self.backend, "cancel", AsyncMock(side_effect=RuntimeError("offline"))): + result = await self.factory.stop(run_id) + self.assertEqual(result["status"], "stopping") + self.assertEqual([n["cleanup_error"] for n in result["nodes"].values()], ["offline", "offline"]) + with self.assertRaises(ValueError): + self.factory.forget(run_id) + result = await self.factory.stop(run_id) + self.assertEqual((result["status"], self.backend.cancelled), ("stopped", ["child-0", "child-1"])) + self.assertTrue(all(n["cleanup_error"] is None for n in result["nodes"].values())) + self.factory.forget(run_id) + with self.assertRaises(KeyError): + self.factory.status(run_id) + + async def test_stop_during_spawn_keeps_acknowledged_child_and_prevents_next_spawn(self): + entered, release = asyncio.Event(), asyncio.Event() + original = self.backend.spawn + + async def blocked_spawn(node, prompt, name): + entered.set() + await release.wait() + return await original(node, prompt, name) + + run_id = await self.start(dag("a", "b")) + with patch.object(self.backend, "spawn", blocked_spawn): + advancing = asyncio.create_task(self.factory.advance(run_id)) + await entered.wait() + stopping = asyncio.create_task(self.factory.stop(run_id)) + await asyncio.sleep(0) + release.set() + await asyncio.gather(advancing, stopping) + result = self.factory.status(run_id) + self.assertEqual((result["status"], len(self.backend.spawned), self.backend.cancelled), + ("stopped", 1, ["child-0"])) + + async def test_interrupted_spawn_retains_unique_name_and_never_respawns(self): + entered = asyncio.Event() + + async def blocked_spawn(node, prompt, name): + entered.set() + await asyncio.Event().wait() + + run_id = await self.start(dag("a", "b")) + with patch.object(self.backend, "spawn", blocked_spawn): + advancing = asyncio.create_task(self.factory.advance(run_id)) + await entered.wait() + advancing.cancel() + with self.assertRaises(asyncio.CancelledError): + await advancing + result = self.factory.status(run_id) + self.assertEqual(result["status"], "stopping") + self.assertTrue(result["nodes"]["a"]["needs_cleanup"]) + self.assertIsNone(result["nodes"]["a"]["child_id"]) + await self.factory.stop(run_id) + self.assertEqual(self.backend.cancelled, [result["nodes"]["a"]["name"]]) + await self.factory.advance(run_id) + self.assertEqual(self.backend.spawned, []) + + async def test_uncertain_spawn_does_not_retry_and_cleanup_failure_is_not_success(self): + run_id = await self.start(dag("a", "b")) + with patch.object(self.backend, "spawn", AsyncMock(side_effect=RuntimeError("connection lost"))) as spawn: + with patch.object(self.backend, "cancel", AsyncMock(side_effect=RuntimeError("unknown outcome"))): + result = await self.factory.advance(run_id) + again = await self.factory.advance(run_id) + self.assertEqual(spawn.await_count, 1) + self.assertEqual((result["status"], again["status"]), ("stopping", "stopping")) + self.assertIn("connection lost", result["nodes"]["a"]["error"]) + self.assertEqual(result["nodes"]["b"]["status"], "cancelled") + + async def test_collect_failure_does_not_lose_children_or_admit_more(self): + run_id = await self.start(dag("a", "b", max_parallel=1)) + await self.factory.advance(run_id) + before = self.factory.status(run_id) + with patch.object(self.backend, "inspect", AsyncMock(side_effect=RuntimeError("offline"))): + with self.assertRaisesRegex(RuntimeError, "offline"): + await self.factory.advance(run_id) + self.assertEqual(self.factory.status(run_id), before) + self.assertEqual(len(self.backend.spawned), 1) + self.backend.settle("child-0") + await self.factory.advance(run_id) + self.assertEqual(len(self.backend.spawned), 2) + + async def test_malformed_snapshot_is_rejected_atomically(self): + run_id = await self.start(dag("a", "b")) + await self.factory.advance(run_id) + before = self.factory.status(run_id) + a = ChildSnapshot("child-0", "completed", True, "A") + b = ChildSnapshot("child-1", "running", False) + invalid = [[], [a, a], [a, replace(b, child_id="stranger")], + [a, replace(b, status="unknown")], [a, replace(b, settled=True)], + [a, replace(b, output=None)]] + for reply in invalid: + with self.subTest(reply=reply), patch.object(self.backend, "inspect", AsyncMock(return_value=reply)): + with self.assertRaises(RuntimeError): + await self.factory.advance(run_id) + self.assertEqual(self.factory.status(run_id), before) + + async def test_wait_timeout_leaves_live_children_for_later_cells(self): + run_id = await self.start(dag("a")) + result = await self.factory.wait(run_id, timeout=0) + self.assertEqual((result["status"], self.backend.cancelled), ("running", [])) + self.backend.settle("child-0") + result = await self.factory.wait(run_id, timeout=0.1, poll_interval=0.01) + self.assertEqual(result["status"], "completed") + + async def test_wait_invalid_bounds_are_side_effect_free(self): + run_id = await self.start(dag("a")) + for options in ({"timeout": True}, {"timeout": -1}, {"timeout": 301}, + {"timeout": float("nan")}, {"poll_interval": float("inf")}, + {"poll_interval": 0}, {"poll_interval": "fast"}): + with self.subTest(options=options), self.assertRaises(ValueError): + await self.factory.wait(run_id, **options) + self.assertEqual(self.backend.spawned, []) + + async def test_oversized_output_and_rendered_prompts_fail_without_truncation(self): + run_id = await self.start(dag("a", {"id": "b", "prompt": "B", "needs": ["a"]})) + await self.factory.advance(run_id) + self.backend.settle("child-0", "x" * (MAX_OUTPUT_CHARS + 1)) + result = await self.factory.advance(run_id) + self.assertEqual((result["status"], result["nodes"]["a"]["output"], len(self.backend.spawned)), + ("failed", None, 1)) + run_id = await self.start(dag("a", {"id": "b", "prompt": "B" * 128000, "needs": ["a"]})) + await self.factory.advance(run_id) + self.backend.settle("child-1", "small") + result = await self.factory.advance(run_id) + self.assertEqual((result["status"], len(self.backend.spawned)), ("failed", 2)) + self.assertIn("rendered prompt", result["nodes"]["b"]["error"]) + + async def test_waiting_for_input_holds_capacity_without_blocking_local_status(self): + run_id = await self.start(dag("a", "b", max_parallel=1)) + await self.factory.advance(run_id) + self.backend.children["child-0"] = ChildSnapshot("child-0", "waiting_for_input", False) + result = await self.factory.advance(run_id) + self.assertEqual((result["nodes"]["a"]["status"], len(self.backend.spawned)), ("waiting_for_input", 1)) + await self.factory.stop(run_id) + self.assertEqual(self.backend.cancelled, ["child-0"]) + + async def test_retained_run_limit_requires_explicit_forget(self): + with patch("rlm.factory._MAX_RUNS", 1): + run_id = await self.start(dag("a")) + with self.assertRaisesRegex(RuntimeError, "limit"): + await self.start(dag("b")) + await self.factory.stop(run_id) + self.factory.forget(run_id) + await self.start(dag("b")) + + +class FactoryAdapterTests(unittest.IsolatedAsyncioTestCase): + async def test_missing_collect_fails_before_registering_or_spawning(self): + factory = FactoryNamespace() + with patch.object(rlm, "collect", None, create=True), patch.object(rlm, "spawn", AsyncMock()) as spawn: + with self.assertRaisesRegex(RuntimeError, "requires.*rlm.collect"): + await factory.start(dag("a"), enabled=True) + self.assertEqual(factory._runs, {}) + spawn.assert_not_awaited() + + async def test_adapter_uses_existing_public_helpers_and_preserves_options(self): + backend = RlmFactoryBackend() + node = FactoryNode("a", "Task", (), "provider/model", "high") + raw = SimpleNamespace(rlm_child_id="id", status="completed", settled=True, + answer_text="answer", error=None) + with patch.object(rlm, "collect", AsyncMock(return_value=[raw]), create=True) as collect: + with patch.object(rlm, "spawn", AsyncMock(return_value=SimpleNamespace(rlm_child_id="id"))) as spawn: + with patch.object(rlm, "delete_subagent", AsyncMock()) as cancel: + backend.check_available() + self.assertEqual(await backend.spawn(node, "Rendered", "unique"), "id") + self.assertEqual(await backend.inspect(["id"]), [ChildSnapshot("id", "completed", True, "answer")]) + await backend.cancel("id") + spawn.assert_awaited_once_with("Rendered", name="unique", model="provider/model", thinking="high") + collect.assert_awaited_once_with(targets=["id"], timeout_ms=0) + cancel.assert_awaited_once_with("id") + + async def test_namespace_and_module_share_one_factory(self): + self.assertIs(rlm.factory, rlm.rlm.factory) + self.assertIn("no loops", rlm.factory.help()) + + +if __name__ == "__main__": + unittest.main() diff --git a/crates/kernel/rlm-runtime/tests/test_factory_spec.py b/crates/kernel/rlm-runtime/tests/test_factory_spec.py new file mode 100644 index 00000000..d1c14c3f --- /dev/null +++ b/crates/kernel/rlm-runtime/tests/test_factory_spec.py @@ -0,0 +1,53 @@ +import copy +import unittest + +from rlm._factory_spec import FactoryNode, FactorySpec, MAX_NODES, validate + + +class FactorySpecTests(unittest.TestCase): + def test_defaults_and_topology_are_immutable(self): + spec = {"version": 1, "nodes": [ + {"id": "review", "prompt": "Review", "needs": ["build"], + "model": "gateway/org/model", "thinking": "high"}, + {"id": "build", "prompt": "Build"}, + ]} + original = copy.deepcopy(spec) + result = validate(spec) + self.assertEqual(result, FactorySpec(( + FactoryNode("review", "Review", ("build",), "gateway/org/model", "high"), + FactoryNode("build", "Build"), + ))) + self.assertEqual(spec, original) + spec["nodes"][0]["needs"].append("another") + self.assertEqual(result.nodes[0].needs, ("build",)) + + def test_invalid_specs(self): + good = {"version": 1, "nodes": [{"id": "one", "prompt": "Task"}]} + bad_specs = [None, [], {}, {**good, "version": True}, {**good, "version": 2}, + {**good, "machine": {}}, {**good, "max_parallel": True}, + {**good, "max_parallel": 0}, {**good, "max_parallel": 17}, + {**good, "max_parallel": 1.0}, {**good, "max_parallel": None}, + {**good, "failure_policy": "escalate"}, {**good, "nodes": []}, + {**good, "nodes": [{}]}, {**good, "nodes": [None]}, + {**good, "nodes": [good["nodes"][0]] * (MAX_NODES + 1)}, + {**good, "nodes": [good["nodes"][0]] * 2}] + node = good["nodes"][0] + for overrides in ({"id": "Bad"}, {"id": "../other"}, {"id": "x" * 33}, + {"prompt": " "}, {"prompt": None}, {"prompt": "x" * 128001}, + {"needs": "one"}, {"needs": [1]}, {"needs": ["missing"]}, + {"needs": ["one"]}, {"needs": ["one", "one"]}, + {"model": "model"}, {"model": "/model"}, {"model": 2}, + {"thinking": ""}, {"thinking": True}, {"retries": 1}, + {"foreach": []}, {"guard": {}}): + bad_specs.append({**good, "nodes": [{**node, **overrides}]}) + bad_specs.append({**good, "nodes": [ + {"id": "one", "prompt": "A", "needs": ["two"]}, + {"id": "two", "prompt": "B", "needs": ["one"]}, + ]}) + for spec in bad_specs: + with self.subTest(spec=str(spec)[:150]), self.assertRaises(ValueError): + validate(spec) + + +if __name__ == "__main__": + unittest.main() diff --git a/docs/factory-workflows.md b/docs/factory-workflows.md new file mode 100644 index 00000000..c43a97bf --- /dev/null +++ b/docs/factory-workflows.md @@ -0,0 +1,163 @@ +# Factory workflows: experimental DAG slice + +Devo Factory v1 composes ordinary direct child agents into a finite DAG. +It provides strict preflight validation, bounded parallel admission, all-parent +joins, plain-text result forwarding, explicit failure policies, local status, +and stop/cleanup. It does not replace the Rust session supervisor or grant +additional permissions. + +## Dependencies and boundaries + +The default backend needs the **subagent-controls implementation** providing +`rlm.collect(targets=[...], timeout_ms=0)` and its host route, in addition to the +existing `rlm.spawn` and `rlm.delete_subagent` helpers. On a baseline without +the Python collection API, `start` fails before creating a run or spawning +children. A mismatched/older host surfaces an error from its existing bridge. +The `FactoryBackend` protocol is isolated so this module and its scheduler +tests can be reviewed independently of that implementation. + +Every run requires explicit `enabled=True`. This is an API opt-in, not a new +persisted session setting, permission grant, or equivalent of Prime's +`/factory on` command. Existing child model selection, resource limits, +approval rules, session ownership, and cancellation remain host-owned. + +**This release advances only inside a live Python cell.** `start` validates and +registers a run without spawning; `advance` performs one pass; `wait` keeps +performing passes in that cell. There is no hidden asyncio task. Children +already spawned are host-owned and may keep running when the cell ends, but +their successors will not start until another `advance` or `wait` call. +This respects Devo's cell-bound host capability lifetime: an idle/detached +kernel request must not reuse an old or a later turn's permissions. + +Run state is in kernel memory, not the durable session journal. A kernel +restart loses the graph, outputs, and cleanup tracking, not necessarily the +host-owned children. After a restart, inspect `rlm.list_subagents()` / collect +and explicitly stop unwanted children. Do not claim automatic run recovery. + +## Example: parallel investigation, joined review + +Run in an active Devo Python cell after the collection API is installed: + +```python +spec = { + "version": 1, + "max_parallel": 2, + "failure_policy": "fail_fast", + "nodes": [ + {"id": "runtime", "prompt": "Inspect runtime behavior; report findings only."}, + {"id": "tests", "prompt": "Inspect relevant test coverage; report findings only."}, + { + "id": "review", + "needs": ["runtime", "tests"], + "prompt": "Combine both findings into a concise implementation recommendation.", + }, + ], +} +run = await rlm.factory.start(spec, enabled=True) +run_id = run["id"] +result = await rlm.factory.wait(run_id, timeout=30) +print(result) +``` + +If still running, use a later cell: + +```python +result = await rlm.factory.wait(run_id, timeout=30) +# Or advance once, returning without waiting for child completion: +result = await rlm.factory.advance(run_id) +# Local status alone does not refresh the host or start work: +print(rlm.factory.status(run_id)) +``` + +To stop: + +```python +result = await rlm.factory.stop(run_id) +print(result) +# Only after completed / failed / stopped: +rlm.factory.forget(run_id) +``` + +`stop` prevents further admissions, then closes known active children using the +existing host API. Completed child transcripts are retained. Cleanup errors +remain visible under each node's `cleanup_error`, and the run remains +`stopping` rather than claiming it stopped. Resolve the host issue and retry +`stop`. A running/stopping run cannot be forgotten. + +## Specification and execution semantics + +- Top-level fields: `version` (exact integer 1), `nodes`, `max_parallel` + (1–16, default 4), and `failure_policy` (`fail_fast` or `continue`). +- Nodes: unique `id` matching `[a-z][a-z0-9_-]{0,31}`, nonempty `prompt`, + optional `needs` list, optional exact `provider/model` selector, optional + `thinking`. Omitted model/thinking inherit the immediate parent through + `rlm.spawn`. The host still validates the resolved model and thinking level. +- Up to 128 nodes. Duplicate IDs/dependencies, missing predecessors, cycles, + unknown fields, and unsupported Prime machine options are rejected, never + silently ignored. Nodes may be declared in any order. +- A node starts only after **all** dependencies have completed successfully. + Predecessor text is appended as a JSON object keyed by predecessor ID and + labeled as task data, not additional authorization. No eval or template + execution occurs. A graph does not authorize its children to exceed the + user's original task. +- Each node is admitted at most once. There are no automatic retries. Admission + is serialized per run, including concurrent `advance`/`wait` callers. + `max_parallel` counts admitted children, including those waiting for input. + This does not increase the host's own global/session child limit. +- `fail_fast` stops admission and closes remaining live children after a node + fails. `continue` allows independent branches to finish; descendants of any + failed node become blocked. A run with any failed node ends `failed`. +- Child `failed`, `interrupted`, `canceled`, or `closed` never becomes success. + Missing, duplicate, or inconsistent collection replies raise without partly + applying observations or spawning more work. +- Outputs are limited to 32,000 characters per node and the rendered successor + prompt to 128,000 characters. Oversized content fails explicitly; it is not + silently truncated. There are at most 32 retained runs; explicitly forget a + terminal run to release its local state. +- `wait` timeout (0–300 seconds) is a polling budget, not a transaction deadline + or child cancellation. An in-flight spawn/collect/close may exceed that budget + so its acknowledgement is not lost. Interruption during spawn is treated as + uncertain admission; the unique child name is retained for cleanup, and the + node is never automatically respawned. If the host cannot resolve that name, + cleanup remains unresolved and the run stays `stopping`. +- Status is a detached local snapshot: `ready`, `running`, `stopping`, + `completed`, `failed`, or `stopped`. Host state may have changed since the last + advance; status is not a live provider-health or process-liveness promise. + +## Prime comparison and next safe milestones + +Reference: [Prime Factory skill at 74591cd](https://github.com/PrimeIntellect-ai/prime-agent/blob/74591cd/skills/factory/SKILL.md) +and [runtime implementation](https://github.com/PrimeIntellect-ai/prime-agent/blob/74591cd/prime-agent-runtime/src/rlm/factory.py). +Devo baseline inspected: [4228c9d](https://github.com/7df-lab/devo/commit/4228c9d). + +| Capability | Prime reference | This Devo slice | +| --- | --- | --- | +| Validated definitions | Harness `factory` entries; DAG compiles to state machine | Inline versioned DAG; no persistent harness kind | +| Fan-out / fan-in | State transitions and joins | Bounded fan-out; all-dependency success joins | +| Child ownership | Ordinary supervisor-owned children | Existing Rust-owned child sessions | +| Failure control | Retries, per-state policies, escalation | Fail-fast/continue; no admission retries | +| Complex transitions | Guards, bounded cycles, foreach, residents | Rejected as unsupported | +| Run lifetime | Background kernel driver; registry lost on kernel restart | Explicit active-cell advance/wait; volatile registry | +| User controls | Persistent gate, graph/watch, stop/resume, TUI factory page | Explicit API opt-in, local status, stop/cleanup | +| Machine library | MACHINE.md seeds and import/export | Not implemented | + +Next milestones should be independently reviewed: (1) integrate and exercise +the real collection API end to end; (2) design a host-owned durable run record +and bounded background scheduler with explicit permission/session lifetime; +(3) add Native-only run status/control events and UI; (4) add guarded bounded +loops, validated retry budgets, and machine-library storage. A background +Python task with a stale host bridge is not a substitute for milestone 2. + +## Verification + +```sh +PYTHONPATH=crates/kernel/rlm-runtime/src python -m unittest discover \ + -s crates/kernel/rlm-runtime/tests -p 'test_factory*.py' -v +``` + +The scheduler tests use an injected backend and cover validation, joins, +concurrency, failure propagation, malformed observations, uncertain admissions, +interruption, stop races/retries, timeouts, capacity, and output bounds. Adapter +tests verify the public helper calls. These are not a substitute for a real +provider-backed Devo session test, nor evidence of background continuation, +restart recovery, UI support, or complete Prime parity. From d2a2ca8d988c5c981ed691c8a7d56c3ac16a825c Mon Sep 17 00:00:00 2001 From: wangtsiao Date: Sun, 8 Nov 2026 23:51:01 +0800 Subject: [PATCH 2/5] fix(factory): preserve waiting children and surface explicit resume --- .../rlm-runtime/src/rlm/_factory_backend.py | 4 +++- crates/kernel/rlm-runtime/src/rlm/factory.py | 22 ++++++++++++------ .../kernel/rlm-runtime/tests/test_factory.py | 23 ++++++++++++++++--- docs/factory-workflows.md | 7 +++++- 4 files changed, 44 insertions(+), 12 deletions(-) diff --git a/crates/kernel/rlm-runtime/src/rlm/_factory_backend.py b/crates/kernel/rlm-runtime/src/rlm/_factory_backend.py index e7627e38..d9b5b70a 100644 --- a/crates/kernel/rlm-runtime/src/rlm/_factory_backend.py +++ b/crates/kernel/rlm-runtime/src/rlm/_factory_backend.py @@ -15,6 +15,7 @@ class ChildSnapshot: settled: bool output: str = "" error: str | None = None + requires_resume: bool = False class FactoryBackend(Protocol): @@ -59,7 +60,8 @@ async def inspect(self, child_ids: list[str]) -> list[ChildSnapshot]: results = await rlm.collect(targets=child_ids, timeout_ms=0) return [ChildSnapshot(item.rlm_child_id, item.status, item.settled, - item.answer_text or "", item.error) for item in results] + item.answer_text or "", item.error, + getattr(item, "requires_resume", False)) for item in results] async def cancel(self, target: str) -> None: import rlm diff --git a/crates/kernel/rlm-runtime/src/rlm/factory.py b/crates/kernel/rlm-runtime/src/rlm/factory.py index 52690217..715609ab 100644 --- a/crates/kernel/rlm-runtime/src/rlm/factory.py +++ b/crates/kernel/rlm-runtime/src/rlm/factory.py @@ -42,6 +42,7 @@ This is not Prime machine compatibility: no loops/guards/foreach/residents, retries, durable specs/runs, restart resume, UI or background advancement. wait timeout leaves children running; call wait/advance again or explicitly stop. +waiting_for_resume returns from wait immediately; explicitly resume the child or stop. Interruption does not imply cancellation. Inspect status and call stop if needed. Host/transport failures are surfaced, never converted to success or respawned. When status is stopping, inspect cleanup_error and retry stop; do not forget it. @@ -116,7 +117,7 @@ async def advance(self, run_id: str) -> dict[str, Any]: return self.status(run_id) run.status = "running" active = {node.child_id: node for node in run.nodes.values() - if node.status in ("running", "waiting_for_input")} + if node.status in ("running", "waiting_for_input", "waiting_for_resume")} if active: # Do not partially apply malformed or incomplete replies. A read # failure leaves all child IDs retained and can be safely retried. @@ -125,16 +126,21 @@ async def advance(self, run_id: str) -> dict[str, Any]: if len(ids) != len(active) or set(ids) != set(active): raise RuntimeError("Factory collect returned missing or duplicate children") for item in snapshots: - if item.status not in {"running", "waiting_for_input", "completed", "failed", + if item.status not in {"running", "waiting_for_input", "waiting_for_resume", "completed", "failed", "interrupted", "canceled", "closed"}: raise RuntimeError(f"Factory collect returned unknown status {item.status!r}") - if type(item.settled) is not bool or item.settled != ( - item.status not in {"running", "waiting_for_input"} - ) or not isinstance(item.output, str): + if (type(item.settled) is not bool + or type(item.requires_resume) is not bool + or (item.requires_resume and item.status != "waiting_for_resume") + or (item.status in {"running", "waiting_for_resume"} and item.settled) + or (item.status not in {"running", "waiting_for_input", "waiting_for_resume"} and not item.settled) + or not isinstance(item.output, str)): raise RuntimeError("Factory collect returned inconsistent child state") for item in snapshots: node = active[item.child_id] - if not item.settled: + # The host may settle a turn while its child still waits + # for input. That is not workflow success or a free slot. + if not item.settled or item.status == "waiting_for_input": node.status = item.status continue node.needs_cleanup = False @@ -201,6 +207,8 @@ async def advance(self, run_id: str) -> dict[str, Any]: await self._cleanup(run) elif all(node.status in {"completed", "failed", "blocked"} for node in run.nodes.values()): run.status = "failed" if run.failed else "completed" + elif any(node.status == "waiting_for_resume" for node in run.nodes.values()): + run.status = "waiting_for_resume" return self.status(run_id) @staticmethod @@ -254,7 +262,7 @@ async def wait(self, run_id: str, *, timeout: float = 30, poll_interval: float = while True: result = await self.advance(run_id) remaining = deadline - asyncio.get_running_loop().time() - if result["status"] in _TERMINAL or result["status"] == "stopping" or remaining <= 0: + if result["status"] in _TERMINAL | {"stopping", "waiting_for_resume"} or remaining <= 0: return result await asyncio.sleep(min(poll_interval, remaining)) diff --git a/crates/kernel/rlm-runtime/tests/test_factory.py b/crates/kernel/rlm-runtime/tests/test_factory.py index 3f049bee..9beecf36 100644 --- a/crates/kernel/rlm-runtime/tests/test_factory.py +++ b/crates/kernel/rlm-runtime/tests/test_factory.py @@ -250,9 +250,10 @@ async def test_oversized_output_and_rendered_prompts_fail_without_truncation(sel async def test_waiting_for_input_holds_capacity_without_blocking_local_status(self): run_id = await self.start(dag("a", "b", max_parallel=1)) await self.factory.advance(run_id) - self.backend.children["child-0"] = ChildSnapshot("child-0", "waiting_for_input", False) - result = await self.factory.advance(run_id) - self.assertEqual((result["nodes"]["a"]["status"], len(self.backend.spawned)), ("waiting_for_input", 1)) + for settled in (False, True): + self.backend.children["child-0"] = ChildSnapshot("child-0", "waiting_for_input", settled) + result = await self.factory.advance(run_id) + self.assertEqual((result["nodes"]["a"]["status"], len(self.backend.spawned)), ("waiting_for_input", 1)) await self.factory.stop(run_id) self.assertEqual(self.backend.cancelled, ["child-0"]) @@ -265,6 +266,22 @@ async def test_retained_run_limit_requires_explicit_forget(self): self.factory.forget(run_id) await self.start(dag("b")) + async def test_restored_child_needing_resume_returns_from_wait_without_replaying(self): + run_id = await self.start(dag("a", "b", max_parallel=1)) + await self.factory.advance(run_id) + self.backend.children["child-0"] = ChildSnapshot( + "child-0", "waiting_for_resume", False, requires_resume=True) + # A very long polling budget must return on this actionable state. + result = await asyncio.wait_for(self.factory.wait(run_id, timeout=300), timeout=1) + self.assertEqual((result["status"], len(self.backend.spawned)), ("waiting_for_resume", 1)) + with self.assertRaises(ValueError): + self.factory.forget(run_id) + self.backend.settle("child-0") + result = await self.factory.advance(run_id) + self.assertEqual((result["status"], len(self.backend.spawned)), ("running", 2)) + await self.factory.stop(run_id) + self.assertEqual(self.backend.cancelled, ["child-1"]) + class FactoryAdapterTests(unittest.IsolatedAsyncioTestCase): async def test_missing_collect_fails_before_registering_or_spawning(self): diff --git a/docs/factory-workflows.md b/docs/factory-workflows.md index c43a97bf..a8ee503e 100644 --- a/docs/factory-workflows.md +++ b/docs/factory-workflows.md @@ -104,6 +104,11 @@ remain visible under each node's `cleanup_error`, and the run remains is serialized per run, including concurrent `advance`/`wait` callers. `max_parallel` counts admitted children, including those waiting for input. This does not increase the host's own global/session child limit. +- A host child requiring explicit resume is reported as `waiting_for_resume`. + `wait` returns that actionable state immediately instead of using its entire + polling budget. The child keeps its slot; no automatic replay occurs. Resume + the child through the existing session controls, then advance again, or stop + the workflow. A settled turn waiting for input also keeps its slot. - `fail_fast` stops admission and closes remaining live children after a node fails. `continue` allows independent branches to finish; descendants of any failed node become blocked. A run with any failed node ends `failed`. @@ -120,7 +125,7 @@ remain visible under each node's `cleanup_error`, and the run remains uncertain admission; the unique child name is retained for cleanup, and the node is never automatically respawned. If the host cannot resolve that name, cleanup remains unresolved and the run stays `stopping`. -- Status is a detached local snapshot: `ready`, `running`, `stopping`, +- Status is a detached local snapshot: `ready`, `running`, `waiting_for_resume`, `stopping`, `completed`, `failed`, or `stopped`. Host state may have changed since the last advance; status is not a live provider-health or process-liveness promise. From 05495fc477b5ed226976a659517e447fe2232ff1 Mon Sep 17 00:00:00 2001 From: wangtsiao Date: Mon, 9 Nov 2026 00:23:41 +0800 Subject: [PATCH 3/5] feat(rlm): add bounded subagent controls and independent root sessions --- crates/core/prompts/rlm/repl_control.md | 4 +- crates/core/src/query/event.rs | 2 + crates/core/src/query/mod.rs | 8 + crates/core/src/rlm_prompts.rs | 32 +- crates/kernel/rlm-runtime/README.md | 10 +- crates/kernel/rlm-runtime/src/rlm/__init__.py | 31 +- crates/kernel/rlm-runtime/src/rlm/controls.py | 113 ++++ crates/kernel/rlm-runtime/src/rlm/harness.py | 7 +- .../kernel/rlm-runtime/tests/test_controls.py | 67 +++ crates/server/src/persistence.rs | 55 +- crates/server/src/runtime.rs | 8 + crates/server/src/runtime/agents.rs | 18 +- crates/server/src/runtime/goal_background.rs | 15 + crates/server/src/runtime/handlers/session.rs | 110 +++- crates/server/src/runtime/kernel_host.rs | 14 +- .../server/src/runtime/kernel_host_bridge.rs | 2 + .../src/runtime/kernel_host_controls.rs | 421 ++++++++++++++ .../server/src/runtime/kernel_host_session.rs | 329 +++++++++++ crates/server/src/runtime/session_cache.rs | 3 + crates/server/src/runtime/session_deletion.rs | 3 + .../src/runtime/turn_exec/approval_resume.rs | 1 + crates/server/src/runtime/turn_exec/query.rs | 3 + crates/server/src/runtime/turn_reservation.rs | 5 +- crates/server/tests/child_model_selection.rs | 130 +++++ .../server/tests/subagent_python_controls.rs | 513 ++++++++++++++++++ docs/rlm-orchestration.md | 93 ++++ 26 files changed, 1949 insertions(+), 48 deletions(-) create mode 100644 crates/kernel/rlm-runtime/src/rlm/controls.py create mode 100644 crates/kernel/rlm-runtime/tests/test_controls.py create mode 100644 crates/server/src/runtime/kernel_host_controls.rs create mode 100644 crates/server/src/runtime/kernel_host_session.rs create mode 100644 crates/server/tests/subagent_python_controls.rs create mode 100644 docs/rlm-orchestration.md diff --git a/crates/core/prompts/rlm/repl_control.md b/crates/core/prompts/rlm/repl_control.md index 7ce588fc..ca1bc3fb 100644 --- a/crates/core/prompts/rlm/repl_control.md +++ b/crates/core/prompts/rlm/repl_control.md @@ -18,7 +18,7 @@ Continual harness state is available as `rlm.harness` and `rlm.get_harness_state Terminology: continual harness names the persisted prompt, memory, skill, and subagent layer; RLM names the runtime, Python REPL kernel, and native call interface exposed to the model. -RLM-native call contract: installed Python skills are pre-imported modules. Read the matching SKILL.md and call its documented function, such as `await .(...)`; when a CLI exists, use ` ...` from shell. Continual harness skill entries are Python REPL skills with an explicit Python `reference` and `arguments` contract. Spawn a reusable delegation spec with `await rlm.spawn('sub-task', name='worker')` (host wire action `rlm.run`); admission returns a child handle immediately. Final assistant output is included in the child completion record; use an available messaging capability for timely updates and files for substantial artifacts. Results are never an `rlm.spawn()` return value. Do not invent non-native wrappers such as `call_skill(...)` or `run_subagent(...)`. Do not call `rlm.create_session` — daemon-backed top-level sessions are out of scope and denied by the host. +RLM-native call contract: installed Python skills are pre-imported modules. Read the matching SKILL.md and call its documented function, such as `await .(...)`; when a CLI exists, use ` ...` from shell. Continual harness skill entries are Python REPL skills with an explicit Python `reference` and `arguments` contract. Spawn a reusable delegation spec with `await rlm.spawn('sub-task', name='worker')` (host wire action `rlm.run`); admission returns a child handle immediately. Final assistant output is included in the child completion record; use an available messaging capability for timely updates and files for substantial artifacts. Results are never an `rlm.spawn()` return value. Do not invent non-native wrappers such as `call_skill(...)` or `run_subagent(...)`. Durable Build roots can call `await rlm.create_session('task', name='independent')` for a separate visible root in the same working directory. It inherits permissions/model settings, has its own journal and no parent lifecycle, and is not addressable by collect/delete_subagent. Child, ephemeral, and Plan callers are rejected. Child model selection: `await rlm.find_models(query='', limit=8)` lists enabled, configured, routable models with `provider`, `id`, `name`, `selector`, and `thinking_levels`; it does not probe live health or credits. Use an exact returned selector with `await rlm.spawn('sub-task', name='worker', model=chosen.selector, thinking='high')`, choosing a supported thinking level. Model IDs may contain slashes. Omitted model/thinking (or `None`) inherit the immediate parent's values. Invalid selectors or unsupported explicit or inherited thinking fail admission without silent fallback; when changing models, check its supported levels. `limit` is an integer from 1 to 100. Catalog descriptors contain no credentials. @@ -26,3 +26,5 @@ RLM host APIs are called from Python through `ipython`, not as separate model to - `await rlm.update_plan([{"step": "Inspect", "status": "in_progress"}])` publishes a session plan during execution. Plan Mode should keep its draft in Python and present the final `` instead. - `await rlm.request_user_input([{"id": "choice", "header": "Choice", "question": "Which option?", "options": [{"label": "A", "description": "..."}, {"label": "B", "description": "..."}]}])` uses the session's question UI. - `await rlm.web_search("query")` and `await rlm.web_fetch("https://...")` use Rust host operations; `rlm.web_search` requires local search configuration. When provider-hosted web tools are configured, use their provider-native capability directly; those are separate from the regular function-tool list. + +Python orchestration controls: `await rlm.collect(targets=None, timeout_ms=0)` snapshots direct children; a positive timeout (up to 300000 ms) waits only until completion, recovery is required, or the deadline. Timeout returns partial snapshots and never cancels work. Results retain status, settled, answer_text, answer_preview, answer_truncated, error, progress_note, and requires_resume. A saved unfinished turn with no live execution reports waiting_for_resume; explicitly recover it rather than replaying blindly. `await rlm.progress_note('running tests')` lets children publish non-steering progress, at most 512 UTF-16 units and one accepted note per 10 seconds. `await rlm.end_turn()` finishes normally after the current tool batch is committed, without an extra model call or canceling children. The rest of the cell and other calls in that batch still run. diff --git a/crates/core/src/query/event.rs b/crates/core/src/query/event.rs index 9b0a8600..826cdb9b 100644 --- a/crates/core/src/query/event.rs +++ b/crates/core/src/query/event.rs @@ -141,6 +141,8 @@ pub struct QueryOptions { /// Acknowledged journal for durable sessions; absent for ephemeral callers. pub journal: Option>, pub cancel_token: Option, + /// Graceful host request: finish after the current durable tool batch. + pub end_turn_requested: Option>, /// Optional provider used only for compaction summaries. Servers use this /// seam to attach Compaction metering without misclassifying the main /// streaming query as compaction overhead. diff --git a/crates/core/src/query/mod.rs b/crates/core/src/query/mod.rs index 16430fcc..9d177d44 100644 --- a/crates/core/src/query/mod.rs +++ b/crates/core/src/query/mod.rs @@ -1323,6 +1323,14 @@ pub async fn query( session.end_turn(); return Err(AgentError::Aborted); } + if options + .end_turn_requested + .as_ref() + .is_some_and(|requested| requested.load(std::sync::atomic::Ordering::Acquire)) + { + session.end_turn(); + return Ok(()); + } } } diff --git a/crates/core/src/rlm_prompts.rs b/crates/core/src/rlm_prompts.rs index 25681931..05a46674 100644 --- a/crates/core/src/rlm_prompts.rs +++ b/crates/core/src/rlm_prompts.rs @@ -1,7 +1,7 @@ //! Immutable RLM system-prompt doctrine for Devo sessions. //! //! Kernel-surface RLM prompt set (reference-agent design, Devo branding) with explicit -//! denial of daemon `rlm.create_session`. Catalog skill progressive disclosure +//! bounded subagent and independent-root controls. Catalog skill progressive disclosure //! uses XML (`format_catalog_skills_xml`); the markdown skill body from //! `devo_skills::render_available_skills_body` remains for non-RLM surfaces. @@ -241,12 +241,14 @@ pub fn build_rlm_base_prompt(options: &RlmPromptOptions) -> String { ); } - // Explicit deny: never teach daemon create_session (depth-0 path is out of scope). - parts.push(String::new()); - parts.push( - "Do not call `rlm.create_session`. Daemon-backed top-level session creation is denied; use `await rlm.spawn(...)` only for child agents." - .to_string(), - ); + if has_ipython { + parts.push(String::new()); + parts.push(if options.depth == 0 { + "Durable Build roots may call `await rlm.create_session('task', name='independent')` to create a separately visible root in the same workspace. It inherits safety and model settings but has its own journal, no parent linkage, and no collect/child lifecycle. Plan and ephemeral callers are rejected." + } else { + "Child sessions cannot call `rlm.create_session`; use `await rlm.spawn(...)` for direct descendants." + }.to_string()); + } if options.allow_recursion && has_ipython { parts.push(String::new()); @@ -259,7 +261,7 @@ pub fn build_rlm_base_prompt(options: &RlmPromptOptions) -> String { .to_string(), ); parts.push( - "Children inherit your model and thinking level. This host does not support `model` or `thinking` overrides on `rlm.spawn`; omit both options." + "Children inherit your model and thinking level by default. Discover exact model selectors and supported thinking levels with `await rlm.find_models()`, then optionally pass `model=chosen.selector` and `thinking` to `rlm.spawn`; unsupported selections fail without fallback." .to_string(), ); if has_agent_observe { @@ -290,12 +292,12 @@ pub fn build_rlm_base_prompt(options: &RlmPromptOptions) -> String { ); } else { parts.push( - "Inspect files a child wrote when you need to collect its work without an observation capability." + "Use `await rlm.collect(child, timeout_ms=0)` to inspect retained child results without an observation capability, or read the files the child wrote." .to_string(), ); } parts.push( - "Spawn independent children in separate calls and end your turn instead of awaiting completion. Multiple replies may arrive over multiple turns. Delete a direct child explicitly with `await rlm.delete_subagent(child)` when it is no longer needed." + "Use `await rlm.collect(children, timeout_ms=1000)` for a short bounded wait: timeout returns partial snapshots without cancellation. Check `settled`, `requires_resume`, and `answer_truncated`; saved unfinished work requires explicit recovery, not automatic replay. Children can report `await rlm.progress_note('running tests')` without steering the parent. `await rlm.end_turn()` ends normally after the current tool batch while children continue. Delete a direct child explicitly with `await rlm.delete_subagent(child)` when it is no longer needed." .to_string(), ); } @@ -424,7 +426,7 @@ mod tests { use super::*; - /// Verifies: root RLM base prompt carries Devo doctrine strings and denies create_session. + /// Root prompt teaches the supported controls and their safety boundaries. #[test] fn build_rlm_base_prompt_contains_key_doctrine() { let mut options = RlmPromptOptions::root("/repo", "/tmp/messages.jsonl"); @@ -441,7 +443,11 @@ mod tests { assert!(prompt.contains("rlm.spawn")); assert!(prompt.contains("rlm.harness")); assert!(prompt.contains("rlm.create_session")); - assert!(prompt.contains("denied")); + assert!(prompt.contains("Durable Build roots")); + assert!(prompt.contains("rlm.collect")); + assert!(prompt.contains("rlm.progress_note")); + assert!(prompt.contains("rlm.end_turn")); + assert!(!prompt.contains("Daemon-backed top-level session creation is denied")); assert!(!prompt.contains("daemon-backed depth-0 session")); assert!(prompt.contains("Recursive agent depth: 0")); assert!(prompt.contains("await refine.run()")); @@ -506,7 +512,7 @@ mod tests { assert!(prompt.contains("Kernel snapshots are separate")); assert!(prompt.contains("`rlm.shell()` defaults to the session workspace")); assert!(prompt.contains("Bash on Unix and PowerShell on Windows")); - assert!(prompt.contains("does not support `model` or `thinking` overrides")); + assert!(prompt.contains("unsupported selections fail without fallback")); assert!(prompt.contains("never relax relative to the parent") || options.depth == 0); assert!( prompt.contains("Never use direct Python process or filesystem APIs to bypass") diff --git a/crates/kernel/rlm-runtime/README.md b/crates/kernel/rlm-runtime/README.md index 220919f8..01ea8232 100644 --- a/crates/kernel/rlm-runtime/README.md +++ b/crates/kernel/rlm-runtime/README.md @@ -167,7 +167,7 @@ The main groups are: | `rlm.web_search`, `rlm.web_fetch` | Host-mediated network access. | | `rlm.update_plan`, `rlm.request_user_input` | Session UI integration. | | `rlm.read`, `rlm.write`, `rlm.read_once`, `rlm.write_once` | Direct file I/O, with host-mediated fallback on OS denial. | -| `rlm.spawn`, `rlm.create_session`, `rlm.find_models`, `rlm.list_subagents`, `rlm.delete_subagent` | Recursive subagent control. | +| `rlm.spawn`, `rlm.create_session`, `rlm.find_models`, `rlm.list_subagents`, `rlm.delete_subagent`, `rlm.collect`, `rlm.progress_note`, `rlm.end_turn` | Recursive subagent control. | | `rlm.harness`, `rlm.get_harness_state` | Cached reads and async host-owned harness CRUD. | | `rlm.emit` | Ship a MIME-keyed display event. | @@ -190,8 +190,12 @@ if chosen is not None: ``` `rlm.spawn` returns when the child is admitted, not when it finishes. The child -runs as a separate session; results arrive through `agent_message` replies or -files, usually on a later turn. +runs as a separate session. Use `await rlm.collect(reviewer, timeout_ms=10000)` +for retained typed results; timeout returns partial snapshots without cancellation. +Children can report `await rlm.progress_note("running tests")` without steering the +parent. `await rlm.end_turn()` finishes normally after the current tool batch. +`create_session` creates a separate durable root in the same workspace. See +[orchestration semantics](../../../docs/rlm-orchestration.md) for limits and recovery. Omitted `model` and `thinking` (or `None`) inherit the immediate parent's values. An unsupported inherited thinking level fails just like an explicit unsupported diff --git a/crates/kernel/rlm-runtime/src/rlm/__init__.py b/crates/kernel/rlm-runtime/src/rlm/__init__.py index 9fff1294..806e48ff 100644 --- a/crates/kernel/rlm-runtime/src/rlm/__init__.py +++ b/crates/kernel/rlm-runtime/src/rlm/__init__.py @@ -50,6 +50,7 @@ class RLMSubagent: session_name: str session_dir: Path status: str + progress_note: str | None = None def _spawn_handle_from_payload(payload: Any) -> RLMSpawnHandle: @@ -327,13 +328,23 @@ async def create_session( thinking: str | None = None, cwd: str | None = None, ) -> RLMCreateSessionHandle: - """Create and prompt a resident depth-0 daemon session. + """Create and prompt an independent, durable root in the same workspace. - Only daemon-backed depth-0 sessions support this operation. The optional - arguments set the session name, model, thinking level, and working directory. + Requires a durable root in Build mode. Model and safety settings inherit; + an explicit model uses an exact selector from find_models. The result is + visible in session/list and has its own journal, with no parent lifecycle. + An explicit cwd must resolve to the current working directory. """ if not isinstance(prompt, str): raise TypeError(f"prompt must be str, got {type(prompt).__name__}") + if not prompt.strip(): + raise ValueError("prompt must be a non-empty string") + for label, value in (("name", name), ("model", model), ("thinking", thinking), ("cwd", cwd)): + if value is not None: + if not isinstance(value, str): + raise TypeError(f"{label} must be str") + if not value.strip() or value.strip() != value: + raise ValueError(f"{label} must be a nonempty string without surrounding whitespace") kwargs: dict[str, Any] = {} if name is not None: kwargs["name"] = name @@ -397,6 +408,7 @@ def _subagent_from_payload(payload: Any, operation: str = "rlm.list_subagents") session_name=session_name, session_dir=Path(session_dir), status=status, + progress_note=payload.get("progress_note"), ) @@ -484,6 +496,15 @@ async def update_plan( async def request_user_input(self, questions: list[dict[str, Any]]) -> dict[str, Any]: return await request_user_input(questions) + async def collect(self, targets=None, *, timeout_ms: int = 0) -> list[RLMChildResult]: + return await collect(targets, timeout_ms=timeout_ms) + + async def progress_note(self, message: str) -> RLMProgressNoteResult: + return await progress_note(message) + + async def end_turn(self) -> dict[str, Any]: + return await end_turn() + async def list_subagents(self) -> list[RLMSubagent]: return await list_subagents() @@ -556,3 +577,7 @@ def __getattr__(name: str) -> Any: # noqa: D401 - module-level lazy attr hook if name == "run": raise AttributeError(_RENAMED_RUN_MESSAGE) raise AttributeError(f"module {__name__!r} has no attribute {name!r}") + +# Imported after handle definitions to keep the orchestration parser independent. +from .controls import RLMChildResult, RLMProgressNoteResult, collect, progress_note, end_turn +__all__ += ["RLMChildResult", "RLMProgressNoteResult", "collect", "progress_note", "end_turn"] diff --git a/crates/kernel/rlm-runtime/src/rlm/controls.py b/crates/kernel/rlm-runtime/src/rlm/controls.py new file mode 100644 index 00000000..53ee5ce2 --- /dev/null +++ b/crates/kernel/rlm-runtime/src/rlm/controls.py @@ -0,0 +1,113 @@ +"""Typed, bounded Python orchestration over the host's session lifecycle.""" +from __future__ import annotations + +from dataclasses import dataclass +from pathlib import Path +from typing import Any + + +@dataclass(frozen=True) +class RLMChildResult: + rlm_child_id: str + session_name: str + session_dir: Path | None + status: str + settled: bool + answer_text: str | None = None + answer_preview: str | None = None + error: str | None = None + progress_note: str | None = None + answer_truncated: bool = False + requires_resume: bool = False + + +@dataclass(frozen=True) +class RLMProgressNoteResult: + accepted: bool + retry_after_ms: int | None = None + + +async def collect(targets=None, *, timeout_ms: int = 0) -> list[RLMChildResult]: + """Snapshot direct children, waiting at most 300000 ms for all to settle. + + Accepts a name, spawn handle, subagent row, or a list/tuple of these. + None/[] snapshots all direct children at admission. Timeout returns partial + snapshots, never cancels children or consumes their retained results. + """ + from . import RLMSpawnHandle, RLMSubagent, host_request + if isinstance(timeout_ms, bool) or not isinstance(timeout_ms, int): + raise TypeError("timeout_ms must be an int") + if not 0 <= timeout_ms <= 300000: + raise ValueError("timeout_ms must be between 0 and 300000") + if targets is None: + targets = [] + elif isinstance(targets, (str, RLMSpawnHandle, RLMSubagent)): + targets = [targets] + elif not isinstance(targets, (list, tuple)): + raise TypeError("targets must be a target or a list/tuple of targets") + selectors = [] + for target in targets: + if isinstance(target, (RLMSpawnHandle, RLMSubagent)): + target = target.rlm_child_id + if not isinstance(target, str): + raise TypeError("each target must be a name, spawn handle, or subagent") + if not target.strip(): + raise ValueError("target must not be empty") + selectors.append(target.strip()) + payload = await host_request("rlm.collect", {"targets": selectors, "timeout_ms": timeout_ms}) + if not isinstance(payload, dict): + raise RuntimeError("rlm.collect returned an invalid payload") + entries = payload.get("results") + if not isinstance(entries, list): + raise RuntimeError("rlm.collect returned an invalid results list") + results = [] + for entry in entries: + if not isinstance(entry, dict): + raise RuntimeError("rlm.collect returned an invalid entry") + if any(not isinstance(entry.get(k), str) or not entry[k] for k in ("rlm_child_id", "session_name", "status")): + raise RuntimeError("rlm.collect entry is missing identity/status") + if not isinstance(entry.get("settled"), bool): + raise RuntimeError("rlm.collect entry has invalid settled flag") + for key in ("answer_truncated", "requires_resume"): + if not isinstance(entry.get(key, False), bool): + raise RuntimeError(f"rlm.collect entry has invalid {key} flag") + for key in ("session_dir", "answer_text", "answer_preview", "error", "progress_note"): + if entry.get(key) is not None and not isinstance(entry[key], str): + raise RuntimeError(f"rlm.collect entry has invalid {key}") + results.append(RLMChildResult( + rlm_child_id=entry["rlm_child_id"], session_name=entry["session_name"], + status=entry["status"], settled=entry["settled"], + session_dir=Path(entry["session_dir"]) if entry.get("session_dir") else None, + answer_text=entry.get("answer_text"), answer_preview=entry.get("answer_preview"), + error=entry.get("error"), progress_note=entry.get("progress_note"), + answer_truncated=entry.get("answer_truncated", False), + requires_resume=entry.get("requires_resume", False), + )) + return results + + +async def progress_note(message: str) -> RLMProgressNoteResult: + """Update a child's progress without steering its parent; throttled to 10s.""" + from . import host_request + if not isinstance(message, str): + raise TypeError("message must be str") + message = message.strip() + if not message or len(message.encode("utf-16-le", "surrogatepass")) // 2 > 512: + raise ValueError("message must contain 1 to 512 UTF-16 code units") + payload = await host_request("rlm.progress.note", {"message": message}) + if not isinstance(payload, dict): + raise RuntimeError("rlm.progress.note returned an invalid payload") + accepted, retry = payload.get("accepted"), payload.get("retry_after_ms") + if not isinstance(accepted, bool) or (retry is not None and (isinstance(retry, bool) or not isinstance(retry, int) or retry < 0)): + raise RuntimeError("rlm.progress.note returned an invalid result") + return RLMProgressNoteResult(accepted, retry) + + +async def end_turn() -> dict[str, Any]: + """Request normal completion after this tool batch, without canceling children. + + The remainder of this cell and other calls in the batch still execute. No + additional model response is requested. Idle/obsolete host bridges are rejected. + """ + from . import host_request + return await host_request("rlm.end_turn", {}) diff --git a/crates/kernel/rlm-runtime/src/rlm/harness.py b/crates/kernel/rlm-runtime/src/rlm/harness.py index 5f5d629a..e53ac0df 100644 --- a/crates/kernel/rlm-runtime/src/rlm/harness.py +++ b/crates/kernel/rlm-runtime/src/rlm/harness.py @@ -633,7 +633,12 @@ def overview(self, *, max_entries_per_kind: int = 20, global_: bool = False, **k "and thinking_levels, not credentials or live health guarantees. Pass model=chosen.selector and a supported " "thinking level to spawn; omitted options inherit the immediate parent. Invalid selectors or unsupported " "explicit/inherited thinking fail without fallback. The handle contains admission metadata " - "and never the child's answer. Results arrive through child completion, explicit agent_message replies or " + "and never the child's answer. Use await rlm.collect(handle, timeout_ms=10000) for retained typed results " + "(0 is nonblocking, timeout returns partial snapshots, inspect settled). Children can call " + "await rlm.progress_note('running tests') without steering the parent. await rlm.end_turn() completes " + "after the current tool batch without canceling children. Durable Build roots can use " + "await rlm.create_session('task', name='independent') for a separate visible root in the same workspace. " + "Results also arrive through child completion, explicit agent_message replies or " "files; children reply with await agent_message.send(message, receiver_role='parent'). Use " "await rlm.list_subagents() to recover direct children. A recovered child uses child.session_name " "(the spawn handle uses handle.name); send await agent_message.send(..., " diff --git a/crates/kernel/rlm-runtime/tests/test_controls.py b/crates/kernel/rlm-runtime/tests/test_controls.py new file mode 100644 index 00000000..2f3ac0e7 --- /dev/null +++ b/crates/kernel/rlm-runtime/tests/test_controls.py @@ -0,0 +1,67 @@ +import unittest +from pathlib import Path +from unittest.mock import AsyncMock, patch +import rlm + + +class ControlsTests(unittest.IsolatedAsyncioTestCase): + async def test_collect_mixed_targets_and_retained_partial_result(self): + entry = dict(rlm_child_id="id", session_name="worker", session_dir="artifacts", status="running", settled=False, progress_note="testing") + host = AsyncMock(return_value={"results": [entry]}) + handle = rlm.RLMSpawnHandle("id", "worker", Path("artifacts"), "model") + with patch.object(rlm, "host_request", host): + result = await rlm.rlm.collect([handle, "worker"], timeout_ms=25) + self.assertEqual(result, [rlm.RLMChildResult("id", "worker", Path("artifacts"), "running", False, progress_note="testing")]) + host.assert_awaited_once_with("rlm.collect", {"targets": ["id", "worker"], "timeout_ms": 25}) + + async def test_invalid_wait_and_target_do_not_call_host(self): + host = AsyncMock() + with patch.object(rlm, "host_request", host): + for timeout in (True, None, "1", 1.0): + with self.assertRaises(TypeError): + await rlm.collect(timeout_ms=timeout) + for timeout in (-1, 300001): + with self.assertRaises(ValueError): + await rlm.collect(timeout_ms=timeout) + for targets in (False, [False], {}): + with self.assertRaises(TypeError): + await rlm.collect(targets) + with self.assertRaises(ValueError): + await rlm.collect([" "]) + host.assert_not_awaited() + + async def test_progress_utf16_boundary_and_throttle(self): + host = AsyncMock(return_value={"accepted": False, "retry_after_ms": 9000}) + with patch.object(rlm, "host_request", host): + self.assertEqual(await rlm.rlm.progress_note(" " + "🎉" * 256 + " "), rlm.RLMProgressNoteResult(False, 9000)) + with self.assertRaises(ValueError): + await rlm.progress_note("🎉" * 257) + with self.assertRaises(ValueError): + await rlm.progress_note(" ") + host.assert_awaited_once_with("rlm.progress.note", {"message": "🎉" * 256}) + + async def test_end_turn_and_host_failure(self): + host = AsyncMock(return_value={"ended": True}) + with patch.object(rlm, "host_request", host): + self.assertEqual(await rlm.rlm.end_turn(), {"ended": True}) + host.assert_awaited_once_with("rlm.end_turn", {}) + with patch.object(rlm, "host_request", AsyncMock(side_effect=RuntimeError("inactive"))): + with self.assertRaisesRegex(RuntimeError, "inactive"): + await rlm.end_turn() + + async def test_invalid_host_payload(self): + for payload in ({}, {"results": [None]}, {"results": [{}]}): + with patch.object(rlm, "host_request", AsyncMock(return_value=payload)): + with self.assertRaises(RuntimeError): + await rlm.collect() + + async def test_create_session_validates_options_before_host(self): + host = AsyncMock() + with patch.object(rlm, "host_request", host): + for kwargs in ({"prompt": " "}, {"prompt": "task", "cwd": " "}, {"prompt": "task", "thinking": " high "}): + with self.assertRaises(ValueError): + await rlm.create_session(**kwargs) + for field in ("name", "model", "thinking", "cwd"): + with self.assertRaises(TypeError): + await rlm.create_session("task", **{field: True}) + host.assert_not_awaited() diff --git a/crates/server/src/persistence.rs b/crates/server/src/persistence.rs index 45ba781e..446056bb 100644 --- a/crates/server/src/persistence.rs +++ b/crates/server/src/persistence.rs @@ -382,6 +382,47 @@ impl RolloutStore { ) } + /// Persist inherited settings before the first metadata header in one locked + /// create batch. A crash leaving only a prefix cannot expose a root with + /// relaxed defaults: either its settings precede its header or no header exists. + pub(crate) fn append_seeded_session_meta_create_at( + &self, + rollout_path: &Path, + session: &devo_protocol::native::session::Session, + extras: devo_core::SessionPersistenceExtras, + settings: &[(SessionSettingsField, serde_json::Value)], + ) -> Result<()> { + let session_id = session.id; + self.append_rollout_lines_with( + rollout_path, + session_id, + JournalWriteMode::CreateIfMissing, + |state| { + let mut lines = Vec::with_capacity(settings.len() + 1); + let session_state = state.session_state_mut(session_id); + for (field, value) in settings { + let epoch = session_state.allocate_settings_epoch(); + lines.push(devo_core::settings_line( + Utc::now(), + session_id, + *field, + value.clone(), + epoch, + )); + } + let merged = + merge_session_extras(state.session_extras.get(&session_id), Some(&extras)); + state.session_extras.insert(session_id, merged.clone()); + lines.push(devo_core::session_line( + session.clone(), + Some(merged), + Utc::now(), + )); + Ok(lines) + }, + ) + } + fn append_session_meta_with_mode( &self, rollout_path: &Path, @@ -2964,10 +3005,22 @@ impl ReplayState { native.active_turn_id = None; native.queued_count = 0; native.last_activity_at = last_activity_at; + // The actor's next turn resolves its selection from these strings. + // Keeping only the base binding silently drops a named variant after + // restart even when the display-only `variant` field still says fast. + let persisted_selection = turn_config.variant.as_ref().map(|variant| { + let base = turn_config + .model_binding_id + .as_ref() + .unwrap_or(&turn_config.model.slug); + format!("{base}/{variant}") + }); native.model.provider = turn_config .model_binding_id + .map(|binding| persisted_selection.clone().unwrap_or(binding)) .unwrap_or_else(|| "unknown".into()); - native.model.model = turn_config.model.slug.clone(); + native.model.model = persisted_selection.unwrap_or_else(|| turn_config.model.slug.clone()); + native.model.variant = turn_config.variant.clone(); native.model.reasoning_effort = summary_reasoning_effort; native.settings.reasoning_effort = summary_reasoning_effort_selection.clone(); native.settings.mode = Some(match core_session.collaboration_mode { diff --git a/crates/server/src/runtime.rs b/crates/server/src/runtime.rs index 5a7e58fd..289b0877 100644 --- a/crates/server/src/runtime.rs +++ b/crates/server/src/runtime.rs @@ -120,8 +120,10 @@ mod items; mod kernel_host; mod kernel_host_agent_message; mod kernel_host_bridge; +mod kernel_host_controls; mod kernel_host_heartbeat; mod kernel_host_models; +mod kernel_host_session; mod kernel_idle_bash; mod lifecycle; mod mcp; @@ -204,6 +206,9 @@ pub struct ServerRuntime { background_work: std::sync::Mutex, /// Per-root-session agent registries for subagent coordination. agent_registries: Mutex>, + rlm_lazy_children: Mutex>, + rlm_progress_notes: Mutex>, + rlm_child_changed: tokio::sync::Notify, /// Per-session inboxes used by agent tools to exchange ordered messages. agent_mailboxes: Mutex>, /// Per-parent child-output buffers used by wait_agent polling. @@ -446,6 +451,9 @@ impl ServerRuntime { goal_stores: Mutex::new(HashMap::new()), background_work: std::sync::Mutex::new(Default::default()), agent_registries: Mutex::new(HashMap::new()), + rlm_lazy_children: Mutex::new(HashSet::new()), + rlm_progress_notes: Mutex::new(HashMap::new()), + rlm_child_changed: tokio::sync::Notify::new(), agent_mailboxes: Mutex::new(HashMap::new()), agent_output_buffers: Mutex::new(HashMap::new()), agent_wait_cursors: Mutex::new(HashMap::new()), diff --git a/crates/server/src/runtime/agents.rs b/crates/server/src/runtime/agents.rs index 966eceb8..fbc25e6b 100644 --- a/crates/server/src/runtime/agents.rs +++ b/crates/server/src/runtime/agents.rs @@ -647,6 +647,7 @@ impl ServerRuntime { .get_mut(&parent_session_id) { registry.update_status(child_session_id, status); + self.rlm_child_changed.notify_waiters(); } } @@ -1870,8 +1871,15 @@ impl ServerRuntime { /// Kernel `host_request("rlm.list_subagents")` — direct children retained /// by this session, in the payload shape `rlm.list_subagents()` parses. - pub(crate) async fn host_rlm_list_subagents(&self, session_id: &str) -> serde_json::Value { + pub(crate) async fn host_rlm_list_subagents( + self: &Arc, + session_id: &str, + ) -> serde_json::Value { let sid = SessionId::from_string(session_id.to_owned()); + if let Err(error) = self.restore_direct_children(sid).await { + return serde_json::json!({"status": "error", "error": error}); + } + let notes = self.rlm_progress_notes.lock().await.clone(); let entries: Vec = { let registries = self.agent_registries.lock().await; registries @@ -1881,7 +1889,13 @@ impl ServerRuntime { .children_of(&sid) .into_iter() .filter_map(|child_id| registry.get(&child_id)) - .map(Self::rlm_subagent_entry) + .map(|meta| { + let mut entry = Self::rlm_subagent_entry(meta); + if let Some((_, note)) = notes.get(&meta.session_id) { + entry["progress_note"] = serde_json::json!(note); + } + entry + }) .collect() }) .unwrap_or_default() diff --git a/crates/server/src/runtime/goal_background.rs b/crates/server/src/runtime/goal_background.rs index a2a3c57b..acf0fa10 100644 --- a/crates/server/src/runtime/goal_background.rs +++ b/crates/server/src/runtime/goal_background.rs @@ -92,6 +92,9 @@ impl ServerRuntime { None } }; + if work_id.starts_with("agent:") { + self.rlm_child_changed.notify_waiters(); + } let Some(deferred) = deferred else { return; }; @@ -201,6 +204,18 @@ impl ServerRuntime { .is_some_and(|state| !state.pending.is_empty()) } + /// Admission tickets bridge mailbox drain and queued-turn activation, so + /// collection cannot report a child settled while accepted work is in transit. + pub(super) fn has_pending_subagent_work(&self, child_id: SessionId) -> bool { + let prefix = format!("agent:{child_id}:"); + self.background_work + .lock() + .expect("background work mutex poisoned") + .sessions + .values() + .any(|state| state.pending.iter().any(|id| id.starts_with(&prefix))) + } + pub(super) fn clear_background_work(&self, session_id: SessionId) { self.background_work .lock() diff --git a/crates/server/src/runtime/handlers/session.rs b/crates/server/src/runtime/handlers/session.rs index 6d58db3a..4de4db35 100644 --- a/crates/server/src/runtime/handlers/session.rs +++ b/crates/server/src/runtime/handlers/session.rs @@ -147,6 +147,24 @@ impl ServerRuntime { request_id: serde_json::Value, params: SessionStartParams, tool_registry: Option>, + ) -> serde_json::Value { + self.start_session_with_seed( + connection_id, + request_id, + params, + tool_registry, + /*seed*/ None, + ) + .await + } + + pub(crate) async fn start_session_with_seed( + &self, + connection_id: u64, + request_id: serde_json::Value, + params: SessionStartParams, + tool_registry: Option>, + seed: Option, ) -> serde_json::Value { let now = Utc::now(); let session_id = devo_protocol::native::ids::SessionId::new(); @@ -164,13 +182,46 @@ impl ServerRuntime { .model_binding_id .as_deref() .or(params.model.as_deref()); - let initial_turn_config = runtime_context.resolve_turn_config(requested_model, None); + let initial_turn_config = runtime_context.resolve_turn_config( + requested_model, + seed.as_ref() + .and_then(|seed| seed.settings.reasoning_effort.clone()), + ); let model = initial_turn_config.model.slug.clone(); - let model_binding_id = initial_turn_config.model_binding_id.clone(); + let mut model_binding_id = initial_turn_config.model_binding_id.clone(); + let persisted_model = if seed.is_some() + && let Some(variant) = initial_turn_config.variant.as_ref() + { + let base = model_binding_id.as_ref().unwrap_or(&model); + let selected = format!("{base}/{variant}"); + model_binding_id = model_binding_id.as_ref().map(|_| selected.clone()); + selected + } else { + model.clone() + }; + let seeded_settings = seed.as_ref().map(|seed| { + let mut settings = vec![ + ( + SessionSettingsField::Model, + serde_json::json!(persisted_model), + ), + ( + SessionSettingsField::ModelBindingId, + serde_json::json!(model_binding_id), + ), + ]; + if let Some(sandbox) = seed.config.sandbox_profile.as_ref() { + settings.push(( + SessionSettingsField::SandboxProfile, + serde_json::json!(sandbox), + )); + } + settings + }); let initial_effort = initial_turn_config.reasoning_effort_selection.clone(); let mut invented_persistence = (!params.ephemeral).then(|| self.rollout_store.invent_session_persistence(&session_id)); - let native_session = devo_protocol::native::session::Session { + let mut native_session = devo_protocol::native::session::Session { id: session_id, version: 1, cwd: params.cwd.clone(), @@ -194,8 +245,10 @@ impl ServerRuntime { .unwrap_or(SessionTitleState::Unset), model: devo_protocol::native::model::ModelBinding { provider: model_binding_id.unwrap_or_else(|| "unknown".to_string()), - model: model.clone(), - variant: None, + model: persisted_model, + variant: seed + .as_ref() + .and_then(|_| initial_turn_config.variant.clone()), reasoning_effort: None, }, settings: devo_protocol::native::session::SessionSettings { @@ -224,6 +277,12 @@ impl ServerRuntime { updated_at: now, }, }; + if let Some(seed) = &seed { + native_session.settings = seed.settings.clone(); + if let Some(invented) = invented_persistence.as_mut() { + invented.extras.collaboration_mode = Some(seed.collaboration_mode); + } + } let applied_compaction_limit = crate::runtime::context_occupancy::resolved_compaction_limit( &initial_turn_config.model, ); @@ -250,6 +309,13 @@ impl ServerRuntime { params.cwd.clone(), params.additional_directories.clone(), ); + if let Some(seed) = seed { + core_session.config.permission_mode = seed.config.permission_mode; + core_session.config.permission_profile = seed.config.permission_profile; + core_session.config.sandbox_profile = seed.config.sandbox_profile; + core_session.collaboration_mode = seed.collaboration_mode; + summary.collaboration_mode = seed.collaboration_mode; + } let permission_preset = protocol_preset_from_safety(core_session.config.permission_profile.preset); summary.set_permission_preset(permission_preset); @@ -298,18 +364,28 @@ impl ServerRuntime { } None => None, }; - if let Some(invented) = &invented_persistence - && let Err(error) = self.rollout_store.append_session_meta_create_at( - &invented.rollout_path, - &summary.native, - Some(invented.extras.clone()), - ) - { - return self.error_response( - request_id, - ProtocolErrorCode::InternalError, - format!("failed to persist session metadata: {error}"), - ); + if let Some(invented) = &invented_persistence { + let persisted = if let Some(settings) = seeded_settings { + self.rollout_store.append_seeded_session_meta_create_at( + &invented.rollout_path, + &summary.native, + invented.extras.clone(), + &settings, + ) + } else { + self.rollout_store.append_session_meta_create_at( + &invented.rollout_path, + &summary.native, + Some(invented.extras.clone()), + ) + }; + if let Err(error) = persisted { + return self.error_response( + request_id, + ProtocolErrorCode::InternalError, + format!("failed to persist session metadata: {error}"), + ); + } } crate::runtime::context_occupancy::apply_resolved_compaction_limit( &mut core_session.config, diff --git a/crates/server/src/runtime/kernel_host.rs b/crates/server/src/runtime/kernel_host.rs index 593727af..311d20d4 100644 --- a/crates/server/src/runtime/kernel_host.rs +++ b/crates/server/src/runtime/kernel_host.rs @@ -46,9 +46,7 @@ pub async fn dispatch_host_request_with_bridge(bridge: &HostBridge, data: Value) action if action.starts_with("harness.") => { crate::runtime::kernel_host_bridge::handle_harness(bridge, action, ¶ms).await } - "rlm.create_session" => { - error_reply("denied: rlm.create_session (daemon out of scope this release)") - } + "rlm.create_session" => super::kernel_host_session::create_session(bridge, ¶ms).await, "rlm_heartbeat.list" => { let Some(runtime) = bridge.runtime() else { return error_reply("runtime not available for rlm_heartbeat.list"); @@ -77,6 +75,9 @@ pub async fn dispatch_host_request_with_bridge(bridge: &HostBridge, data: Value) error_reply(format!("unknown rlm_heartbeat action: {a}")) } "rlm.run" => super::kernel_host_models::spawn(bridge, ¶ms).await, + "rlm.collect" => super::kernel_host_controls::collect(bridge, ¶ms).await, + "rlm.progress.note" => super::kernel_host_controls::progress_note(bridge, ¶ms).await, + "rlm.end_turn" => super::kernel_host_controls::end_turn(bridge).await, "rlm.find_models" => super::kernel_host_models::find_models(bridge, ¶ms).await, "rlm.list_subagents" => { let Some(runtime) = bridge.runtime() else { @@ -379,9 +380,9 @@ mod tests { use devo_core::tools::PermissionChecker; use devo_protocol::CollaborationMode; - /// Verifies: rlm.create_session is denied. + /// Invalid root creation fails before side effects. #[tokio::test] - async fn denies_create_session() { + async fn create_session_requires_a_prompt() { let reply = dispatch_host_request( "ses_00000000-0000-0000-0000-000000000001", json!({ "type": "rlm.create_session" }), @@ -389,7 +390,7 @@ mod tests { .await; assert_eq!(reply["status"], "error"); assert!( - reply["error"].as_str().unwrap_or("").contains("denied"), + reply["error"].as_str().unwrap_or("").contains("prompt"), "{reply}" ); } @@ -520,6 +521,7 @@ mod tests { file_read_ledger: Arc::new(FileReadLedger::new()), sandbox_profile: None, cancel_token: tokio_util::sync::CancellationToken::new(), + end_turn_requested: None, kernel_lifetime: None, mcp_manager: None, agent_scope: ToolAgentScope::Parent, diff --git a/crates/server/src/runtime/kernel_host_bridge.rs b/crates/server/src/runtime/kernel_host_bridge.rs index a221f464..2a6f7ae6 100644 --- a/crates/server/src/runtime/kernel_host_bridge.rs +++ b/crates/server/src/runtime/kernel_host_bridge.rs @@ -53,6 +53,7 @@ pub(crate) struct HostBridge { pub file_read_ledger: Arc, pub sandbox_profile: Option, pub cancel_token: CancellationToken, + pub end_turn_requested: Option>, pub kernel_lifetime: Option, pub mcp_manager: Option>, pub agent_scope: ToolAgentScope, @@ -82,6 +83,7 @@ impl HostBridge { file_read_ledger: Arc::new(FileReadLedger::new()), sandbox_profile: None, cancel_token: CancellationToken::new(), + end_turn_requested: None, kernel_lifetime: None, mcp_manager: None, agent_scope: ToolAgentScope::Parent, diff --git a/crates/server/src/runtime/kernel_host_controls.rs b/crates/server/src/runtime/kernel_host_controls.rs new file mode 100644 index 00000000..a12f252c --- /dev/null +++ b/crates/server/src/runtime/kernel_host_controls.rs @@ -0,0 +1,421 @@ +//! Non-steering orchestration. Waits run outside actors and never own admission locks. + +use std::collections::HashSet; +use std::sync::Arc; +use std::time::Duration; + +use devo_protocol::native::ids::SessionId; +use devo_protocol::native::item::Item; +use serde_json::{Value, json}; +use tokio::time::Instant; + +use super::ServerRuntime; +use super::kernel_host_bridge::HostBridge; +use crate::subagent::SubagentStatus; + +pub(super) async fn collect(bridge: &HostBridge, params: &Value) -> Value { + let result = async { + let timeout_ms = match params.get("timeout_ms") { + None => 0, + Some(value) => value.as_u64().filter(|ms| *ms <= 300_000) + .ok_or("timeout_ms must be an integer between 0 and 300000")?, + }; + let targets = match params.get("targets") { + None => Vec::new(), + Some(Value::Array(values)) => values.iter().map(|value| { + value.as_str().filter(|target| !target.trim().is_empty()) + .ok_or("targets must contain nonempty strings") + }).collect::, _>>()?, + Some(_) => return Err("targets must be an array".to_string()), + }; + let runtime = bridge.runtime().ok_or("runtime not available for rlm.collect")?; + // Rehydrate only durable direct children, not arbitrary user sessions. The + // registry is process-local; the index and journal survive host restarts. + runtime.restore_direct_children(bridge.session_id).await?; + let ids = if targets.is_empty() { + runtime.agent_registries.lock().await.get(&bridge.session_id) + .map(|registry| registry.children_of(&bridge.session_id)).unwrap_or_default() + } else { + let mut seen = HashSet::new(); + let mut ids = Vec::new(); + for target in targets { + let child = runtime.resolve_child_agent(bridge.session_id, target).await + .map_err(|error| error.to_string())?; + if child.parent_session_id != bridge.session_id { + return Err("collect can only read direct children".to_string()); + } + if seen.insert(child.session_id) { ids.push(child.session_id); } + } + ids + }; + let deadline = Instant::now() + Duration::from_millis(timeout_ms); + loop { + // Register before observing state so a concurrent merge cannot be lost. + let changed = runtime.rlm_child_changed.notified(); + tokio::pin!(changed); + changed.as_mut().enable(); + if bridge.cancel_token.is_cancelled() { + return Err("rlm.collect interrupted".to_string()); + } + let mut results = Vec::with_capacity(ids.len()); + for id in &ids { results.push(runtime.collect_child(bridge.session_id, *id).await?); } + if results.iter().all(|result| result["settled"] == true || result["requires_resume"] == true) || Instant::now() >= deadline { + return Ok(json!({"results": results})); + } + tokio::select! { + _ = changed => {}, + _ = tokio::time::sleep_until(deadline) => {}, + _ = bridge.cancel_token.cancelled() => return Err("rlm.collect interrupted".to_string()), + } + } + }.await; + match result { + Ok(result) => json!({"status": "ok", "result": result}), + Err(error) => json!({"status": "error", "error": error}), + } +} + +impl ServerRuntime { + pub(super) async fn restore_direct_children( + self: &Arc, + parent: SessionId, + ) -> Result<(), String> { + let rows = self + .deps + .db + .list_sessions() + .map_err(|error| error.to_string())?; + for row in rows + .into_iter() + .filter(|row| row.parent_session_id == Some(parent) && row.agent_path.is_some()) + { + if self.session(row.session_id).await.is_none() { + self.get_or_load_parent_session(row.session_id) + .await + .map_err(|error| format!("restore child {}: {error}", row.session_id))?; + } + } + Ok(()) + } + + async fn collect_child(&self, parent: SessionId, child: SessionId) -> Result { + let meta = self + .resolve_child_agent(parent, child.as_str()) + .await + .map_err(|error| error.to_string())?; + let handle = self + .session(child) + .await + .ok_or("child session is unavailable")?; + let (snapshot, queued, active) = { + // Queue drain pops an input before registering its next turn. A + // short admission-consistent snapshot must not mistake that gap + // for completion. Release the gate before transcript/recovery I/O + // or any wait for child progress. + let _admission = handle.lock_state_change().await; + let snapshot = handle + .turn_reservation_snapshot() + .await + .ok_or("child snapshot is unavailable")?; + let queued = !snapshot + .pending_turn_queue + .lock() + .map_err(|_| "child queue is unavailable")? + .is_empty() + || self.has_pending_subagent_work(child); + let active = self.active_turns.active_turn_id(child).await.is_some(); + (snapshot, queued, active) + }; + // Terminal registry status is published before MergeTurn. Keep it running + // until the working set is installed and the execution reservation clears. + // Recovery is a journal-owned state, not just an unfinished status. + // Provider failures and shutdown interruptions can also be resumable. + // Queued follow-ups must not mask an abandoned current turn. + let requires_resume = if !meta.close_requested && !active && snapshot.active_turn.is_none() + { + self.turn_recovery(child) + .await + .map_err(|error| format!("read child recovery: {error}"))? + .is_some() + || (queued && self.rlm_lazy_children.lock().await.contains(&child)) + || snapshot.latest_turn.as_ref().is_some_and(|turn| { + turn.native.status == devo_protocol::native::turn::TurnStatus::WaitingApproval + }) + } else { + false + }; + let settled = !matches!( + meta.status, + SubagentStatus::Spawning | SubagentStatus::Running + ) && snapshot.active_turn.is_none() + && !queued + && !active + && !requires_resume; + let answer = if settled { + let latest_id = snapshot.latest_turn.as_ref().map(|turn| turn.turn_id()); + handle + .transcript_items() + .await + .unwrap_or_default() + .into_iter() + .filter(|item| Some(item.turn_id) == latest_id) + .filter_map(|item| match item.item { + Item::AssistantMessage { text, .. } => Some(text), + _ => None, + }) + .collect::>() + .join("\n") + } else { + String::new() + }; + // Explicit host bound; callers can read the retained journal for larger output. + let truncated = answer.chars().count() > 128 * 1024; + let answer: String = answer.chars().take(128 * 1024).collect(); + let error = if (settled || requires_resume) && meta.status == SubagentStatus::Failed { + if let Some(turn) = snapshot.latest_turn.as_ref() { + // Native turn/read includes the canonical persisted terminal + // error; the actor's latest-turn mirror can omit its details. + let read = self + .handle_native_turn_read( + json!("rlm.collect"), + json!({ + "sessionId": child, "turnId": turn.turn_id(), + }), + ) + .await; + Some( + read["result"]["turn"]["error"]["message"] + .as_str() + .unwrap_or("child turn failed; inspect its transcript") + .to_string(), + ) + } else { + Some("child failed before a turn was admitted".to_string()) + } + } else { + None + }; + let note = self + .rlm_progress_notes + .lock() + .await + .get(&child) + .map(|(_, note)| note.clone()); + Ok(json!({ + "rlm_child_id": child.to_string(), "session_name": meta.nickname, + "session_dir": meta.session_dir, "status": if requires_resume { "waiting_for_resume" } else if settled { meta.status.as_str() } else { "running" }, + "settled": settled, "answer_preview": if answer.is_empty() { None } else { Some(answer.chars().take(512).collect::()) }, + "answer_text": if answer.is_empty() { None } else { Some(answer) }, + "answer_truncated": truncated, "error": error, "progress_note": note, + "requires_resume": requires_resume, + })) + } +} + +pub(super) async fn progress_note(bridge: &HostBridge, params: &Value) -> Value { + let result = async { + let note = params + .get("message") + .and_then(Value::as_str) + .map(str::trim) + .filter(|note| !note.is_empty() && note.encode_utf16().count() <= 512) + .ok_or("message must contain 1 to 512 UTF-16 code units")?; + let runtime = bridge + .runtime() + .ok_or("runtime not available for rlm.progress.note")?; + if runtime + .child_parent_and_path(bridge.session_id) + .await + .is_none() + { + return Err("progress_note requires a child session"); + } + let mut notes = runtime.rlm_progress_notes.lock().await; + if let Some((at, _)) = notes.get(&bridge.session_id) { + let remaining = Duration::from_secs(/*secs*/ 10).saturating_sub(at.elapsed()); + if !remaining.is_zero() { + return Ok( + json!({"accepted": false, "retry_after_ms": remaining.as_millis().max(1)}), + ); + } + } + notes.insert( + bridge.session_id, + (std::time::Instant::now(), note.to_string()), + ); + Ok(json!({"accepted": true, "retry_after_ms": null})) + } + .await; + match result { + Ok(result) => json!({"status": "ok", "result": result}), + Err(error) => json!({"status": "error", "error": error}), + } +} + +/// The bridge may outlive a turn when a Python task is detached. Never let an +/// obsolete bridge end a later turn or claim success after its turn merged. +pub(super) async fn end_turn(bridge: &HostBridge) -> Value { + if let (Some(runtime), Some(turn_id), Some(requested)) = + (bridge.runtime(), bridge.turn_id, &bridge.end_turn_requested) + && !bridge.cancel_token.is_cancelled() + && runtime.active_turns.active_turn_id(bridge.session_id).await == Some(turn_id) + { + requested.store(true, std::sync::atomic::Ordering::Release); + json!({"status": "ok", "result": {"ended": true}}) + } else { + json!({"status": "error", "error": "rlm.end_turn requires an active model turn"}) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + + #[tokio::test] + async fn malformed_bounds_and_targets_are_rejected_before_runtime_access() { + let bridge = HostBridge::for_tests(SessionId::new()); + for timeout in [ + json!(-1), + json!(true), + json!(1.5), + json!(300001), + json!(null), + ] { + assert_eq!( + collect(&bridge, &json!({"timeout_ms":timeout})).await, + json!({"status":"error","error":"timeout_ms must be an integer between 0 and 300000"}) + ); + } + for target in [json!([null]), json!([false]), json!([""]), json!([" "])] { + assert_eq!( + collect(&bridge, &json!({"targets":target})).await, + json!({"status":"error","error":"targets must contain nonempty strings"}) + ); + } + assert_eq!( + end_turn(&bridge).await, + json!({"status":"error","error":"rlm.end_turn requires an active model turn"}) + ); + } + + #[tokio::test] + async fn progress_limit_counts_utf16_not_unicode_scalars() { + let bridge = HostBridge::for_tests(SessionId::new()); + for message in [String::new(), " ".to_string(), "🎉".repeat(257)] { + assert_eq!( + progress_note(&bridge, &json!({"message":message})).await, + json!({"status":"error","error":"message must contain 1 to 512 UTF-16 code units"}) + ); + } + assert_eq!( + progress_note(&bridge, &json!({"message":"🎉".repeat(256)})).await, + json!({"status":"error","error":"runtime not available for rlm.progress.note"}) + ); + } + #[tokio::test] + async fn saved_unfinished_child_requires_explicit_recovery_without_waiting() { + use devo_core::tools::AgentToolCoordinator; + let home = tempfile::TempDir::new().unwrap(); + let runtime = crate::test_support::TestRuntime::noop().runtime(home.path()); + let started = runtime + .start_session_with_registry( + /*connection_id*/ 0, + json!(1), + devo_protocol::SessionStartParams { + cwd: home.path().to_path_buf(), + additional_directories: Vec::new(), + ephemeral: false, + title: None, + model: None, + model_binding_id: None, + }, + /*tool_registry*/ None, + ) + .await; + let parent: SessionId = + serde_json::from_value(started["result"]["session"]["id"].clone()).unwrap(); + let child = Arc::clone(&runtime) + .spawn_agent( + serde_json::from_value(json!({ + "session_id":parent,"message":"task","nickname":"worker","fork_turns":"none" + })) + .unwrap(), + ) + .await + .unwrap(); + let handle = runtime.session(child.child_session_id).await.unwrap(); + let mut saved = tokio::time::timeout(Duration::from_secs(/*secs*/ 5), async { + loop { + let snapshot = handle.turn_reservation_snapshot().await.unwrap(); + if runtime + .active_turns + .active_turn_id(child.child_session_id) + .await + .is_none() + && let Some(turn) = snapshot.latest_turn + { + break turn; + } + tokio::task::yield_now().await; + } + }) + .await + .unwrap(); + let mut completed = saved.clone(); + completed.native.status = devo_protocol::native::turn::TurnStatus::Completed; + handle + .enqueue_pending_turn_input(devo_protocol::PendingInputItem::new( + devo_protocol::PendingInputKind::UserText { + text: "queued follow-up".to_string(), + }, + None, + chrono::Utc::now(), + )) + .await; + // Simulate the durable state exposed after a crash, without starting or + // silently replaying the saved turn's tool intents. Pending follow-ups + // must not mask the abandoned current turn. + saved.native.status = devo_protocol::native::turn::TurnStatus::InProgress; + handle.set_runtime_session_idle(Some(saved)).await; + runtime + .set_agent_status(parent, child.child_session_id, SubagentStatus::Running) + .await; + let mut bridge = HostBridge::for_tests(parent); + bridge.runtime = Arc::downgrade(&runtime); + let result = tokio::time::timeout( + Duration::from_secs(/*secs*/ 1), + collect(&bridge, &json!({"targets":["worker"],"timeout_ms":300000})), + ) + .await + .unwrap(); + let item = &result["result"]["results"][0]; + assert_eq!( + json!({"status":item["status"],"settled":item["settled"], + "requires_resume":item["requires_resume"],"answer_text":item["answer_text"]}), + json!({"status":"waiting_for_resume","settled":false,"requires_resume":true,"answer_text":null}) + ); + // A normal live post-merge scheduling gap is not orphaned work. + handle.set_runtime_session_idle(Some(completed)).await; + runtime + .set_agent_status(parent, child.child_session_id, SubagentStatus::Completed) + .await; + let live_gap = collect(&bridge, &json!({"targets":["worker"],"timeout_ms":0})).await; + assert_eq!( + live_gap["result"]["results"][0]["requires_resume"], + json!(false) + ); + // Cold hydration marks the same queued-only state as needing explicit + // session resume. Starting any real turn clears this runtime marker. + runtime + .rlm_lazy_children + .lock() + .await + .insert(child.child_session_id); + let dormant = collect(&bridge, &json!({"targets":["worker"],"timeout_ms":300000})).await; + assert_eq!( + dormant["result"]["results"][0]["requires_resume"], + json!(true) + ); + } +} diff --git a/crates/server/src/runtime/kernel_host_session.rs b/crates/server/src/runtime/kernel_host_session.rs new file mode 100644 index 00000000..64a9c99f --- /dev/null +++ b/crates/server/src/runtime/kernel_host_session.rs @@ -0,0 +1,329 @@ +//! Independent root creation uses the canonical session constructor and Native turn admission. + +use devo_core::SessionConfig; +use devo_protocol::native::session::SessionSettings; +use devo_protocol::{CollaborationMode, SessionStartParams}; +use serde_json::{Value, json}; + +use super::kernel_host_bridge::HostBridge; + +/// Initial safety/settings seed, installed before persistence and SessionStart hooks. +/// This is creation state, not an alternate settings-update protocol. +pub(crate) struct RootSessionSeed { + pub config: SessionConfig, + pub settings: SessionSettings, + pub collaboration_mode: CollaborationMode, +} + +pub(super) async fn create_session(bridge: &HostBridge, params: &Value) -> Value { + let result = async { + let prompt = params + .get("prompt") + .and_then(Value::as_str) + .filter(|prompt| !prompt.trim().is_empty()) + .ok_or("create_session requires a nonempty prompt")?; + let kwargs = params + .get("kwargs") + .and_then(Value::as_object) + .ok_or("create_session requires kwargs")?; + for key in kwargs.keys() { + if !["name", "model", "thinking", "cwd"].contains(&key.as_str()) { + return Err(format!("unsupported create_session option: {key}")); + } + } + let option = |key: &str| -> Result, String> { + match kwargs.get(key) { + None => Ok(None), + Some(Value::String(value)) if !value.trim().is_empty() && value.trim() == value => { + Ok(Some(value)) + } + _ => Err(format!( + "{key} must be a nonempty string without surrounding whitespace" + )), + } + }; + let name = option("name")?; + let selected_model = option("model")?; + let thinking = option("thinking")?; + let cwd = option("cwd")?; + let runtime = bridge + .runtime() + .ok_or("runtime not available for rlm.create_session")?; + if matches!(bridge.collaboration_mode, CollaborationMode::Plan) { + return Err("create_session is blocked in Plan mode".to_string()); + } + let caller = runtime + .session(bridge.session_id) + .await + .ok_or("caller session is unavailable")?; + let snapshot = caller + .spawn_snapshot() + .await + .ok_or("caller snapshot is unavailable")?; + if matches!( + snapshot.parent_summary.collaboration_mode, + CollaborationMode::Plan + ) { + return Err("create_session is blocked in Plan mode".to_string()); + } + if snapshot.parent_summary.agent_path.is_some() + || snapshot.parent_summary.ephemeral + || matches!( + snapshot.parent_summary.native.parent, + Some(devo_protocol::native::session::SessionParent::Agent { .. }) + ) + { + return Err("create_session requires a durable root session".to_string()); + } + // Do not use independent sessions to escape the caller's filesystem + // capability. A cross-workspace creation needs a separate approval flow. + if let Some(cwd) = cwd { + let requested = std::path::Path::new(cwd); + let requested = if requested.is_absolute() { + requested.to_path_buf() + } else { + bridge.cwd.join(requested) + }; + if std::fs::canonicalize(requested).map_err(|error| error.to_string())? + != std::fs::canonicalize(&bridge.cwd).map_err(|error| error.to_string())? + { + return Err( + "create_session currently requires the caller's working directory".to_string(), + ); + } + } + // Like child spawn, inherit the current decision-point configuration, + // including named variants and mid-turn model/thinking changes. + let live_config = if let Some(stream) = runtime.active_stream_state(bridge.session_id).await + { + let stream = stream.lock().await; + stream.turn_inline.as_ref().and_then(|inline| { + inline + .live_turn_settings + .lock() + .ok() + .and_then(|live| live.turn_config.clone()) + }) + } else { + None + }; + let parent_model = live_config + .as_ref() + .map(|config| { + let base = config + .model_binding_id + .as_ref() + .unwrap_or(&config.model.slug); + config + .variant + .as_ref() + .map(|variant| format!("{base}/{variant}")) + .unwrap_or_else(|| base.clone()) + }) + .or_else(|| { + super::session_model_selection(&snapshot.parent_summary).map(str::to_string) + }) + .ok_or("caller has no model binding")?; + let parent_thinking = match live_config.as_ref() { + Some(config) => config.reasoning_effort_selection.as_deref(), + None => snapshot.parent_summary.settings.reasoning_effort.as_deref(), + }; + let (_, resolved) = snapshot + .runtime_context + .resolve_child_model(selected_model, &parent_model, thinking, parent_thinking) + .map_err(|error| error.to_string())?; + let model = resolved + .model_binding_id + .clone() + .unwrap_or_else(|| resolved.model.slug.clone()); + let model_selection = resolved + .variant + .as_ref() + .map(|variant| format!("{model}/{variant}")) + .unwrap_or_else(|| model.clone()); + let mut settings = snapshot.parent_summary.native.settings.clone(); + settings.reasoning_effort = resolved.reasoning_effort_selection; + // A root is independently durable and visible; it has no parent usage + // ledger, ancestry, shared journal, or implicit parent completion notice. + let connection = runtime + .active_turns + .active_connection_id(bridge.session_id) + .await; + let response = runtime + .start_session_with_seed( + connection.unwrap_or(0), + json!("rlm.create_session"), + SessionStartParams { + cwd: bridge.cwd.clone(), + additional_directories: snapshot.parent_summary.additional_directories.clone(), + ephemeral: false, + title: name.map(str::to_string), + model: Some(model_selection), + model_binding_id: None, + }, + snapshot.parent_tool_registry, + Some(RootSessionSeed { + config: snapshot.parent_config, + settings, + collaboration_mode: bridge.collaboration_mode, + }), + ) + .await; + if let Some(error) = response.get("error") { + return Err(format!("create_session failed: {error}")); + } + let session: devo_protocol::native::session::Session = + serde_json::from_value(response["result"]["session"].clone()) + .map_err(|error| format!("invalid created session: {error}"))?; + let session_id = session.id; + let handle = runtime + .session(session_id) + .await + .ok_or_else(|| format!("created session {session_id} is unavailable"))?; + let session_file = handle + .rollout_path() + .await + .flatten() + .ok_or_else(|| format!("created session {session_id} has no journal"))?; + let turn = runtime + .handle_turn_start_for_connection( + connection, + json!("rlm.create_session.turn"), + json!({ + "sessionId": session_id, "input": [{"type":"text", "text":prompt}], + "idempotencyKey": format!("rlm-create-{session_id}"), + }), + ) + .await; + if let Some(error) = turn.get("error") { + // Creation is durable. Do not hide the id or retry with another root. + return Err(format!( + "created session {session_id}, but its initial turn was not admitted: {error}" + )); + } + Ok::<_, String>(json!({ + "active_session_id": session_id.to_string(), "session_id": session_id.to_string(), + "name": name.map(str::to_string).unwrap_or_else(|| session_id.to_string()), + "session_file": session_file, "model": model, + })) + } + .await; + match result { + Ok(result) => json!({"status":"ok", "result":result}), + Err(error) => json!({"status":"error", "error":error}), + } +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + use std::sync::Arc; + + #[tokio::test] + async fn root_inherits_restrictive_safety_and_invalid_requests_create_nothing() { + let home = tempfile::TempDir::new().unwrap(); + let runtime = crate::test_support::TestRuntime::noop().runtime(home.path()); + let started = runtime + .start_session_with_registry( + 0, + json!(1), + SessionStartParams { + cwd: home.path().to_path_buf(), + additional_directories: Vec::new(), + ephemeral: false, + title: Some("caller".into()), + model: None, + model_binding_id: None, + }, + None, + ) + .await; + let caller_id: devo_protocol::SessionId = + serde_json::from_value(started["result"]["session"]["id"].clone()).unwrap(); + let updated = runtime + .handle_native_session_metadata_update( + json!(2), + json!({ + "sessionId": caller_id, "expectedVersion":0, + "settings":{"permissionProfile":"default", "sandboxProfile":"strict"} + }), + ) + .await; + assert!(updated.get("error").is_none(), "{updated}"); + let mut bridge = HostBridge::for_tests(caller_id); + bridge.cwd = home.path().to_path_buf(); + bridge.runtime = Arc::downgrade(&runtime); + let parent = runtime + .session(caller_id) + .await + .unwrap() + .spawn_snapshot() + .await + .unwrap(); + assert_eq!( + parent.parent_summary.native.settings.permission_profile, + devo_protocol::native::model::PermissionProfile::Default + ); + for params in [ + json!({"prompt":"task","kwargs":{"cwd":".."}}), + json!({"prompt":"task","kwargs":{"model":"unavailable/model"}}), + json!({"prompt":"task","kwargs":{"unexpected":true}}), + ] { + assert_eq!( + create_session(&bridge, ¶ms).await["status"], + json!("error") + ); + assert_eq!(runtime.deps.db.list_root_sessions().unwrap().len(), 1); + } + bridge.collaboration_mode = CollaborationMode::Plan; + assert_eq!( + create_session(&bridge, &json!({"prompt":"task","kwargs":{}})).await, + json!({"status":"error","error":"create_session is blocked in Plan mode"}) + ); + assert_eq!(runtime.deps.db.list_root_sessions().unwrap().len(), 1); + bridge.collaboration_mode = CollaborationMode::Build; + let created = create_session( + &bridge, + &json!({"prompt":"task","kwargs":{"name":"independent"}}), + ) + .await; + assert_eq!(created["status"], json!("ok"), "{created}"); + let id = serde_json::from_value(created["result"]["session_id"].clone()).unwrap(); + let child = runtime + .session(id) + .await + .unwrap() + .spawn_snapshot() + .await + .unwrap(); + assert_eq!( + child.parent_config.permission_mode, + parent.parent_config.permission_mode + ); + assert_eq!( + child.parent_config.permission_profile, + parent.parent_config.permission_profile + ); + assert_eq!( + child.parent_config.sandbox_profile, + parent.parent_config.sandbox_profile + ); + assert_eq!(child.parent_summary.native.parent, None); + assert_eq!(runtime.deps.db.list_root_sessions().unwrap().len(), 2); + runtime.shutdown().await; + drop(runtime); + let restored = crate::test_support::TestRuntime::noop().runtime(home.path()); + restored.load_persisted_sessions().await.unwrap(); + let resumed = restored.get_or_load_parent_session(id).await.unwrap(); + let resumed = resumed.spawn_snapshot().await.unwrap(); + assert_eq!( + resumed.parent_config.permission_profile, + parent.parent_config.permission_profile + ); + assert_eq!( + resumed.parent_config.sandbox_profile.as_deref(), + Some("strict") + ); + } +} diff --git a/crates/server/src/runtime/session_cache.rs b/crates/server/src/runtime/session_cache.rs index 42b5decd..4b1229eb 100644 --- a/crates/server/src/runtime/session_cache.rs +++ b/crates/server/src/runtime/session_cache.rs @@ -209,6 +209,9 @@ impl ServerRuntime { .await; let mut state = SessionActorState::from_runtime_session(runtime_session); state.session_file_lock = Some(session_lock); + // This cold child has no queue scheduler yet. Set under the load + // gate before exposing the actor; a real turn admission clears it. + self.rlm_lazy_children.lock().await.insert(session_id); self.insert_session_actor(state).await } else { self.insert_root_session_actor(runtime_session, Some(session_lock)) diff --git a/crates/server/src/runtime/session_deletion.rs b/crates/server/src/runtime/session_deletion.rs index ce3f0809..e4e31b4a 100644 --- a/crates/server/src/runtime/session_deletion.rs +++ b/crates/server/src/runtime/session_deletion.rs @@ -721,6 +721,9 @@ impl ServerRuntime { self.clear_background_work(session_id); self.agent_mailboxes.lock().await.remove(&session_id); self.agent_output_buffers.lock().await.remove(&session_id); + self.rlm_lazy_children.lock().await.remove(&session_id); + self.rlm_progress_notes.lock().await.remove(&session_id); + self.rlm_child_changed.notify_waiters(); self.agent_wait_cursors.lock().await.remove(&session_id); { let mut registries = self.agent_registries.lock().await; diff --git a/crates/server/src/runtime/turn_exec/approval_resume.rs b/crates/server/src/runtime/turn_exec/approval_resume.rs index 228b69f7..f75bf24a 100644 --- a/crates/server/src/runtime/turn_exec/approval_resume.rs +++ b/crates/server/src/runtime/turn_exec/approval_resume.rs @@ -756,6 +756,7 @@ impl ServerRuntime { file_read_ledger: Arc::clone(&working.state.file_read_ledger), sandbox_profile: working.state.core.config.sandbox_profile.clone(), cancel_token: host_cancel_token.clone(), + end_turn_requested: None, kernel_lifetime: Some(kernel.lifetime_token()), mcp_manager: Some(Arc::clone(&working.state.runtime_context.mcp_manager)), agent_scope, diff --git a/crates/server/src/runtime/turn_exec/query.rs b/crates/server/src/runtime/turn_exec/query.rs index 7cd07f17..d986bbb1 100644 --- a/crates/server/src/runtime/turn_exec/query.rs +++ b/crates/server/src/runtime/turn_exec/query.rs @@ -166,6 +166,7 @@ impl ServerRuntime { .await .unwrap_or_else(CancellationToken::new); let query_cancel_token = turn_cancel_token.clone(); + let end_turn_requested = Arc::new(std::sync::atomic::AtomicBool::new(/*v*/ false)); let provider_http = runtime_context .config_store .lock() @@ -324,6 +325,7 @@ impl ServerRuntime { file_read_ledger: Arc::clone(&state.file_read_ledger), sandbox_profile: state.core.config.sandbox_profile.clone(), cancel_token: turn_cancel_token.clone(), + end_turn_requested: Some(Arc::clone(&end_turn_requested)), kernel_lifetime: Some(kernel.lifetime_token()), mcp_manager: Some(Arc::clone(&runtime_context.mcp_manager)), agent_scope, @@ -473,6 +475,7 @@ impl ServerRuntime { as Arc }), cancel_token: Some(query_cancel_token.clone()), + end_turn_requested: Some(end_turn_requested), compaction_provider: Some(compaction_provider), live_settings: live_turn_settings.clone(), live_goal, diff --git a/crates/server/src/runtime/turn_reservation.rs b/crates/server/src/runtime/turn_reservation.rs index 7702aacd..68875cb2 100644 --- a/crates/server/src/runtime/turn_reservation.rs +++ b/crates/server/src/runtime/turn_reservation.rs @@ -77,13 +77,16 @@ impl ServerRuntime { self.active_turns .register_turn(session_id, turn.native) .await; + self.rlm_lazy_children.lock().await.remove(&session_id); } pub(super) async fn clear_active_turn_runtime_handles( &self, session_id: devo_protocol::native::ids::SessionId, ) -> bool { - self.active_turns.clear_runtime_handles(session_id).await + let activation = self.active_turns.clear_runtime_handles(session_id).await; + self.rlm_child_changed.notify_waiters(); + activation } } diff --git a/crates/server/tests/child_model_selection.rs b/crates/server/tests/child_model_selection.rs index ef50bfa3..e34bb97b 100644 --- a/crates/server/tests/child_model_selection.rs +++ b/crates/server/tests/child_model_selection.rs @@ -567,3 +567,133 @@ async fn invalid_selection_does_not_poison_later_spawn_during_active_parent_turn wait_for_parent_turn_completed(&mut notifications, root).await?; Ok(()) } + +#[tokio::test] +async fn independent_root_inherits_named_non_effort_variant() -> Result<()> { + let home = TempDir::new()?; + write_config(home.path())?; + let path = home.path().join("providers.json"); + let mut config: serde_json::Value = serde_json::from_slice(&std::fs::read(&path)?)?; + config["provider"]["parent"]["models"]["vendor/main"]["variants"] = json!({ + "fast": {"request": {"speed":"fast"}} + }); + std::fs::write(&path, serde_json::to_vec(&config)?)?; + let parent = Arc::new(ScriptedProvider::new([ + ScriptedProvider::ipython_code_tool_call( + r#" +from pathlib import Path +root = await rlm.create_session('Use the inherited variant', name='variant') +Path('variant-root').write_text(root.session_id) +await rlm.end_turn() +"#, + ), + ScriptedProvider::completed("variant answer"), + ])); + let runtime = runtime( + home.path(), + parent.clone(), + Arc::new(ScriptedProvider::pending()), + ); + let (connection, mut notifications) = initialize_connection(&runtime).await?; + let session = start_parent_session(&runtime, connection, home.path()).await?; + let response = runtime + .handle_incoming( + connection, + json!({ + "id": 91, "method": "session/metadata/update", "params": { + "sessionId": session, "expectedVersion":0, + "model":{"provider":"parent", "model":"vendor/main", "variant":"fast"} + } + }), + ) + .await + .context("select named variant")?; + assert!(response.get("error").is_none(), "{response}"); + start_turn_with_approval_policy( + &runtime, + connection, + session, + "Create an independent root", + Some("never"), + ) + .await?; + let completed = + wait_for_session_notification(&mut notifications, "turn/completed", session).await?; + assert_eq!( + completed["params"]["turn"]["status"], + json!("completed"), + "{completed}" + ); + let child: SessionId = std::fs::read_to_string(home.path().join("variant-root"))?.parse()?; + wait_for_stream_calls(&parent, 2).await?; + assert_eq!( + parent + .requests() + .iter() + .map(|request| request.extra_body.clone()) + .collect::>(), + vec![Some(json!({"speed":"fast"})); 2] + ); + let read = runtime + .handle_incoming( + connection, + json!({ + "id":92, "method":"session/read", "params":{"sessionId": child} + }), + ) + .await + .context("read variant root")?; + assert_eq!(read["result"]["session"]["model"]["variant"], json!("fast")); + runtime.shutdown().await; + drop(runtime); + let resumed_provider = Arc::new(ScriptedProvider::new([ScriptedProvider::completed( + "after restart", + )])); + let restored = self::runtime( + home.path(), + resumed_provider.clone(), + Arc::new(ScriptedProvider::pending()), + ); + restored.load_persisted_sessions().await?; + let (connection, mut notifications) = initialize_connection(&restored).await?; + let resumed = restored + .handle_incoming( + connection, + json!({ + "id":93, "method":"session/resume", "params":{"sessionId": child} + }), + ) + .await + .context("resume variant root")?; + assert!(resumed.get("error").is_none(), "{resumed}"); + assert_eq!( + resumed["result"]["session"]["model"]["variant"], + json!("fast") + ); + while notifications.try_recv().is_ok() {} + let started = start_turn( + &restored, + connection, + child, + "Keep the variant after restart", + ) + .await?; + let completed = loop { + let notification = + wait_for_session_notification(&mut notifications, "turn/completed", child).await?; + if notification["params"]["turn"]["id"] == json!(started.turn.id) { + break notification; + } + }; + assert_eq!( + completed["params"]["turn"]["status"], + json!("completed"), + "{completed}" + ); + wait_for_stream_calls(&resumed_provider, 1).await?; + assert_eq!( + resumed_provider.requests()[0].extra_body, + Some(json!({"speed":"fast"})) + ); + Ok(()) +} diff --git a/crates/server/tests/subagent_python_controls.rs b/crates/server/tests/subagent_python_controls.rs new file mode 100644 index 00000000..b3a99850 --- /dev/null +++ b/crates/server/tests/subagent_python_controls.rs @@ -0,0 +1,513 @@ +//! Real CPython cells through the production host bridge and session actors. +#[path = "support/subagent_lifecycle.rs"] +mod support; + +use anyhow::{Context, Result}; +use pretty_assertions::assert_eq; +use serde_json::json; +use std::sync::Arc; +use std::time::Duration; +use support::*; +use tempfile::TempDir; + +#[tokio::test] +async fn python_collect_progress_and_graceful_end_turn() -> Result<()> { + let home = TempDir::new()?; + let provider = Arc::new(ScriptedProvider::new([ + ScriptedProvider::ipython_code_tool_call( + r#" +from pathlib import Path +worker = await rlm.spawn('Report progress, then finish', name='worker') +import asyncio +results, concurrent = await asyncio.gather( + rlm.collect([worker, 'worker'], timeout_ms=10000), + rlm.collect(worker, timeout_ms=10000), +) +assert results == concurrent +assert len(results) == 1, results +result = results[0] +assert result.settled and result.status == 'completed', result +assert result.answer_text == 'child final answer', result +assert result.progress_note == 'testing', result +assert (await rlm.collect(worker))[0] == result +assert (await rlm.list_subagents())[0].progress_note == 'testing' +assert (await rlm.end_turn())['ended'] +Path('controls-ok').write_text('complete') +"#, + ), + ScriptedProvider::ipython_code_tool_call( + r#" +assert (await rlm.progress_note('testing')).accepted +again = await rlm.progress_note('too soon') +assert not again.accepted and 0 < again.retry_after_ms <= 10000 +"#, + ), + ScriptedProvider::completed("child final answer"), + ])); + let runtime = build_runtime(home.path(), provider.clone())?; + let (connection, mut notifications) = initialize_connection(&runtime).await?; + let parent = start_parent_session(&runtime, connection, home.path()).await?; + start_turn_with_approval_policy( + &runtime, + connection, + parent, + "Coordinate children", + Some("never"), + ) + .await?; + let completed = + wait_for_session_notification(&mut notifications, "turn/completed", parent).await?; + assert_eq!( + completed["params"]["turn"]["status"], + json!("completed"), + "{completed}" + ); + assert_eq!( + std::fs::read_to_string(home.path().join("controls-ok"))?, + "complete" + ); + // No synthetic parent message and no fourth model request after end_turn. + assert_eq!(provider.stream_calls(), 3); + Ok(()) +} + +#[tokio::test] +async fn collect_timeout_does_not_cancel_child_or_block_read_rpcs() -> Result<()> { + let home = TempDir::new()?; + let provider = Arc::new(ScriptedProvider::new([ + ScriptedProvider::ipython_code_tool_call( + r#" +from pathlib import Path +worker = await rlm.spawn('Keep working', name='worker') +partial = await rlm.collect(worker, timeout_ms=50) +assert len(partial) == 1 and not partial[0].settled, partial +Path('timeout-ok').write_text('still running') +await rlm.collect(worker, timeout_ms=300000) +"#, + ), + StreamScript::Pending, + ])); + let runtime = build_runtime(home.path(), provider.clone())?; + let (connection, mut notifications) = initialize_connection(&runtime).await?; + let parent = start_parent_session(&runtime, connection, home.path()).await?; + start_turn_with_approval_policy( + &runtime, + connection, + parent, + "Wait for child", + Some("never"), + ) + .await?; + wait_for_stream_calls(&provider, 2).await?; + // Observable file proves the real kernel returned a partial snapshot. + tokio::time::timeout(Duration::from_secs(10), async { + while !home.path().join("timeout-ok").exists() { + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await?; + for (index, method, params) in [ + (41, "runtime/ping", json!({})), + (42, "session/read", json!({"sessionId":parent})), + (43, "session/list", json!({})), + ] { + let result = tokio::time::timeout( + Duration::from_secs(1), + runtime.handle_incoming( + connection, + json!({"id":index, "method":method, "params":params}), + ), + ) + .await? + .context("read response")?; + assert!(result.get("error").is_none(), "{result}"); + } + let agents = request_agent_list(&runtime, connection, parent).await?; + assert_eq!(agents.agents[0].status, "running"); + let interrupt = runtime + .handle_incoming( + connection, + json!({ + "id":44, "method":"session/interrupt", "params":{"scope":{"scope":"session", "sessionId":parent}} + }), + ) + .await + .context("interrupt")?; + assert!(interrupt.get("error").is_none(), "{interrupt}"); + let completed = + wait_for_session_notification(&mut notifications, "turn/completed", parent).await?; + assert_eq!(completed["params"]["turn"]["status"], json!("interrupted")); + // Explicit session interruption retains the existing cascading child policy. + request_agent_close(&runtime, connection, parent, "worker").await?; + Ok(()) +} + +#[tokio::test] +async fn collect_restores_direct_children_and_answers_after_restart() -> Result<()> { + let home = TempDir::new()?; + let first_provider = Arc::new(ScriptedProvider::new([ScriptedProvider::completed( + "durable answer", + )])); + let first = build_runtime(home.path(), first_provider)?; + let (connection, mut notifications) = initialize_connection(&first).await?; + let parent = start_parent_session(&first, connection, home.path()).await?; + let child = spawn_child_named( + &first, + parent, + "Complete the task", + Some("none"), + Some("retained"), + ) + .await?; + wait_for_session_notification(&mut notifications, "turn/completed", child.child_session_id) + .await?; + drop(first); + let provider = Arc::new(ScriptedProvider::new([ + ScriptedProvider::ipython_code_tool_call( + r#" +from pathlib import Path +result = (await rlm.collect('retained', timeout_ms=1000))[0] +assert result.settled and result.answer_text == 'durable answer', result +Path('restored-ok').write_text(result.answer_text) +await rlm.end_turn() +"#, + ), + ])); + let restored = build_runtime(home.path(), provider.clone())?; + restored.load_persisted_sessions().await?; + let (connection, mut notifications) = initialize_connection(&restored).await?; + let response = restored + .handle_incoming( + connection, + json!({ + "id":45,"method":"session/resume","params":{"sessionId":parent} + }), + ) + .await + .context("resume parent")?; + assert!(response.get("error").is_none(), "{response}"); + start_turn_with_approval_policy( + &restored, + connection, + parent, + "Collect retained answer", + Some("never"), + ) + .await?; + let completed = + wait_for_session_notification(&mut notifications, "turn/completed", parent).await?; + assert_eq!( + completed["params"]["turn"]["status"], + json!("completed"), + "{completed}" + ); + assert_eq!( + std::fs::read_to_string(home.path().join("restored-ok"))?, + "durable answer" + ); + assert_eq!(provider.stream_calls(), 1); + Ok(()) +} + +#[tokio::test] +async fn python_create_session_is_an_independent_visible_durable_root() -> Result<()> { + let home = TempDir::new()?; + let provider = Arc::new(ScriptedProvider::new([ + ScriptedProvider::ipython_code_tool_call( + r#" +from pathlib import Path +import json +root = await rlm.create_session('Work independently', name='independent', cwd='.') +assert root.active_session_id == root.session_id +assert root.session_file.is_file() +assert not await rlm.list_subagents() +Path('root-handle.json').write_text(json.dumps({'id': root.session_id, 'journal': str(root.session_file)})) +await rlm.end_turn() +"#, + ), + ScriptedProvider::completed_after(Duration::from_millis(100), "independent answer"), + ])); + let runtime = build_runtime(home.path(), provider.clone())?; + let (connection, mut notifications) = initialize_connection(&runtime).await?; + let parent = start_parent_session(&runtime, connection, home.path()).await?; + start_turn_with_approval_policy( + &runtime, + connection, + parent, + "Create an independent root", + Some("never"), + ) + .await?; + let completed = + wait_for_session_notification(&mut notifications, "turn/completed", parent).await?; + assert_eq!( + completed["params"]["turn"]["status"], + json!("completed"), + "{completed}" + ); + let handle: serde_json::Value = + serde_json::from_slice(&std::fs::read(home.path().join("root-handle.json"))?)?; + let created_id: devo_protocol::SessionId = handle["id"].as_str().context("root id")?.parse()?; + assert_ne!(created_id, parent); + tokio::time::timeout(Duration::from_secs(10), async { + loop { + let read = runtime + .handle_incoming( + connection, + json!({"id":46, "method":"session/read", "params":{"sessionId":created_id}}), + ) + .await + .context("read independent root")?; + assert!(read.get("error").is_none(), "{read}"); + let session = &read["result"]["session"]; + assert!(session["parent"].is_null(), "{session}"); + assert_eq!(session["title"], json!("independent")); + assert_eq!( + session["settings"]["permissionProfile"], + json!("fullAccess") + ); + if session["status"] == "idle" { + return Ok::<_, anyhow::Error>(()); + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await??; + let list = runtime + .handle_incoming( + connection, + json!({"id":47,"method":"session/list","params":{}}), + ) + .await + .context("list roots")?; + assert!(list.to_string().contains(created_id.as_str()), "{list}"); + assert!( + request_agent_list(&runtime, connection, parent) + .await? + .agents + .is_empty() + ); + assert_eq!(provider.stream_calls(), 2); + let journal = std::fs::read_to_string(handle["journal"].as_str().context("journal")?)?; + assert!(journal.contains("independent answer")); + Ok(()) +} + +#[tokio::test] +async fn end_turn_keeps_a_running_child_alive() -> Result<()> { + let home = TempDir::new()?; + let provider = Arc::new(ScriptedProvider::new([ + ScriptedProvider::ipython_code_tool_call( + r#" +from pathlib import Path +worker = await rlm.spawn('Continue after parent returns', name='worker') +partial = await rlm.collect(worker, timeout_ms=10) +assert not partial[0].settled +await rlm.end_turn() +Path('ended-ok').write_text('ended') +"#, + ), + StreamScript::Pending, + ])); + let runtime = build_runtime(home.path(), provider.clone())?; + let (connection, mut notifications) = initialize_connection(&runtime).await?; + let parent = start_parent_session(&runtime, connection, home.path()).await?; + start_turn_with_approval_policy( + &runtime, + connection, + parent, + "Yield normally", + Some("never"), + ) + .await?; + let completed = + wait_for_session_notification(&mut notifications, "turn/completed", parent).await?; + assert_eq!(completed["params"]["turn"]["status"], json!("completed")); + assert_eq!( + std::fs::read_to_string(home.path().join("ended-ok"))?, + "ended" + ); + let agents = request_agent_list(&runtime, connection, parent).await?; + assert_eq!(agents.agents[0].status, "running"); + wait_for_stream_calls(&provider, 2).await?; + assert_eq!(provider.stream_calls(), 2); + request_agent_close(&runtime, connection, parent, "worker").await?; + Ok(()) +} + +#[tokio::test] +async fn collect_reports_recoverable_failure_with_its_canonical_error() -> Result<()> { + let home = TempDir::new()?; + let provider = Arc::new(ScriptedProvider::new([ + ScriptedProvider::ipython_code_tool_call( + r#" +from pathlib import Path +worker = await rlm.spawn('Fail the model request', name='failure') +result = (await rlm.collect(worker, timeout_ms=10000))[0] +assert not result.settled and result.status == 'waiting_for_resume', result +assert result.requires_resume and result.answer_text is None, result +assert 'scripted provider stream script exhausted' in result.error, result +await rlm.delete_subagent(worker.rlm_child_id) +closed = (await rlm.collect(worker))[0] +assert closed.settled and not closed.requires_resume and closed.status == 'failed', closed +assert closed.error == result.error, closed +Path('failure-ok').write_text('reported') +await rlm.end_turn() +"#, + ), + ])); + let runtime = build_runtime(home.path(), provider.clone())?; + let (connection, mut notifications) = initialize_connection(&runtime).await?; + let parent = start_parent_session(&runtime, connection, home.path()).await?; + start_turn_with_approval_policy( + &runtime, + connection, + parent, + "Collect failure", + Some("never"), + ) + .await?; + let completed = + wait_for_session_notification(&mut notifications, "turn/completed", parent).await?; + assert_eq!(completed["params"]["turn"]["status"], json!("completed")); + assert_eq!( + std::fs::read_to_string(home.path().join("failure-ok"))?, + "reported" + ); + Ok(()) +} + +#[tokio::test] +async fn collect_recognizes_shutdown_interrupted_child_recovery() -> Result<()> { + let home = TempDir::new()?; + let first_provider = Arc::new(ScriptedProvider::pending()); + let first = build_runtime(home.path(), first_provider.clone())?; + let (connection, _) = initialize_connection(&first).await?; + let parent = start_parent_session(&first, connection, home.path()).await?; + spawn_child_named( + &first, + parent, + "Keep working", + Some("none"), + Some("recoverable"), + ) + .await?; + wait_for_stream_calls(&first_provider, 1).await?; + first.shutdown().await; + drop(first); + let provider = Arc::new(ScriptedProvider::new([ + ScriptedProvider::ipython_code_tool_call( + r#" +from pathlib import Path +result = (await rlm.collect('recoverable', timeout_ms=300000))[0] +assert result.status == 'waiting_for_resume' and result.requires_resume, result +assert not result.settled and result.answer_text is None, result +Path('shutdown-recovery-ok').write_text('no replay') +await rlm.end_turn() +"#, + ), + ])); + let restored = build_runtime(home.path(), provider.clone())?; + restored.load_persisted_sessions().await?; + let (connection, mut notifications) = initialize_connection(&restored).await?; + let response = restored + .handle_incoming( + connection, + json!({ + "id": 81, "method": "session/resume", "params": {"sessionId": parent} + }), + ) + .await + .context("resume parent")?; + assert!(response.get("error").is_none(), "{response}"); + start_turn_with_approval_policy( + &restored, + connection, + parent, + "Inspect recovery", + Some("never"), + ) + .await?; + let completed = + wait_for_session_notification(&mut notifications, "turn/completed", parent).await?; + assert_eq!( + completed["params"]["turn"]["status"], + json!("completed"), + "{completed}" + ); + assert_eq!( + std::fs::read_to_string(home.path().join("shutdown-recovery-ok"))?, + "no replay" + ); + assert_eq!(provider.stream_calls(), 1); + Ok(()) +} + +#[tokio::test] +async fn collect_waits_through_a_live_queued_followup() -> Result<()> { + let home = TempDir::new()?; + let provider = Arc::new(ScriptedProvider::new([ + ScriptedProvider::ipython_code_tool_call( + r#" +from pathlib import Path +worker = await rlm.spawn('Do the first task', name='queued') +result = (await rlm.collect(worker, timeout_ms=10000))[0] +print(result) +assert result.settled and not result.requires_resume, result +assert result.answer_text == 'queued answer', result +Path('queued-ok').write_text('joined both turns') +await rlm.end_turn() +"#, + ), + ScriptedProvider::ipython_code_tool_call( + r#" +import asyncio +from pathlib import Path +while not Path('queue-installed').exists(): + await asyncio.sleep(0.01) +"#, + ), + ScriptedProvider::completed("first answer"), + ScriptedProvider::completed("queued answer"), + ])); + let runtime = build_runtime(home.path(), provider.clone())?; + let (connection, mut notifications) = initialize_connection(&runtime).await?; + let parent = start_parent_session(&runtime, connection, home.path()).await?; + start_turn_with_approval_policy( + &runtime, + connection, + parent, + "Join queued work", + Some("never"), + ) + .await?; + wait_for_stream_calls(&provider, 2).await?; + request_agent_send_message(&runtime, connection, parent, "queued", "Do the follow-up").await?; + std::fs::write(home.path().join("queue-installed"), "ready")?; + let completed = + wait_for_session_notification(&mut notifications, "turn/completed", parent).await?; + assert_eq!( + completed["params"]["turn"]["status"], + json!("completed"), + "{completed}" + ); + let outputs = provider + .requests() + .into_iter() + .flat_map(|r| r.messages) + .flat_map(|m| m.content) + .filter_map(|content| match content { + devo_protocol::RequestContent::ToolResult { content, .. } => Some(content), + _ => None, + }) + .collect::>(); + assert!( + home.path().join("queued-ok").exists(), + "Python outputs: {outputs:?}" + ); + assert_eq!( + std::fs::read_to_string(home.path().join("queued-ok"))?, + "joined both turns" + ); + Ok(()) +} diff --git a/docs/rlm-orchestration.md b/docs/rlm-orchestration.md new file mode 100644 index 00000000..75347270 --- /dev/null +++ b/docs/rlm-orchestration.md @@ -0,0 +1,93 @@ +# Python subagent orchestration + +These operations run through the real kernel host bridge. They use existing +session actors, durable journals and Native turn admission; they do not create a +second execution engine or require model-provider network access to coordinate. + +```python +worker = await rlm.spawn("Run the focused tests", name="tests") +results = await rlm.collect([worker], timeout_ms=10_000) +for result in results: + print(result.status, result.progress_note, result.answer_text) +await rlm.end_turn() +``` + +## Collect and progress + +`collect(targets=None, *, timeout_ms=0)` accepts a child name/id, spawn handle, +subagent row, or a list/tuple mixing those selectors. Empty selection snapshots +all direct children at admission. Duplicate selectors are returned once. Foreign +sessions and unknown targets fail rather than being silently skipped. + +Timeout is an integer from 0 to 300000 milliseconds. Zero is a snapshot. Positive +values wait until all selected children settle or the deadline expires, then +return current snapshots. Timeout does not cancel work. Interrupted waits fail +promptly. Waiting does not hold the session mailbox or an admission lock, does +not send steering messages, and does not consume results or other wait cursors. +The notification subscription precedes the snapshot to avoid lost wakeups. + +Each `RLMChildResult` contains `rlm_child_id`, `session_name`, `session_dir`, +`status`, `settled`, `answer_text`, `answer_preview`, `answer_truncated`, `error`, +`progress_note`, and `requires_resume`. Status preserves native terminal distinctions (`completed`, +`failed`, `interrupted`, `canceled`, `closed`, `waiting_for_input`); unfinished +snapshots report `running`. Settlement waits for the completed working set to +merge and execution admission to clear, and excludes queued follow-ups and accepted mailbox messages still in transit. Results +read the latest completed turn's assistant text, bounded to 128 Ki characters +with a truncation flag and a 512-character preview. The journal retains larger +output. A resumed child never presents its previous answer as the new answer. + +The host rehydrates durable direct children from the index/journal after restart. +Corrupt or missing child journals are explicit restore errors. Saved unfinished +turns, recoverable provider failures, and shutdown interruptions with no live execution report `waiting_for_resume`, `requires_resume=True`, +and `settled=False`. Collect stops waiting once every target is settled or needs +explicit Native recovery; it never silently starts a new run or replays tools. +An idle restored child with queued follow-ups also requires resume. Use Native +turn/recovery/read to identify resumable turns; approval recovery and queued-only +work use their normal approval/session resume controls. In-process +progress is intentionally transient, not recovered as if it were fresh. + +Children call `await rlm.progress_note("running tests")`. Notes contain 1–512 +UTF-16 code units after trimming and are accepted at most once per 10 seconds. +The result has `accepted` and an optional `retry_after_ms`. Notes appear in +`collect` and `list_subagents`, without messaging or waking the parent model. +Root calls fail. Use `agent_message` for a message that needs a response. + +## End the current turn + +`await rlm.end_turn()` requests normal completion at the next tool-batch boundary. +The rest of the Python cell and sibling calls in that batch still run. Tool +results are committed before completion; no additional model response is +requested. This is not cancellation and does not stop children or background +jobs. Calling it from an idle or obsolete host bridge fails. Approval-replayed +tools do not support this control until normal model execution resumes. + +## Independent root sessions + +```python +root = await rlm.create_session("Investigate the next issue", name="Next issue") +print(root.session_id, root.session_file) +``` + +`create_session(prompt, name=None, model=None, thinking=None, cwd=None)` requires +a durable root in Build mode. It creates a separately visible, durable root with +its own journal, no parent link, no inherited transcript, and no parent usage or +completion lifecycle. Normal session/list, session/resume and turn controls apply. +It is not a child and cannot be collected or deleted through subagent controls. + +Model/thinking use the same exact validated resolver as spawn, with inherited +defaults and no silent fallback. Permissions and sandbox settings are installed +before the first durable metadata write and before SessionStart hooks. Initial +turn admission uses the Native handler. The handle returns only after admission, +not after the initial prompt completes. If creation succeeds but admission +fails, the error includes the existing session id; callers must not blindly retry. + +For this slice, an explicit cwd must canonicalize to the caller's current working +directory. Cross-workspace roots need a separate authorization flow. Child, +ephemeral and Plan-mode callers are rejected before any root is created. + +## Verification + +`crates/server/tests/subagent_python_controls.rs` exercises real CPython cells, +production host dispatch, durable actor merge, nonblocking read RPCs, bounded +waits and interruption, root isolation, and child result recovery after restart. +Python wire and malformed-input cases live in `test_controls.py`. From f7ab273828a564f506bf59c8fe7e8c0270c584dd Mon Sep 17 00:00:00 2001 From: wangtsiao Date: Mon, 9 Nov 2026 01:03:10 +0800 Subject: [PATCH 4/5] feat(factory): persist guarded workflows and reconcile durable child admissions --- crates/kernel/rlm-runtime/README.md | 2 +- .../rlm-runtime/src/rlm/_factory_backend.py | 15 + .../src/rlm/_factory_persistence.py | 131 ++++++ .../rlm-runtime/src/rlm/_factory_spec.py | 67 ++- crates/kernel/rlm-runtime/src/rlm/factory.py | 200 ++++++++- .../kernel/rlm-runtime/tests/test_factory.py | 2 +- .../rlm-runtime/tests/test_factory_durable.py | 388 ++++++++++++++++++ .../rlm-runtime/tests/test_factory_spec.py | 2 +- crates/server/src/runtime/kernel_host.rs | 3 + .../server/src/runtime/kernel_host_bridge.rs | 2 + .../src/runtime/kernel_host_bridge/factory.rs | 146 +++++++ .../kernel_host_bridge/factory/files.rs | 298 ++++++++++++++ .../kernel_host_bridge/factory/store.rs | 279 +++++++++++++ .../kernel_host_bridge/factory/store/tests.rs | 213 ++++++++++ .../kernel_host_bridge/factory/tests.rs | 181 ++++++++ crates/server/tests/factory_workflows.rs | 177 ++++++++ docs/factory-durable-workflows.md | 150 +++++++ docs/factory-workflows.md | 4 + 18 files changed, 2232 insertions(+), 28 deletions(-) create mode 100644 crates/kernel/rlm-runtime/src/rlm/_factory_persistence.py create mode 100644 crates/kernel/rlm-runtime/tests/test_factory_durable.py create mode 100644 crates/server/src/runtime/kernel_host_bridge/factory.rs create mode 100644 crates/server/src/runtime/kernel_host_bridge/factory/files.rs create mode 100644 crates/server/src/runtime/kernel_host_bridge/factory/store.rs create mode 100644 crates/server/src/runtime/kernel_host_bridge/factory/store/tests.rs create mode 100644 crates/server/src/runtime/kernel_host_bridge/factory/tests.rs create mode 100644 crates/server/tests/factory_workflows.rs create mode 100644 docs/factory-durable-workflows.md diff --git a/crates/kernel/rlm-runtime/README.md b/crates/kernel/rlm-runtime/README.md index 51164fd5..e7ae2c6b 100644 --- a/crates/kernel/rlm-runtime/README.md +++ b/crates/kernel/rlm-runtime/README.md @@ -168,7 +168,7 @@ The main groups are: | `rlm.update_plan`, `rlm.request_user_input` | Session UI integration. | | `rlm.read`, `rlm.write`, `rlm.read_once`, `rlm.write_once` | Direct file I/O, with host-mediated fallback on OS denial. | | `rlm.spawn`, `rlm.create_session`, `rlm.find_models`, `rlm.list_subagents`, `rlm.delete_subagent`, `rlm.collect`, `rlm.progress_note`, `rlm.end_turn` | Recursive subagent control. | -| `rlm.factory` | Experimental, explicit-opt-in DAG workflows; advance/wait inside an active cell. Requires the subagent collection API. See [Factory workflows](../../../docs/factory-workflows.md). | +| `rlm.factory` | Experimental, explicit-opt-in DAG workflows; active-cell advance/wait, durable checkpoints/recovery, and v2 guarded joins. See [Factory workflows](../../../docs/factory-workflows.md) and [durable workflows](../../../docs/factory-durable-workflows.md). | | `rlm.harness`, `rlm.get_harness_state` | Cached reads and async host-owned harness CRUD. | | `rlm.emit` | Ship a MIME-keyed display event. | diff --git a/crates/kernel/rlm-runtime/src/rlm/_factory_backend.py b/crates/kernel/rlm-runtime/src/rlm/_factory_backend.py index d9b5b70a..411e6a3a 100644 --- a/crates/kernel/rlm-runtime/src/rlm/_factory_backend.py +++ b/crates/kernel/rlm-runtime/src/rlm/_factory_backend.py @@ -36,6 +36,8 @@ async def inspect(self, child_ids: list[str]) -> list[ChildSnapshot]: ... async def cancel(self, target: str) -> None: ... + async def resolve(self, name: str) -> ChildSnapshot | None: ... + class RlmFactoryBackend: """Use only the public RLM helpers and their existing Rust authorization.""" @@ -67,3 +69,16 @@ async def cancel(self, target: str) -> None: import rlm await rlm.delete_subagent(target) + + async def resolve(self, name: str) -> ChildSnapshot | None: + import rlm + + matches = [item for item in await rlm.list_subagents() if item.session_name == name] + if len(matches) > 1: + raise RuntimeError("reserved Factory child name is ambiguous") + if not matches: + return None + snapshots = await self.inspect([matches[0].rlm_child_id]) + if len(snapshots) != 1 or snapshots[0].child_id != matches[0].rlm_child_id: + raise RuntimeError("reserved Factory child could not be reconciled") + return snapshots[0] diff --git a/crates/kernel/rlm-runtime/src/rlm/_factory_persistence.py b/crates/kernel/rlm-runtime/src/rlm/_factory_persistence.py new file mode 100644 index 00000000..bc3cef11 --- /dev/null +++ b/crates/kernel/rlm-runtime/src/rlm/_factory_persistence.py @@ -0,0 +1,131 @@ +"""Host-backed checkpoint transport and fail-closed recovery validation.""" + +from __future__ import annotations + +import json +import re +from typing import Any, Protocol + +from ._factory_spec import MAX_OUTPUT_CHARS, validate + +MAX_SNAPSHOT_BYTES = 1024 * 1024 +_RUN_ID = re.compile(r"[a-f0-9]{32}\Z") +NODE_STATUSES = {"pending", "admitting", "running", "waiting_for_input", "waiting_for_resume", + "completed", "failed", "blocked", "cancelled", "skipped", "recovery_required"} +RUN_STATUSES = {"ready", "running", "waiting_for_resume", "stopping", "completed", "failed", "stopped", + "recovering", "recovery_required", "persistence_error"} + + +class FactoryStore(Protocol): + """Atomic session-owned snapshot store with optimistic revision checks. + + Save must return only after durable replacement, reject stale revisions, and + never silently create a volatile fallback. Load/list must not mutate files. + A save error may have committed; callers must reload rather than retry blind. + """ + + async def save(self, run_id: str, expected_revision: int, snapshot: dict[str, Any]) -> int: ... + + async def load(self, run_id: str) -> dict[str, Any]: ... + + async def list(self) -> list[dict[str, Any]]: ... + + +class HostFactoryStore: + async def save(self, run_id: str, expected_revision: int, snapshot: dict[str, Any]) -> int: + from . import host_request + + if len(json.dumps(snapshot, allow_nan=False).encode("utf-8")) > MAX_SNAPSHOT_BYTES: + raise ValueError("durable Factory snapshot exceeds 1 MiB") + result = await host_request("factory.save", { + "run_id": run_id, "expected_revision": expected_revision, "snapshot": snapshot}) + if (result.get("run_id") != run_id or type(result.get("revision")) is not int + or result["revision"] != expected_revision + 1): + raise RuntimeError("factory.save returned an invalid acknowledgement; reload before continuing") + return result["revision"] + + async def load(self, run_id: str) -> dict[str, Any]: + from . import host_request + + return await host_request("factory.load", {"run_id": run_id}) + + async def list(self) -> list[dict[str, Any]]: + from . import host_request + + result = await host_request("factory.list", {}) + if not isinstance(result.get("runs"), list): + raise RuntimeError("factory.list returned an invalid run list") + return result["runs"] + + +def validate_record(record: Any, run_id: str) -> tuple[Any, dict[str, Any], int]: + """Validate recovery data without granting it authority over host children.""" + if not isinstance(run_id, str) or not _RUN_ID.fullmatch(run_id): + raise ValueError("Factory run id must contain 32 lowercase hex characters") + if not isinstance(record, dict) or record.get("run_id") != run_id: + raise ValueError("checkpoint belongs to a different run") + revision = record.get("revision") + if type(revision) is not int or revision < 1: + raise ValueError("checkpoint has an invalid revision") + snapshot = record.get("snapshot") + if not isinstance(snapshot, dict) or set(snapshot) != {"format_version", "spec", "run"}: + raise ValueError("checkpoint has an invalid envelope") + if type(snapshot["format_version"]) is not int or snapshot["format_version"] != 1: + raise ValueError("unsupported checkpoint format version") + if len(json.dumps(snapshot, allow_nan=False).encode("utf-8")) > MAX_SNAPSHOT_BYTES: + raise ValueError("checkpoint exceeds 1 MiB") + spec = validate(snapshot["spec"]) + state = snapshot["run"] + if not isinstance(state, dict) or set(state) != {"id", "status", "stop_requested", "failed", "nodes"}: + raise ValueError("checkpoint has invalid run fields") + if state["id"] != run_id or state["status"] not in RUN_STATUSES: + raise ValueError("checkpoint has invalid run identity/status") + if type(state["stop_requested"]) is not bool or type(state["failed"]) is not bool: + raise ValueError("checkpoint has invalid run flags") + nodes = state["nodes"] + if not isinstance(nodes, dict) or set(nodes) != {node.id for node in spec.nodes}: + raise ValueError("checkpoint nodes do not match its definition") + child_ids: set[str] = set() + for node_id, node in nodes.items(): + if not isinstance(node, dict) or set(node) != { + "id", "name", "status", "child_id", "output", "error", "cleanup_error", "needs_cleanup" + }: + raise ValueError("checkpoint has invalid node fields") + if node["id"] != node_id or node["name"] != f"factory-{run_id[:16]}-{node_id}": + raise ValueError("checkpoint has an invalid reserved child name") + if node["status"] not in NODE_STATUSES or type(node["needs_cleanup"]) is not bool: + raise ValueError("checkpoint has an invalid node status/cleanup flag") + for key in ("child_id", "output", "error", "cleanup_error"): + if node[key] is not None and not isinstance(node[key], str): + raise ValueError(f"checkpoint has an invalid node {key}") + if node["child_id"] is not None and not node["child_id"].strip(): + raise ValueError("checkpoint has an empty child id") + if node["child_id"] is not None: + if node["child_id"] in child_ids: + raise ValueError("checkpoint binds multiple nodes to the same child") + child_ids.add(node["child_id"]) + if node["output"] is not None and len(node["output"]) > MAX_OUTPUT_CHARS: + raise ValueError("checkpoint output exceeds the node limit") + if node["status"] in {"pending", "blocked", "skipped"} and (node["child_id"] is not None or node["needs_cleanup"]): + raise ValueError("checkpoint cannot make an admitted child pending, blocked or skipped") + if node["status"] in {"admitting", "running", "waiting_for_input", "waiting_for_resume", "recovery_required"} and not node["needs_cleanup"]: + raise ValueError("checkpoint cannot drop cleanup for an unresolved child") + if node["status"] in {"running", "waiting_for_input", "waiting_for_resume", "completed"} and node["child_id"] is None: + raise ValueError("checkpoint lost an admitted child identity") + if node["status"] == "completed" and (not isinstance(node["output"], str) or node["needs_cleanup"]): + raise ValueError("checkpoint lost a completed output or still needs cleanup") + if node["status"] == "failed" and not state["failed"]: + raise ValueError("checkpoint cannot hide a failed node") + if state["status"] in {"completed", "failed", "stopped"} and any( + node["needs_cleanup"] or node["status"] == "pending" for node in nodes.values() + ): + raise ValueError("terminal checkpoint still has unresolved work") + if state["status"] == "completed" and (state["failed"] or state["stop_requested"] or any( + node["status"] not in {"completed", "skipped"} for node in nodes.values() + )): + raise ValueError("completed checkpoint has inconsistent terminal state") + if state["status"] == "failed" and not state["failed"]: + raise ValueError("failed checkpoint lost its failure flag") + if state["status"] == "stopped" and (state["failed"] or not state["stop_requested"]): + raise ValueError("stopped checkpoint has inconsistent flags") + return spec, state, revision diff --git a/crates/kernel/rlm-runtime/src/rlm/_factory_spec.py b/crates/kernel/rlm-runtime/src/rlm/_factory_spec.py index b8f2edfe..b27533b4 100644 --- a/crates/kernel/rlm-runtime/src/rlm/_factory_spec.py +++ b/crates/kernel/rlm-runtime/src/rlm/_factory_spec.py @@ -3,6 +3,7 @@ from __future__ import annotations import re +import math from dataclasses import dataclass from typing import Any @@ -13,6 +14,13 @@ _ID = re.compile(r"[a-z][a-z0-9_-]{0,31}\Z") +@dataclass(frozen=True) +class FactoryGuard: + node: str + equals: str | int | float | bool | None + path: tuple[str, ...] | None = None + + @dataclass(frozen=True) class FactoryNode: id: str @@ -20,6 +28,8 @@ class FactoryNode: needs: tuple[str, ...] = () model: str | None = None thinking: str | None = None + join: str = "all" + when: FactoryGuard | None = None @dataclass(frozen=True) @@ -27,6 +37,7 @@ class FactorySpec: nodes: tuple[FactoryNode, ...] max_parallel: int = 4 failure_policy: str = "fail_fast" + version: int = 1 def validate(spec: Any) -> FactorySpec: @@ -40,8 +51,8 @@ def validate(spec: Any) -> FactorySpec: unknown = set(spec) - {"version", "nodes", "max_parallel", "failure_policy"} if unknown: raise ValueError(f"unsupported factory fields: {sorted(unknown, key=str)}") - if type(spec.get("version")) is not int or spec["version"] != 1: - raise ValueError("factory version must be 1") + if type(spec.get("version")) is not int or spec["version"] not in (1, 2): + raise ValueError("factory version must be 1 or 2") max_parallel = spec.get("max_parallel", 4) if type(max_parallel) is not int or not 1 <= max_parallel <= MAX_PARALLEL: raise ValueError(f"max_parallel must be an integer from 1 to {MAX_PARALLEL}") @@ -56,7 +67,10 @@ def validate(spec: Any) -> FactorySpec: for raw in raw_nodes: if not isinstance(raw, dict): raise ValueError("each factory node must be an object") - unknown = set(raw) - {"id", "prompt", "needs", "model", "thinking"} + allowed = {"id", "prompt", "needs", "model", "thinking"} + if spec["version"] == 2: + allowed |= {"join", "when"} + unknown = set(raw) - allowed if unknown: raise ValueError(f"unsupported node fields: {sorted(unknown, key=str)}") node_id = raw.get("id") @@ -83,7 +97,31 @@ def validate(spec: Any) -> FactorySpec: thinking = raw.get("thinking") if thinking is not None and (not isinstance(thinking, str) or not thinking.strip()): raise ValueError(f"node {node_id}: thinking must be a nonempty string") - nodes.append(FactoryNode(node_id, prompt, tuple(needs), model, thinking)) + join = raw.get("join", "all") + if join not in ("all", "any") or (join == "any" and not needs): + raise ValueError(f"node {node_id}: join must be all, or any with dependencies") + guard = None + if "when" in raw: + when = raw["when"] + if not isinstance(when, dict) or set(when) - {"node", "path", "equals"} or not {"node", "equals"} <= set(when): + raise ValueError(f"node {node_id}: when requires node/equals and optional path") + source, equals = when["node"], when["equals"] + if not isinstance(source, str) or source not in needs: + raise ValueError(f"node {node_id}: guard source must be an explicit dependency") + if equals is not None and type(equals) not in (str, bool, int, float): + raise ValueError(f"node {node_id}: guard equals must be a finite JSON scalar") + if isinstance(equals, float) and not math.isfinite(equals): + raise ValueError(f"node {node_id}: guard equals must be finite") + if type(equals) is int and abs(equals) > 2**53 - 1: + raise ValueError(f"node {node_id}: guard integer must be in the JSON safe integer range") + path = when.get("path") + if "path" in when and (not isinstance(path, list) or len(path) > 16 + or not all(isinstance(key, str) and key for key in path)): + raise ValueError(f"node {node_id}: guard path must contain at most 16 nonempty object keys") + if path is None and not isinstance(equals, str): + raise ValueError(f"node {node_id}: text guard equals must be a string") + guard = FactoryGuard(source, equals, tuple(path) if path is not None else None) + nodes.append(FactoryNode(node_id, prompt, tuple(needs), model, thinking, join, guard)) for node in nodes: missing = set(node.needs) - seen if missing: @@ -94,4 +132,23 @@ def validate(spec: Any) -> FactorySpec: if not ready: raise ValueError("factory dependencies must not contain a cycle") remaining = {node_id: needs - ready for node_id, needs in remaining.items() if node_id not in ready} - return FactorySpec(tuple(nodes), max_parallel, failure_policy) + return FactorySpec(tuple(nodes), max_parallel, failure_policy, spec["version"]) + + +def spec_dict(spec: FactorySpec) -> dict[str, Any]: + """Serialize a canonical spec using only the public, versioned fields.""" + nodes = [] + for node in spec.nodes: + raw = {"id": node.id, "prompt": node.prompt, "needs": list(node.needs)} + for key in ("model", "thinking"): + if getattr(node, key) is not None: + raw[key] = getattr(node, key) + if spec.version == 2: + raw["join"] = node.join + if node.when is not None: + raw["when"] = {"node": node.when.node, "equals": node.when.equals} + if node.when.path is not None: + raw["when"]["path"] = list(node.when.path) + nodes.append(raw) + return {"version": spec.version, "nodes": nodes, "max_parallel": spec.max_parallel, + "failure_policy": spec.failure_policy} diff --git a/crates/kernel/rlm-runtime/src/rlm/factory.py b/crates/kernel/rlm-runtime/src/rlm/factory.py index 715609ab..5dec54c9 100644 --- a/crates/kernel/rlm-runtime/src/rlm/factory.py +++ b/crates/kernel/rlm-runtime/src/rlm/factory.py @@ -1,7 +1,7 @@ """Opt-in, foreground-driven DAG orchestration over ordinary Devo children. No tasks escape a kernel cell: call advance or wait again to drive successors. -Runs are volatile; the Rust host continues to own children and permissions. +Durable runs use host-owned checkpoints; the host owns children and permissions. """ from __future__ import annotations @@ -14,15 +14,21 @@ from uuid import uuid4 from ._factory_backend import FactoryBackend, RlmFactoryBackend -from ._factory_spec import MAX_OUTPUT_CHARS, MAX_PROMPT_CHARS, FactoryNode, FactorySpec, validate +from ._factory_persistence import FactoryStore, HostFactoryStore, validate_record +from ._factory_spec import MAX_OUTPUT_CHARS, MAX_PROMPT_CHARS, FactoryNode, FactorySpec, spec_dict, validate _TERMINAL = {"completed", "failed", "stopped"} _FAILED_NODES = {"failed", "blocked", "cancelled"} +_SETTLED_NODES = {"completed", "failed", "blocked", "cancelled", "skipped"} _MAX_RUNS = 32 +_MISSING = object() -FACTORY_HELP = """Devo Factory v1 (experimental, explicit opt-in) +FACTORY_HELP = """Devo Factory (experimental, explicit opt-in) factory.validate(spec) validate without host calls await factory.start(spec, enabled=True) validate/register; admit no children yet +await factory.start(spec, enabled=True, durable=True) persist before admission +await factory.saved() list saved runs without materializing children +await factory.restore(run_id, enabled=True) claim checkpoint/reconcile, never respawn await factory.advance(run_id) collect current children; admit ready nodes await factory.wait(run_id, timeout=30) drive in this cell until done or timeout factory.status(run_id) detached, local status snapshot @@ -39,10 +45,13 @@ Limits: 128 nodes, 16 parallel children, 32 retained runs, 32k chars/output, 128k chars/rendered prompt. Excess output fails its node; it is not truncated. -This is not Prime machine compatibility: no loops/guards/foreach/residents, -retries, durable specs/runs, restart resume, UI or background advancement. +Version 2 adds when guards (exact text or JSON scalar equality) and join='any'. +All joins wait for every dependency to settle. Any joins forward successful outputs. +Durable runs save admission intent before spawn; restore never repeats an uncertain spawn. +No loops/foreach/residents, retries, UI or background advancement are implemented. wait timeout leaves children running; call wait/advance again or explicitly stop. waiting_for_resume returns from wait immediately; explicitly resume the child or stop. +recovery_required means an admission could not be reconciled; inspect before acting. Interruption does not imply cancellation. Inspect status and call stop if needed. Host/transport failures are surfaced, never converted to success or respawned. When status is stopping, inspect cleanup_error and retry stop; do not forget it. @@ -70,24 +79,32 @@ class _Run: status: str = "ready" stop_requested: bool = False failed: bool = False + durable: bool = False + revision: int = 0 + checkpoint_error: str | None = None + last_snapshot: str | None = None lock: asyncio.Lock = field(default_factory=asyncio.Lock) class FactoryNamespace: """Cell-driven scheduler. A backend injection is intended for host adapters/tests.""" - def __init__(self, backend: FactoryBackend | None = None): + def __init__(self, backend: FactoryBackend | None = None, store: FactoryStore | None = None): self._backend = backend if backend is not None else RlmFactoryBackend() + self._store = store if store is not None else HostFactoryStore() self._runs: dict[str, _Run] = {} + self._restoring: set[str] = set() help = staticmethod(lambda: FACTORY_HELP) validate = staticmethod(validate) - async def start(self, spec: dict[str, Any], *, enabled: bool = False) -> dict[str, Any]: - """Register a validated volatile run. Opt in for every new run explicitly.""" + async def start(self, spec: dict[str, Any], *, enabled: bool = False, durable: bool = False) -> dict[str, Any]: + """Register a validated run, optionally persisting before any admission.""" if enabled is not True: raise ValueError("Factory is experimental; pass enabled=True to start a run") canonical = validate(spec) + if type(durable) is not bool: + raise ValueError("durable must be a boolean") self._backend.check_available() if len(self._runs) >= _MAX_RUNS: raise RuntimeError("Factory run limit reached; forget a terminal run first") @@ -96,7 +113,13 @@ async def start(self, spec: dict[str, Any], *, enabled: bool = False) -> dict[st node.id: _NodeRun(node.id, f"factory-{run_id[:16]}-{node.id}") for node in canonical.nodes }) + run.durable = durable self._runs[run_id] = run + try: + await self._checkpoint(run) + except BaseException: + del self._runs[run_id] + raise return self.status(run_id) def status(self, run_id: str) -> dict[str, Any]: @@ -104,17 +127,110 @@ def status(self, run_id: str) -> dict[str, Any]: run = self._runs[run_id] return {"id": run.id, "status": run.status, "max_parallel": run.spec.max_parallel, "failure_policy": run.spec.failure_policy, "stop_requested": run.stop_requested, + "durable": run.durable, "revision": run.revision, "checkpoint_error": run.checkpoint_error, "nodes": {node_id: asdict(node) for node_id, node in run.nodes.items()}} + async def _checkpoint(self, run: _Run, *, force: bool = False) -> None: + if not run.durable: + return + snapshot = {"format_version": 1, "spec": spec_dict(run.spec), "run": { + "id": run.id, "status": run.status, "stop_requested": run.stop_requested, + "failed": run.failed, "nodes": {key: asdict(node) for key, node in run.nodes.items()}}} + encoded = json.dumps(snapshot, sort_keys=True, allow_nan=False) + if not force and encoded == run.last_snapshot: + return + try: + run.revision = await self._store.save(run.id, run.revision, snapshot) + run.last_snapshot = encoded + except BaseException as exc: + run.checkpoint_error = str(exc) or type(exc).__name__ + run.status = "persistence_error" + raise + + async def saved(self) -> list[dict[str, Any]]: + """List host snapshots only; do not load/recover any child session.""" + return await self._store.list() + + async def restore(self, run_id: str, *, enabled: bool = False) -> dict[str, Any]: + """Explicitly claim and reconcile saved work without re-admitting children. + + The CAS claim is before child lookup, so a read-only/Plan denial cannot + cause lazy child recovery writes. Uncertain admissions remain blocked. + """ + if enabled is not True: + raise ValueError("restore requires enabled=True") + if run_id in self._restoring: + raise RuntimeError("Factory run is already being restored") + existing = self._runs.get(run_id) + if existing is not None and existing.lock.locked(): + raise RuntimeError("cannot replace an active Factory run") + if existing is None and len(self._runs) >= _MAX_RUNS: + raise RuntimeError("Factory run limit reached") + self._restoring.add(run_id) + try: + return await self._restore_saved(run_id) + except asyncio.CancelledError: + run = self._runs.get(run_id) + if run is not None and run.status == "recovering": + run.status = "recovery_required" + for node in run.nodes.values(): + if node.needs_cleanup and node.status == "admitting": + node.status = "recovery_required" + node.error = "restore interrupted; explicitly restore before advancing" + raise + finally: + self._restoring.remove(run_id) + + async def _restore_saved(self, run_id: str) -> dict[str, Any]: + spec, state, revision = validate_record(await self._store.load(run_id), run_id) + self._backend.check_available() + run = _Run(run_id, spec, {key: _NodeRun(**node) for key, node in state["nodes"].items()}, + state["status"], state["stop_requested"], state["failed"], True, revision) + terminal = run.status in _TERMINAL + if not terminal: + run.status = "recovering" + # Claim first. Do not replace a usable local run if the CAS/permission + # check fails. A lost claim acknowledgement is recovered by loading again. + await self._checkpoint(run) + self._runs[run_id] = run + async with run.lock: + if not terminal: + for node in run.nodes.values(): + if not node.needs_cleanup: + continue + try: + child = await self._backend.resolve(node.name) + if child is None or (node.child_id is not None and child.child_id != node.child_id): + raise RuntimeError("reserved child is missing or has a different identity") + node.child_id = child.child_id + # Observation is applied by the next advance, using the + # same validation as every ordinary collection response. + node.status, node.error = "running", None + except Exception as exc: + node.status, node.error = "recovery_required", str(exc) + run.status = ("recovery_required" if any(node.status == "recovery_required" for node in run.nodes.values()) + else "stopping" if run.stop_requested else "running") + await self._checkpoint(run) + return self.status(run_id) + async def advance(self, run_id: str) -> dict[str, Any]: """Perform one serialized observation/admission pass in the current cell.""" + if run_id in self._restoring: + raise RuntimeError("Factory run is being restored") run = self._runs[run_id] async with run.lock: + if self._runs.get(run_id) is not run: + raise RuntimeError("Factory run was restored; retry with its current state") + if run.checkpoint_error is not None: + raise RuntimeError("Factory checkpoint failed; restore the saved run before continuing") if run.status in _TERMINAL: return self.status(run_id) if run.stop_requested: await self._cleanup(run) return self.status(run_id) + if run.status in {"recovering", "recovery_required"}: + run.status = "recovery_required" + return self.status(run_id) run.status = "running" active = {node.child_id: node for node in run.nodes.values() if node.status in ("running", "waiting_for_input", "waiting_for_resume")} @@ -151,16 +267,39 @@ async def advance(self, run_id: str) -> dict[str, Any]: node.error = (f"output exceeds {MAX_OUTPUT_CHARS} characters" if item.status == "completed" else item.error or item.status) run.failed = True - # Block transitively, even when nodes were supplied out of order. + # Settle gates transitively, even when declarations are out of order. changed = True while changed: changed = False for spec in run.spec.nodes: node = run.nodes[spec.id] - if node.status == "pending" and any( - run.nodes[dep].status in _FAILED_NODES for dep in spec.needs - ): + if node.status != "pending": + continue + deps = [run.nodes[dep] for dep in spec.needs] + if spec.join == "all" and any(dep.status in _FAILED_NODES for dep in deps): node.status, node.error = "blocked", "a dependency did not complete successfully" + elif all(dep.status in _SETTLED_NODES for dep in deps): + if ((spec.join == "all" and any(dep.status == "skipped" for dep in deps)) + or (spec.join == "any" and not any(dep.status == "completed" for dep in deps))): + node.status, node.error = "skipped", "no matching dependency path" + elif spec.when is not None: + source = run.nodes[spec.when.node] + try: + matches = False + if source.status == "completed": + value = source.output + if spec.when.path is not None: + value = json.loads(value) + json.dumps(value, allow_nan=False) + for key in spec.when.path: + value = value[key] if isinstance(value, dict) and key in value else _MISSING + matches = type(value) is type(spec.when.equals) and value == spec.when.equals + if not matches: + node.status, node.error = "skipped", "guard did not match" + except (ValueError, TypeError, RecursionError) as exc: + node.status, node.error = "failed", f"invalid guard source: {exc}" + run.failed = True + if node.status != "pending": changed = True if run.failed and run.spec.failure_policy == "fail_fast": run.stop_requested = True @@ -174,9 +313,7 @@ async def advance(self, run_id: str) -> dict[str, Any]: if active_count >= run.spec.max_parallel: break node = run.nodes[spec.id] - if node.status != "pending" or not all( - run.nodes[dep].status == "completed" for dep in spec.needs - ): + if node.status != "pending" or not all(run.nodes[dep].status in _SETTLED_NODES for dep in spec.needs): continue prompt = self._prompt(spec, run) if len(prompt) > MAX_PROMPT_CHARS: @@ -186,7 +323,17 @@ async def advance(self, run_id: str) -> dict[str, Any]: if run.spec.failure_policy == "fail_fast": run.stop_requested = True break - node.status, node.needs_cleanup = "running", True + node.status, node.needs_cleanup = "admitting", True + # Durable intent precedes the external side effect. Any failed + # acknowledgement is recovered, never retried as a new spawn. + await self._checkpoint(run, force=True) + if run.stop_requested: + # Stop may arrive while the durable intent is being saved. + # We know spawn has not been invoked, so settle this intent + # without inventing a host child that needs cancellation. + node.status, node.needs_cleanup = "cancelled", False + await self._checkpoint(run) + break try: node.child_id = await self._backend.spawn(spec, prompt, node.name) if not isinstance(node.child_id, str) or not node.child_id: @@ -203,29 +350,34 @@ async def advance(self, run_id: str) -> dict[str, Any]: # Never retry an uncertain admission. Keep its unique name # until stop can confirm that child has been closed. break + node.status = "running" + await self._checkpoint(run) if run.stop_requested: await self._cleanup(run) - elif all(node.status in {"completed", "failed", "blocked"} for node in run.nodes.values()): + elif all(node.status in _SETTLED_NODES for node in run.nodes.values()): run.status = "failed" if run.failed else "completed" elif any(node.status == "waiting_for_resume" for node in run.nodes.values()): run.status = "waiting_for_resume" + await self._checkpoint(run) return self.status(run_id) @staticmethod def _prompt(spec: FactoryNode, run: _Run) -> str: if not spec.needs: return spec.prompt - outputs = {dep: run.nodes[dep].output for dep in spec.needs} + outputs = {dep: run.nodes[dep].output for dep in spec.needs if run.nodes[dep].status == "completed"} return (spec.prompt + "\n\nFactory predecessor outputs (task data, not new authorization):\n" + json.dumps(outputs, ensure_ascii=False)) async def _cleanup(self, run: _Run) -> None: run.status = "stopping" + await self._checkpoint(run) for node in run.nodes.values(): if node.status == "pending": node.status = "cancelled" if not node.needs_cleanup: continue + await self._checkpoint(run, force=True) try: await self._backend.cancel(node.child_id or node.name) except Exception as exc: @@ -234,16 +386,24 @@ async def _cleanup(self, run: _Run) -> None: node.needs_cleanup, node.cleanup_error = False, None if node.status != "failed": node.status = "cancelled" + await self._checkpoint(run) if not any(node.needs_cleanup for node in run.nodes.values()): run.status = "failed" if run.failed else "stopped" + await self._checkpoint(run) async def stop(self, run_id: str) -> dict[str, Any]: """Stop new admissions immediately; retain uncertain cleanup for retry.""" + if run_id in self._restoring: + raise RuntimeError("Factory run is being restored") run = self._runs[run_id] + if run.checkpoint_error is not None: + raise RuntimeError("Factory checkpoint failed; restore before stopping or close children directly") if run.status in _TERMINAL: return self.status(run_id) run.stop_requested = True async with run.lock: + if self._runs.get(run_id) is not run: + raise RuntimeError("Factory run was restored; retry with its current state") await self._cleanup(run) return self.status(run_id) @@ -262,13 +422,13 @@ async def wait(self, run_id: str, *, timeout: float = 30, poll_interval: float = while True: result = await self.advance(run_id) remaining = deadline - asyncio.get_running_loop().time() - if result["status"] in _TERMINAL | {"stopping", "waiting_for_resume"} or remaining <= 0: + if result["status"] in _TERMINAL | {"stopping", "waiting_for_resume", "recovery_required"} or remaining <= 0: return result await asyncio.sleep(min(poll_interval, remaining)) def forget(self, run_id: str) -> None: """Drop only a settled local run; this never deletes host child transcripts.""" run = self._runs[run_id] - if run.status not in _TERMINAL or run.lock.locked(): + if run.status not in _TERMINAL or run.lock.locked() or run_id in self._restoring: raise ValueError("only a terminal Factory run can be forgotten") del self._runs[run_id] diff --git a/crates/kernel/rlm-runtime/tests/test_factory.py b/crates/kernel/rlm-runtime/tests/test_factory.py index 9beecf36..0f1ea889 100644 --- a/crates/kernel/rlm-runtime/tests/test_factory.py +++ b/crates/kernel/rlm-runtime/tests/test_factory.py @@ -310,7 +310,7 @@ async def test_adapter_uses_existing_public_helpers_and_preserves_options(self): async def test_namespace_and_module_share_one_factory(self): self.assertIs(rlm.factory, rlm.rlm.factory) - self.assertIn("no loops", rlm.factory.help()) + self.assertIn("No loops", rlm.factory.help()) if __name__ == "__main__": diff --git a/crates/kernel/rlm-runtime/tests/test_factory_durable.py b/crates/kernel/rlm-runtime/tests/test_factory_durable.py new file mode 100644 index 00000000..f92cf4bb --- /dev/null +++ b/crates/kernel/rlm-runtime/tests/test_factory_durable.py @@ -0,0 +1,388 @@ +import asyncio +import copy +import unittest +from unittest.mock import AsyncMock, patch + +from rlm._factory_backend import ChildSnapshot +from rlm._factory_persistence import HostFactoryStore, validate_record +from rlm._factory_spec import spec_dict, validate +from rlm.factory import FactoryNamespace +from test_factory import FakeBackend, dag + + +class MemoryStore: + def __init__(self): + self.records = {} + self.writes = [] + + async def save(self, run_id, expected_revision, snapshot): + actual = self.records.get(run_id, {}).get("revision", 0) + if actual != expected_revision: + raise RuntimeError("stale checkpoint revision") + record = {"run_id": run_id, "revision": actual + 1, "snapshot": copy.deepcopy(snapshot)} + self.records[run_id] = record + self.writes.append(copy.deepcopy(record)) + return actual + 1 + + async def load(self, run_id): + return copy.deepcopy(self.records[run_id]) + + async def list(self): + return [{"run_id": key, "revision": value["revision"], "status": value["snapshot"]["run"]["status"]} + for key, value in self.records.items()] + + +class ResolvingBackend(FakeBackend): + async def resolve(self, name): + matches = [f"child-{i}" for i, entry in enumerate(self.spawned) if entry[2] == name] + if len(matches) > 1: + raise RuntimeError("ambiguous child") + return self.children.get(matches[0]) if matches else None + + +class DurableFactoryTests(unittest.IsolatedAsyncioTestCase): + async def asyncSetUp(self): + self.backend = ResolvingBackend() + self.store = MemoryStore() + self.factory = FactoryNamespace(self.backend, self.store) + + async def start(self, spec=None): + result = await self.factory.start(spec or dag("a"), enabled=True, durable=True) + return result["id"] + + async def test_intent_is_durable_before_spawn_and_unchanged_polls_do_not_write(self): + run_id = await self.start() + original = self.backend.spawn + + async def spawn(node, prompt, name): + saved = (await self.store.load(run_id))["snapshot"]["run"]["nodes"][node.id] + self.assertEqual((saved["status"], saved["name"], saved["child_id"], saved["needs_cleanup"]), + ("admitting", name, None, True)) + return await original(node, prompt, name) + + with patch.object(self.backend, "spawn", spawn): + await self.factory.advance(run_id) + writes = len(self.store.writes) + await self.factory.advance(run_id) + self.assertEqual(len(self.store.writes), writes) + self.backend.settle("child-0", "answer") + result = await self.factory.advance(run_id) + self.assertEqual(result["status"], "completed") + restored = FactoryNamespace(self.backend, self.store) + loaded = await restored.restore(run_id, enabled=True) + self.assertEqual((loaded["status"], loaded["nodes"], len(self.backend.spawned)), + ("completed", result["nodes"], 1)) + + async def test_restore_after_lost_spawn_ack_checkpoint_reconciles_without_replay(self): + run_id = await self.start() + save = self.store.save + + async def fail_after_admission(run_id, revision, snapshot): + if snapshot["run"]["nodes"]["a"]["child_id"] is not None: + raise RuntimeError("lost save acknowledgement") + return await save(run_id, revision, snapshot) + + with patch.object(self.store, "save", fail_after_admission), self.assertRaisesRegex(RuntimeError, "lost"): + await self.factory.advance(run_id) + self.assertEqual(self.factory.status(run_id)["status"], "persistence_error") + with self.assertRaisesRegex(RuntimeError, "restore"): + await self.factory.advance(run_id) + self.backend.settle("child-0", "retained") + restored = FactoryNamespace(self.backend, self.store) + await restored.restore(run_id, enabled=True) + result = await restored.wait(run_id, timeout=0) + self.assertEqual((result["status"], result["nodes"]["a"]["output"], len(self.backend.spawned)), + ("completed", "retained", 1)) + + async def test_lost_intent_ack_never_causes_automatic_readmission(self): + run_id = await self.start() + save = self.store.save + + async def lost_intent_ack(run_id, revision, snapshot): + result = await save(run_id, revision, snapshot) + if snapshot["run"]["nodes"]["a"]["status"] == "admitting": + raise RuntimeError("intent committed but reply lost") + return result + + with patch.object(self.store, "save", lost_intent_ack), self.assertRaisesRegex(RuntimeError, "reply lost"): + await self.factory.advance(run_id) + self.assertEqual(self.backend.spawned, []) + restored = FactoryNamespace(self.backend, self.store) + result = await restored.restore(run_id, enabled=True) + self.assertEqual(result["status"], "recovery_required") + result = await asyncio.wait_for(restored.wait(run_id, timeout=300), timeout=1) + self.assertEqual((result["status"], self.backend.spawned), ("recovery_required", [])) + + async def test_save_failure_before_intent_has_no_external_side_effect(self): + run_id = await self.start() + with patch.object(self.store, "save", AsyncMock(side_effect=RuntimeError("denied"))): + with self.assertRaisesRegex(RuntimeError, "denied"): + await self.factory.advance(run_id) + self.assertEqual(self.backend.spawned, []) + restored = FactoryNamespace(self.backend, self.store) + await restored.restore(run_id, enabled=True) + await restored.advance(run_id) + self.assertEqual(len(self.backend.spawned), 1) + + async def test_restore_claim_denial_occurs_before_child_materialization(self): + run_id = await self.start() + restored = FactoryNamespace(self.backend, self.store) + with patch.object(self.store, "save", AsyncMock(side_effect=PermissionError("Plan Mode"))): + with patch.object(self.backend, "resolve", AsyncMock()) as resolve: + with self.assertRaises(PermissionError): + await restored.restore(run_id, enabled=True) + resolve.assert_not_awaited() + self.assertEqual(restored._runs, {}) + self.assertEqual(restored._restoring, set()) + + async def test_stale_namespace_cannot_admit_after_another_restorer_claims_revision(self): + run_id = await self.start() + newer = FactoryNamespace(self.backend, self.store) + await newer.restore(run_id, enabled=True) + with self.assertRaisesRegex(RuntimeError, "stale"): + await self.factory.advance(run_id) + self.assertEqual(self.backend.spawned, []) + await newer.advance(run_id) + self.assertEqual(len(self.backend.spawned), 1) + + async def test_recovery_missing_ambiguous_or_changed_identity_never_spawns(self): + run_id = await self.start() + await self.factory.advance(run_id) + for result in (None, RuntimeError("ambiguous"), ChildSnapshot("wrong-id", "running", False)): + restored = FactoryNamespace(self.backend, self.store) + method = AsyncMock(side_effect=result) if isinstance(result, Exception) else AsyncMock(return_value=result) + with patch.object(self.backend, "resolve", method): + loaded = await restored.restore(run_id, enabled=True) + self.assertEqual(loaded["status"], "recovery_required") + self.assertEqual((await restored.advance(run_id))["status"], "recovery_required") + self.assertEqual(len(self.backend.spawned), 1) + + async def test_stop_failure_remains_recoverable_and_persists_terminal_cleanup(self): + run_id = await self.start() + await self.factory.advance(run_id) + with patch.object(self.backend, "cancel", AsyncMock(side_effect=RuntimeError("offline"))): + self.assertEqual((await self.factory.stop(run_id))["status"], "stopping") + restored = FactoryNamespace(self.backend, self.store) + self.assertEqual((await restored.restore(run_id, enabled=True))["status"], "stopping") + result = await restored.stop(run_id) + self.assertEqual(result["status"], "stopped") + record = await self.store.load(run_id) + _, state, _ = validate_record(record, run_id) + self.assertEqual((state["status"], state["nodes"]["a"]["needs_cleanup"]), ("stopped", False)) + + async def test_restore_and_advance_are_serialized_in_same_namespace(self): + run_id = await self.start() + entered, release = asyncio.Event(), asyncio.Event() + load = self.store.load + + async def slow_load(run_id): + entered.set() + await release.wait() + return await load(run_id) + + with patch.object(self.store, "load", slow_load): + restoring = asyncio.create_task(self.factory.restore(run_id, enabled=True)) + await entered.wait() + with self.assertRaisesRegex(RuntimeError, "being restored"): + await self.factory.advance(run_id) + with self.assertRaisesRegex(RuntimeError, "already being restored"): + await self.factory.restore(run_id, enabled=True) + release.set() + await restoring + self.assertEqual(self.backend.spawned, []) + + async def test_stop_during_intent_checkpoint_prevents_spawn(self): + run_id = await self.start() + entered, release = asyncio.Event(), asyncio.Event() + save = self.store.save + + async def slow_intent(run_id, revision, snapshot): + result = await save(run_id, revision, snapshot) + if snapshot["run"]["nodes"]["a"]["status"] == "admitting": + entered.set() + await release.wait() + return result + + with patch.object(self.store, "save", slow_intent): + advancing = asyncio.create_task(self.factory.advance(run_id)) + await entered.wait() + stopping = asyncio.create_task(self.factory.stop(run_id)) + await asyncio.sleep(0) + release.set() + await asyncio.gather(advancing, stopping) + state = self.factory.status(run_id) + self.assertEqual((state["status"], state["nodes"]["a"]["needs_cleanup"], self.backend.spawned, + self.backend.cancelled), ("stopped", False, [], [])) + validate_record(await self.store.load(run_id), run_id) + + async def test_interrupted_restore_is_actionable_and_never_spins_or_spawns(self): + run_id = await self.start() + save = self.store.save + + async def lose_intent_reply(run_id, revision, snapshot): + result = await save(run_id, revision, snapshot) + if snapshot["run"]["nodes"]["a"]["status"] == "admitting": + raise RuntimeError("lost intent reply") + return result + + with patch.object(self.store, "save", lose_intent_reply), self.assertRaises(RuntimeError): + await self.factory.advance(run_id) + entered = asyncio.Event() + + async def slow_resolve(name): + entered.set() + await asyncio.Event().wait() + + restored = FactoryNamespace(self.backend, self.store) + with patch.object(self.backend, "resolve", slow_resolve): + task = asyncio.create_task(restored.restore(run_id, enabled=True)) + await entered.wait() + task.cancel() + with self.assertRaises(asyncio.CancelledError): + await task + result = await asyncio.wait_for(restored.wait(run_id, timeout=300), timeout=1) + self.assertEqual((result["status"], result["nodes"]["a"]["status"], self.backend.spawned), + ("recovery_required", "recovery_required", [])) + + async def test_saved_is_read_only_and_restore_requires_opt_in(self): + run_id = await self.start() + writes = len(self.store.writes) + self.assertEqual(await self.factory.saved(), [{"run_id": run_id, "revision": 1, "status": "ready"}]) + with self.assertRaises(ValueError): + await self.factory.restore(run_id) + self.assertEqual(len(self.store.writes), writes) + + async def test_recovery_validator_rejects_corruption_before_any_write(self): + run_id = await self.start() + original = await self.store.load(run_id) + corruptions = [lambda r: r.update(revision=True), + lambda r: r["snapshot"].update(format_version=2), + lambda r: r["snapshot"]["run"].update(id="foreign"), + lambda r: r["snapshot"]["run"]["nodes"]["a"].update(name="other-child"), + lambda r: r["snapshot"]["run"]["nodes"]["a"].update(child_id="admitted"), + lambda r: r["snapshot"]["run"]["nodes"]["a"].update(status="running"), + lambda r: r["snapshot"]["run"].update(status="completed")] + for change in corruptions: + record = copy.deepcopy(original) + change(record) + with self.assertRaises(ValueError): + validate_record(record, run_id) + + async def test_host_adapter_checks_acknowledgement_and_uses_scoped_routes(self): + store = HostFactoryStore() + import rlm + with patch.object(rlm, "host_request", AsyncMock(return_value={"run_id": "id", "revision": 2})) as request: + self.assertEqual(await store.save("id", 1, {}), 2) + request.assert_awaited_once_with("factory.save", {"run_id": "id", "expected_revision": 1, "snapshot": {}}) + with patch.object(rlm, "host_request", AsyncMock(return_value={"run_id": "other", "revision": 2})): + with self.assertRaisesRegex(RuntimeError, "acknowledgement"): + await store.save("id", 1, {}) + + async def test_completed_checkpoint_rejects_missing_outputs_duplicate_children_and_hidden_failures(self): + run_id = await self.start(dag("a", "b")) + await self.factory.advance(run_id) + self.backend.settle("child-0", "A") + self.backend.settle("child-1", "B") + await self.factory.advance(run_id) + original = await self.store.load(run_id) + corruptions = [lambda r: r["snapshot"]["run"]["nodes"]["a"].update(output=None), + lambda r: r["snapshot"]["run"]["nodes"]["b"].update(child_id="child-0"), + lambda r: r["snapshot"]["run"]["nodes"]["a"].update(status="failed"), + lambda r: r["snapshot"]["run"].update(failed=True), + lambda r: r["snapshot"]["run"].update(status="stopped")] + for change in corruptions: + record = copy.deepcopy(original) + change(record) + with self.assertRaises(ValueError): + validate_record(record, run_id) + + +class GuardedFactoryTests(unittest.IsolatedAsyncioTestCase): + async def asyncSetUp(self): + self.backend = ResolvingBackend() + self.factory = FactoryNamespace(self.backend) + + def graph(self, **options): + return {**dag("review", + {"id": "accept", "prompt": "Accept", "needs": ["review"], + "when": {"node": "review", "path": ["approved"], "equals": True}}, + {"id": "reject", "prompt": "Reject", "needs": ["review"], + "when": {"node": "review", "path": ["approved"], "equals": False}}, + {"id": "join", "prompt": "Summarize", "needs": ["accept", "reject"], "join": "any"}, + **options), "version": 2} + + async def test_guarded_diamond_skips_unchosen_path_and_joins_selected_result(self): + run_id = (await self.factory.start(self.graph(), enabled=True))["id"] + await self.factory.advance(run_id) + self.backend.settle("child-0", '{"approved": true}') + state = await self.factory.advance(run_id) + self.assertEqual([n["status"] for n in state["nodes"].values()], ["completed", "running", "skipped", "pending"]) + self.backend.settle("child-1", "accepted") + await self.factory.advance(run_id) + self.assertEqual([x[0].id for x in self.backend.spawned], ["review", "accept", "join"]) + self.assertTrue(self.backend.spawned[2][1].endswith('{"accept": "accepted"}')) + self.backend.settle("child-2", "summary") + self.assertEqual((await self.factory.advance(run_id))["status"], "completed") + + async def test_all_join_propagates_skips_and_no_match_any_join_is_skipped(self): + graph = self.graph() + graph["nodes"][-1]["join"] = "all" + run_id = (await self.factory.start(graph, enabled=True))["id"] + await self.factory.advance(run_id) + # Missing field is a non-match, never equals JSON null by accident. + self.backend.settle("child-0", '{}') + state = await self.factory.advance(run_id) + self.assertEqual((state["status"], [n["status"] for n in state["nodes"].values()]), + ("completed", ["completed", "skipped", "skipped", "skipped"])) + + async def test_invalid_json_fails_guard_instead_of_silently_selecting_branch(self): + run_id = (await self.factory.start(self.graph(), enabled=True))["id"] + await self.factory.advance(run_id) + self.backend.settle("child-0", "not JSON") + state = await self.factory.advance(run_id) + self.assertEqual(state["status"], "failed") + self.assertEqual(len(self.backend.spawned), 1) + self.assertIn("invalid guard source", state["nodes"]["accept"]["error"]) + + async def test_any_join_waits_for_all_branches_and_continue_keeps_successful_output(self): + graph = {**dag("a", "b", {"id": "join", "prompt": "Join", "needs": ["a", "b"], "join": "any"}, + failure_policy="continue"), "version": 2} + run_id = (await self.factory.start(graph, enabled=True))["id"] + await self.factory.advance(run_id) + self.backend.settle("child-0", "A") + await self.factory.advance(run_id) + self.assertEqual(len(self.backend.spawned), 2) + self.backend.settle("child-1", status="failed") + await self.factory.advance(run_id) + self.assertEqual(len(self.backend.spawned), 3) + self.assertTrue(self.backend.spawned[2][1].endswith('{"a": "A"}')) + self.backend.settle("child-2", "joined") + self.assertEqual((await self.factory.advance(run_id))["status"], "failed") + + async def test_version_two_roundtrip_and_strict_guard_validation(self): + spec = validate(self.graph()) + self.assertEqual(validate(spec_dict(spec)), spec) + invalid = [{"node": "missing", "equals": "yes"}, {"node": "review", "equals": True}, + {"node": "review", "path": [1], "equals": True}, + {"node": "review", "path": ["approved"], "equals": float("nan")}, + {"node": "review", "path": ["approved"], "equals": 2**53}, + {"node": "review", "path": ["approved"], "equals": -(2**53)}, + {"node": "review", "path": [], "equals": []}, + {"node": "review", "equals": "ok", "eval": "dangerous"}] + for guard in invalid: + graph = self.graph() + graph["nodes"][1]["when"] = guard + with self.subTest(guard=guard), self.assertRaises(ValueError): + validate(graph) + for value in (-(2**53 - 1), 2**53 - 1): + graph = self.graph() + graph["nodes"][1]["when"]["equals"] = value + self.assertEqual(validate(spec_dict(validate(graph))), validate(graph)) + graph = self.graph() + graph["version"] = 1 + with self.assertRaises(ValueError): + validate(graph) + + +if __name__ == "__main__": + unittest.main() diff --git a/crates/kernel/rlm-runtime/tests/test_factory_spec.py b/crates/kernel/rlm-runtime/tests/test_factory_spec.py index d1c14c3f..951ce5d8 100644 --- a/crates/kernel/rlm-runtime/tests/test_factory_spec.py +++ b/crates/kernel/rlm-runtime/tests/test_factory_spec.py @@ -23,7 +23,7 @@ def test_defaults_and_topology_are_immutable(self): def test_invalid_specs(self): good = {"version": 1, "nodes": [{"id": "one", "prompt": "Task"}]} - bad_specs = [None, [], {}, {**good, "version": True}, {**good, "version": 2}, + bad_specs = [None, [], {}, {**good, "version": True}, {**good, "version": 3}, {**good, "machine": {}}, {**good, "max_parallel": True}, {**good, "max_parallel": 0}, {**good, "max_parallel": 17}, {**good, "max_parallel": 1.0}, {**good, "max_parallel": None}, diff --git a/crates/server/src/runtime/kernel_host.rs b/crates/server/src/runtime/kernel_host.rs index 311d20d4..f92acafa 100644 --- a/crates/server/src/runtime/kernel_host.rs +++ b/crates/server/src/runtime/kernel_host.rs @@ -43,6 +43,9 @@ pub async fn dispatch_host_request_with_bridge(bridge: &HostBridge, data: Value) match action.as_str() { "" => error_reply("missing host_request type/action"), + action if action.starts_with("factory.") => { + crate::runtime::kernel_host_bridge::handle_factory(bridge, action, ¶ms).await + } action if action.starts_with("harness.") => { crate::runtime::kernel_host_bridge::handle_harness(bridge, action, ¶ms).await } diff --git a/crates/server/src/runtime/kernel_host_bridge.rs b/crates/server/src/runtime/kernel_host_bridge.rs index 2a6f7ae6..3da44ec6 100644 --- a/crates/server/src/runtime/kernel_host_bridge.rs +++ b/crates/server/src/runtime/kernel_host_bridge.rs @@ -20,6 +20,8 @@ use uuid::Uuid; use super::ServerRuntime; use devo_core::McpManager; use devo_core::tools::contracts::ToolBudgets; +mod factory; +pub(crate) use factory::handle_factory; mod harness; pub(crate) use harness::handle_harness; mod shell; diff --git a/crates/server/src/runtime/kernel_host_bridge/factory.rs b/crates/server/src/runtime/kernel_host_bridge/factory.rs new file mode 100644 index 00000000..dd4ff1ad --- /dev/null +++ b/crates/server/src/runtime/kernel_host_bridge/factory.rs @@ -0,0 +1,146 @@ +//! Host-bound Factory checkpoints. Reads never create files or hydrate children. + +use std::path::PathBuf; + +use serde::Deserialize; +use serde_json::{Value, json}; + +use super::{ + HostBridge, check_or_error, deny_plan_mutation, error_reply, permission_request, + plan_mode_denies_mutation, synthetic_tool_call_id, +}; +use devo_safety::ResourceKind; + +mod files; +mod store; + +#[derive(Deserialize)] +#[serde(deny_unknown_fields)] +struct SaveParams { + run_id: String, + expected_revision: u64, + snapshot: Value, +} + +#[derive(Deserialize)] +#[serde(deny_unknown_fields)] +struct LoadParams { + run_id: String, +} + +#[derive(Deserialize)] +#[serde(deny_unknown_fields)] +struct ListParams {} + +enum Operation { + Save(SaveParams), + Load(LoadParams), + List, +} + +pub(crate) async fn handle_factory(bridge: &HostBridge, action: &str, params: &Value) -> Value { + if action == "factory.save" && plan_mode_denies_mutation(bridge) { + return deny_plan_mutation(action); + } + let parsed = match action { + "factory.save" => serde_json::from_value(params.clone()).map(Operation::Save), + "factory.load" => serde_json::from_value(params.clone()).map(Operation::Load), + "factory.list" => { + serde_json::from_value::(params.clone()).map(|_| Operation::List) + } + _ => return error_reply(format!("unknown Factory action: {action}")), + }; + let operation = match parsed { + Ok(operation) => operation, + Err(error) => return error_reply(format!("invalid {action} request: {error}")), + }; + let validation = match &operation { + Operation::Save(params) => store::validate_id(¶ms.run_id) + .and_then(|_| store::validate_snapshot(¶ms.run_id, ¶ms.snapshot)), + Operation::Load(params) => store::validate_id(¶ms.run_id), + Operation::List => Ok(()), + }; + if let Err(error) = validation { + return error_reply(error.to_string()); + } + let Some(root) = &bridge.session_dir else { + return match operation { + Operation::List => json!({"status":"ok","result":{"runs":[]}}), + Operation::Load(params) => { + error_reply(format!("Factory run not found: {}", params.run_id)) + } + Operation::Save(_) => error_reply("This session has no persistent Factory store"), + }; + }; + // Open the host-owned directory before permission waits. On Unix every + // subsequent operation is descriptor-relative and refuses symlinks. + let root = match files::Directory::open(root) { + Ok(root) => root, + Err(error) => return error_reply(format!("open Factory session directory: {error}")), + }; + let directory = match root.child("factory") { + Ok(directory) => Some(directory), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => None, + Err(error) => return error_reply(format!("open Factory store: {error}")), + }; + let mutation = matches!(operation, Operation::Save(_)); + // Approval covers the store directory, including its stable lock and + // unique temporary files. No unapproved sidecar path is opened for write. + let path: PathBuf = root.path.join("factory"); + let request = permission_request( + bridge, + synthetic_tool_call_id(action), + if mutation { "write" } else { "read" }, + params.clone(), + if mutation { + ResourceKind::FileWrite + } else { + ResourceKind::FileRead + }, + format!( + "{action} {} (Factory snapshots and transaction sidecars)", + path.display() + ), + Some(path), + /*target*/ None, + ); + if let Err(reply) = check_or_error(bridge, request).await { + return reply; + } + if bridge.cancel_token.is_cancelled() { + return error_reply("cancelled"); + } + let owner = bridge.session_id; + match tokio::task::spawn_blocking(move || { + let directory = match (directory, &operation) { + (Some(directory), _) => directory, + (None, Operation::Save(_)) => root.create_child("factory")?, + (None, Operation::List) => return Ok(json!({"runs":[]})), + (None, Operation::Load(params)) => { + anyhow::bail!("Factory run not found: {}", params.run_id) + } + }; + match operation { + Operation::Save(params) => store::save( + &directory, + owner, + params.run_id, + params.expected_revision, + params.snapshot, + ), + Operation::Load(params) => { + store::load(&directory, owner, ¶ms.run_id).map(|run| run.response()) + } + Operation::List => store::list(&directory, owner), + } + }) + .await + { + Ok(Ok(result)) => json!({"status":"ok","result":result}), + Ok(Err(error)) => error_reply(format!("{error:#}")), + Err(error) => error_reply(format!("Factory persistence worker failed: {error}")), + } +} + +#[cfg(test)] +mod tests; diff --git a/crates/server/src/runtime/kernel_host_bridge/factory/files.rs b/crates/server/src/runtime/kernel_host_bridge/factory/files.rs new file mode 100644 index 00000000..02454515 --- /dev/null +++ b/crates/server/src/runtime/kernel_host_bridge/factory/files.rs @@ -0,0 +1,298 @@ +//! Descriptor-relative storage: approval cannot be redirected by a later path swap. + +use std::fs::{File, OpenOptions}; +use std::io; +use std::path::{Path, PathBuf}; + +#[cfg(unix)] +use std::os::fd::{AsRawFd, FromRawFd}; + +pub(super) struct Directory { + file: File, + pub path: PathBuf, +} + +#[derive(Clone, Copy)] +pub(super) enum FileMode { + Read, + Lock, + Create, +} + +impl Directory { + pub fn open(path: &Path) -> io::Result { + if std::fs::symlink_metadata(path)?.file_type().is_symlink() { + return Err(io::Error::other("Factory directory cannot be a symlink")); + } + let path = std::fs::canonicalize(path)?; + let mut options = OpenOptions::new(); + options.read(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + options.custom_flags(libc::O_DIRECTORY | libc::O_NOFOLLOW | libc::O_CLOEXEC); + } + #[cfg(windows)] + { + use std::os::windows::fs::OpenOptionsExt; + use windows_sys::Win32::Storage::FileSystem::{ + FILE_FLAG_BACKUP_SEMANTICS, FILE_FLAG_OPEN_REPARSE_POINT, FILE_SHARE_READ, + FILE_SHARE_WRITE, + }; + options.custom_flags(FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT); + // Prevent directory replacement while an approved operation owns it. + options.share_mode(FILE_SHARE_READ | FILE_SHARE_WRITE); + } + let file = options.open(&path)?; + let metadata = file.metadata()?; + if !metadata.is_dir() || metadata.file_type().is_symlink() { + return Err(io::Error::other("Factory store is not a regular directory")); + } + #[cfg(windows)] + { + use std::os::windows::fs::MetadataExt; + use windows_sys::Win32::Storage::FileSystem::FILE_ATTRIBUTE_REPARSE_POINT; + if metadata.file_attributes() & FILE_ATTRIBUTE_REPARSE_POINT != 0 { + return Err(io::Error::other("Factory store cannot be a reparse point")); + } + } + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + let current = std::fs::metadata(&path)?; + if metadata.dev() != current.dev() || metadata.ino() != current.ino() { + return Err(io::Error::other("Factory directory changed during open")); + } + } + Ok(Self { file, path }) + } + + pub fn child(&self, name: &str) -> io::Result { + #[cfg(unix)] + { + let name = std::ffi::CString::new(name)?; + // SAFETY: the parent descriptor is owned and the name is NUL-terminated. + let fd = unsafe { + libc::openat( + self.file.as_raw_fd(), + name.as_ptr(), + libc::O_RDONLY | libc::O_DIRECTORY | libc::O_NOFOLLOW | libc::O_CLOEXEC, + ) + }; + if fd < 0 { + return Err(io::Error::last_os_error()); + } + // SAFETY: openat returned a new owned descriptor. + let file = unsafe { File::from_raw_fd(fd) }; + Ok(Self { + file, + path: self.path.join(name.to_str().expect("ASCII store name")), + }) + } + #[cfg(not(unix))] + { + Self::open(&self.path.join(name)) + } + } + + pub fn create_child(&self, name: &str) -> io::Result { + #[cfg(unix)] + let result = { + let name = std::ffi::CString::new(name)?; + // SAFETY: the parent descriptor is owned and the name is NUL-terminated. + if unsafe { libc::mkdirat(self.file.as_raw_fd(), name.as_ptr(), 0o700) } < 0 { + Err(io::Error::last_os_error()) + } else { + Ok(()) + } + }; + #[cfg(not(unix))] + let result = std::fs::create_dir(self.path.join(name)); + match result { + Ok(()) => self.sync()?, + Err(error) if error.kind() == io::ErrorKind::AlreadyExists => {} + Err(error) => return Err(error), + } + self.child(name) + } + + pub fn open_file(&self, name: &str, mode: FileMode) -> io::Result { + #[cfg(unix)] + let file = { + let name = std::ffi::CString::new(name)?; + let flags = match mode { + FileMode::Read => libc::O_RDONLY, + FileMode::Lock => libc::O_RDWR | libc::O_CREAT, + FileMode::Create => libc::O_WRONLY | libc::O_CREAT | libc::O_EXCL, + } | libc::O_NOFOLLOW + | libc::O_NONBLOCK + | libc::O_CLOEXEC; + // SAFETY: descriptor and C string remain valid; mode applies only to creation. + let fd = unsafe { libc::openat(self.file.as_raw_fd(), name.as_ptr(), flags, 0o600) }; + if fd < 0 { + return Err(io::Error::last_os_error()); + } + // SAFETY: openat returned a new owned descriptor. + unsafe { File::from_raw_fd(fd) } + }; + #[cfg(not(unix))] + let file = { + let mut options = OpenOptions::new(); + match mode { + FileMode::Read => { + options.read(true); + } + FileMode::Lock => { + options.read(true).write(true).create(true).truncate(false); + } + FileMode::Create => { + options.write(true).create_new(true); + } + } + #[cfg(windows)] + { + use std::os::windows::fs::OpenOptionsExt; + use windows_sys::Win32::Storage::FileSystem::FILE_FLAG_OPEN_REPARSE_POINT; + options.custom_flags(FILE_FLAG_OPEN_REPARSE_POINT); + } + options.open(self.path.join(name))? + }; + let metadata = file.metadata()?; + if !metadata.is_file() || metadata.file_type().is_symlink() { + return Err(io::Error::other( + "Factory store entry is not a regular file", + )); + } + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + if metadata.nlink() != 1 { + return Err(io::Error::other( + "Factory store entry cannot have hard links", + )); + } + } + #[cfg(windows)] + { + use std::os::windows::fs::MetadataExt; + use windows_sys::Win32::Storage::FileSystem::FILE_ATTRIBUTE_REPARSE_POINT; + if metadata.file_attributes() & FILE_ATTRIBUTE_REPARSE_POINT != 0 { + return Err(io::Error::other( + "Factory store entry cannot be a reparse point", + )); + } + } + Ok(file) + } + + pub fn replace(&self, source: &str, target: &str) -> io::Result<()> { + #[cfg(unix)] + { + let source = std::ffi::CString::new(source)?; + let target = std::ffi::CString::new(target)?; + // SAFETY: directory is owned; both names are validated, NUL-terminated basenames. + if unsafe { + libc::renameat( + self.file.as_raw_fd(), + source.as_ptr(), + self.file.as_raw_fd(), + target.as_ptr(), + ) + } < 0 + { + return Err(io::Error::last_os_error()); + } + } + #[cfg(not(unix))] + std::fs::rename(self.path.join(source), self.path.join(target))?; + self.sync() + } + + pub fn remove(&self, name: &str) -> io::Result<()> { + #[cfg(unix)] + { + let name = std::ffi::CString::new(name)?; + // SAFETY: directory is owned; only a temporary basename is removed. + if unsafe { libc::unlinkat(self.file.as_raw_fd(), name.as_ptr(), 0) } < 0 { + return Err(io::Error::last_os_error()); + } + Ok(()) + } + #[cfg(not(unix))] + { + std::fs::remove_file(self.path.join(name)) + } + } + + pub fn entries(&self) -> io::Result> { + #[cfg(unix)] + { + // Open a fresh directory description so repeated scans have independent offsets. + let scan = self.child(".")?; + use std::os::fd::IntoRawFd; + let fd = scan.file.into_raw_fd(); + // SAFETY: fd is a fresh owned directory descriptor; fdopendir takes ownership on success. + let directory = unsafe { libc::fdopendir(fd) }; + if directory.is_null() { + let error = io::Error::last_os_error(); + // SAFETY: ownership was not transferred on fdopendir failure. + unsafe { libc::close(fd) }; + return Err(error); + } + let mut names = Vec::new(); + let mut read_error = None; + loop { + // SAFETY: errno is thread-local; clearing it distinguishes end-of-stream. + unsafe { *errno_location() = 0 }; + // SAFETY: directory is live and used only on this thread. + let entry = unsafe { libc::readdir(directory) }; + if entry.is_null() { + let error = io::Error::last_os_error(); + if error.raw_os_error() != Some(0) { + read_error = Some(error); + } + break; + } + // SAFETY: readdir supplies a NUL-terminated name valid until the next call. + let name = unsafe { std::ffi::CStr::from_ptr((*entry).d_name.as_ptr()) }; + names.push(name.to_string_lossy().into_owned()); + } + // SAFETY: closes the stream and its descriptor exactly once. + unsafe { libc::closedir(directory) }; + match read_error { + Some(error) => Err(error), + None => Ok(names), + } + } + #[cfg(not(unix))] + { + std::fs::read_dir(&self.path)? + .map(|entry| Ok(entry?.file_name().to_string_lossy().into_owned())) + .collect() + } + } + + fn sync(&self) -> io::Result<()> { + #[cfg(unix)] + self.file.sync_all()?; + Ok(()) + } +} + +#[cfg(any(target_os = "linux", target_os = "emscripten"))] +unsafe fn errno_location() -> *mut libc::c_int { + // SAFETY: the platform exposes the calling thread's errno storage. + unsafe { libc::__errno_location() } +} + +#[cfg(any(target_os = "macos", target_os = "ios", target_os = "freebsd"))] +unsafe fn errno_location() -> *mut libc::c_int { + // SAFETY: the platform exposes the calling thread's errno storage. + unsafe { libc::__error() } +} + +#[cfg(target_os = "android")] +unsafe fn errno_location() -> *mut libc::c_int { + // SAFETY: the platform exposes the calling thread's errno storage. + unsafe { libc::__errno() } +} diff --git a/crates/server/src/runtime/kernel_host_bridge/factory/store.rs b/crates/server/src/runtime/kernel_host_bridge/factory/store.rs new file mode 100644 index 00000000..654056b3 --- /dev/null +++ b/crates/server/src/runtime/kernel_host_bridge/factory/store.rs @@ -0,0 +1,279 @@ +//! Versioned recovery data, not child lifecycle or permission authority. + +use std::collections::BTreeSet; +use std::io::{Read, Write}; + +use anyhow::{Context, Result, ensure}; +use devo_protocol::native::ids::SessionId; +use serde::{Deserialize, Serialize}; +use serde_json::{Value, json}; + +use super::files::{Directory, FileMode}; + +pub(super) const MAX_SNAPSHOT_BYTES: usize = 1024 * 1024; +const MAX_RUNS: usize = 128; +const MAX_FILE_BYTES: usize = MAX_SNAPSHOT_BYTES + 4096; + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[serde(deny_unknown_fields)] +pub(super) struct StoredRun { + store_version: u32, + owner_session_id: SessionId, + pub run_id: String, + pub revision: u64, + pub snapshot: Value, +} + +impl StoredRun { + pub fn response(&self) -> Value { + json!({"run_id":self.run_id,"revision":self.revision,"snapshot":self.snapshot}) + } +} + +pub(super) fn validate_id(id: &str) -> Result<()> { + ensure!( + id.len() == 32 + && id + .bytes() + .all(|byte| byte.is_ascii_digit() || (b'a'..=b'f').contains(&byte)), + "Factory run_id must contain exactly 32 lowercase hexadecimal characters" + ); + Ok(()) +} + +pub(super) fn validate_snapshot(run_id: &str, snapshot: &Value) -> Result<()> { + ensure!( + serde_json::to_vec(snapshot)?.len() <= MAX_SNAPSHOT_BYTES, + "Factory snapshot exceeds 1 MiB" + ); + ensure!( + snapshot.is_object() && snapshot["format_version"].as_u64() == Some(1), + "Factory snapshot format_version must be 1" + ); + let spec = snapshot + .get("spec") + .and_then(Value::as_object) + .context("Factory snapshot spec must be an object")?; + ensure!( + matches!(spec.get("version").and_then(Value::as_u64), Some(1 | 2)), + "Factory spec version must be 1 or 2" + ); + let nodes = spec + .get("nodes") + .and_then(Value::as_array) + .context("Factory spec nodes must be an array")?; + ensure!( + (1..=128).contains(&nodes.len()), + "Factory spec must contain 1 to 128 nodes" + ); + let mut ids = BTreeSet::new(); + for node in nodes { + let id = node + .get("id") + .and_then(Value::as_str) + .context("Factory spec node id must be a string")?; + ensure!( + !id.is_empty() && id.len() <= 32 && ids.insert(id), + "Factory spec node ids must be unique and bounded" + ); + ensure!( + node.get("prompt") + .and_then(Value::as_str) + .is_some_and( + |prompt| !prompt.trim().is_empty() && prompt.chars().count() <= 128_000 + ), + "Factory spec node prompt must contain 1 to 128000 characters" + ); + } + let run = snapshot + .get("run") + .and_then(Value::as_object) + .context("Factory snapshot run must be an object")?; + ensure!( + run.get("id").and_then(Value::as_str) == Some(run_id), + "Factory snapshot run.id must match run_id" + ); + ensure!( + run.get("status") + .and_then(Value::as_str) + .is_some_and(|status| !status.is_empty() && status.len() <= 64), + "Factory run status must be a bounded string" + ); + ensure!( + run.get("stop_requested").and_then(Value::as_bool).is_some() + && run.get("failed").and_then(Value::as_bool).is_some(), + "Factory run stop_requested and failed must be booleans" + ); + let states = run + .get("nodes") + .and_then(Value::as_object) + .context("Factory run nodes must be an object")?; + ensure!( + states.keys().map(String::as_str).collect::>() == ids, + "Factory run nodes must match spec node ids" + ); + for (id, node) in states { + ensure!( + node.is_object() && node.get("id").and_then(Value::as_str) == Some(id), + "Factory run node id must match its key" + ); + for key in ["name", "status"] { + ensure!( + node.get(key) + .and_then(Value::as_str) + .is_some_and(|text| !text.is_empty() && text.len() <= 128), + "Factory node {key} must be a bounded string" + ); + } + ensure!( + node.get("needs_cleanup").and_then(Value::as_bool).is_some(), + "Factory node needs_cleanup must be a boolean" + ); + for key in ["child_id", "output", "error", "cleanup_error"] { + ensure!( + node.get(key) + .is_some_and(|value| value.is_null() || value.is_string()), + "Factory node {key} must be null or a string" + ); + } + ensure!( + node["child_id"] + .as_str() + .is_none_or(|id| !id.trim().is_empty() && id.len() <= 256), + "Factory child_id must be nonempty and bounded" + ); + } + Ok(()) +} + +fn validate_transition(previous: &Value, next: &Value) -> Result<()> { + ensure!( + previous["spec"] == next["spec"], + "Factory run spec is immutable" + ); + let previous_nodes = previous["run"]["nodes"] + .as_object() + .context("invalid previous nodes")?; + for (id, previous) in previous_nodes { + let next = &next["run"]["nodes"][id]; + ensure!( + previous["name"] == next["name"], + "Factory node name is immutable: {id}" + ); + ensure!( + previous["child_id"].is_null() || previous["child_id"] == next["child_id"], + "Factory child binding is immutable: {id}" + ); + ensure!( + previous["status"] == "pending" || next["status"] != "pending", + "Factory admitted or settled node cannot return to pending: {id}" + ); + } + Ok(()) +} + +pub(super) fn load(directory: &Directory, owner: SessionId, run_id: &str) -> Result { + let file = directory + .open_file(&format!("{run_id}.json"), FileMode::Read) + .with_context(|| format!("Factory run not found or unreadable: {run_id}"))?; + let mut bytes = Vec::new(); + file.take((MAX_FILE_BYTES + 1) as u64) + .read_to_end(&mut bytes)?; + ensure!( + bytes.len() <= MAX_FILE_BYTES, + "Factory run file exceeds size limit" + ); + let record: StoredRun = + serde_json::from_slice(&bytes).context("corrupt Factory run file (fail closed)")?; + ensure!( + record.store_version == 1 + && record.owner_session_id == owner + && record.run_id == run_id + && record.revision > 0, + "corrupt Factory run envelope or session ownership mismatch" + ); + validate_snapshot(run_id, &record.snapshot).context("corrupt Factory run snapshot")?; + Ok(record) +} + +pub(super) fn list(directory: &Directory, owner: SessionId) -> Result { + let mut runs = Vec::new(); + for name in directory.entries()? { + let Some(id) = name.strip_suffix(".json") else { + continue; + }; + validate_id(id).context("invalid Factory run filename")?; + ensure!(runs.len() < MAX_RUNS, "Factory stored run limit exceeded"); + let run = load(directory, owner, id)?; + runs.push(json!({"run_id":run.run_id,"revision":run.revision,"status":run.snapshot["run"]["status"]})); + } + runs.sort_by(|a, b| a["run_id"].as_str().cmp(&b["run_id"].as_str())); + Ok(json!({"runs":runs})) +} + +pub(super) fn save( + directory: &Directory, + owner: SessionId, + run_id: String, + expected_revision: u64, + snapshot: Value, +) -> Result { + validate_id(&run_id)?; + validate_snapshot(&run_id, &snapshot)?; + let lock = directory.open_file("store.lock", FileMode::Lock)?; + fs2::FileExt::lock_exclusive(&lock)?; + // Keep the stable sidecar lock, not the replaceable destination inode. + let filename = format!("{run_id}.json"); + let current = match directory.open_file(&filename, FileMode::Read) { + Ok(_) => Some(load(directory, owner, &run_id)?), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => None, + Err(error) => return Err(error.into()), + }; + let current_revision = current.as_ref().map_or(0, |run| run.revision); + ensure!( + current_revision == expected_revision, + "Factory revision conflict: expected {expected_revision}, found {current_revision}" + ); + if let Some(current) = ¤t { + validate_transition(¤t.snapshot, &snapshot)?; + } + if current_revision == 0 { + let count = list(directory, owner)?["runs"] + .as_array() + .expect("list returns array") + .len(); + ensure!( + count < MAX_RUNS, + "Factory session run limit reached ({MAX_RUNS})" + ); + } + let revision = current_revision + .checked_add(1) + .context("Factory revision exhausted")?; + let record = StoredRun { + store_version: 1, + owner_session_id: owner, + run_id, + revision, + snapshot, + }; + let temporary_name = format!(".{}.tmp", uuid::Uuid::new_v4().simple()); + let result = (|| { + let mut temporary = directory.open_file(&temporary_name, FileMode::Create)?; + serde_json::to_writer(&mut temporary, &record)?; + temporary.flush()?; + temporary.sync_all()?; + drop(temporary); + directory.replace(&temporary_name, &filename)?; + Ok::<_, anyhow::Error>(record.response()) + })(); + if result.is_err() { + let _ = directory.remove(&temporary_name); + } + // A rename/fsync error has an uncertain commit outcome. Callers must reload, + // never silently retry the expected revision or assume the old file survived. + result +} + +#[cfg(test)] +mod tests; diff --git a/crates/server/src/runtime/kernel_host_bridge/factory/store/tests.rs b/crates/server/src/runtime/kernel_host_bridge/factory/store/tests.rs new file mode 100644 index 00000000..7da58e7b --- /dev/null +++ b/crates/server/src/runtime/kernel_host_bridge/factory/store/tests.rs @@ -0,0 +1,213 @@ +use super::*; +use pretty_assertions::assert_eq; + +const RUN: &str = "0123456789abcdef0123456789abcdef"; + +fn owner() -> SessionId { + SessionId::from_string("ses_00000000-0000-0000-0000-000000000001".into()) +} + +pub(in crate::runtime::kernel_host_bridge::factory) fn snapshot(id: &str) -> Value { + json!({"format_version":1,"spec":{"version":1,"nodes":[{"id":"work","prompt":"task"}],"max_parallel":1,"failure_policy":"fail_fast"}, + "run":{"id":id,"status":"ready","stop_requested":false,"failed":false, + "nodes":{"work":{"id":"work","name":"reserved-name","status":"pending","child_id":null,"output":null,"error":null,"cleanup_error":null,"needs_cleanup":false}}}}) +} + +#[test] +fn roundtrip_cas_owner_and_corruption_fail_closed() { + let home = tempfile::tempdir().unwrap(); + let directory = Directory::open(home.path()).unwrap(); + let first = save(&directory, owner(), RUN.into(), 0, snapshot(RUN)).unwrap(); + assert_eq!( + first, + json!({"run_id":RUN,"revision":1,"snapshot":snapshot(RUN)}) + ); + assert_eq!(load(&directory, owner(), RUN).unwrap().response(), first); + assert_eq!( + list(&directory, owner()).unwrap(), + json!({"runs":[{"run_id":RUN,"revision":1,"status":"ready"}]}) + ); + let before = std::fs::read(home.path().join(format!("{RUN}.json"))).unwrap(); + assert!( + save(&directory, owner(), RUN.into(), 0, snapshot(RUN)) + .unwrap_err() + .to_string() + .contains("revision conflict") + ); + assert_eq!( + std::fs::read(home.path().join(format!("{RUN}.json"))).unwrap(), + before + ); + assert!(load(&directory, SessionId::new(), RUN).is_err()); + std::fs::write(home.path().join(format!("{RUN}.json")), "broken").unwrap(); + assert!( + save(&directory, owner(), RUN.into(), 1, snapshot(RUN)) + .unwrap_err() + .to_string() + .contains("corrupt") + ); + assert!(list(&directory, owner()).is_err()); + assert_eq!( + std::fs::read(home.path().join(format!("{RUN}.json"))).unwrap(), + b"broken" + ); +} + +#[test] +fn invalid_ids_and_schema_fail_before_writes() { + let home = tempfile::tempdir().unwrap(); + let directory = Directory::open(home.path()).unwrap(); + for id in [ + "", + "../escape", + "ABCDEF0123456789abcdef0123456789ab", + "0000000000000000000000000000000g", + ] { + assert!(save(&directory, owner(), id.into(), 0, snapshot(id)).is_err()); + } + let mut malformed = snapshot(RUN); + malformed["run"]["id"] = json!("wrong"); + assert!(save(&directory, owner(), RUN.into(), 0, malformed).is_err()); + let mut oversized = snapshot(RUN); + oversized["run"]["nodes"]["work"]["output"] = json!("x".repeat(MAX_SNAPSHOT_BYTES)); + assert!(save(&directory, owner(), RUN.into(), 0, oversized).is_err()); + assert_eq!(std::fs::read_dir(home.path()).unwrap().count(), 0); +} + +#[test] +fn concurrent_same_revision_has_exactly_one_winner() { + let home = tempfile::tempdir().unwrap(); + let barrier = std::sync::Arc::new(std::sync::Barrier::new(8)); + let results = std::thread::scope(|scope| { + let workers = (0..8) + .map(|_| { + let barrier = barrier.clone(); + let path = home.path(); + scope.spawn(move || { + let directory = Directory::open(path).unwrap(); + barrier.wait(); + save(&directory, owner(), RUN.into(), 0, snapshot(RUN)).is_ok() + }) + }) + .collect::>(); + workers + .into_iter() + .map(|worker| worker.join().unwrap()) + .collect::>() + }); + assert_eq!( + results.into_iter().filter(|succeeded| *succeeded).count(), + 1 + ); + let directory = Directory::open(home.path()).unwrap(); + assert_eq!(load(&directory, owner(), RUN).unwrap().revision, 1); + assert_eq!( + list(&directory, owner()).unwrap()["runs"] + .as_array() + .unwrap() + .len(), + 1 + ); +} + +#[test] +fn retained_run_limit_is_transactional() { + let home = tempfile::tempdir().unwrap(); + let directory = Directory::open(home.path()).unwrap(); + for index in 0..MAX_RUNS { + let id = format!("{index:032x}"); + save(&directory, owner(), id.clone(), 0, snapshot(&id)).unwrap(); + } + assert!( + save(&directory, owner(), RUN.into(), 0, snapshot(RUN)) + .unwrap_err() + .to_string() + .contains("run limit") + ); + let existing = format!("{:032x}", 0); + assert_eq!( + save( + &directory, + owner(), + existing.clone(), + 1, + snapshot(&existing) + ) + .unwrap()["revision"], + 2 + ); +} + +#[cfg(unix)] +#[test] +fn private_atomic_files_and_symlink_rejection() { + use std::os::unix::fs::{PermissionsExt, symlink}; + let home = tempfile::tempdir().unwrap(); + let other = tempfile::tempdir().unwrap(); + let directory = Directory::open(home.path()).unwrap(); + let target = home.path().join(format!("{RUN}.json")); + save(&directory, owner(), RUN.into(), 0, snapshot(RUN)).unwrap(); + for name in [format!("{RUN}.json"), "store.lock".into()] { + assert_eq!( + std::fs::metadata(home.path().join(name)) + .unwrap() + .permissions() + .mode() + & 0o777, + 0o600 + ); + } + std::fs::remove_file(&target).unwrap(); + let outside = other.path().join("untouched"); + std::fs::write(&outside, "original").unwrap(); + symlink(&outside, &target).unwrap(); + assert!(load(&directory, owner(), RUN).is_err()); + assert!(save(&directory, owner(), RUN.into(), 0, snapshot(RUN)).is_err()); + assert_eq!(std::fs::read_to_string(outside).unwrap(), "original"); + std::fs::remove_file(home.path().join("store.lock")).unwrap(); + symlink( + other.path().join("new-lock"), + home.path().join("store.lock"), + ) + .unwrap(); + assert!(save(&directory, owner(), RUN.into(), 0, snapshot(RUN)).is_err()); + assert!(!other.path().join("new-lock").exists()); +} + +#[test] +fn checkpoints_cannot_rewrite_definition_identity_or_admission_history() { + let home = tempfile::tempdir().unwrap(); + let directory = Directory::open(home.path()).unwrap(); + let mut original = snapshot(RUN); + original["run"]["nodes"]["work"]["status"] = json!("admitting"); + save(&directory, owner(), RUN.into(), 0, original.clone()).unwrap(); + let mut admitted = original.clone(); + admitted["run"]["nodes"]["work"]["child_id"] = json!("ses_host_owned_child"); + admitted["run"]["nodes"]["work"]["status"] = json!("running"); + save(&directory, owner(), RUN.into(), 1, admitted.clone()).unwrap(); + let before = load(&directory, owner(), RUN).unwrap(); + for (path, replacement) in [ + (vec!["spec", "version"], json!(2)), + (vec!["run", "nodes", "work", "name"], json!("another-child")), + (vec!["run", "nodes", "work", "child_id"], Value::Null), + ( + vec!["run", "nodes", "work", "child_id"], + json!("different-child"), + ), + (vec!["run", "nodes", "work", "status"], json!("pending")), + ] { + let mut changed = admitted.clone(); + let mut target = &mut changed; + for key in path { + target = &mut target[key]; + } + *target = replacement; + assert!(save(&directory, owner(), RUN.into(), 2, changed).is_err()); + assert_eq!(load(&directory, owner(), RUN).unwrap(), before); + } + admitted["run"]["nodes"]["work"]["status"] = json!("recovery_required"); + assert_eq!( + save(&directory, owner(), RUN.into(), 2, admitted).unwrap()["revision"], + 3 + ); +} diff --git a/crates/server/src/runtime/kernel_host_bridge/factory/tests.rs b/crates/server/src/runtime/kernel_host_bridge/factory/tests.rs new file mode 100644 index 00000000..5b879561 --- /dev/null +++ b/crates/server/src/runtime/kernel_host_bridge/factory/tests.rs @@ -0,0 +1,181 @@ +use super::*; +use crate::runtime::kernel_host::dispatch_host_request_with_bridge; +use devo_core::tools::PermissionChecker; +use devo_protocol::CollaborationMode; +use pretty_assertions::assert_eq; + +const RUN: &str = "0123456789abcdef0123456789abcdef"; + +fn snapshot() -> Value { + json!({"format_version":1,"spec":{"version":2,"nodes":[{"id":"work","prompt":"task"}],"max_parallel":1,"failure_policy":"fail_fast"}, + "run":{"id":RUN,"status":"admitting","stop_requested":false,"failed":false, + "nodes":{"work":{"id":"work","name":"reserved-name","status":"admitting","child_id":null,"output":null,"error":null,"cleanup_error":null,"needs_cleanup":true}}}}) +} + +#[tokio::test] +async fn dispatch_roundtrip_and_session_isolation() { + let home = tempfile::tempdir().unwrap(); + let other = tempfile::tempdir().unwrap(); + let mut bridge = HostBridge::for_tests("ses_00000000-0000-0000-0000-000000000001"); + bridge.session_dir = Some(home.path().to_path_buf()); + let saved = dispatch_host_request_with_bridge( + &bridge, + json!({"type":"factory.save","run_id":RUN,"expected_revision":0,"snapshot":snapshot()}), + ) + .await; + assert_eq!( + saved, + json!({"status":"ok","result":{"run_id":RUN,"revision":1,"snapshot":snapshot()}}) + ); + assert_eq!( + dispatch_host_request_with_bridge(&bridge, json!({"type":"factory.load","run_id":RUN})) + .await, + saved + ); + assert_eq!( + dispatch_host_request_with_bridge(&bridge, json!({"type":"factory.list"})).await, + json!({"status":"ok","result":{"runs":[{"run_id":RUN,"revision":1,"status":"admitting"}]}}) + ); + bridge.session_dir = Some(other.path().to_path_buf()); + assert_eq!( + dispatch_host_request_with_bridge(&bridge, json!({"type":"factory.load","run_id":RUN})) + .await["status"], + "error" + ); + assert_eq!(std::fs::read_dir(other.path()).unwrap().count(), 0); +} + +#[tokio::test] +async fn plan_reads_and_denied_save_create_nothing() { + let home = tempfile::tempdir().unwrap(); + let mut bridge = HostBridge::for_tests("ses_00000000-0000-0000-0000-000000000001"); + bridge.session_dir = Some(home.path().to_path_buf()); + bridge.collaboration_mode = CollaborationMode::Plan; + assert_eq!( + dispatch_host_request_with_bridge( + &bridge, + json!({"type":"factory.save","run_id":RUN,"expected_revision":0,"snapshot":snapshot()}) + ) + .await, + json!({"status":"error","error":"denied: factory.save blocked in Plan mode"}) + ); + assert_eq!( + dispatch_host_request_with_bridge(&bridge, json!({"type":"factory.list"})).await, + json!({"status":"ok","result":{"runs":[]}}) + ); + assert_eq!( + dispatch_host_request_with_bridge(&bridge, json!({"type":"factory.load","run_id":RUN})) + .await["status"], + "error" + ); + bridge.collaboration_mode = CollaborationMode::Build; + bridge.permission = PermissionChecker::new(|_| Box::pin(async { Err("test denial".into()) })); + assert_eq!( + dispatch_host_request_with_bridge( + &bridge, + json!({"type":"factory.save","run_id":RUN,"expected_revision":0,"snapshot":snapshot()}) + ) + .await, + json!({"status":"error","error":"test denial"}) + ); + assert_eq!(std::fs::read_dir(home.path()).unwrap().count(), 0); +} + +#[tokio::test] +async fn rejects_unknown_routing_fields_and_bad_revision_without_writes() { + let home = tempfile::tempdir().unwrap(); + let mut bridge = HostBridge::for_tests("ses_00000000-0000-0000-0000-000000000001"); + bridge.session_dir = Some(home.path().to_path_buf()); + for request in [ + json!({"type":"factory.list","scope":"global"}), + json!({"type":"factory.load","run_id":RUN,"session_id":"other"}), + json!({"type":"factory.save","run_id":RUN,"expected_revision":true,"snapshot":snapshot()}), + json!({"type":"factory.save","run_id":"../escape","expected_revision":0,"snapshot":snapshot()}), + json!({"type":"factory.save","run_id":RUN,"expected_revision":0,"snapshot":snapshot(),"path":"elsewhere"}), + ] { + assert_eq!( + dispatch_host_request_with_bridge(&bridge, request).await["status"], + "error" + ); + } + assert_eq!(std::fs::read_dir(home.path()).unwrap().count(), 0); +} + +#[cfg(unix)] +#[tokio::test] +async fn approval_path_swap_cannot_redirect_checkpoint_or_lock() { + use std::os::unix::fs::symlink; + let home = tempfile::tempdir().unwrap(); + let other = tempfile::tempdir().unwrap(); + let original = home.path().join("session"); + let moved = home.path().join("moved"); + std::fs::create_dir(&original).unwrap(); + let mut bridge = HostBridge::for_tests("ses_00000000-0000-0000-0000-000000000001"); + bridge.session_dir = Some(original.clone()); + let outside = other.path().to_path_buf(); + bridge.permission = PermissionChecker::new(move |request| { + assert_eq!(request.path, Some(original.join("factory"))); + std::fs::rename(&original, &moved).unwrap(); + symlink(&outside, &original).unwrap(); + Box::pin(async { Ok(Default::default()) }) + }); + let response = dispatch_host_request_with_bridge( + &bridge, + json!({"type":"factory.save","run_id":RUN,"expected_revision":0,"snapshot":snapshot()}), + ) + .await; + assert_eq!(response["status"], "ok", "{response}"); + assert_eq!(std::fs::read_dir(other.path()).unwrap().count(), 0); + assert!( + home.path() + .join("moved/factory") + .join(format!("{RUN}.json")) + .exists() + ); +} + +#[tokio::test] +async fn live_kernel_checkpoint_survives_kernel_replacement() -> anyhow::Result<()> { + use devo_kernel::{KernelSession, KernelSessionConfig}; + use std::sync::Arc; + + let home = tempfile::tempdir()?; + let runtime_path = + devo_kernel::default_runtime_pythonpath(std::path::Path::new(env!("CARGO_MANIFEST_DIR"))) + .expect("repository includes the Python runtime"); + let config = KernelSessionConfig { + cwd: home.path().to_path_buf(), + python_path_entries: vec![runtime_path], + ..Default::default() + }; + let mut bridge = HostBridge::for_tests("ses_00000000-0000-0000-0000-000000000001"); + bridge.session_dir = Some(home.path().to_path_buf()); + let bridge = Arc::new(bridge); + let kernel = KernelSession::spawn(config.clone()).await?; + kernel + .set_host_handler(Some(super::super::host_handler_for_bridge(bridge.clone()))) + .await; + let request = + json!({"type":"factory.save","run_id":RUN,"expected_revision":0,"snapshot":snapshot()}) + .to_string(); + let code = format!( + "import json\nfrom rlm.repl import host_request\nreply = await host_request(json.loads({request:?}))\nassert reply['status'] == 'ok', reply\nassert reply['result']['revision'] == 1\nprint('checkpoint-saved')" + ); + let result = kernel.execute(&code).await?; + assert_eq!(result.status, "ok", "{result:?}"); + assert!(result.stdout.contains("checkpoint-saved")); + kernel.shutdown().await?; + + let restored = KernelSession::spawn(config).await?; + restored + .set_host_handler(Some(super::super::host_handler_for_bridge(bridge))) + .await; + let code = format!( + "from rlm.repl import host_request\nreply = await host_request({{'type':'factory.load','run_id':'{RUN}'}})\nassert reply['status'] == 'ok', reply\nassert reply['result']['revision'] == 1\nassert reply['result']['snapshot']['run']['nodes']['work']['status'] == 'admitting'\nprint('checkpoint-restored')" + ); + let result = restored.execute(&code).await?; + assert_eq!(result.status, "ok", "{result:?}"); + assert!(result.stdout.contains("checkpoint-restored")); + restored.shutdown().await?; + Ok(()) +} diff --git a/crates/server/tests/factory_workflows.rs b/crates/server/tests/factory_workflows.rs new file mode 100644 index 00000000..32ef5d37 --- /dev/null +++ b/crates/server/tests/factory_workflows.rs @@ -0,0 +1,177 @@ +//! Factory checkpoints and guards through real CPython, host RPC and session actors. +#[path = "support/subagent_lifecycle.rs"] +mod support; + +use anyhow::{Context, Result}; +use pretty_assertions::assert_eq; +use serde_json::json; +use std::sync::Arc; +use support::*; + +#[tokio::test] +async fn durable_guarded_diamond_runs_only_selected_branch_and_joins_output() -> Result<()> { + let home = tempfile::TempDir::new()?; + let provider = Arc::new(ScriptedProvider::new([ + ScriptedProvider::ipython_code_tool_call( + r#" +from pathlib import Path +spec = {'version': 2, 'nodes': [ + {'id': 'review', 'prompt': 'Return an approval verdict as JSON.'}, + {'id': 'accept', 'prompt': 'Handle the approved branch.', 'needs': ['review'], + 'when': {'node': 'review', 'path': ['approved'], 'equals': True}}, + {'id': 'reject', 'prompt': 'Handle the rejected branch.', 'needs': ['review'], + 'when': {'node': 'review', 'path': ['approved'], 'equals': False}}, + {'id': 'join', 'prompt': 'Summarize the selected result.', 'needs': ['accept', 'reject'], 'join': 'any'} +]} +run = await rlm.factory.start(spec, enabled=True, durable=True) +result = await rlm.factory.wait(run['id'], timeout=10, poll_interval=0.01) +assert result['status'] == 'completed', result +assert result['nodes']['reject']['status'] == 'skipped', result +assert result['nodes']['join']['output'] == 'joined result', result +assert len(await rlm.list_subagents()) == 3 +saved = await rlm.factory.saved() +assert len(saved) == 1 and saved[0]['status'] == 'completed', saved +assert await rlm.factory.stop(run['id']) == result +Path('factory-guard-ok').write_text(run['id']) +await rlm.end_turn() +"#, + ), + ScriptedProvider::completed(r#"{"approved": true}"#), + ScriptedProvider::completed("accepted result"), + ScriptedProvider::completed("joined result"), + ])); + let runtime = build_runtime(home.path(), provider.clone())?; + let (connection, mut notifications) = initialize_connection(&runtime).await?; + let parent = start_parent_session(&runtime, connection, home.path()).await?; + start_turn_with_approval_policy( + &runtime, + connection, + parent, + "Run a guarded workflow", + Some("never"), + ) + .await?; + let completed = + wait_for_session_notification(&mut notifications, "turn/completed", parent).await?; + assert_eq!( + completed["params"]["turn"]["status"], + json!("completed"), + "{completed}" + ); + assert_eq!( + std::fs::read_to_string(home.path().join("factory-guard-ok"))?.len(), + 32 + ); + let requests = provider.requests(); + assert_eq!(requests.len(), 4); + let join = message_texts(&requests[3]).join("\n"); + assert!(join.contains(r#""accept": "accepted result""#), "{join}"); + assert!(!join.contains(r#""reject":"#), "{join}"); + Ok(()) +} + +#[tokio::test] +async fn durable_factory_recovers_existing_child_after_runtime_restart_without_respawn() +-> Result<()> { + let home = tempfile::TempDir::new()?; + let first_provider = Arc::new(ScriptedProvider::new([ + ScriptedProvider::ipython_code_tool_call( + r#" +from pathlib import Path +run = await rlm.factory.start({'version': 1, 'nodes': [ + {'id': 'research', 'prompt': 'Research once.'}, + {'id': 'join', 'prompt': 'Summarize the retained research.', 'needs': ['research']} +]}, enabled=True, durable=True) +state = await rlm.factory.advance(run['id']) +child = state['nodes']['research']['child_id'] +assert (await rlm.collect(child, timeout_ms=10000))[0].answer_text == 'retained research' +# The durable snapshot still says running. Recovery must reconcile the retained +# child, not trust stale status or dispatch the research a second time. +Path('factory-resume-id').write_text(run['id']) +await rlm.end_turn() +"#, + ), + ScriptedProvider::completed("retained research"), + ])); + let first = build_runtime(home.path(), first_provider.clone())?; + let (connection, mut notifications) = initialize_connection(&first).await?; + let parent = start_parent_session(&first, connection, home.path()).await?; + start_turn_with_approval_policy( + &first, + connection, + parent, + "Start durable work", + Some("never"), + ) + .await?; + let completed = + wait_for_session_notification(&mut notifications, "turn/completed", parent).await?; + assert_eq!( + completed["params"]["turn"]["status"], + json!("completed"), + "{completed}" + ); + assert_eq!(first_provider.stream_calls(), 2); + drop(first); + + let provider = Arc::new(ScriptedProvider::new([ + ScriptedProvider::ipython_code_tool_call( + r#" +from pathlib import Path +run_id = Path('factory-resume-id').read_text() +saved = await rlm.factory.saved() +assert len(saved) == 1 and saved[0]['run_id'] == run_id, saved +restored = await rlm.factory.restore(run_id, enabled=True) +assert restored['durable'] and restored['status'] == 'running', restored +result = await rlm.factory.wait(run_id, timeout=10, poll_interval=0.01) +assert result['status'] == 'completed', result +assert result['nodes']['research']['output'] == 'retained research', result +assert result['nodes']['join']['output'] == 'resumed summary', result +assert len(await rlm.list_subagents()) == 2 +Path('factory-resumed-ok').write_text('no duplicate research') +await rlm.end_turn() +"#, + ), + ScriptedProvider::completed("resumed summary"), + ])); + let restored = build_runtime(home.path(), provider.clone())?; + restored.load_persisted_sessions().await?; + let (connection, mut notifications) = initialize_connection(&restored).await?; + let response = restored + .handle_incoming( + connection, + json!({ + "id":45, "method":"session/resume", "params":{"sessionId":parent} + }), + ) + .await + .context("resume parent")?; + assert!(response.get("error").is_none(), "{response}"); + start_turn_with_approval_policy( + &restored, + connection, + parent, + "Restore durable work", + Some("never"), + ) + .await?; + let completed = + wait_for_session_notification(&mut notifications, "turn/completed", parent).await?; + assert_eq!( + completed["params"]["turn"]["status"], + json!("completed"), + "{completed}" + ); + assert_eq!( + std::fs::read_to_string(home.path().join("factory-resumed-ok"))?, + "no duplicate research" + ); + assert_eq!(provider.stream_calls(), 2); + let requests = provider.requests(); + let join = message_texts(&requests[1]).join("\n"); + assert!( + join.contains(r#""research": "retained research""#), + "{join}" + ); + Ok(()) +} diff --git a/docs/factory-durable-workflows.md b/docs/factory-durable-workflows.md new file mode 100644 index 00000000..2aaa9bd2 --- /dev/null +++ b/docs/factory-durable-workflows.md @@ -0,0 +1,150 @@ +# Durable Factory workflows and guarded joins + +This slice extends the experimental DAG scheduler in [Factory workflows](factory-workflows.md). +It depends on that scheduler and the RLM child collection/lifecycle controls. +Execution still advances explicitly inside active Python cells. It does not add +a background supervisor, automatic child restart, or Prime machine compatibility. + +## Persist before dispatching + +```python +run = await rlm.factory.start(spec, enabled=True, durable=True) +result = await rlm.factory.wait(run["id"], timeout=30) +``` + +`durable=False` remains the default and preserves the original volatile behavior. +For a durable run, the host saves a checkpoint before returning from `start`. +Before each spawn, it saves an `admitting` intent containing the reserved child +name. After admission, it saves the returned child ID. Observations, graph +decisions, and stop/cleanup results are checkpointed too. Unchanged observation +polls do not write a new revision. Each external mutation still checks the +current revision first, including retried cancellation. + +The host resolves storage under the current session's artifact directory: +`factory/.json`. The caller cannot supply a path, another session, or a +global scope. Records include their owning session ID and a monotonic revision. +CAS writes reject stale callers; the definition, reserved names and bound child +IDs cannot be changed, and admitted/settled nodes cannot return to pending. + +Writes use a stable sidecar lock, unique temporary file, file fsync, atomic +replacement, and directory fsync on Unix. Unix files are private and operations +are relative to held directory descriptors, so a later symlink/path swap does +not redirect an approved operation. Windows directory handles disallow deletion +while the operation holds them, and reparse-point entries are rejected. + +The ordinary host permission pipeline covers the store and transaction sidecars. +Plan Mode rejects saves before creating files. Read-only load/list never create +directories/locks and never load or recover child sessions. Stored content is +recovery data, not a permission grant or authoritative child lifecycle state. + +Limits are 1 MiB per serialized snapshot and 128 saved runs per session, in +addition to the scheduler's limits. A size, permission, corruption, fsync, or +revision error is surfaced. There is no volatile fallback or silent truncation. +Large definitions/output can reach this bound; avoid large artifacts in prompts +and keep them in separately managed files. There is no snapshot deletion API in +this slice. `forget` drops only a terminal in-memory run, not its durable record. + +## Explicit recovery + +After replacing the kernel or restarting the host, first inspect the saved list: + +```python +saved = await rlm.factory.saved() +print(saved) # run_id, revision, status; no child loading or provider calls +``` + +Then opt in to restoring the selected run in the same parent session: + +```python +run = await rlm.factory.restore(saved[0]["run_id"], enabled=True) +print(run) +# Inspect before proceeding. Only pending nodes can be newly admitted. +result = await rlm.factory.wait(run["id"], timeout=30) +``` + +Restore validates the checkpoint and claims a new revision before looking up +children. That claim goes through the write/Plan gate, before any lazy child +recovery can materialize journal state. It reconciles each unresolved admission +against exactly one direct child with its reserved name, and requires an already +bound child ID to match. It never spawns, replays an earlier prompt, or turns an +uncertain admission back into pending. + +- A retained completed child is collected on the next advance. Its existing + answer can unblock a successor without rerunning the completed work. +- A child that requires explicit session resume yields `waiting_for_resume` on + advance/wait. Resume it through existing session controls, then advance again. +- A missing, ambiguous, or mismatched child leaves `recovery_required`. `wait` + returns immediately. Inspect the host's direct children and the node error; + do not infer that a missing acknowledgement means the child was never created. +- A lost checkpoint acknowledgement leaves `persistence_error` in the local + view. Reload/restore rather than retrying the old revision. A persisted + admission intent can remain unresolved even when no child actually started; + this conservative outcome prevents duplicate work. +- Stop intent survives recovery. Further advancement closes outstanding children + instead of dispatching successors. Failed cleanup stays visible and retryable. +- Stale in-memory schedulers cannot dispatch new work after another restorer + claims the revision. Checkpoint failures prevent further scheduler mutations. + +If persistence is unavailable while an already admitted child needs an urgent +stop, use the existing child controls directly; after storage is available, +restore and reconcile the run. Neither interruption nor a wait timeout promises +that host-owned children have stopped. + +## Version 2 guards and joins + +Version 1 keeps its previous schema and rejects these new fields. Version 2 adds +`when` and `join` while retaining the same finite acyclic graph and admission cap: + +```python +spec = { + "version": 2, + "nodes": [ + {"id": "review", "prompt": 'Return JSON with an "approved" boolean.'}, + {"id": "accept", "prompt": "Summarize the approved outcome.", + "needs": ["review"], + "when": {"node": "review", "path": ["approved"], "equals": True}}, + {"id": "reject", "prompt": "Summarize the requested corrections.", + "needs": ["review"], + "when": {"node": "review", "path": ["approved"], "equals": False}}, + {"id": "summary", "prompt": "Summarize the selected branch.", + "needs": ["accept", "reject"], "join": "any"}, + ], +} +``` + +Guards name an explicit dependency. Without `path`, `equals` is an exact text +comparison. With `path`, the predecessor answer must be finite JSON; the path +contains up to 16 nonempty object keys and the comparison value is a JSON scalar. +Integer comparison values must be in `[-(2**53 - 1), 2**53 - 1]` so the host and +Python preserve their type/value through checkpoint serialization. +Comparison is type-sensitive: `True`, `1`, and `1.0` are distinct. Missing paths +do not match, including when comparing to null. Invalid JSON fails the guarded +node explicitly. There is no eval, code expression, implicit prompt template, +network lookup, or authorization inherited from output text. + +All joins wait for **every** dependency to settle, so branch timing does not +select a winner and no losing branch is abandoned: + +- `join="all"` is the default. Failed predecessors block it; skipped predecessors + skip it. All successful predecessors contribute their outputs. +- `join="any"` proceeds if at least one predecessor completed successfully, + forwarding only successful outputs. If none succeeded it is skipped. With + `failure_policy="continue"`, an independent successful branch may contribute + even when another failed, but the overall run still reports failure. +- A false guard marks its node `skipped`, which is distinct from successful + completion or failure. Skipped nodes spawn no child. All-skipped paths can + complete without inventing a result. + +Loops, dynamic foreach, resident agents, retry budgets, background continuation, +TUI/desktop workflow views, and a machine library remain future work. + +## Verification scope + +Python tests cover guarded joins, persisted admission intent, save-before-effect +failures, lost acknowledgements, stale revisions, missing/mismatched recovery, +Plan claim denial before child access, stop recovery, and corrupt snapshots. +Rust tests cover the host store, permissions, atomic/CAS behavior, filesystem +boundaries, and fresh-kernel loading. Real CPython → host → child-session tests +exercise a guarded branch/join and restoration after rebuilding the runtime. +Provider responses in these integration tests are scripted; they are not paid +model tests or evidence of a long-running background daemon. diff --git a/docs/factory-workflows.md b/docs/factory-workflows.md index a8ee503e..d0183307 100644 --- a/docs/factory-workflows.md +++ b/docs/factory-workflows.md @@ -1,5 +1,9 @@ # Factory workflows: experimental DAG slice +For opt-in durable checkpoints, explicit recovery, and version 2 guards/joins, +see [Durable Factory workflows](factory-durable-workflows.md). The baseline API +and version 1 contract below remain available. + Devo Factory v1 composes ordinary direct child agents into a finite DAG. It provides strict preflight validation, bounded parallel admission, all-parent joins, plain-text result forwarding, explicit failure policies, local status, From 50230e154e892e248f69d022e764ea37b0d229c2 Mon Sep 17 00:00:00 2001 From: wangtsiao Date: Mon, 9 Nov 2026 01:04:31 +0800 Subject: [PATCH 5/5] fix(factory): retain Windows directory guard without dead-code warning --- .../kernel_host_bridge/factory/files.rs | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/crates/server/src/runtime/kernel_host_bridge/factory/files.rs b/crates/server/src/runtime/kernel_host_bridge/factory/files.rs index 02454515..25cfad72 100644 --- a/crates/server/src/runtime/kernel_host_bridge/factory/files.rs +++ b/crates/server/src/runtime/kernel_host_bridge/factory/files.rs @@ -8,7 +8,8 @@ use std::path::{Path, PathBuf}; use std::os::fd::{AsRawFd, FromRawFd}; pub(super) struct Directory { - file: File, + // Retained on Windows to prevent replacement even when no descriptor API is used. + _file: File, pub path: PathBuf, } @@ -64,7 +65,7 @@ impl Directory { return Err(io::Error::other("Factory directory changed during open")); } } - Ok(Self { file, path }) + Ok(Self { _file: file, path }) } pub fn child(&self, name: &str) -> io::Result { @@ -74,7 +75,7 @@ impl Directory { // SAFETY: the parent descriptor is owned and the name is NUL-terminated. let fd = unsafe { libc::openat( - self.file.as_raw_fd(), + self._file.as_raw_fd(), name.as_ptr(), libc::O_RDONLY | libc::O_DIRECTORY | libc::O_NOFOLLOW | libc::O_CLOEXEC, ) @@ -85,7 +86,7 @@ impl Directory { // SAFETY: openat returned a new owned descriptor. let file = unsafe { File::from_raw_fd(fd) }; Ok(Self { - file, + _file: file, path: self.path.join(name.to_str().expect("ASCII store name")), }) } @@ -100,7 +101,7 @@ impl Directory { let result = { let name = std::ffi::CString::new(name)?; // SAFETY: the parent descriptor is owned and the name is NUL-terminated. - if unsafe { libc::mkdirat(self.file.as_raw_fd(), name.as_ptr(), 0o700) } < 0 { + if unsafe { libc::mkdirat(self._file.as_raw_fd(), name.as_ptr(), 0o700) } < 0 { Err(io::Error::last_os_error()) } else { Ok(()) @@ -128,7 +129,7 @@ impl Directory { | libc::O_NONBLOCK | libc::O_CLOEXEC; // SAFETY: descriptor and C string remain valid; mode applies only to creation. - let fd = unsafe { libc::openat(self.file.as_raw_fd(), name.as_ptr(), flags, 0o600) }; + let fd = unsafe { libc::openat(self._file.as_raw_fd(), name.as_ptr(), flags, 0o600) }; if fd < 0 { return Err(io::Error::last_os_error()); } @@ -193,9 +194,9 @@ impl Directory { // SAFETY: directory is owned; both names are validated, NUL-terminated basenames. if unsafe { libc::renameat( - self.file.as_raw_fd(), + self._file.as_raw_fd(), source.as_ptr(), - self.file.as_raw_fd(), + self._file.as_raw_fd(), target.as_ptr(), ) } < 0 @@ -213,7 +214,7 @@ impl Directory { { let name = std::ffi::CString::new(name)?; // SAFETY: directory is owned; only a temporary basename is removed. - if unsafe { libc::unlinkat(self.file.as_raw_fd(), name.as_ptr(), 0) } < 0 { + if unsafe { libc::unlinkat(self._file.as_raw_fd(), name.as_ptr(), 0) } < 0 { return Err(io::Error::last_os_error()); } Ok(()) @@ -230,7 +231,7 @@ impl Directory { // Open a fresh directory description so repeated scans have independent offsets. let scan = self.child(".")?; use std::os::fd::IntoRawFd; - let fd = scan.file.into_raw_fd(); + let fd = scan._file.into_raw_fd(); // SAFETY: fd is a fresh owned directory descriptor; fdopendir takes ownership on success. let directory = unsafe { libc::fdopendir(fd) }; if directory.is_null() { @@ -274,7 +275,7 @@ impl Directory { fn sync(&self) -> io::Result<()> { #[cfg(unix)] - self.file.sync_all()?; + self._file.sync_all()?; Ok(()) } }