From f955e8305d2141f5626b5c807379b1ade1c287f5 Mon Sep 17 00:00:00 2001 From: song Date: Fri, 2 Oct 2026 22:49:16 +0800 Subject: [PATCH 1/3] fix(runtime): stop treating route locks as a second authority Goal configuration locks every candidate runtime registry before writing, and the native effect runtime locks the same registry file. On a fresh machine the untouched legacy root then holds only `registry.global.json.ts-effect.lock`, which `_is_route_observation` did not recognize, so the first configured Goal failed with "Both default LoopX runtime roots contain state". - Recognize the native effect lock as a route observation, like the Python lock it sits beside. - Isolate the default runtime routes per test in `tests/conftest.py`, and fail the session when tests create a real default route that was absent. - Follow the `collect_doctor` signature added in #5457 in two test mocks. - Keep `cancel-in-progress` on pull requests only, so a `main` push run is not cancelled by the next merge. Verified: the added regression test fails on the unpatched `paths.py` and passes with it; the 54 files that failed on CI pass 1149/1152 here, and the 3 remaining failures reproduce unchanged at this branch's base. Signed-off-by: song --- .github/workflows/postgresql-integration.yml | 2 +- .github/workflows/python-tests.yml | 2 +- loopx/paths.py | 5 ++ tests/conftest.py | 56 ++++++++++++++++++++ tests/test_cli_argument_diagnostics.py | 2 +- tests/test_cli_entrypoint.py | 5 +- tests/test_local_state_migration.py | 23 ++++++++ 7 files changed, 91 insertions(+), 4 deletions(-) diff --git a/.github/workflows/postgresql-integration.yml b/.github/workflows/postgresql-integration.yml index a6e103a455..e80d1adcf6 100644 --- a/.github/workflows/postgresql-integration.yml +++ b/.github/workflows/postgresql-integration.yml @@ -31,7 +31,7 @@ permissions: concurrency: group: postgresql-integration-${{ github.ref }} - cancel-in-progress: true + cancel-in-progress: ${{ github.event_name == 'pull_request' }} jobs: postgresql-authority: diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml index eee57938aa..704e76c46c 100644 --- a/.github/workflows/python-tests.yml +++ b/.github/workflows/python-tests.yml @@ -30,7 +30,7 @@ permissions: concurrency: group: python-tests-${{ github.ref }} - cancel-in-progress: true + cancel-in-progress: ${{ github.event_name == 'pull_request' }} env: LOOPX_USAGE_PING: "0" diff --git a/loopx/paths.py b/loopx/paths.py index fab5086aba..7dbbfbe0cb 100644 --- a/loopx/paths.py +++ b/loopx/paths.py @@ -147,6 +147,11 @@ def _is_route_observation(path: Path) -> bool: return False if path.name == GLOBAL_REGISTRY_FILENAME + ".lock": return path.is_file() + # The native effect runtime locks every registry it writes through, so a + # mere lock file must not turn a default root into a second authority. + # Spelled here because file_lock imports this module. + if path.name == GLOBAL_REGISTRY_FILENAME + ".ts-effect.lock": + return path.is_file() if path.name == "lark-consumers" and path.is_dir(): import re diff --git a/tests/conftest.py b/tests/conftest.py index 156af8272f..5a7cd9f8aa 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,5 +1,6 @@ from __future__ import annotations +import importlib import os import sys from pathlib import Path @@ -12,10 +13,65 @@ if str(REPO_ROOT) not in sys.path: sys.path.insert(0, str(REPO_ROOT)) +import pytest # noqa: E402 + +from loopx import paths # noqa: E402 from loopx.canary.runner import SMOKE_SUITE_CHOICES # noqa: E402 from loopx.semantics.production import NPM_DEV_DEPENDENCIES_MISSING # noqa: E402 +# Default runtime routes resolve from HOME at import time. A test that writes +# either one leaves state behind, and once both hold state every later implicit +# default route fails as a conflict, so each test gets its own disposable pair. +_DEFAULT_ROUTE_REFERENCES = ( + ("loopx.paths", "DEFAULT_RUNTIME_ROOT", (".loopx",)), + ("loopx.paths", "LEGACY_RUNTIME_ROOT", (".codex", "loopx")), + ("loopx.contract", "DEFAULT_RUNTIME_ROOT", (".loopx",)), + ("loopx.contract", "LEGACY_RUNTIME_ROOT", (".codex", "loopx")), + ("loopx.cli_commands.registry_admin_lifecycle", "DEFAULT_RUNTIME_ROOT", (".loopx",)), + ("loopx.cli_commands.registry_admin_lifecycle", "LEGACY_LOCAL_RUNTIME_ROOT", (".codex", "loopx")), + ("loopx.control_plane.runtime.local_state_migration", "DEFAULT_RUNTIME_ROOT", (".loopx",)), + ("loopx.control_plane.runtime.local_state_migration", "LEGACY_RUNTIME_ROOT", (".codex", "loopx")), +) +_REAL_DEFAULT_ROUTES = (paths.DEFAULT_RUNTIME_ROOT, paths.LEGACY_RUNTIME_ROOT) + + +@pytest.fixture(autouse=True) +def _isolated_default_runtime_routes(tmp_path_factory, monkeypatch): + home = tmp_path_factory.mktemp("home") + monkeypatch.setenv("HOME", str(home)) + monkeypatch.setenv("USERPROFILE", str(home)) + monkeypatch.delenv("CODEX_HOME", raising=False) + for module_name, attribute, parts in _DEFAULT_ROUTE_REFERENCES: + monkeypatch.setattr(importlib.import_module(module_name), attribute, home.joinpath(*parts)) + yield + + +def pytest_sessionstart(session) -> None: + # Only routes absent at start are guarded; existing developer state may be + # changed by other local processes during the run. + session.config._loopx_absent_routes = [ + root for root in _REAL_DEFAULT_ROUTES if not os.path.lexists(root) + ] + + +def pytest_sessionfinish(session, exitstatus) -> None: + created = [ + root for root in getattr(session.config, "_loopx_absent_routes", []) + if os.path.lexists(root) + ] + if not created: + return + reporter = session.config.pluginmanager.get_plugin("terminalreporter") + if reporter is not None: + reporter.write_sep("=", "loopx default runtime route leak", red=True) + reporter.write_line( + f"Tests created real default runtime routes {', '.join(map(str, created))}; " + "isolate the writer." + ) + session.exitstatus = pytest.ExitCode.TESTS_FAILED + + def pytest_addoption(parser) -> None: group = parser.getgroup("loopx-smoke-suite") group.addoption( diff --git a/tests/test_cli_argument_diagnostics.py b/tests/test_cli_argument_diagnostics.py index 1a476b50d0..1a3fe22c94 100644 --- a/tests/test_cli_argument_diagnostics.py +++ b/tests/test_cli_argument_diagnostics.py @@ -1089,7 +1089,7 @@ def test_doctor_accepts_subcommand_json_format( monkeypatch.setattr( doctor_command, "collect_doctor", - lambda *, deep=False, agent_type=None, installation_only=False: { + lambda *, deep=False, agent_type=None, installation_only=False, **_routes: { "ok": True, "deep": deep, "agent_type": agent_type, diff --git a/tests/test_cli_entrypoint.py b/tests/test_cli_entrypoint.py index 364e4911d2..425392cce4 100644 --- a/tests/test_cli_entrypoint.py +++ b/tests/test_cli_entrypoint.py @@ -369,7 +369,10 @@ def collect(**kwargs): code = main(["--format", "markdown", "doctor", "--format", "json", "--deep", "--installation-only"]) assert code == {0 if healthy else 1} -assert observed == [{{"deep": True, "agent_type": None, "installation_only": True}}] +assert len(observed) == 1 +assert {{key: observed[0][key] for key in ("deep", "agent_type", "installation_only", "runtime_root_override")}} == {{ + "deep": True, "agent_type": None, "installation_only": True, "runtime_root_override": None, +}} assert json.loads(output.getvalue()) == {{"ok": {healthy!r}, "scope": "installation_only"}} """ completed = run_isolated_script(script) diff --git a/tests/test_local_state_migration.py b/tests/test_local_state_migration.py index 2c538b2f73..fb5a7e2ce1 100644 --- a/tests/test_local_state_migration.py +++ b/tests/test_local_state_migration.py @@ -159,6 +159,29 @@ def reparse_lstat(path, *args, **kwargs): assert paths.configured_runtime_route(runtime_root_override=str(redirected))["status"] == "invalid" +@pytest.mark.parametrize("relative", ["registry.global.json.lock", + "registry.global.json.ts-effect.lock"]) +def test_own_registry_locks_do_not_declare_a_second_authority( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, relative: str, +) -> None: + """A route must not read its own write-locks back as machine state. + + Goal configuration locks every candidate runtime registry before writing, + and the native effect runtime locks the same file too. On a fresh machine + those locks are the only entries in the untouched legacy root, so treating + them as state made the first configured Goal fail as a two-root conflict. + """ + + source, target = tmp_path / "home" / ".codex" / "loopx", tmp_path / "home" / ".loopx" + monkeypatch.setattr(paths, "LEGACY_RUNTIME_ROOT", source) + monkeypatch.setattr(paths, "DEFAULT_RUNTIME_ROOT", target) + for root in (source, target): + root.mkdir(parents=True) + (root / relative).touch() + assert paths.default_runtime_route()["status"] == "fresh" + assert paths.select_default_runtime_root() == target + + def test_global_service_selector_keeps_a_registered_route_amid_real_conflict(tmp_path, monkeypatch): from loopx.cli_commands.support_control_registry import explicit_global_registry source, target, projects = _fixture(tmp_path, projects=1) From ab4d5d1feee3d15c0cf2c699092536f55fb1eac5 Mon Sep 17 00:00:00 2001 From: song Date: Sat, 3 Oct 2026 01:17:44 +0800 Subject: [PATCH 2/3] test: align isolated fixtures with current skill and Turn contracts Signed-off-by: song --- tests/test_chat_codex_goal.py | 5 +++++ tests/test_skill_delivery_parity.py | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/test_chat_codex_goal.py b/tests/test_chat_codex_goal.py index 6d888ec4d1..047dc6bc9c 100644 --- a/tests/test_chat_codex_goal.py +++ b/tests/test_chat_codex_goal.py @@ -415,6 +415,11 @@ def test_external_queued_message_cannot_activate_local_owner_continuation( message="/goal start --tokens 1000 Analyze", origin="lark", ) + # Production dispatch claims the exact queued Turn before starting it. + # Without that claim the execution fence correctly refuses this worker + # before it can evaluate whether the external command is authorized. + claimed = store.claim_next_queued_turn(session["session_id"]) + assert claimed is not None and claimed["turn_id"] == turn["turn_id"] controller._run_turn( session_id=session["session_id"], turn_id=turn["turn_id"], diff --git a/tests/test_skill_delivery_parity.py b/tests/test_skill_delivery_parity.py index ff5704ec26..661251f46f 100644 --- a/tests/test_skill_delivery_parity.py +++ b/tests/test_skill_delivery_parity.py @@ -82,8 +82,8 @@ def test_packaged_skill_dirs_exist(self): encoding="utf-8" ).strip() == "global", skill_id - def test_repo_has_seven_skills(self): - assert len(REQUIRED_HOST_SKILL_IDS) == 7 # loopx + 6 packaged + def test_required_skills_match_the_shipped_catalog(self): + assert set(REQUIRED_HOST_SKILL_IDS) == {"loopx", *PACKAGED_HOST_SKILL_IDS} # -- Skill install readback lifecycle ----------------------------------------- From 2033749e65cfe38dae04496eb34b4559ce818890 Mon Sep 17 00:00:00 2001 From: huangruiteng Date: Sat, 3 Oct 2026 02:52:08 +0800 Subject: [PATCH 3/3] Refresh registry I/O location after route-lock classifier change Signed-off-by: huangruiteng --- loopx/semantics/project_registry_io_manifest_v1.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/loopx/semantics/project_registry_io_manifest_v1.json b/loopx/semantics/project_registry_io_manifest_v1.json index b6edd7aa33..c9a0d2d46f 100644 --- a/loopx/semantics/project_registry_io_manifest_v1.json +++ b/loopx/semantics/project_registry_io_manifest_v1.json @@ -1959,7 +1959,7 @@ }, { "site": "loopx/paths.py::.configured_runtime_route::codec_read:load_registry#1", - "line": 262, + "line": 267, "column": 20, "kind": "codec_read", "api": "load_registry",