From 5edf913d22550e97fc3731e5e4403d54f99740cb Mon Sep 17 00:00:00 2001 From: Godopu Date: Fri, 2 Oct 2026 08:09:46 +0900 Subject: [PATCH] fix(followups): F2 - Python entry points report an unreadable env file in one line A .mam.env that exists but cannot be read or decoded stopped the Python entry points with an EnvFileError traceback, while the wrapper printed one `ERROR:` line and exited 1 (PR #43 review claude S1). The mains that load the env file - publish_event, registry, job_subscriber, cli, bench, heartbeat and remote - now call mqtt_common.load_env_or_report(), which prints `ERROR: : cannot read ()` and lets the main return 1. No value or file content is printed. ensure_env_loaded() still raises: no library function exits the process. Test: every entry point, for a 000 file and an undecodable one, ends with exactly that line and rc 1 and creates nothing (14 cases; all 14 fail at 0051fd6 with a traceback). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01HCGpJXhSZasJLU38ksDghe --- .agents/skills/lib_py/delegation/bench.py | 3 +- .agents/skills/lib_py/delegation/cli.py | 3 +- .agents/skills/lib_py/delegation/heartbeat.py | 3 +- .../lib_py/delegation/job_subscriber.py | 3 +- .../skills/lib_py/delegation/mqtt_common.py | 20 ++++++++- .../skills/lib_py/delegation/publish_event.py | 3 +- .agents/skills/lib_py/delegation/registry.py | 3 +- .agents/skills/lib_py/delegation/remote.py | 3 +- tests/test_issue26_envfile.py | 41 +++++++++++++++++++ 9 files changed, 73 insertions(+), 9 deletions(-) diff --git a/.agents/skills/lib_py/delegation/bench.py b/.agents/skills/lib_py/delegation/bench.py index 51db2df..14a3c0a 100644 --- a/.agents/skills/lib_py/delegation/bench.py +++ b/.agents/skills/lib_py/delegation/bench.py @@ -163,7 +163,8 @@ def run_benchmark(n=50, transport="fake", connect_latency_ms=50.0, def main(argv: Optional[List[str]] = None) -> int: - mqtt_common.ensure_env_loaded() + if not mqtt_common.load_env_or_report(): + return 1 parser = argparse.ArgumentParser(prog="lib_py.delegation.bench") parser.add_argument("--n", type=int, default=50) parser.add_argument("--transport", default="fake", choices=["fake", "broker"]) diff --git a/.agents/skills/lib_py/delegation/cli.py b/.agents/skills/lib_py/delegation/cli.py index 27f860b..9c0fd5a 100644 --- a/.agents/skills/lib_py/delegation/cli.py +++ b/.agents/skills/lib_py/delegation/cli.py @@ -57,7 +57,8 @@ def _build_parser() -> argparse.ArgumentParser: def main(argv: Optional[List[str]] = None) -> int: - mqtt_common.ensure_env_loaded() + if not mqtt_common.load_env_or_report(): + return 1 args = _build_parser().parse_args(argv) if args.command == "submit": diff --git a/.agents/skills/lib_py/delegation/heartbeat.py b/.agents/skills/lib_py/delegation/heartbeat.py index 997ea1f..e4ee4ab 100644 --- a/.agents/skills/lib_py/delegation/heartbeat.py +++ b/.agents/skills/lib_py/delegation/heartbeat.py @@ -493,7 +493,8 @@ def main(argv=None) -> int: help="stop once this process (the agent) is gone") parser.add_argument("-v", "--verbose", action="store_true") args = parser.parse_args(argv) - mqtt_common.ensure_env_loaded() + if not mqtt_common.load_env_or_report(): + return 1 mqtt_common.setup_logging(logging.DEBUG if args.verbose else logging.INFO) return run(args.job, args.registry_dir, args.interval, args.expect_dispatch_token, args.attempt_id, args.max_lifetime, args.watch_pid) diff --git a/.agents/skills/lib_py/delegation/job_subscriber.py b/.agents/skills/lib_py/delegation/job_subscriber.py index ef8cb83..a718fc1 100755 --- a/.agents/skills/lib_py/delegation/job_subscriber.py +++ b/.agents/skills/lib_py/delegation/job_subscriber.py @@ -587,7 +587,8 @@ def main(argv=None) -> int: parser.add_argument("-v", "--verbose", action="store_true") args = parser.parse_args(argv) - mqtt_common.ensure_env_loaded() + if not mqtt_common.load_env_or_report(): + return 1 mqtt_common.setup_logging(logging.DEBUG if args.verbose else logging.WARNING) try: diff --git a/.agents/skills/lib_py/delegation/mqtt_common.py b/.agents/skills/lib_py/delegation/mqtt_common.py index 89445bf..3b58114 100644 --- a/.agents/skills/lib_py/delegation/mqtt_common.py +++ b/.agents/skills/lib_py/delegation/mqtt_common.py @@ -21,6 +21,7 @@ import hmac import json import logging import os +import sys import tempfile import time import uuid @@ -110,9 +111,10 @@ def ensure_env_loaded(workspace_dir: Optional[str] = None) -> None: """Load the workspace env file exactly once per process (D6, issue #5 phase 1). This used to run at import time. Importing a library must not mutate - ``os.environ``, so every *process entry point* calls this instead: + ``os.environ``, so every *process entry point* calls this instead - ``publish_event.main``, ``registry.main``, ``job_subscriber.main``, - ``cli.main``, ``bench.main``, and reconcile.sh's embedded subscriber. + ``cli.main``, ``bench.main``, ``heartbeat.main`` and ``remote.main`` + through :func:`load_env_or_report`, and reconcile.sh's embedded subscriber. Production semantics are unchanged because every production path passes through one of those, and OS env still wins over the file (see :func:`_parse_env_file`). @@ -123,6 +125,20 @@ def ensure_env_loaded(workspace_dir: Optional[str] = None) -> None: _env_loaded = True _load_dotenv(workspace_dir) + +def load_env_or_report() -> bool: + """ensure_env_loaded() for a process entry point: an env file that exists + but cannot be read ends the process the way the wrapper does - one + `ERROR: : cannot read ()` line, no traceback, no + value - and the caller returns 1. False after printing; library code keeps + calling ensure_env_loaded() and never exits.""" + try: + ensure_env_loaded() + except envfile.EnvFileError as e: + print(f"ERROR: {e}", file=sys.stderr) + return False + return True + # -------------------------------------------------------------------------- # Constants # -------------------------------------------------------------------------- diff --git a/.agents/skills/lib_py/delegation/publish_event.py b/.agents/skills/lib_py/delegation/publish_event.py index 4b00c04..4dc9365 100755 --- a/.agents/skills/lib_py/delegation/publish_event.py +++ b/.agents/skills/lib_py/delegation/publish_event.py @@ -334,7 +334,8 @@ def main(argv=None, _beat: bool = False) -> int: logger.error("--heartbeat needs a positive period and a non-terminal event") return 1 - mqtt_common.ensure_env_loaded() + if not mqtt_common.load_env_or_report(): + return 1 mqtt_common.setup_logging(logging.DEBUG if args.verbose else logging.WARNING) # --- parse optional data JSON (parameter error -> exit 1) --- diff --git a/.agents/skills/lib_py/delegation/registry.py b/.agents/skills/lib_py/delegation/registry.py index 0beff05..29fe970 100755 --- a/.agents/skills/lib_py/delegation/registry.py +++ b/.agents/skills/lib_py/delegation/registry.py @@ -533,7 +533,8 @@ def _prompt_from_args(parser: argparse.ArgumentParser, args: argparse.Namespace) def main(argv: Optional[List[str]] = None) -> int: - mqtt_common.ensure_env_loaded() + if not mqtt_common.load_env_or_report(): + return 1 mqtt_common.setup_logging(logging.INFO) parser = _build_parser() args = parser.parse_args(argv) diff --git a/.agents/skills/lib_py/delegation/remote.py b/.agents/skills/lib_py/delegation/remote.py index 0866689..31e63fc 100644 --- a/.agents/skills/lib_py/delegation/remote.py +++ b/.agents/skills/lib_py/delegation/remote.py @@ -460,7 +460,8 @@ def main(argv=None) -> int: p_r.add_argument("--python", default=None) parser.add_argument("-v", "--verbose", action="store_true") args = parser.parse_args(argv) - mqtt_common.ensure_env_loaded() + if not mqtt_common.load_env_or_report(): + return 1 mqtt_common.setup_logging(logging.DEBUG if args.verbose else logging.INFO) if args.command == "dispatch": try: diff --git a/tests/test_issue26_envfile.py b/tests/test_issue26_envfile.py index 190451f..f7b6c76 100644 --- a/tests/test_issue26_envfile.py +++ b/tests/test_issue26_envfile.py @@ -180,6 +180,47 @@ def test_a1_a_file_that_cannot_be_read_stops_both_loaders(tmp_path, kind): f.chmod(0o600) +# Follow-up F2 (PR #43 claude S1): every main that loads the env file, with +# arguments its parser accepts when it parses before loading. +ENTRY_POINTS = { + "publish_event": ["--job", "abcd1234", "--event", "progress", "--registry-dir", "reg"], + "registry": [], + "job_subscriber": ["--job", "abcd1234", "--registry-dir", "reg"], + "cli": [], + "bench": [], + "heartbeat": ["--job", "abcd1234", "--interval", "30", "--registry-dir", "reg"], + "remote": ["run", "--session", "s", "--registry-dir", "reg"], +} + + +@pytest.mark.parametrize("kind", ["permission", "utf8"]) +@pytest.mark.parametrize("module", sorted(ENTRY_POINTS)) +def test_a1_python_entry_points_report_an_unreadable_file_in_one_line(tmp_path, module, kind): + """The wrapper's shape at every Python entry point: one `ERROR:` line + naming the path and the exception type, rc 1, no traceback, no value.""" + f = tmp_path / "broken.env" + if kind == "permission": + if os.geteuid() == 0: + pytest.skip("root reads a 000 file") + f.write_text("PROBE_A=one\n") + f.chmod(0) + cause = "PermissionError" + else: + f.write_bytes(b"PROBE_A=one\nPROBE_B=\xff\xfe\n") + cause = "UnicodeDecodeError" + try: + # main() itself: `-m` adds runpy's warning (the package __init__ imports these modules) + code = f"import sys; from lib_py.delegation import {module} as m; sys.exit(m.main(sys.argv[1:]))" + res = subprocess.run([sys.executable, "-c", code, *ENTRY_POINTS[module]], + capture_output=True, text=True, env=_env(tmp_path, f), cwd=str(tmp_path), timeout=60) + finally: + f.chmod(0o600) + assert res.returncode == 1, (res.returncode, res.stderr[-500:]) + assert res.stderr.strip().splitlines() == [f"ERROR: {f}: cannot read ({cause})"], res.stderr[-500:] + assert "one" not in res.stdout + res.stderr + assert not (tmp_path / "reg").exists(), "nothing ran past the env file" + + def test_a1_a_nul_line_is_skipped_by_both_loaders(tmp_path): """Review R4: no environment can carry a NUL byte. Both loaders skip that line with a warning naming it (not its value) and keep the lines around it."""