From acc67ea022e5fc09d8251c54c4c3e5c3635605e2 Mon Sep 17 00:00:00 2001 From: Dbisina Date: Mon, 26 Oct 2026 20:06:32 +0100 Subject: [PATCH] fix(config): honor ARTEMIS_VIDEO_RECORDING_ENABLED=false when the alias is unset The override was resolved with `_parse_bool(A) or _parse_bool(B)`. With A explicitly false and B unset that evaluates to None, so the override was dropped and automatic tool detection kept recording enabled. Collect the values that are set and apply them; for conflicting aliases an explicit true still wins, as before. Fixes #202 --- artemis/config/agent.py | 15 ++++++++++++--- tests/unit/test_config_package.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/artemis/config/agent.py b/artemis/config/agent.py index 6fb00d398..ef7f39178 100644 --- a/artemis/config/agent.py +++ b/artemis/config/agent.py @@ -1005,9 +1005,18 @@ def _parse_bool(var_name: str) -> bool | None: if out_env is not None: data.setdefault("outputter", {})["enabled"] = out_env - vid_rec_env = _parse_bool("ARTEMIS_VIDEO_RECORDING_ENABLED") or _parse_bool( - "ARTEMIS_WITH_VIDEO_RECORDING_TOOLS" - ) + # `a or b` would turn an explicit False into None when the other alias + # is unset, silently dropping the override. Keep "any True wins" for + # conflicting aliases, but let a lone False through. + vid_rec_values = [ + v + for v in ( + _parse_bool("ARTEMIS_VIDEO_RECORDING_ENABLED"), + _parse_bool("ARTEMIS_WITH_VIDEO_RECORDING_TOOLS"), + ) + if v is not None + ] + vid_rec_env = any(vid_rec_values) if vid_rec_values else None if vid_rec_env is not None: data.setdefault("video_analyzer", {})["enabled"] = vid_rec_env data.setdefault("pro", {}).setdefault("video_analyzer", {})["enabled"] = vid_rec_env diff --git a/tests/unit/test_config_package.py b/tests/unit/test_config_package.py index 5b8c56ad8..28aad33b2 100644 --- a/tests/unit/test_config_package.py +++ b/tests/unit/test_config_package.py @@ -668,6 +668,34 @@ def test_agent_config_environment_variable_overrides(monkeypatch): assert cfg.pro.video_analyzer.enable_ledger is False +@pytest.mark.parametrize( + ("recording", "alias", "expected"), + [ + ("false", None, False), + (None, "false", False), + ("false", "false", False), + ("true", None, True), + (None, "true", True), + ("true", "false", True), + ("false", "true", True), + ], +) +def test_video_recording_environment_override(monkeypatch, recording, alias, expected): + """An explicit false must disable video even when the other alias is unset.""" + from artemis.config.agent import load_agent_config + + monkeypatch.delenv("ARTEMIS_VIDEO_RECORDING_ENABLED", raising=False) + monkeypatch.delenv("ARTEMIS_WITH_VIDEO_RECORDING_TOOLS", raising=False) + if recording is not None: + monkeypatch.setenv("ARTEMIS_VIDEO_RECORDING_ENABLED", recording) + if alias is not None: + monkeypatch.setenv("ARTEMIS_WITH_VIDEO_RECORDING_TOOLS", alias) + + cfg = load_agent_config() + assert cfg.video_analyzer.enabled is expected + assert cfg.pro.video_analyzer.enabled is expected + + def test_consolidated_workspace_and_admin_paths(): """Verify consolidated workspace and admin paths exported from artemis.config.""" from artemis.config import (