fix(followups): F2 review R1 - a failed env load is not cached as loaded

ensure_env_loaded() set _env_loaded before _load_dotenv() ran. With F2,
load_env_or_report() turns the EnvFileError into a return value, so a second
main() in the same process skipped the unreadable file and ran unconfigured
(codex R1: registry.main twice -> rc 1 then rc 0 with a job registered; a
repaired file was never read). The flag is now set only after a successful
load: a failure is retried on the next call, a success still loads once.
envfile parses the whole file before applying, so a failed load sets nothing.

MESSAGING.md: the one-line ERROR is the Python entry points'; the wrapper
adds its own "ERROR: cannot load <path>" line (codex, minor).

Test: in one process, registry.main twice on an undecodable file is rc 1
both times, prints the ERROR line twice and registers nothing; the repaired
file is applied on the next call; a later change is not re-read. At 95eabc6
it fails with rc [1, 0], one job registered and the value unset.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HCGpJXhSZasJLU38ksDghe
This commit is contained in:
GodopuandClaude Opus 5.5 committed 2026-10-02 09:29:11 +09:00
1 parent 95eabc693f
commit 75f364e4b4
3 files changed
+41 -2

No files matched your search

@@ -122,8 +122,10 @@ def ensure_env_loaded(workspace_dir: Optional[str] = None) -> None:
global _env_loaded global _env_loaded
if _env_loaded: if _env_loaded:
return return
_env_loaded = True
_load_dotenv(workspace_dir) _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: def load_env_or_report() -> bool:
+1 -1
View File
@@ -366,7 +366,7 @@ The subscriber acts as the central execution watchdog:
* **`.mam.env` Resolution Hierarchy (`_load_dotenv`)**: * **`.mam.env` Resolution Hierarchy (`_load_dotenv`)**:
Configuration files are resolved with strict precedence rules: 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. 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: <path>: cannot read (<exception type>)` 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: <path>: cannot read (<exception type>)` line and exit 1; the wrapper prints that line followed by `ERROR: cannot load <path> (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()`. 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. 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`)**: * **Broker Config Resolution (`broker_config_from_job`)**:
+37
View File
@@ -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": "<unset>"} assert _python_values(env, ["PROBE_AMBIENT"]) == {"PROBE_AMBIENT": "<unset>"}
doc = next(l for l in (REPO_ROOT / "MESSAGING.md").read_text().splitlines() if "Explicit Override (`MAM_ENV_FILE`)" in l) 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 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"