From 17497de43667f6c0ab887c2836c96da8a4ec5e0d Mon Sep 17 00:00:00 2001 From: Felipe Date: Thu, 10 Sep 2026 16:17:27 -0300 Subject: [PATCH] Detect the .car layout on the host the way the runtime does Claude-Session: https://claude.ai/code/session_01GNQXMh1CKBn6krXbq9ZJ5S --- cli/canyonos/config.py | 9 +++-- cli/canyonos/constants.py | 28 +++++++++++++-- cli/canyonos/deploy.py | 18 +++++++--- cli/canyonos/test.py | 10 ++++-- tests/test_canyonos_deploy.py | 28 +++++++++++++-- tests/test_canyonos_test.py | 15 ++++++++ tests/test_cli.py | 68 +++++++++++++++++++++++++++++++++++ 7 files changed, 157 insertions(+), 19 deletions(-) diff --git a/cli/canyonos/config.py b/cli/canyonos/config.py index 4a0327a7..abeefb19 100644 --- a/cli/canyonos/config.py +++ b/cli/canyonos/config.py @@ -4,12 +4,10 @@ View merely prints out the config, while change opens up a separate temp screen for easy changes. """ -import os - import yaml from rich.table import Table -from canyonos.constants import default_config_path, round_trip_yaml +from canyonos.constants import default_config_path, missing_config_message, round_trip_yaml from canyonos.theme import GREEN, WHITE from canyonos import ui from utils.tui import DELETE_ACTION, QUIT_ACTION, select_menu @@ -103,8 +101,9 @@ def _kv_table(title, data): def _require_config(config_path): """Resolved config path, or None after reporting that it's missing.""" config_path = config_path or default_config_path() - if not os.path.isfile(config_path): - ui.fail(f"Config file not found: {config_path}") + missing = missing_config_message(config_path) + if missing: + ui.fail(missing) return None return config_path diff --git a/cli/canyonos/constants.py b/cli/canyonos/constants.py index f7c9993f..c45b5058 100644 --- a/cli/canyonos/constants.py +++ b/cli/canyonos/constants.py @@ -27,9 +27,31 @@ def default_config_path(): - """Global controller config for the current directory, preferring the .car artifact layout.""" - car = os.path.join(".car", "config", "global_controller.yaml") - return car if os.path.isfile(car) else os.path.join("config", "global_controller.yaml") + """Global controller config for the current directory. + + The layout is picked off the `.car` directory, exactly as canyonos_core + picks it inside the container. Keying on the config file instead would let + the two disagree: with a .car directory but no config in it the host would + read config/global_controller.yaml while the container still insisted on + the .car one, and the deploy would fail naming a path that exists here. + + The path is returned whether or not anything is at it, so the commands that + only read a port out of it stay usable in a half-built project. + """ + prefix = ".car" if os.path.isdir(".car") else "" + return os.path.join(prefix, "config", "global_controller.yaml") + + +def missing_config_message(config_path): + """Why `config_path` is unusable, or None when the file is there. + + Shared so every command names the missing file the same way, while each + still reports through its own channel -- `canyonos test` folds the message + into its `--json` payload, the others print it and stop. + """ + if os.path.isfile(config_path): + return None + return f"Config file not found: {config_path}. Run `canyonos build` to generate it." def public_ip(timeout=0.3): diff --git a/cli/canyonos/deploy.py b/cli/canyonos/deploy.py index 2aed2ec4..37464db1 100644 --- a/cli/canyonos/deploy.py +++ b/cli/canyonos/deploy.py @@ -31,6 +31,7 @@ DEFAULT_QUERY_PARAM, WORKFLOW_ROUTE, default_config_path, + missing_config_message, port_in_use, public_ip, workflow_api_port, @@ -162,6 +163,15 @@ def run_deploy(config_path=None, serve=True, verbose=False, quiet=False, extra_e if config_path is None: raise RuntimeError("Config must be inside the project directory being synced.") + # The same path canyonos will resolve in the container. Checked before + # run_init(), so a project that has nothing to deploy is turned away + # without first tearing down whatever is running. + resolved_config = config_path or default_config_path() + missing = missing_config_message(resolved_config) + if missing: + ui.fail(missing) + return None + run_init(banner=banner, extra_env=extra_env) # Copy the current project into the container before building/deploying. @@ -171,14 +181,14 @@ def run_deploy(config_path=None, serve=True, verbose=False, quiet=False, extra_e state = load_state() # Read for display only -- canyonos resolves the path it actually deploys. - api_port = workflow_api_port(config_path or default_config_path()) + api_port = workflow_api_port(resolved_config) # Checked here, after run_init() has already torn down any previous deploy, # so a still-live prior run doesn't read as an unrelated conflict. if api_port is not None and port_in_use(api_port): raise RuntimeError( f"Port {api_port} is already in use, and the workflow needs it. Free it " - f"or change `api_port` in {config_path or default_config_path()}." + f"or change `api_port` in {resolved_config}." ) try: @@ -194,9 +204,7 @@ def run_deploy(config_path=None, serve=True, verbose=False, quiet=False, extra_e _start_dashboard() return state - _stream_logs_and_autoserve( - state, api_port, config_path or default_config_path(), serve=serve, verbose=verbose - ) + _stream_logs_and_autoserve(state, api_port, resolved_config, serve=serve, verbose=verbose) return state diff --git a/cli/canyonos/test.py b/cli/canyonos/test.py index 47e589f4..4f3d4485 100644 --- a/cli/canyonos/test.py +++ b/cli/canyonos/test.py @@ -13,7 +13,6 @@ """ import json -import os import subprocess import time import urllib.error @@ -26,6 +25,7 @@ from canyonos.constants import ( WORKFLOW_ROUTE, default_config_path, + missing_config_message, round_trip_yaml, workflow_api_port, workspace_relative, @@ -234,8 +234,12 @@ def _run_test(run, llm_stub=DEFAULT_LLM_STUB): config_path = workspace_relative(default_config_path()) if config_path is None: raise RuntimeError("Config must be inside the project directory being synced.") - if not os.path.isfile(config_path): - raise RuntimeError(f"No config at {config_path}. Run `canyonos build` first.") + # Raised rather than printed: `run_test` renders every failure itself, and + # under `--json` a `ui.fail` would be silenced and leave the payload saying + # the run passed. + missing = missing_config_message(config_path) + if missing: + raise RuntimeError(missing) api_port = workflow_api_port(config_path) if api_port is None: diff --git a/tests/test_canyonos_deploy.py b/tests/test_canyonos_deploy.py index ac62c88d..9ffa9bb1 100644 --- a/tests/test_canyonos_deploy.py +++ b/tests/test_canyonos_deploy.py @@ -1,15 +1,20 @@ +import os + import pytest from canyonos import deploy as deploy_cmd from canyonos.gc import GCError -CONFIG_PATH = "config/global_controller.yaml" +CONFIG_PATH = os.path.join("config", "global_controller.yaml") STATE = {"container_id": "abc", "port": 8000} @pytest.fixture -def deployable(monkeypatch): - """Every step run_deploy drives succeeds unless overridden.""" +def deployable(monkeypatch, tmp_path): + """Every step run_deploy drives succeeds unless overridden, in a project whose config exists.""" + (tmp_path / "config").mkdir() + (tmp_path / CONFIG_PATH).write_text("agents: []\n") + monkeypatch.chdir(tmp_path) monkeypatch.setattr(deploy_cmd, "workspace_relative", lambda p: p) monkeypatch.setattr(deploy_cmd, "run_init", lambda banner=True, extra_env=None: None) monkeypatch.setattr(deploy_cmd, "run_sync", lambda: True) @@ -26,6 +31,23 @@ def test_a_config_path_outside_the_project_raises(monkeypatch, deployable): deploy_cmd.run_deploy(CONFIG_PATH, quiet=True) +def test_a_half_built_car_project_fails_before_tearing_anything_down( + monkeypatch, tmp_path, deployable, capsys +): + """An interrupted `canyonos build` leaves a .car directory with no config in it. + The container would resolve the .car path, so the CLI has to name that one.""" + reached = [] + monkeypatch.setattr(deploy_cmd, "run_init", lambda **_kwargs: reached.append("init")) + (tmp_path / ".car").mkdir() + + assert deploy_cmd.run_deploy(quiet=True) is None + + printed = " ".join(capsys.readouterr().out.split()) + assert "Config file not found" in printed + assert os.path.join(".car", "config", "global_controller.yaml") in printed + assert reached == [] + + def test_a_sync_failure_raises(monkeypatch, deployable): monkeypatch.setattr(deploy_cmd, "run_sync", lambda: False) diff --git a/tests/test_canyonos_test.py b/tests/test_canyonos_test.py index 05b7a11c..821d9b5a 100644 --- a/tests/test_canyonos_test.py +++ b/tests/test_canyonos_test.py @@ -1,4 +1,5 @@ import json +import os import subprocess import pytest @@ -244,6 +245,20 @@ def test_a_flat_layout_project_deploys_fine_with_no_car_directory(monkeypatch, t ] +def test_a_half_built_car_project_is_reported_as_a_failure( + monkeypatch, tmp_path, deployable, capsys +): + """An interrupted `canyonos build` leaves a .car directory with no config in it.""" + (tmp_path / "half-built" / ".car").mkdir(parents=True) + monkeypatch.chdir(tmp_path / "half-built") + + assert test_cmd.run_test("hi", as_json=True) == 1 + payload = json.loads(capsys.readouterr().out) + + assert os.path.join(".car", "config", "global_controller.yaml") in payload["error"] + assert deployable["run_deploy"] == 0 + + # ------------------------------------------------------------------ # # Docker plumbing # # ------------------------------------------------------------------ # diff --git a/tests/test_cli.py b/tests/test_cli.py index f07a39db..18e16609 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -9,6 +9,12 @@ from unittest.mock import MagicMock, patch import yaml +from canyonos.constants import ( + DEFAULT_DASHBOARD_PORT, + dashboard_port, + default_config_path, + workflow_api_port, +) sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))) @@ -464,5 +470,67 @@ def test_clean_uses_car_when_present(self): self.assertTrue((project_dir / "stubs").exists()) +class DefaultConfigPathTests(unittest.TestCase): + """The host CLI must pick the layout the runtime picks, or a deploy fails inside + the container naming a config path that exists on the host.""" + + def _resolve_in(self, project_dir): + cwd = os.getcwd() + os.chdir(project_dir) + try: + return default_config_path() + finally: + os.chdir(cwd) + + def test_car_layout_is_used_when_the_car_config_exists(self): + with tempfile.TemporaryDirectory() as tmpdir: + project_dir = Path(tmpdir) + (project_dir / ".car" / "config").mkdir(parents=True) + (project_dir / ".car" / "config" / "global_controller.yaml").write_text("agents: []\n") + + self.assertEqual( + self._resolve_in(project_dir), + os.path.join(".car", "config", "global_controller.yaml"), + ) + + def test_root_layout_is_used_when_there_is_no_car_directory(self): + with tempfile.TemporaryDirectory() as tmpdir: + project_dir = Path(tmpdir) + (project_dir / "config").mkdir() + (project_dir / "config" / "global_controller.yaml").write_text("agents: []\n") + + self.assertEqual( + self._resolve_in(project_dir), + os.path.join("config", "global_controller.yaml"), + ) + + def test_car_layout_wins_over_a_root_config_when_the_car_config_is_missing(self): + with tempfile.TemporaryDirectory() as tmpdir: + project_dir = Path(tmpdir) + (project_dir / ".car").mkdir() + (project_dir / "config").mkdir() + (project_dir / "config" / "global_controller.yaml").write_text("agents: []\n") + + self.assertEqual(cli._artifact_prefix(str(project_dir)), ".car") + self.assertEqual( + self._resolve_in(project_dir), + os.path.join(".car", "config", "global_controller.yaml"), + ) + + def test_the_port_readers_tolerate_a_half_built_project(self): + """`serve` and `status` only read ports out of the config, so an interrupted + build must leave them working rather than stopping the user.""" + with tempfile.TemporaryDirectory() as tmpdir: + project_dir = Path(tmpdir) + (project_dir / ".car").mkdir() + cwd = os.getcwd() + os.chdir(project_dir) + try: + self.assertEqual(dashboard_port(default_config_path()), DEFAULT_DASHBOARD_PORT) + self.assertIsNone(workflow_api_port(default_config_path())) + finally: + os.chdir(cwd) + + if __name__ == "__main__": unittest.main()