feat(paths): remove SLOP_CONFIG env-var fallback; add --config CLI flag (FR2)
This commit is contained in:
@@ -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: <project_root>/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
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
+25
-2
@@ -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
|
||||
<project_root>/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 <project_root>/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:
|
||||
"""
|
||||
|
||||
+61
-17
@@ -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_<RUN_ID>/ 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,
|
||||
|
||||
@@ -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)
|
||||
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)"
|
||||
Reference in New Issue
Block a user