diff --git a/loopx/cli_commands/start_goal.py b/loopx/cli_commands/start_goal.py index 9655b7aeb6..736aac42b1 100644 --- a/loopx/cli_commands/start_goal.py +++ b/loopx/cli_commands/start_goal.py @@ -65,7 +65,15 @@ ) -def _effect_runtime_startup_recommended_action(diagnostic_code: str) -> str: +def _effect_runtime_startup_recommended_action( + diagnostic_code: str, + message: str, +) -> str: + if diagnostic_code == "invalid_idle_timeout": + # The TypeScript runtime owns the idle-timeout validation rules and + # publishes them as the typed startup message, so the projection must + # reuse that message rather than restate the accepted range here. + return f"{message} Then retry `loopx start-goal --guided`." return _EFFECT_RUNTIME_STARTUP_REMEDIATION_BY_CODE.get( diagnostic_code, _EFFECT_RUNTIME_STARTUP_DEFAULT_REMEDIATION, @@ -87,7 +95,8 @@ def _effect_runtime_startup_failure_payload( "required_for": ["start-goal", "control_plane"], }, "recommended_action": _effect_runtime_startup_recommended_action( - diagnostic_code + diagnostic_code, + str(exc), ), } diff --git a/loopx/control_plane/effect_runtime.py b/loopx/control_plane/effect_runtime.py index 00c251747b..525549bf22 100644 --- a/loopx/control_plane/effect_runtime.py +++ b/loopx/control_plane/effect_runtime.py @@ -14,7 +14,7 @@ from collections.abc import Mapping from functools import lru_cache from pathlib import Path -from typing import Any +from typing import IO, Any from ..file_lock import process_is_alive @@ -22,10 +22,14 @@ EFFECT_RUNTIME_RESPONSE_SCHEMA_VERSION = "loopx_effect_runtime_response_v1" EFFECT_RUNTIME_INFO_SCHEMA_VERSION = "loopx_effect_runtime_info_v0" EFFECT_RUNTIME_READINESS_SCHEMA_VERSION = "loopx_effect_runtime_readiness_v0" +EFFECT_RUNTIME_STARTUP_ERROR_SCHEMA_VERSION = ( + "loopx_effect_runtime_startup_error_v0" +) MINIMUM_NODE_VERSION = (22, 18, 0) MINIMUM_NODE_VERSION_TEXT = ".".join(str(part) for part in MINIMUM_NODE_VERSION) MAX_RESPONSE_BYTES = 2 * 1024 * 1024 MAX_REQUEST_BYTES = 2 * 1024 * 1024 +MAX_STARTUP_DIAGNOSTIC_BYTES = 8 * 1024 STARTUP_LOCK_TIMEOUT_SECONDS = 15.0 STARTUP_READY_TIMEOUT_SECONDS = 15.0 STARTUP_POLL_SECONDS = 0.025 @@ -407,6 +411,53 @@ def _remote_runtime_error(value: object) -> EffectRuntimeRemoteError: ) +def _read_startup_stderr(capture: IO[bytes]) -> bytes: + """Return the bounded stderr a managed runtime wrote before it exited.""" + + try: + capture.seek(0) + return capture.read(MAX_STARTUP_DIAGNOSTIC_BYTES) + except (OSError, ValueError): + return b"" + + +def _startup_diagnostic(raw: bytes) -> tuple[str, str] | None: + """Return the typed diagnostic a rejected managed runtime published. + + A server that rejects its own startup configuration writes one JSON + envelope to stderr and exits before it listens, so a matching envelope is + the authoritative configuration error. Any other stderr content, such as a + Node.js stack trace, is not a typed diagnostic and must not be reported as + one. + """ + + if not raw: + return None + for line in reversed(raw.decode("utf-8", errors="replace").splitlines()): + candidate = line.strip() + if not candidate.startswith("{"): + continue + try: + payload = json.loads(candidate) + except json.JSONDecodeError: + continue + if not isinstance(payload, dict): + continue + if ( + payload.get("schema_version") + != EFFECT_RUNTIME_STARTUP_ERROR_SCHEMA_VERSION + ): + continue + code = payload.get("code") + if not isinstance(code, str) or not code: + continue + rendered = " ".join(str(payload.get("message") or "").split())[:240] + return code, rendered or ( + "TypeScript Effect runtime rejected its startup configuration" + ) + return None + + def _start_runtime(*, fingerprint: str, info_path: Path) -> dict[str, Any]: runtime_dir = info_path.parent runtime_dir.mkdir(parents=True, exist_ok=True, mode=0o700) @@ -458,49 +509,63 @@ def _start_runtime(*, fingerprint: str, info_path: Path) -> dict[str, Any]: token = secrets.token_urlsafe(32) environment = os.environ.copy() environment["LOOPX_EFFECT_RUNTIME_TOKEN"] = token - try: - process = subprocess.Popen( - [ - _node_executable(), - "--no-warnings", - "--experimental-strip-types", - str(_runtime_server_path()), - "--info", - str(info_path), - "--fingerprint", - fingerprint, - ], - env=environment, - stdin=subprocess.DEVNULL, - stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL, - start_new_session=os.name != "nt", - close_fds=True, - ) - except OSError as exc: - raise EffectRuntimeStartupError( - "TypeScript Effect runtime process could not be launched", - diagnostic_code="runtime_launch_failed", - ) from exc - ready_deadline = time.monotonic() + STARTUP_READY_TIMEOUT_SECONDS - while time.monotonic() < ready_deadline: - info = _read_info(info_path, fingerprint=fingerprint) - if info is not None: - return info - exit_code = process.poll() - if exit_code is not None: - raise EffectRuntimeStartupError( - "TypeScript Effect runtime exited before becoming ready " - f"(exit_code={exit_code})", - diagnostic_code="runtime_exited_before_ready", + # Capture stderr so a rejected startup can publish a typed + # configuration diagnostic instead of a bare exit status. The capture + # is an unlinked temporary file, so it cannot deadlock the child on a + # full pipe and it leaves no stale path behind. + with tempfile.TemporaryFile() as startup_stderr: + try: + process = subprocess.Popen( + [ + _node_executable(), + "--no-warnings", + "--experimental-strip-types", + str(_runtime_server_path()), + "--info", + str(info_path), + "--fingerprint", + fingerprint, + ], + env=environment, + stdin=subprocess.DEVNULL, + stdout=subprocess.DEVNULL, + stderr=startup_stderr, + start_new_session=os.name != "nt", + close_fds=True, ) - time.sleep(STARTUP_POLL_SECONDS) - if process.poll() is None: - process.terminate() - raise EffectRuntimeStartupError( - "TypeScript Effect runtime did not become ready before the startup deadline", - diagnostic_code="runtime_startup_timeout", - ) + except OSError as exc: + raise EffectRuntimeStartupError( + "TypeScript Effect runtime process could not be launched", + diagnostic_code="runtime_launch_failed", + ) from exc + ready_deadline = time.monotonic() + STARTUP_READY_TIMEOUT_SECONDS + while time.monotonic() < ready_deadline: + info = _read_info(info_path, fingerprint=fingerprint) + if info is not None: + return info + exit_code = process.poll() + if exit_code is not None: + diagnostic = _startup_diagnostic( + _read_startup_stderr(startup_stderr) + ) + if diagnostic is not None: + code, message = diagnostic + raise EffectRuntimeStartupError( + message, + diagnostic_code=code, + ) + raise EffectRuntimeStartupError( + "TypeScript Effect runtime exited before becoming ready " + f"(exit_code={exit_code})", + diagnostic_code="runtime_exited_before_ready", + ) + time.sleep(STARTUP_POLL_SECONDS) + if process.poll() is None: + process.terminate() + raise EffectRuntimeStartupError( + "TypeScript Effect runtime did not become ready before the startup deadline", + diagnostic_code="runtime_startup_timeout", + ) finally: lock.unlink(missing_ok=True) diff --git a/loopx/control_plane/effect_runtime_server.ts b/loopx/control_plane/effect_runtime_server.ts index f18aef8a3e..7c6a7f294f 100644 --- a/loopx/control_plane/effect_runtime_server.ts +++ b/loopx/control_plane/effect_runtime_server.ts @@ -1,3 +1,4 @@ +import { writeSync } from "node:fs"; import { createServer, type Socket } from "node:net"; import { chmod, rm } from "node:fs/promises"; @@ -19,8 +20,18 @@ import { const REQUEST_SCHEMA = "loopx_effect_runtime_request_v0"; const RESPONSE_SCHEMA = "loopx_effect_runtime_response_v1"; const INFO_SCHEMA = "loopx_effect_runtime_info_v0"; +const STARTUP_ERROR_SCHEMA = "loopx_effect_runtime_startup_error_v0"; const MAX_REQUEST_BYTES = 2 * 1024 * 1024; const DEFAULT_IDLE_MS = 5 * 60 * 1_000; +// Bounds for LOOPX_EFFECT_RUNTIME_IDLE_MS. The upper bound is the largest +// delay `setTimeout` accepts: a larger delay overflows and fires immediately, +// which is the same silent immediate-shutdown failure as a NaN or negative +// value. +const MIN_IDLE_MS = 1; +const MAX_IDLE_MS = 2 ** 31 - 1; +const STARTUP_CONFIG_EXIT_CODE = 2; +const INVALID_IDLE_TIMEOUT_CODE = "invalid_idle_timeout"; +const MAX_REPORTED_RAW_LENGTH = 32; let shutdownRequested = false; function asObject(value: unknown): JsonObject { @@ -37,10 +48,51 @@ function parseArg(name: string): string { return requiredString(process.argv[index + 1], name); } +function failStartup(code: string, message: string): never { + writeSync( + 2, + `${JSON.stringify({ + schema_version: STARTUP_ERROR_SCHEMA, + code, + message, + })}\n`, + ); + process.exit(STARTUP_CONFIG_EXIT_CODE); +} + +function idleTimeoutGuidance(): string { + return "LOOPX_EFFECT_RUNTIME_IDLE_MS must be a base-10 integer between " + + `${MIN_IDLE_MS} and ${MAX_IDLE_MS} milliseconds; leave it unset to use ` + + `the ${DEFAULT_IDLE_MS} ms default`; +} + +function parseIdleMs(raw: string | undefined): number { + if (raw === undefined) return DEFAULT_IDLE_MS; + const received = JSON.stringify(raw.slice(0, MAX_REPORTED_RAW_LENGTH)); + if (!/^[0-9]+$/.test(raw)) { + failStartup( + INVALID_IDLE_TIMEOUT_CODE, + `${idleTimeoutGuidance()} (received ${received})`, + ); + } + const parsed = Number(raw); + if ( + !Number.isSafeInteger(parsed) || + parsed < MIN_IDLE_MS || + parsed > MAX_IDLE_MS + ) { + failStartup( + INVALID_IDLE_TIMEOUT_CODE, + `${idleTimeoutGuidance()} (received ${received})`, + ); + } + return parsed; +} + const infoPath = parseArg("--info"); const fingerprint = parseArg("--fingerprint"); const token = requiredString(process.env.LOOPX_EFFECT_RUNTIME_TOKEN, "runtime token"); -const idleMs = Number(process.env.LOOPX_EFFECT_RUNTIME_IDLE_MS ?? DEFAULT_IDLE_MS); +const idleMs = parseIdleMs(process.env.LOOPX_EFFECT_RUNTIME_IDLE_MS); let idleTimer: NodeJS.Timeout; const handlers = createEffectRuntimeHandlers({ fingerprint, diff --git a/tests/control_plane/test_effect_runtime_integration.py b/tests/control_plane/test_effect_runtime_integration.py index f35c7e9750..93651583e8 100644 --- a/tests/control_plane/test_effect_runtime_integration.py +++ b/tests/control_plane/test_effect_runtime_integration.py @@ -765,6 +765,68 @@ def test_managed_runtime_releases_memory_after_idle_timeout( ) +@pytest.mark.parametrize( + "raw_idle_ms", + [ + "", + "not-a-number", + "0", + "-1", + "1.5", + "1e3", + "0x10", + " 150", + "150 ", + "2147483648", + "9007199254740993", + ], +) +def test_invalid_idle_timeout_configuration_fails_closed( + tmp_path: Path, + monkeypatch, + raw_idle_ms: str, +) -> None: + runtime_dir = tmp_path / "runtime" + monkeypatch.setattr(effect_runtime, "_runtime_dir", lambda: runtime_dir) + monkeypatch.setenv("LOOPX_EFFECT_RUNTIME_IDLE_MS", raw_idle_ms) + + with pytest.raises( + effect_runtime.EffectRuntimeStartupError, + match="LOOPX_EFFECT_RUNTIME_IDLE_MS", + ) as exc_info: + effect_runtime.effect_runtime_result("runtime.ping", {}) + + assert exc_info.value.diagnostic_code == "invalid_idle_timeout" + assert list(runtime_dir.glob("runtime-*.json")) == [] + + +@pytest.mark.parametrize("raw_idle_ms", [None, "150", "2147483647"]) +def test_valid_idle_timeout_configuration_serves_requests( + tmp_path: Path, + monkeypatch, + raw_idle_ms: str | None, +) -> None: + runtime_dir = tmp_path / "runtime" + monkeypatch.setattr(effect_runtime, "_runtime_dir", lambda: runtime_dir) + if raw_idle_ms is None: + monkeypatch.delenv("LOOPX_EFFECT_RUNTIME_IDLE_MS", raising=False) + else: + monkeypatch.setenv("LOOPX_EFFECT_RUNTIME_IDLE_MS", raw_idle_ms) + + try: + result = effect_runtime.effect_runtime_result("runtime.ping", {}) + assert int(result["pid"]) > 0 + finally: + try: + effect_runtime.effect_runtime_result( + "runtime.shutdown", + {}, + retry_safe=False, + ) + except Exception: + pass + + def test_oversized_request_is_rejected_before_runtime_dispatch( tmp_path: Path, monkeypatch, diff --git a/tests/control_plane/test_start_goal_compact_projection.py b/tests/control_plane/test_start_goal_compact_projection.py index 66e9768495..f52c6dcf31 100644 --- a/tests/control_plane/test_start_goal_compact_projection.py +++ b/tests/control_plane/test_start_goal_compact_projection.py @@ -153,10 +153,11 @@ def _block_effect_runtime_startup( monkeypatch: pytest.MonkeyPatch, *, diagnostic_code: str = "node_unavailable", + message: str = "TypeScript Effect runtime could not serve the request", ) -> None: def unavailable(*_args: object, **_kwargs: object) -> object: raise effect_runtime.EffectRuntimeStartupError( - "TypeScript Effect runtime could not serve the request", + message, diagnostic_code=diagnostic_code, ) @@ -168,19 +169,26 @@ def unavailable(*_args: object, **_kwargs: object) -> object: @pytest.mark.parametrize( - ("diagnostic_code", "expected_action_fragment", "mentions_node_installation"), + ( + "diagnostic_code", + "expected_action_fragment", + "mentions_node_installation", + "mentions_generic_runtime_recovery", + ), [ - ("node_unavailable", "Install or activate Node.js", True), + ("node_unavailable", "Install or activate Node.js", True, True), ( "startup_lock_timeout", "wait for the active startup to settle", False, + True, ), - ("runtime_launch_failed", "runtime still cannot start", False), - ("runtime_exited_before_ready", "runtime exits again", False), - ("runtime_startup_timeout", "startup continues to time out", False), - ("runtime_request_failed", "requests continue to fail", False), - ("future_runtime_diagnostic", "runtime remains unavailable", False), + ("runtime_launch_failed", "runtime still cannot start", False, True), + ("runtime_exited_before_ready", "runtime exits again", False, True), + ("runtime_startup_timeout", "startup continues to time out", False, True), + ("runtime_request_failed", "requests continue to fail", False, True), + ("invalid_idle_timeout", "could not serve the request", False, False), + ("future_runtime_diagnostic", "runtime remains unavailable", False, True), ], ) def test_cli_reports_effect_runtime_startup_failure_without_traceback( @@ -189,6 +197,7 @@ def test_cli_reports_effect_runtime_startup_failure_without_traceback( diagnostic_code: str, expected_action_fragment: str, mentions_node_installation: bool, + mentions_generic_runtime_recovery: bool, ) -> None: project = _write_connected_project(tmp_path) _block_effect_runtime_startup(monkeypatch, diagnostic_code=diagnostic_code) @@ -222,11 +231,16 @@ def test_cli_reports_effect_runtime_startup_failure_without_traceback( assert payload["error"] == "LoopX TypeScript control-plane runtime is unavailable" assert payload["diagnostic_code"] == diagnostic_code assert payload["runtime_requirement"]["minimum_node_version"] == "22.18.0" - assert "loopx doctor --deep" in payload["recommended_action"] - assert expected_action_fragment in payload["recommended_action"] - assert ("Node.js" in payload["recommended_action"]) is mentions_node_installation + action = payload["recommended_action"] + assert expected_action_fragment in action + assert ("Node.js" in action) is mentions_node_installation if not mentions_node_installation: - assert "Install or activate Node.js" not in payload["recommended_action"] + assert "Install or activate Node.js" not in action + if mentions_generic_runtime_recovery: + assert "loopx doctor --deep" in action + else: + assert "loopx doctor --deep" not in action + assert "reinstall" not in action assert "Traceback" not in raw_output @@ -265,6 +279,111 @@ def test_cli_reports_effect_runtime_startup_failure_in_default_markdown( assert "Traceback" not in raw_output +def test_guided_cli_keeps_actionable_invalid_idle_timeout_remediation( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The real guided entrypoint must keep the actionable configuration fix. + + This drives the real managed Node runtime instead of stubbing the startup + failure, so it also proves the typed stderr diagnostic survives the public + projection end to end. + """ + + project = _write_connected_project(tmp_path) + monkeypatch.setattr(effect_runtime, "_runtime_dir", lambda: tmp_path / "runtime") + monkeypatch.setenv("LOOPX_EFFECT_RUNTIME_IDLE_MS", "0") + + output = io.StringIO() + with contextlib.redirect_stdout(output): + exit_code = cli_main( + [ + "--format", + "json", + "start-goal", + "--guided", + "--project", + str(project), + "--goal-id", + GOAL_ID, + "--agent-id", + AGENT_ID, + "--host-surface", + "shell", + "--goal-text", + GOAL_TEXT, + ] + ) + + payload = json.loads(output.getvalue()) + assert exit_code == 1 + assert payload["ok"] is False + assert payload["diagnostic_code"] == "invalid_idle_timeout" + + action = payload["recommended_action"] + assert "LOOPX_EFFECT_RUNTIME_IDLE_MS" in action + assert "between 1 and 2147483647 milliseconds" in action + assert '(received "0")' in action + assert "unset" in action.lower() + assert "Then retry `loopx start-goal --guided`." in action + assert "doctor" not in action + assert "reinstall" not in action + + +def test_invalid_idle_timeout_remediation_reuses_typed_runtime_message( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The projection reuses the runtime-owned guidance instead of restating it. + + The TypeScript parser owns the accepted range and publishes it as the + typed startup message, so the guided CLI must surface whatever bounded + message the runtime produced and append only its own retry text. A varied + message proves no range is re-hardcoded on the Python side. + """ + + varied_message = ( + "LOOPX_EFFECT_RUNTIME_IDLE_MS must be a base-10 integer between " + '7 and 9001 milliseconds (received "nope")' + ) + project = _write_connected_project(tmp_path) + _block_effect_runtime_startup( + monkeypatch, + diagnostic_code="invalid_idle_timeout", + message=varied_message, + ) + + output = io.StringIO() + with contextlib.redirect_stdout(output): + exit_code = cli_main( + [ + "--format", + "json", + "start-goal", + "--guided", + "--project", + str(project), + "--goal-id", + GOAL_ID, + "--agent-id", + AGENT_ID, + "--host-surface", + "shell", + "--goal-text", + GOAL_TEXT, + ] + ) + + payload = json.loads(output.getvalue()) + assert exit_code == 1 + assert payload["diagnostic_code"] == "invalid_idle_timeout" + action = payload["recommended_action"] + assert action == f"{varied_message} Then retry `loopx start-goal --guided`." + assert "2147483647" not in action + assert "doctor" not in action + assert "reinstall" not in action + + def test_default_projection_preserves_host_actions_and_json_anchors( tmp_path: Path, ) -> None: