From 02fef00470047db2ec065b75aceb7feab5dfa34e Mon Sep 17 00:00:00 2001 From: Ed_ Date: Fri, 19 Jun 2026 07:45:10 -0400 Subject: [PATCH] feat(paths): remove SLOP_CONFIG env-var fallback; add --config CLI flag (FR2) --- sloppy.py | 5 +++ src/models.py | 2 - src/paths.py | 27 ++++++++++++- tests/conftest.py | 78 +++++++++++++++++++++++++++++--------- tests/test_test_sandbox.py | 46 +++++++++++++++++++++- 5 files changed, 136 insertions(+), 22 deletions(-) diff --git a/sloppy.py b/sloppy.py index 7c85ab0e..ca3f62ff 100644 --- a/sloppy.py +++ b/sloppy.py @@ -33,6 +33,7 @@ parser.add_argument("--headless", action="store_true", help="Run in headless mod parser.add_argument("--web-host", default=None, help="Enable web mode and bind to this host (e.g., 0.0.0.0)") parser.add_argument("--web-port", type=int, default=8080, help="Web mode port (default: 8080)") parser.add_argument("--enable-test-hooks", action="store_true", help="Enable the HookServer on :8999 for external automation") +parser.add_argument("--config", default=None, help="Override config.toml path (replaces historical SLOP_CONFIG env var; default: /config.toml)") # Defer parse_args() so `import sloppy` (for _SLOPPY_COLD_START_TS) doesn't # require CLI args. parse_args() runs at the start of __main__ only. args: argparse.Namespace = argparse.Namespace() # type: ignore[assignment] @@ -40,6 +41,10 @@ args: argparse.Namespace = argparse.Namespace() # type: ignore[assignment] if __name__ == "__main__": args = parser.parse_args() + from pathlib import Path + from src.paths import set_config_override + if args.config: + set_config_override(Path(args.config).resolve()) if args.web_host is not None: with startup_profiler.phase("web_host_imports"): from imgui_bundle import hello_imgui diff --git a/src/models.py b/src/models.py index af71ee24..9e2f0e50 100644 --- a/src/models.py +++ b/src/models.py @@ -191,8 +191,6 @@ def _save_config_to_disk(config: dict[str, Any]) -> None: # only when the user actually saves config. import tomli_w config = _clean_nones(config) - sys.stderr.write(f"[DEBUG] Saving config. Theme: {config.get('theme')}\n") - sys.stderr.flush() with open(get_config_path(), "wb") as f: tomli_w.dump(config, f) diff --git a/src/paths.py b/src/paths.py index ad9c8a5d..416ca9ed 100644 --- a/src/paths.py +++ b/src/paths.py @@ -49,12 +49,35 @@ from typing import Optional, Any _RESOLVED: dict[str, Path] = {} +_CONFIG_OVERRIDE: Path | None = None + + +def set_config_override(path: Path | None) -> None: + """ + Set the active config.toml path. Pass None to use the default + /config.toml. The CLI --config flag is the ONLY + supported override mechanism; the historical SLOP_CONFIG env var + fallback has been removed (per test_sandbox_hardening_20260619 FR2). + [C: sloppy.py:main, tests/conftest.py] + """ + global _CONFIG_OVERRIDE + _CONFIG_OVERRIDE = path + _RESOLVED.clear() + + def get_config_path() -> Path: """ - [C: tests/test_paths.py:test_default_paths] + Returns the active config.toml. If a CLI override is set, returns it. + Otherwise returns the default /config.toml. + [C: src/app_controller.py:AppController.load_config, + src/app_controller.py:AppController.init_state, + src/models.py:_load_config_from_disk, + tests/test_test_sandbox.py] """ + if _CONFIG_OVERRIDE is not None: + return _CONFIG_OVERRIDE root_dir = Path(__file__).resolve().parent.parent - return Path(os.environ.get("SLOP_CONFIG", root_dir / "config.toml")) + return root_dir / "config.toml" def get_global_presets_path() -> Path: """ diff --git a/tests/conftest.py b/tests/conftest.py index 2772bbdc..834319a1 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -17,6 +17,25 @@ if project_root not in sys.path: _RUN_ID = datetime.datetime.now().strftime("%Y%m%d_%H%M%S") _RUN_WORKSPACE = Path(f"tests/artifacts/live_gui_workspace_{_RUN_ID}") +_ISOLATION_WORKSPACE = Path(f"tests/artifacts/_isolation_workspace_{_RUN_ID}") +_ISOLATION_WORKSPACE.mkdir(parents=True, exist_ok=True) + +def _parse_config_arg(argv: list[str]) -> Path | None: + for i in range(1, len(argv)): + arg = argv[i] + if arg == "--config" and i + 1 < len(argv): + return Path(argv[i + 1]).resolve() + if arg.startswith("--config="): + return Path(arg.split("=", 1)[1]).resolve() + return None + +_config_override_arg = _parse_config_arg(sys.argv) +if _config_override_arg is None: + _config_override_arg = _ISOLATION_WORKSPACE / "config_overrides.toml" + +from src import paths as _paths # noqa: E402 +_paths.set_config_override(_config_override_arg) + thirdparty_dir = os.path.join(os.path.dirname(__file__), "..", "thirdparty") if thirdparty_dir not in sys.path: sys.path.insert(0, thirdparty_dir) @@ -178,6 +197,17 @@ if not _warmup_app_controller.wait_for_warmup(timeout=60.0): import threading _pytest_finished_event: threading.Event = threading.Event() +def pytest_addoption(parser) -> None: + """ + Register the --config flag so pytest does not warn about an unknown + option. Parsing happens in module body BEFORE any src/ import (see + _parse_config_arg above). + [C: tests/conftest.py:_parse_config_arg] + """ + parser.addoption("--config", action="store", default=None, + help="Manual Slop: override config.toml path for tests") + + def pytest_configure(config: object) -> None: """ Pytest session-start hook. Installs the runtime sandbox audit hook @@ -335,28 +365,40 @@ def _enforce_test_sandbox() -> Generator[None, None, None]: yield @pytest.fixture(autouse=True) -def isolate_workspace(tmp_path_factory, monkeypatch) -> Generator[None, None, None]: +def isolate_workspace(monkeypatch) -> Generator[None, None, None]: """ Autouse fixture to isolate tests from the active user workspace. - Protects the real config.toml and manual_slop.toml from being overwritten. + Workspace lives under tests/artifacts/_isolation_workspace_/ per + conductor/code_styleguides/workspace_paths.md (replaces the historical + tmp_path_factory.mktemp pattern). Writes placeholder TOMLs so src/ code + that reads these files at startup does not crash. + [C: tests/conftest.py:_ISOLATION_WORKSPACE, src/paths.py:set_config_override] """ - test_workspace = tmp_path_factory.mktemp("isolated_workspace") - - config_path = test_workspace / "config.toml" - import tomli_w - with open(config_path, "wb") as f: - tomli_w.dump({ - 'ai': {'provider': 'gemini', 'model': 'gemini-2.5-flash-lite'}, - 'projects': {'paths': [], 'active': ''}, - 'gui': {'show_windows': {}} - }, f) + test_workspace = _ISOLATION_WORKSPACE + + config_path = test_workspace / "config_overrides.toml" + if not config_path.exists(): + import tomli_w + with open(config_path, "wb") as f: + tomli_w.dump({ + 'ai': {'provider': 'gemini', 'model': 'gemini-2.5-flash-lite'}, + 'projects': {'paths': [], 'active': ''}, + 'gui': {'show_windows': {}} + }, f) + + for name in ( + "presets.toml", "tool_presets.toml", "personas.toml", + "workspace_profiles.toml", "credentials.toml", "mcp_env.toml", + ): + (test_workspace / name).touch() - monkeypatch.setenv("SLOP_CONFIG", str(config_path)) monkeypatch.setenv("SLOP_GLOBAL_PRESETS", str(test_workspace / "presets.toml")) monkeypatch.setenv("SLOP_GLOBAL_TOOL_PRESETS", str(test_workspace / "tool_presets.toml")) monkeypatch.setenv("SLOP_GLOBAL_PERSONAS", str(test_workspace / "personas.toml")) monkeypatch.setenv("SLOP_GLOBAL_WORKSPACE_PROFILES", str(test_workspace / "workspace_profiles.toml")) - + monkeypatch.setenv("SLOP_CREDENTIALS", str(test_workspace / "credentials.toml")) + monkeypatch.setenv("SLOP_MCP_ENV", str(test_workspace / "mcp_env.toml")) + yield @pytest.fixture(autouse=True) @@ -691,8 +733,6 @@ def live_gui(request) -> Generator["_LiveGuiHandle", None, None]: # or just run from that CWD. env = os.environ.copy() env["PYTHONPATH"] = str(project_root.absolute()) - if config_file.exists(): - env["SLOP_CONFIG"] = str(config_file.absolute()) if cred_file.exists(): env["SLOP_CREDENTIALS"] = str(cred_file.absolute()) if mcp_file.exists(): @@ -700,8 +740,12 @@ def live_gui(request) -> Generator["_LiveGuiHandle", None, None]: env["SLOP_GLOBAL_PRESETS"] = str((temp_workspace / "presets.toml").absolute()) env["SLOP_GLOBAL_TOOL_PRESETS"] = str((temp_workspace / "tool_presets.toml").absolute()) + gui_args = ["uv", "run", "python", "-u", gui_script, "--enable-test-hooks"] + if config_file.exists(): + gui_args.append(f"--config={config_file.absolute()}") + _process = subprocess.Popen( - ["uv", "run", "python", "-u", gui_script, "--enable-test-hooks"], + gui_args, stdout=log_file, stderr=log_file, text=True, diff --git a/tests/test_test_sandbox.py b/tests/test_test_sandbox.py index fe8ea5cf..4073c68f 100644 --- a/tests/test_test_sandbox.py +++ b/tests/test_test_sandbox.py @@ -147,4 +147,48 @@ def test_sandbox_allows_pytest_cache_write() -> None: probe.write_text("ok", encoding="utf-8") assert probe.read_text(encoding="utf-8") == "ok" finally: - probe.unlink(missing_ok=True) \ No newline at end of file + probe.unlink(missing_ok=True) + + +def test_config_override_via_cli_flag(tmp_path) -> None: + """paths.set_config_override(path) makes get_config_path() return that path.""" + from src import paths + config_path = tmp_path / "my_config.toml" + config_path.write_text("[ai]\nprovider='gemini'\n", encoding="utf-8") + original = paths._CONFIG_OVERRIDE + try: + paths.set_config_override(config_path) + assert paths.get_config_path() == config_path + finally: + paths.set_config_override(original) + + +def test_paths_get_config_path_no_env_fallback(monkeypatch) -> None: + """Without an override AND without SLOP_CONFIG, get_config_path returns default.""" + monkeypatch.delenv("SLOP_CONFIG", raising=False) + from src import paths + original = paths._CONFIG_OVERRIDE + try: + paths.set_config_override(None) + expected = Path(__file__).resolve().parent.parent / "config.toml" + assert paths.get_config_path() == expected + finally: + paths.set_config_override(original) + + +def test_sloppy_py_parses_config_flag() -> None: + """sloppy.py has a --config argparse argument that calls set_config_override.""" + import ast + sloppy = Path(__file__).resolve().parent.parent / "sloppy.py" + tree = ast.parse(sloppy.read_text(encoding="utf-8")) + found_config_arg = False + found_set_override_call = False + for node in ast.walk(tree): + if isinstance(node, ast.Constant) and node.value == "--config": + found_config_arg = True + if isinstance(node, ast.Call): + func = node.func + if isinstance(func, ast.Name) and func.id == "set_config_override": + found_set_override_call = True + assert found_config_arg, "sloppy.py must have a --config argparse argument" + assert found_set_override_call, "sloppy.py must call paths.set_config_override(args.config)" \ No newline at end of file