diff --git a/packages/cli/canyonos/config.py b/packages/cli/canyonos/config.py index 089f8214..b641703d 100644 --- a/packages/cli/canyonos/config.py +++ b/packages/cli/canyonos/config.py @@ -4,12 +4,14 @@ 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 @@ -119,8 +121,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/packages/cli/canyonos/constants.py b/packages/cli/canyonos/constants.py index edee9698..0e75290e 100644 --- a/packages/cli/canyonos/constants.py +++ b/packages/cli/canyonos/constants.py @@ -29,11 +29,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/packages/cli/canyonos/deploy.py b/packages/cli/canyonos/deploy.py index de69a2ca..193cd902 100644 --- a/packages/cli/canyonos/deploy.py +++ b/packages/cli/canyonos/deploy.py @@ -32,6 +32,7 @@ DEFAULT_QUERY_PARAM, WORKFLOW_ROUTE, default_config_path, + missing_config_message, port_in_use, public_ip, workflow_api_port, @@ -217,8 +218,17 @@ def run_deploy( "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 + # `canyonos test` (quiet) deploys the config as-is, without the picker. - if not quiet and not configure_resources(config_path or default_config_path()): + if not quiet and not configure_resources(resolved_config): ui.say("Deploy cancelled.") return None @@ -234,14 +244,14 @@ def run_deploy( 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: @@ -260,7 +270,7 @@ def run_deploy( _stream_logs_and_autoserve( state, api_port, - config_path or default_config_path(), + resolved_config, serve=serve, verbose=verbose, on_ready=ready.append, diff --git a/packages/cli/canyonos/test.py b/packages/cli/canyonos/test.py index 00fd9b13..b5069a74 100644 --- a/packages/cli/canyonos/test.py +++ b/packages/cli/canyonos/test.py @@ -13,7 +13,6 @@ """ import json -import os import subprocess import time import urllib.error @@ -27,6 +26,7 @@ DEFAULT_QUERY_PARAM, WORKFLOW_ROUTE, default_config_path, + missing_config_message, round_trip_yaml, workflow_api_port, workflow_entrypoint, @@ -275,8 +275,12 @@ def _run_test(run, llm_stub=None, timeout=REQUEST_TIMEOUT): 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/packages/cli/tests/test_canyonos_deploy.py b/packages/cli/tests/test_canyonos_deploy.py index 3162affe..1c814ec2 100644 --- a/packages/cli/tests/test_canyonos_deploy.py +++ b/packages/cli/tests/test_canyonos_deploy.py @@ -8,8 +8,11 @@ @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 @@ -33,6 +36,27 @@ 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.""" + monkeypatch.setattr( + deploy_cmd, + "configure_resources", + lambda _path: pytest.fail("the picker should not run"), + ) + monkeypatch.setattr( + deploy_cmd, "run_init", lambda **_k: pytest.fail("run_init should not run") + ) + (tmp_path / ".car").mkdir() + + assert deploy_cmd.run_deploy() is None + + printed = " ".join(capsys.readouterr().out.split()) + assert "Config file not found: .car/config/global_controller.yaml" in printed + + def test_a_sync_failure_raises(monkeypatch, deployable): cleaned = [] monkeypatch.setattr(deploy_cmd, "run_sync", lambda: False) diff --git a/packages/cli/tests/test_canyonos_test.py b/packages/cli/tests/test_canyonos_test.py index 31542eaa..cd7408f2 100644 --- a/packages/cli/tests/test_canyonos_test.py +++ b/packages/cli/tests/test_canyonos_test.py @@ -282,6 +282,21 @@ def test_a_flat_layout_project_deploys_fine_with_no_car_directory( ] +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 payload["ok"] is False + assert ".car/config/global_controller.yaml" in payload["error"] + assert deployable["run_deploy"] == 0 + + # ------------------------------------------------------------------ # # Query body derived from the workflow signature # # ------------------------------------------------------------------ # diff --git a/packages/cli/tests/test_default_config_path.py b/packages/cli/tests/test_default_config_path.py new file mode 100644 index 00000000..0d25d5e5 --- /dev/null +++ b/packages/cli/tests/test_default_config_path.py @@ -0,0 +1,99 @@ +"""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.""" + +import pytest + +from canyonos import serve as serve_cmd +from canyonos import status as status_cmd +from canyonos.constants import ( + DEFAULT_DASHBOARD_PORT, + default_config_path, + missing_config_message, +) +from canyonos.dashboard_stack import ServeResult + +CAR_CONFIG = ".car/config/global_controller.yaml" +ROOT_CONFIG = "config/global_controller.yaml" + + +@pytest.fixture +def project(monkeypatch, tmp_path): + monkeypatch.chdir(tmp_path) + return tmp_path + + +@pytest.fixture +def half_built(project): + """What an interrupted `canyonos build` leaves: a .car directory, no config in it.""" + (project / ".car").mkdir() + return project + + +def test_the_car_layout_is_used_when_the_car_config_exists(project): + (project / ".car" / "config").mkdir(parents=True) + (project / CAR_CONFIG).write_text("agents: []\n") + + assert default_config_path() == CAR_CONFIG + + +def test_the_root_layout_is_used_when_there_is_no_car_directory(project): + (project / "config").mkdir() + (project / ROOT_CONFIG).write_text("agents: []\n") + + assert default_config_path() == ROOT_CONFIG + + +def test_a_car_directory_wins_over_a_root_config(half_built): + (half_built / "config").mkdir() + (half_built / ROOT_CONFIG).write_text("agents: []\n") + + assert default_config_path() == CAR_CONFIG + + +def test_the_missing_message_names_the_path_and_the_fix(half_built): + assert missing_config_message(CAR_CONFIG) == ( + f"Config file not found: {CAR_CONFIG}. Run `canyonos build` to generate it." + ) + + +def test_an_existing_config_has_no_missing_message(project): + (project / "config").mkdir() + (project / ROOT_CONFIG).write_text("agents: []\n") + + assert missing_config_message(ROOT_CONFIG) is None + + +def test_serve_starts_on_the_default_port_in_a_half_built_project( + monkeypatch, half_built +): + """`serve` only reads a port out of the config, so it must not need one.""" + seen = [] + + def run_dashboard(_report, preferred_port): + seen.append(preferred_port) + return ServeResult(ok=True, phase="ready", message="up") + + monkeypatch.setattr(serve_cmd, "run_dashboard", run_dashboard) + + assert serve_cmd.serve_dashboard().ok + assert seen == [DEFAULT_DASHBOARD_PORT] + + +def test_status_reports_a_running_deploy_in_a_half_built_project( + monkeypatch, half_built, capsys +): + """`status` only reads a port out of the config, so it must not need one.""" + seen = [] + monkeypatch.setattr(status_cmd, "require_state", lambda: {"port": 8000}) + monkeypatch.setattr(status_cmd, "deploy_status", lambda _port: {"running": True}) + monkeypatch.setattr( + status_cmd, + "workflow_targets", + lambda _gc_port, api_port: seen.append(api_port) or [], + ) + monkeypatch.setattr(status_cmd, "_existing_dashboard_port", lambda: None) + + status_cmd.run_status() + + assert seen == [None] + assert "Deploy is running." in capsys.readouterr().out