diff --git a/loopx/control_plane/effect_runtime.py b/loopx/control_plane/effect_runtime.py index 7e592ed798..248c37a586 100644 --- a/loopx/control_plane/effect_runtime.py +++ b/loopx/control_plane/effect_runtime.py @@ -1000,6 +1000,11 @@ def _start_runtime(*, fingerprint: str, info_path: Path) -> dict[str, Any]: stderr=startup_stderr, start_new_session=os.name != "nt", close_fds=True, + # The host outlives the command that started it. On Windows + # a live child whose working directory is the caller's + # directory keeps that directory (and its ancestors) from + # being renamed or removed, so the host must not inherit it. + cwd=tempfile.gettempdir(), ) except PermissionError as exc: raise EffectRuntimeHostPermissionError() from exc diff --git a/loopx/usage_ping.py b/loopx/usage_ping.py index d2202ceaf5..ca2edea245 100644 --- a/loopx/usage_ping.py +++ b/loopx/usage_ping.py @@ -6,6 +6,7 @@ import shutil import subprocess import sys +import tempfile import time from contextvars import ContextVar from datetime import datetime, timezone @@ -199,9 +200,32 @@ def observe_verified_return() -> None: pass +# Request fields that name a caller-owned filesystem path. The child runs in a +# stable directory instead of the caller's, so a relative value must be fixed +# here or it would resolve against the wrong directory. +_CALLER_PATH_FIELDS = ("path", "registry", "runtime") + + +def _pin_caller_paths(request: dict[str, Any]) -> dict[str, Any]: + """Freeze caller-owned request paths before the child loses the caller cwd.""" + pinned = dict(request) + for field in _CALLER_PATH_FIELDS: + value = pinned.get(field) + if isinstance(value, str) and value: + pinned[field] = str(Path(value).expanduser().resolve()) + return pinned + + def _detach(request: dict[str, Any], *, command: list[str] | None = None) -> None: + request = _pin_caller_paths(request) kwargs: dict[str, Any] = {"stdin": subprocess.PIPE, "stdout": subprocess.DEVNULL, - "stderr": subprocess.DEVNULL, "close_fds": True} + "stderr": subprocess.DEVNULL, "close_fds": True, + # The sender outlives the command. On Windows a + # live child whose working directory is the + # caller's directory keeps that directory (and its + # ancestors) from being renamed or removed, so the + # child must not inherit it. + "cwd": tempfile.gettempdir()} if os.name == "nt": kwargs["creationflags"] = getattr(subprocess, "DETACHED_PROCESS", 0) | getattr(subprocess, "CREATE_NEW_PROCESS_GROUP", 0) else: diff --git a/tests/control_plane/test_effect_runtime_listener_startup.py b/tests/control_plane/test_effect_runtime_listener_startup.py index 47164ab749..b381556948 100644 --- a/tests/control_plane/test_effect_runtime_listener_startup.py +++ b/tests/control_plane/test_effect_runtime_listener_startup.py @@ -1,6 +1,7 @@ from __future__ import annotations import json +import subprocess import tempfile from pathlib import Path @@ -58,3 +59,41 @@ def test_listener_failure_preserves_safe_diagnostic_and_retry_boundary( runtime_dir = effect_runtime._runtime_dir() assert not list(runtime_dir.glob("runtime-*.json")) assert not list(runtime_dir.glob("start-*.lock")) + + +def test_runtime_host_spawn_leaves_the_callers_directory_free( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + for variable in ("TMPDIR", "TEMP", "TMP"): + monkeypatch.setenv(variable, str(tmp_path)) + monkeypatch.setattr(tempfile, "tempdir", str(tmp_path)) + hook = tmp_path / "denied-listener.mjs" + hook.write_text( + 'import { Server } from "node:net";\n' + 'Server.prototype.listen = function () {\n' + ' process.nextTick(() => this.emit("error", Object.assign(\n' + ' new Error("private-listener-fixture"),\n' + ' { code: "EPERM", syscall: "listen" })));\n' + ' return this;\n' + '};\n', + encoding="utf-8", + ) + monkeypatch.setenv("NODE_OPTIONS", f"--import={hook.as_uri()}") + recorded: list[dict[str, object]] = [] + original = subprocess.Popen + + def record(args, *rest, **kwargs): + if "effect_runtime_server.ts" in str(args): + recorded.append(kwargs) + return original(args, *rest, **kwargs) + + monkeypatch.setattr(effect_runtime.subprocess, "Popen", record) + + with pytest.raises(effect_runtime.EffectRuntimeStartupError): + effect_runtime.effect_runtime_result("runtime.ping", {}) + + # The host outlives the command; on Windows an inherited caller directory + # stays locked for the whole host lifetime. + assert len(recorded) == 1 + assert Path(str(recorded[0]["cwd"])).resolve() == tmp_path.resolve() diff --git a/tests/test_usage_goal.py b/tests/test_usage_goal.py index 053d3fccac..a8f1d99f0b 100644 --- a/tests/test_usage_goal.py +++ b/tests/test_usage_goal.py @@ -156,12 +156,15 @@ def test_metadata_cannot_redirect_timing_read_outside_selected_home(tmp_path, mo assert usage_goal._bound_codex_session(registry, "goal", "agent") is None -def _bound_cycle_through_detached_ts(tmp_path, monkeypatch, *, header_characters: int = 0, numeric_seconds: bool = False): +def _bound_cycle_through_detached_ts(tmp_path, monkeypatch, *, header_characters: int = 0, numeric_seconds: bool = False, + caller_relative_paths: bool = False): """Drive the real quota observer -> detached TS chain over one bound session. `header_characters` widens the session_meta line the way Codex's recorder does with base_instructions, so the same entry point covers a header that - does not fit in one fixed read. + does not fit in one fixed read. `caller_relative_paths` names the registry + and runtime root the way an ordinary `loopx --registry .loopx/registry.json` + call does: relative to the caller, not to the detached child. """ from datetime import datetime, timezone from pathlib import Path @@ -188,8 +191,15 @@ def wait_for(predicate): pytest.fail("detached timing did not reach TS owner") now = time.time_ns() // 1_000_000 + registry_path, runtime_root = registry, tmp_path / "runtime" + if caller_relative_paths: + # The observer is a caller: its paths are relative to this directory, + # while the detached child runs in the stable OS temp directory. + monkeypatch.chdir(tmp_path) + registry_path, runtime_root = Path("registry.json"), Path("runtime") + def publish(phase): - usage_goal.observe_quota_cycle(registry_path=registry, runtime_root=tmp_path / "runtime", goal_id="goal", + usage_goal.observe_quota_cycle(registry_path=registry_path, runtime_root=runtime_root, goal_id="goal", agent_id="agent", turn_id="turn-a", phase=phase, at=time.time_ns() // 1_000_000, host="unknown") publish("start") @@ -226,6 +236,14 @@ def test_real_detached_cycle_and_bound_codex_event_reach_shared_ts_aggregator(tm assert not cycle_path.exists() and not goals.exists() +def test_caller_relative_registry_and_runtime_reach_the_detached_observer(tmp_path, monkeypatch): + """Leaving the caller's directory must not re-resolve the caller's own paths.""" + preview, _, _, _ = _bound_cycle_through_detached_ts(tmp_path, monkeypatch, caller_relative_paths=True) + assert {row["measurement"] for row in preview["counters"]} == {"quota_cycle", "codex_turn"} + assert all(row["host"] == "codex_app" for row in preview["counters"]) + usage_ping.control("disable") + + def test_provider_numeric_timing_reaches_real_detached_installation_aggregator(tmp_path, monkeypatch): from pathlib import Path preview, _, _, _ = _bound_cycle_through_detached_ts(tmp_path, monkeypatch, numeric_seconds=True) diff --git a/tests/test_usage_ping.py b/tests/test_usage_ping.py index 09480d830e..18f3d016fa 100644 --- a/tests/test_usage_ping.py +++ b/tests/test_usage_ping.py @@ -9,6 +9,7 @@ import socket import subprocess import sys +import tempfile import threading import time from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer @@ -366,6 +367,25 @@ def test_detached_sender_honors_proxy_and_no_proxy(isolated, collector, monkeypa release.set() +def test_detached_sender_leaves_the_callers_directory_free(isolated, tmp_path): + """On Windows a live child whose cwd is the caller's directory pins it.""" + observed = tmp_path / 'sender-cwd.json' + code = ("import json, os, sys\n" + "json.load(sys.stdin)\n" + f"open({str(observed)!r}, 'w', encoding='utf-8').write(json.dumps(os.getcwd()))\n") + usage_ping._detach(usage_ping._request('observe', isolated / 'usage-ping.json'), + command=[sys.executable, '-c', code]) + deadline = time.monotonic() + 30 + child_cwd = None + while time.monotonic() < deadline and child_cwd is None: + try: + child_cwd = json.loads(observed.read_text(encoding='utf-8')) + except (OSError, json.JSONDecodeError): + time.sleep(0.05) + assert child_cwd is not None, 'detached sender did not report its working directory' + assert Path(child_cwd).resolve() == Path(tempfile.gettempdir()).resolve() + + @pytest.mark.parametrize('upgrade', [False, True]) def test_real_chat_settings_share_cli_choice_and_reject_cross_origin(isolated, upgrade): if upgrade: