Skip to content

Commit 8969eb7

Browse files
Jammy2211claude
authored andcommitted
fix(check_dataset_allowlist): import env_config in both invocation contexts
`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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F11sMzmaVWfU6NCz1PKVVb
1 parent ce51277 commit 8969eb7

2 files changed

Lines changed: 67 additions & 5 deletions

File tree

autohands/check_dataset_allowlist.py

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,29 @@ def tracked_dataset_files():
7474
UNRESOLVED = None
7575

7676

77+
def _env_config():
78+
"""Import the sibling ``env_config`` module in either invocation context.
79+
80+
This module is reached two ways and they put different things on the path:
81+
82+
- as a **CLI verb**, ``bin/autohands`` (``_python_in_autohands``) runs it as a
83+
script with ``autohands/`` ITSELF on ``PYTHONPATH``, so siblings are
84+
top-level modules -- the flat ``from env_config import ...`` idiom the other
85+
guards in this package use;
86+
- as a **library import** (pytest, or anything importing
87+
``autohands.check_dataset_allowlist``), the package's PARENT is on the path
88+
and the flat name does not resolve.
89+
90+
Supporting only the package-qualified form silently broke the CLI verb the
91+
moment it was registered. Supporting only the flat form breaks the tests.
92+
"""
93+
try:
94+
import env_config # CLI: autohands/ is on PYTHONPATH
95+
except ImportError:
96+
from autohands import env_config # library: imported as a package
97+
return env_config
98+
99+
77100
def _releasing_tokens():
78101
"""Tokens whose declaration unsets ``PYAUTO_SMALL_DATASETS``.
79102
@@ -82,16 +105,19 @@ def _releasing_tokens():
82105
protecting scripts without an edit here. Falls back to the known pair only if
83106
the import is unavailable (the guard must never hard-fail on an env_config
84107
refactor -- it would block a release).
108+
109+
That fallback is a genuine last resort, not a routine path: before
110+
:func:`_env_config` existed this swallowed the CLI's ImportError and quietly
111+
returned the hardcoded pair, so the verb never actually consulted the map --
112+
a silent degradation that still produced a green run.
85113
"""
86114
try:
87-
from autohands.env_config import ENV_DECLARATION_TOKENS
115+
tokens = _env_config().ENV_DECLARATION_TOKENS
88116
except Exception:
89117
return {"full_datasets", "real_output"}
90118

91119
return {
92-
tok
93-
for tok, vars_ in ENV_DECLARATION_TOKENS.items()
94-
if "PYAUTO_SMALL_DATASETS" in vars_
120+
tok for tok, vars_ in tokens.items() if "PYAUTO_SMALL_DATASETS" in vars_
95121
}
96122

97123

@@ -243,7 +269,7 @@ def check_capped_deletion(prefixes, tracked) -> int:
243269
``prefixes``/``tracked`` are leg 1's already-computed allowlist and tracked
244270
file list, so this adds no extra git calls beyond the Python file listing.
245271
"""
246-
from autohands.env_config import read_env_declaration
272+
read_env_declaration = _env_config().read_env_declaration
247273

248274
# The invariant is NOT "the path sits under an allowlist prefix" -- it is
249275
# "rmtree(path) would delete committed files". Those differ, and the prefix

tests/test_dataset_allowlist_capped_deletion.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,3 +219,39 @@ def test_unresolvable_call_site_is_reported_and_skipped(
219219
assert code == 0
220220
assert "skipped" in captured.out
221221
assert "script.py:3" in captured.out
222+
223+
224+
# --- dual invocation context ------------------------------------------------
225+
226+
227+
def test_env_config_resolves_when_only_the_package_dir_is_importable(monkeypatch):
228+
"""The CLI context: `bin/autohands` puts `autohands/` ITSELF on PYTHONPATH,
229+
so `autohands.env_config` does NOT resolve and the flat name does.
230+
231+
Supporting only the package-qualified form silently broke the CLI verb the
232+
moment it was registered — and `_releasing_tokens` swallowed the ImportError
233+
and returned its hardcoded fallback, so the breakage still looked green.
234+
"""
235+
import builtins
236+
237+
from autohands import check_dataset_allowlist as guard
238+
from autohands import env_config as real_env_config
239+
240+
real_import = builtins.__import__
241+
242+
def no_package(name, *args, **kwargs):
243+
if name == "autohands" or name.startswith("autohands."):
244+
raise ImportError("simulated CLI context: autohands/ is on the path")
245+
if name == "env_config":
246+
return real_env_config
247+
return real_import(name, *args, **kwargs)
248+
249+
monkeypatch.setattr(builtins, "__import__", no_package)
250+
251+
assert guard._env_config() is real_env_config
252+
# Derived from the map, NOT the hardcoded fallback.
253+
assert guard._releasing_tokens() == {
254+
tok
255+
for tok, vars_ in real_env_config.ENV_DECLARATION_TOKENS.items()
256+
if "PYAUTO_SMALL_DATASETS" in vars_
257+
}

0 commit comments

Comments
 (0)