diff --git a/.agents/skills/lib_py/delegation/mqtt_common.py b/.agents/skills/lib_py/delegation/mqtt_common.py index 3b58114..8cc30b4 100644 --- a/.agents/skills/lib_py/delegation/mqtt_common.py +++ b/.agents/skills/lib_py/delegation/mqtt_common.py @@ -122,8 +122,10 @@ def ensure_env_loaded(workspace_dir: Optional[str] = None) -> None: global _env_loaded if _env_loaded: return - _env_loaded = True _load_dotenv(workspace_dir) + # Only a load that succeeded is cached: after an EnvFileError the next + # call reads the file again (load_env_or_report returns, it does not exit). + _env_loaded = True def load_env_or_report() -> bool: diff --git a/MESSAGING.md b/MESSAGING.md index ada0492..11f5b34 100644 --- a/MESSAGING.md +++ b/MESSAGING.md @@ -366,7 +366,7 @@ The subscriber acts as the central execution watchdog: * **`.mam.env` Resolution Hierarchy (`_load_dotenv`)**: Configuration files are resolved with strict precedence rules: 1. **OS Environment Precedence**: Any variable already defined in `os.environ` is preserved and never overwritten by file-based configs. - 2. **Explicit Override (`MAM_ENV_FILE`)**: If `MAM_ENV_FILE` is set, only that specific file is parsed. If the specified file does not exist, it is skipped silently and ambient search is refused (no fallback to unintended parent configs): the process runs on its environment and the defaults. If it exists but cannot be read or decoded, that is an error (`EnvFileError`): the wrapper and the Python entry points print one `ERROR: : cannot read ()` line and exit 1. + 2. **Explicit Override (`MAM_ENV_FILE`)**: If `MAM_ENV_FILE` is set, only that specific file is parsed. If the specified file does not exist, it is skipped silently and ambient search is refused (no fallback to unintended parent configs): the process runs on its environment and the defaults. If it exists but cannot be read or decoded, that is an error (`EnvFileError`): the Python entry points print one `ERROR: : cannot read ()` line and exit 1; the wrapper prints that line followed by `ERROR: cannot load (see the line above)` and exits 1. 3. **Workspace Root Auto-Discovery**: If `MAM_ENV_FILE` is not set, the resolver searches candidate paths in order: `MAM_REAL_ROOT`, `WORKSPACE_ROOT`, upward directory walk searching for `.agents` or `.git` boundary markers, and `os.getcwd()`. 4. **Public Broker Security Alert (B-17)**: If the final resolved host falls back to the public sandbox `broker.hivemq.com`, a prominent security warning is emitted. * **Broker Config Resolution (`broker_config_from_job`)**: diff --git a/tests/test_issue26_envfile.py b/tests/test_issue26_envfile.py index 4017832..b37a7ce 100644 --- a/tests/test_issue26_envfile.py +++ b/tests/test_issue26_envfile.py @@ -449,3 +449,40 @@ def test_a7_a_missing_explicit_file_loads_nothing_and_says_nothing(tmp_path): assert _python_values(env, ["PROBE_AMBIENT"]) == {"PROBE_AMBIENT": ""} doc = next(l for l in (REPO_ROOT / "MESSAGING.md").read_text().splitlines() if "Explicit Override (`MAM_ENV_FILE`)" in l) assert "does not exist, it is skipped silently" in doc and "an error is logged" not in doc, doc + + +_RETRY_PROBE = r""" +import json, os, sys +from pathlib import Path +from lib_py.delegation import mqtt_common, registry +env_file, jobs = Path(sys.argv[1]), sys.argv[2] +argv = ["--registry-dir", jobs, "register", "--prompt", "retry probe"] +out = {"rc": [registry.main(argv), registry.main(argv)]} +out["registered"] = sorted(p.name for p in Path(jobs).glob("*.json")) if Path(jobs).exists() else [] +env_file.write_text("PROBE_CONFIG=from-file\n") +out["recovered"] = [mqtt_common.load_env_or_report(), os.environ.get("PROBE_CONFIG")] +env_file.write_text("PROBE_CONFIG=changed\n") +out["cached"] = [mqtt_common.load_env_or_report(), os.environ.get("PROBE_CONFIG")] +print(json.dumps(out)) +""" + + +def test_a1_a_failed_load_is_not_cached_as_loaded(tmp_path): + """Review R1 (codex): load_env_or_report returns instead of exiting, so the + failure must not mark the env as loaded - in one process, a second main() + on the same unreadable file is rc 1 again and registers nothing, a repaired + file is read on the next call, and only a success is loaded once.""" + f = tmp_path / "broken.env" + f.write_bytes(b"PROBE_CONFIG=from-file\nBAD=\xff\n") + jobs = tmp_path / "jobs" + env = _env(tmp_path, f) + env.pop("PROBE_CONFIG", None) + res = subprocess.run([sys.executable, "-c", _RETRY_PROBE, str(f), str(jobs)], capture_output=True, text=True, + env=env, cwd=str(tmp_path), timeout=60) + assert res.returncode == 0, res.stderr[-800:] + got = json.loads(res.stdout.strip().splitlines()[-1]) + assert got["rc"] == [1, 1], (got, res.stderr[-800:]) + assert got["registered"] == [], got + assert res.stderr.count(f"ERROR: {f}: cannot read (UnicodeDecodeError)") == 2, res.stderr[-800:] + assert got["recovered"] == [True, "from-file"], got + assert got["cached"] == [True, "from-file"], "a successful load stays once per process"