From 8969eb70bf2b87c2659a922ccbfa6f143d4c11fc Mon Sep 17 00:00:00 2001 From: Jammy2211 Date: Sat, 22 Aug 2026 20:08:12 -0400 Subject: [PATCH] fix(check_dataset_allowlist): import env_config in both invocation contexts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `autohands check_dataset_allowlist`, newly registered as a CLI verb in #254, crashed with ModuleNotFoundError when run from a workspace root. The two merges combined to expose it: #253 added `from autohands.env_config import ...` to this module, and #254 made it reachable from the dispatcher. `bin/autohands` (`_python_in_autohands`) runs these tools as scripts with `autohands/` ITSELF on PYTHONPATH, so siblings are top-level modules — the flat `from env_config import ...` idiom the other guards here already use. As a library import (pytest, or anything importing `autohands.check_dataset_allowlist`) the package's PARENT is on the path and the flat name does not resolve. Supporting only one form breaks the other, so `_env_config()` tries flat first and falls back to package-qualified. Also fixes a quieter instance of the same bug. `_releasing_tokens` wrapped its import in `except Exception` and returned the hardcoded `{full_datasets, real_output}` fallback, so under the CLI it swallowed the ImportError and never consulted ENV_DECLARATION_TOKENS at all — a silent degradation that still produced a green run, and would have stopped honouring any future releasing token without failing. The fallback is now a genuine last resort. Verified in both contexts: the CLI verb runs clean from a workspace root and still reports the originating defect (exact file, line, resolved path) when that workspace is reverted to its pre-fix state. Suite 375 passed; firewall gate OK. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F11sMzmaVWfU6NCz1PKVVb --- autohands/check_dataset_allowlist.py | 36 ++++++++++++++++--- .../test_dataset_allowlist_capped_deletion.py | 36 +++++++++++++++++++ 2 files changed, 67 insertions(+), 5 deletions(-) diff --git a/autohands/check_dataset_allowlist.py b/autohands/check_dataset_allowlist.py index a727923..2dba172 100644 --- a/autohands/check_dataset_allowlist.py +++ b/autohands/check_dataset_allowlist.py @@ -74,6 +74,29 @@ def tracked_dataset_files(): UNRESOLVED = None +def _env_config(): + """Import the sibling ``env_config`` module in either invocation context. + + This module is reached two ways and they put different things on the path: + + - as a **CLI verb**, ``bin/autohands`` (``_python_in_autohands``) runs it as a + script with ``autohands/`` ITSELF on ``PYTHONPATH``, so siblings are + top-level modules -- the flat ``from env_config import ...`` idiom the other + guards in this package use; + - as a **library import** (pytest, or anything importing + ``autohands.check_dataset_allowlist``), the package's PARENT is on the path + and the flat name does not resolve. + + Supporting only the package-qualified form silently broke the CLI verb the + moment it was registered. Supporting only the flat form breaks the tests. + """ + try: + import env_config # CLI: autohands/ is on PYTHONPATH + except ImportError: + from autohands import env_config # library: imported as a package + return env_config + + def _releasing_tokens(): """Tokens whose declaration unsets ``PYAUTO_SMALL_DATASETS``. @@ -82,16 +105,19 @@ def _releasing_tokens(): protecting scripts without an edit here. Falls back to the known pair only if the import is unavailable (the guard must never hard-fail on an env_config refactor -- it would block a release). + + That fallback is a genuine last resort, not a routine path: before + :func:`_env_config` existed this swallowed the CLI's ImportError and quietly + returned the hardcoded pair, so the verb never actually consulted the map -- + a silent degradation that still produced a green run. """ try: - from autohands.env_config import ENV_DECLARATION_TOKENS + tokens = _env_config().ENV_DECLARATION_TOKENS except Exception: return {"full_datasets", "real_output"} return { - tok - for tok, vars_ in ENV_DECLARATION_TOKENS.items() - if "PYAUTO_SMALL_DATASETS" in vars_ + tok for tok, vars_ in tokens.items() if "PYAUTO_SMALL_DATASETS" in vars_ } @@ -243,7 +269,7 @@ def check_capped_deletion(prefixes, tracked) -> int: ``prefixes``/``tracked`` are leg 1's already-computed allowlist and tracked file list, so this adds no extra git calls beyond the Python file listing. """ - from autohands.env_config import read_env_declaration + read_env_declaration = _env_config().read_env_declaration # The invariant is NOT "the path sits under an allowlist prefix" -- it is # "rmtree(path) would delete committed files". Those differ, and the prefix diff --git a/tests/test_dataset_allowlist_capped_deletion.py b/tests/test_dataset_allowlist_capped_deletion.py index 7c71bb4..921cd1e 100644 --- a/tests/test_dataset_allowlist_capped_deletion.py +++ b/tests/test_dataset_allowlist_capped_deletion.py @@ -219,3 +219,39 @@ def test_unresolvable_call_site_is_reported_and_skipped( assert code == 0 assert "skipped" in captured.out assert "script.py:3" in captured.out + + +# --- dual invocation context ------------------------------------------------ + + +def test_env_config_resolves_when_only_the_package_dir_is_importable(monkeypatch): + """The CLI context: `bin/autohands` puts `autohands/` ITSELF on PYTHONPATH, + so `autohands.env_config` does NOT resolve and the flat name does. + + Supporting only the package-qualified form silently broke the CLI verb the + moment it was registered — and `_releasing_tokens` swallowed the ImportError + and returned its hardcoded fallback, so the breakage still looked green. + """ + import builtins + + from autohands import check_dataset_allowlist as guard + from autohands import env_config as real_env_config + + real_import = builtins.__import__ + + def no_package(name, *args, **kwargs): + if name == "autohands" or name.startswith("autohands."): + raise ImportError("simulated CLI context: autohands/ is on the path") + if name == "env_config": + return real_env_config + return real_import(name, *args, **kwargs) + + monkeypatch.setattr(builtins, "__import__", no_package) + + assert guard._env_config() is real_env_config + # Derived from the map, NOT the hardcoded fallback. + assert guard._releasing_tokens() == { + tok + for tok, vars_ in real_env_config.ENV_DECLARATION_TOKENS.items() + if "PYAUTO_SMALL_DATASETS" in vars_ + }