From bdabbbe4c41ee28e6bee2f8ff34456c9caf010aa Mon Sep 17 00:00:00 2001 From: Jack McCarthy Date: Mon, 5 Oct 2026 21:09:04 -0400 Subject: [PATCH 1/3] Reuse scaffold identity helpers in pin and gitignore checks Build the two clean-checkout git diff commands from shared parts, call hashlib constructors directly (requires-python is 3.10), rely on _committed_scaffold_entries for the single hash length, and drop the shadowed _cross_interface_identity plus _file_identity in favour of the existing _node_identity and _is_link helpers. --- autoform_cli/scaffold.py | 129 ++++++++++----------------------------- 1 file changed, 32 insertions(+), 97 deletions(-) diff --git a/autoform_cli/scaffold.py b/autoform_cli/scaffold.py index 564460ea..81a536a8 100644 --- a/autoform_cli/scaffold.py +++ b/autoform_cli/scaffold.py @@ -202,50 +202,19 @@ def _normalize_autoform_source(source: str, *, allow_github_scp: bool = False) - def _git_checkout_clean(root: Path) -> bool: """Whether *root* has no tracked or staged changes from ``HEAD``.""" - commands = ( - [ - "git", - "--no-optional-locks", - "--no-replace-objects", - "-c", - "core.fsmonitor=false", - "-C", - str(root), - "diff", - "--quiet", - "--no-ext-diff", - "--no-textconv", - "HEAD", - "--", - ], - [ - "git", - "--no-optional-locks", - "--no-replace-objects", - "-c", - "core.fsmonitor=false", - "-C", - str(root), - "diff", - "--cached", - "--quiet", - "--no-ext-diff", - "--no-textconv", - "HEAD", - "--", - ], - ) + git = ["git", "--no-optional-locks", "--no-replace-objects", "-c", "core.fsmonitor=false"] + options = ["--quiet", "--no-ext-diff", "--no-textconv", "HEAD", "--"] try: return all( subprocess.run( - command, + [*git, "-C", str(root), "diff", *cached, *options], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, timeout=10, check=False, ).returncode == 0 - for command in commands + for cached in ((), ("--cached",)) ) except (OSError, subprocess.SubprocessError): return False @@ -386,17 +355,10 @@ def _committed_scaffold_entries( def _git_blob_id(content: bytes, length: int) -> str | None: - algorithm = {40: "sha1", 64: "sha256"}.get(length) + algorithm = {40: hashlib.sha1, 64: hashlib.sha256}.get(length) if algorithm is None: return None - try: - digest = hashlib.new(algorithm, usedforsecurity=False) - except (TypeError, ValueError): - try: - digest = hashlib.new(algorithm) - except ValueError: - return None - digest.update(f"blob {len(content)}\0".encode("ascii")) + digest = algorithm(f"blob {len(content)}\0".encode("ascii"), usedforsecurity=False) digest.update(content) return digest.hexdigest() @@ -424,27 +386,23 @@ def _matches_committed_scaffold( ) -> bool: """Whether retained bytes and output-affecting modes equal the Git tree.""" - lengths = {len(object_name) for _mode, object_name in committed.values()} - if len(lengths) != 1: - return False - object_length = lengths.pop() - actual: dict[str, tuple[str, str]] = {} - for path, content, mode in ( - ("autoform_cli/scaffold.py", scaffold_source, scaffold_mode), - *( - (f"autoform_cli/templates/{relative}", content, mode) - for relative, content, mode in templates - ), - ): - git_mode = ( + # _committed_scaffold_entries guarantees one hash length and the renderer entry. + object_length = len(committed["autoform_cli/scaffold.py"][1]) + actual = { + path: ( committed.get(path, ("", ""))[0] if _WINDOWS_STAT_VIEWS - else {0o644: "100644", 0o755: "100755"}.get(_canonical_template_mode(mode)) + else ("100755" if mode & 0o111 else "100644"), + _git_blob_id(content, object_length), ) - object_name = _git_blob_id(content, object_length) - if git_mode is None or object_name is None or path in actual: - return False - actual[path] = (git_mode, object_name) + for path, content, mode in ( + ("autoform_cli/scaffold.py", scaffold_source, scaffold_mode), + *( + (f"autoform_cli/templates/{relative}", content, mode) + for relative, content, mode in templates + ), + ) + } return actual == committed @@ -609,12 +567,6 @@ def _node_identity(metadata: os.stat_result) -> tuple[int, ...]: ) -def _cross_interface_identity(identity: tuple[int, ...]) -> tuple[int, ...]: - """Normalize fields Windows exposes differently through stat and fstat.""" - - return identity[:-1] if _WINDOWS_STAT_VIEWS else identity - - def _is_link(metadata: os.stat_result) -> bool: attributes = getattr(metadata, "st_file_attributes", 0) reparse = getattr(stat, "FILE_ATTRIBUTE_REPARSE_POINT", 0x400) @@ -838,20 +790,7 @@ def _scaffold_plan( return tuple(files), tuple(skipped) -def _file_identity(metadata: os.stat_result) -> tuple[int, int, int, int, int, int]: - return ( - metadata.st_dev, - metadata.st_ino, - metadata.st_mode, - metadata.st_size, - metadata.st_mtime_ns, - metadata.st_ctime_ns, - ) - - -def _cross_interface_identity( - identity: tuple[int, int, int, int, int, int], -) -> tuple[int, ...]: +def _cross_interface_identity(identity: tuple[int, ...]) -> tuple[int, ...]: """Normalize Windows path-stat birth time versus fstat change time.""" return identity[:-1] if _WINDOWS_STAT_VIEWS else identity @@ -860,13 +799,11 @@ def _cross_interface_identity( def _read_gitignore_descriptor( descriptor: int, path: Path, -) -> tuple[bytes, tuple[int, int, int, int, int, int]]: +) -> tuple[bytes, tuple[int, ...]]: """Read a stable bounded snapshot from one retained regular file.""" opened = os.fstat(descriptor) - attributes = getattr(opened, "st_file_attributes", 0) - reparse = getattr(stat, "FILE_ATTRIBUTE_REPARSE_POINT", 0x400) - if not stat.S_ISREG(opened.st_mode) or attributes & reparse: + if not stat.S_ISREG(opened.st_mode) or _is_link(opened): raise ScaffoldError([f"refusing to merge non-regular .gitignore: {path}"]) if opened.st_nlink != 1: raise ScaffoldError([f"refusing to merge hard-linked .gitignore: {path}"]) @@ -889,14 +826,13 @@ def _read_gitignore_descriptor( ) after = os.fstat(descriptor) named = os.stat(path, follow_symlinks=False) - opened_identity = _file_identity(opened) - after_identity = _file_identity(after) - named_identity = _file_identity(named) - named_attributes = getattr(named, "st_file_attributes", 0) + opened_identity = _node_identity(opened) + after_identity = _node_identity(after) + named_identity = _node_identity(named) if ( after.st_nlink != 1 or named.st_nlink != 1 - or named_attributes & reparse + or _is_link(named) or after_identity != opened_identity or _cross_interface_identity(named_identity) != _cross_interface_identity(opened_identity) @@ -923,24 +859,23 @@ def _gitignore_suffix(existing: bytes, required: bytes) -> bytes | None: def _gitignore_identity_matches( descriptor: int, path: Path, - expected: tuple[int, int, int, int, int, int], + expected: tuple[int, ...], ) -> bool: try: opened = os.fstat(descriptor) named = os.stat(path, follow_symlinks=False) except OSError: return False - reparse = getattr(stat, "FILE_ATTRIBUTE_REPARSE_POINT", 0x400) return ( stat.S_ISREG(opened.st_mode) and stat.S_ISREG(named.st_mode) - and not getattr(opened, "st_file_attributes", 0) & reparse - and not getattr(named, "st_file_attributes", 0) & reparse + and not _is_link(opened) + and not _is_link(named) and opened.st_nlink == 1 and named.st_nlink == 1 - and _cross_interface_identity(_file_identity(opened)) + and _cross_interface_identity(_node_identity(opened)) == _cross_interface_identity(expected) - == _cross_interface_identity(_file_identity(named)) + == _cross_interface_identity(_node_identity(named)) ) From c1fc7025953648b94d7aff75d9190abb5f154b71 Mon Sep 17 00:00:00 2001 From: Jack McCarthy Date: Mon, 5 Oct 2026 19:38:44 -0400 Subject: [PATCH 2/3] Share plugin_pin test checkout setup Factor the repeated scaffold-copy, commit, and _here/_TEMPLATES patching into _git_output, _autoform_checkout, and _run_from. Each test keeps its own assertions. --- tests/test_scaffold.py | 152 ++++++++++++----------------------------- 1 file changed, 45 insertions(+), 107 deletions(-) diff --git a/tests/test_scaffold.py b/tests/test_scaffold.py index 2c927d82..f1ec0420 100644 --- a/tests/test_scaffold.py +++ b/tests/test_scaffold.py @@ -895,30 +895,33 @@ def _repository(path: Path, remote: str, *, advertise: bool = True) -> str: return head +def _git_output(path: Path, *args: str) -> str: + run = ["git", "-c", "user.email=t@test", "-c", "user.name=Test", *args] + return subprocess.run(run, cwd=path, capture_output=True, text=True, check=True).stdout.strip() + + +def _autoform_checkout(path: Path, remote: str, *, advertise: bool = True) -> str: + """Commit a copy of this scaffold and its templates at *path*; return HEAD.""" + (path / "autoform_cli").mkdir(parents=True) + shutil.copy2(Path(scaffold_module.__file__), path / "autoform_cli" / "scaffold.py") + shutil.copytree(scaffold_module._TEMPLATES, path / "autoform_cli" / "templates") + return _repository(path, remote, advertise=advertise) + + +def _run_from(monkeypatch: pytest.MonkeyPatch, root: Path) -> None: + monkeypatch.setattr(scaffold_module, "_here", lambda: root) + monkeypatch.setattr(scaffold_module, "_TEMPLATES", root / "autoform_cli" / "templates") + + def _fake_plugin_install( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> tuple[Path, Path, str]: """Lay out a plugin cache copy and the real checkout it was copied from.""" - from autoform_cli import scaffold as scaffold_module - checkout = tmp_path / "src" / "autoform-bot" - (checkout / "autoform_cli").mkdir(parents=True) - shutil.copy2(Path(scaffold_module.__file__), checkout / "autoform_cli" / "scaffold.py") - shutil.copytree(scaffold_module._TEMPLATES, checkout / "autoform_cli" / "templates") - head = _repository(checkout, "git@github.com:owner/autoform-bot.git") - + head = _autoform_checkout(checkout, "git@github.com:owner/autoform-bot.git") copied = tmp_path / ".claude/plugins/cache/autoform/autoform/0.5.0" - (copied / "autoform_cli").mkdir(parents=True) - shutil.copy2( - checkout / "autoform_cli" / "scaffold.py", - copied / "autoform_cli" / "scaffold.py", - ) - shutil.copytree( - checkout / "autoform_cli" / "templates", - copied / "autoform_cli" / "templates", - ) - monkeypatch.setattr(scaffold_module, "_here", lambda: copied) - monkeypatch.setattr(scaffold_module, "_TEMPLATES", copied / "autoform_cli" / "templates") + shutil.copytree(checkout / "autoform_cli", copied / "autoform_cli") + _run_from(monkeypatch, copied) registry = tmp_path / "known_marketplaces.json" registry.write_text( @@ -950,15 +953,8 @@ def test_an_installed_plugin_pins_from_the_marketplace_checkout( def test_plugin_pin_omits_a_local_commit_no_remote_tracking_ref_contains( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: - from autoform_cli import scaffold as scaffold_module - - checkout = tmp_path / "checkout" - (checkout / "autoform_cli").mkdir(parents=True) - shutil.copy2(Path(scaffold_module.__file__), checkout / "autoform_cli" / "scaffold.py") - shutil.copytree(scaffold_module._TEMPLATES, checkout / "autoform_cli" / "templates") - _repository(checkout, "https://example.test/fork.git", advertise=False) - monkeypatch.setattr(scaffold_module, "_here", lambda: checkout) - monkeypatch.setattr(scaffold_module, "_TEMPLATES", checkout / "autoform_cli" / "templates") + _autoform_checkout(tmp_path / "checkout", "https://example.test/fork.git", advertise=False) + _run_from(monkeypatch, tmp_path / "checkout") assert scaffold_module.plugin_pin() == ("", "") @@ -966,37 +962,21 @@ def test_plugin_pin_omits_a_local_commit_no_remote_tracking_ref_contains( def test_plugin_pin_ignores_git_replacement_objects( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: - from autoform_cli import scaffold as scaffold_module - checkout = tmp_path / "checkout" - (checkout / "autoform_cli").mkdir(parents=True) - shutil.copy2(Path(scaffold_module.__file__), checkout / "autoform_cli" / "scaffold.py") - shutil.copytree(scaffold_module._TEMPLATES, checkout / "autoform_cli" / "templates") - original = _repository(checkout, "https://example.test/autoform.git") - run = ["git", "-c", "user.email=t@test", "-c", "user.name=Test"] + original = _autoform_checkout(checkout, "https://example.test/autoform.git") renderer = checkout / "autoform_cli" / "scaffold.py" renderer.write_bytes(renderer.read_bytes() + b"\n# replacement behavior\n") - subprocess.run([*run, "add", "autoform_cli/scaffold.py"], cwd=checkout, check=True) - subprocess.run([*run, "commit", "-q", "-m", "replacement"], cwd=checkout, check=True) - replacement = subprocess.run( - [*run, "rev-parse", "HEAD"], - cwd=checkout, - capture_output=True, - text=True, - check=True, - ).stdout.strip() + _git_output(checkout, "commit", "-q", "-m", "replacement", "autoform_cli/scaffold.py") + replacement = _git_output(checkout, "rev-parse", "HEAD") replacement_bytes = renderer.read_bytes() - subprocess.run([*run, "checkout", "-q", "--detach", original], cwd=checkout, check=True) + _git_output(checkout, "checkout", "-q", "--detach", original) renderer.write_bytes(replacement_bytes) - subprocess.run([*run, "add", "autoform_cli/scaffold.py"], cwd=checkout, check=True) - subprocess.run([*run, "replace", original, replacement], cwd=checkout, check=True) - assert subprocess.run([*run, "diff", "--quiet", "HEAD", "--"], cwd=checkout).returncode == 0 - assert ( - subprocess.run([*run, "diff", "--cached", "--quiet", "HEAD", "--"], cwd=checkout).returncode - == 0 - ) - monkeypatch.setattr(scaffold_module, "_here", lambda: checkout) - monkeypatch.setattr(scaffold_module, "_TEMPLATES", checkout / "autoform_cli" / "templates") + _git_output(checkout, "add", "autoform_cli/scaffold.py") + _git_output(checkout, "replace", original, replacement) + for cached in ((), ("--cached",)): + diff = subprocess.run(["git", "diff", *cached, "--quiet", "HEAD", "--"], cwd=checkout) + assert diff.returncode == 0 + _run_from(monkeypatch, checkout) assert scaffold_module.plugin_pin() == ("", "") @@ -1004,45 +984,15 @@ def test_plugin_pin_ignores_git_replacement_objects( def test_plugin_pin_prefers_canonical_upstream_over_a_containing_fork_origin( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: - from autoform_cli import scaffold as scaffold_module - checkout = tmp_path / "checkout" - (checkout / "autoform_cli").mkdir(parents=True) - shutil.copy2(Path(scaffold_module.__file__), checkout / "autoform_cli" / "scaffold.py") - shutil.copytree(scaffold_module._TEMPLATES, checkout / "autoform_cli" / "templates") - _repository(checkout, "https://example.test/fork.git") - run = ["git", "-c", "user.email=t@test", "-c", "user.name=Test"] - subprocess.run( - [*run, "commit", "-q", "--allow-empty", "-m", "upstream head"], - cwd=checkout, - check=True, - ) - head = subprocess.run( - [*run, "rev-parse", "HEAD"], - cwd=checkout, - capture_output=True, - text=True, - check=True, - ).stdout.strip() - subprocess.run( - [*run, "update-ref", "refs/remotes/origin/main", head], cwd=checkout, check=True - ) - subprocess.run( - [ - *run, - "remote", - "add", - "upstream", - "https://github.com/facebookresearch/autoform-bot.git", - ], - cwd=checkout, - check=True, - ) - subprocess.run( - [*run, "update-ref", "refs/remotes/upstream/main", head], cwd=checkout, check=True - ) - monkeypatch.setattr(scaffold_module, "_here", lambda: checkout) - monkeypatch.setattr(scaffold_module, "_TEMPLATES", checkout / "autoform_cli" / "templates") + _autoform_checkout(checkout, "https://example.test/fork.git") + _git_output(checkout, "commit", "-q", "--allow-empty", "-m", "upstream head") + head = _git_output(checkout, "rev-parse", "HEAD") + _git_output(checkout, "update-ref", "refs/remotes/origin/main", head) + canonical = "https://github.com/facebookresearch/autoform-bot.git" + _git_output(checkout, "remote", "add", "upstream", canonical) + _git_output(checkout, "update-ref", "refs/remotes/upstream/main", head) + _run_from(monkeypatch, checkout) assert scaffold_module.plugin_pin() == (scaffold_module.DEFAULT_AUTOFORM_SOURCE, head) @@ -1089,15 +1039,9 @@ def test_plugin_pin_fails_closed_when_checkout_or_copy_bytes_do_not_match_head( def test_direct_checkout_pin_requires_a_matching_tracked_surface( change: str, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: - from autoform_cli import scaffold as scaffold_module - checkout = tmp_path / "checkout" - (checkout / "autoform_cli").mkdir(parents=True) - shutil.copy2(Path(scaffold_module.__file__), checkout / "autoform_cli" / "scaffold.py") - shutil.copytree(scaffold_module._TEMPLATES, checkout / "autoform_cli" / "templates") - head = _repository(checkout, "https://example.test/autoform.git") - monkeypatch.setattr(scaffold_module, "_here", lambda: checkout) - monkeypatch.setattr(scaffold_module, "_TEMPLATES", checkout / "autoform_cli" / "templates") + head = _autoform_checkout(checkout, "https://example.test/autoform.git") + _run_from(monkeypatch, checkout) assert scaffold_module.plugin_pin() == ("https://example.test/autoform.git", head) if change == "tracked": @@ -1122,16 +1066,10 @@ def test_direct_checkout_pin_requires_a_matching_tracked_surface( def test_direct_checkout_pin_ignores_unrelated_untracked_files( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: - from autoform_cli import scaffold as scaffold_module - checkout = tmp_path / "checkout" - (checkout / "autoform_cli").mkdir(parents=True) - shutil.copy2(Path(scaffold_module.__file__), checkout / "autoform_cli" / "scaffold.py") - shutil.copytree(scaffold_module._TEMPLATES, checkout / "autoform_cli" / "templates") - head = _repository(checkout, "https://example.test/autoform.git") + head = _autoform_checkout(checkout, "https://example.test/autoform.git") (checkout / "large-unrelated-output").write_bytes(b"x" * (1024 * 1024)) - monkeypatch.setattr(scaffold_module, "_here", lambda: checkout) - monkeypatch.setattr(scaffold_module, "_TEMPLATES", checkout / "autoform_cli" / "templates") + _run_from(monkeypatch, checkout) assert scaffold_module.plugin_pin() == ("https://example.test/autoform.git", head) From a38dbb494443dba687c145a2f384d459f7def0ad Mon Sep 17 00:00:00 2001 From: Jack McCarthy Date: Mon, 5 Oct 2026 22:23:32 -0400 Subject: [PATCH 3/3] Leave the scaffold pin unset when a Git digest is unavailable Calling hashlib.sha1/sha256 directly raised when the interpreter could not provide the digest, so plugin_pin failed instead of omitting the workflows as main does. Go back to hashlib.new and treat ValueError as an unidentified blob; only the TypeError fallback for interpreters without usedforsecurity was dead under requires-python 3.10. --- autoform_cli/scaffold.py | 8 ++++++-- tests/test_scaffold.py | 13 +++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/autoform_cli/scaffold.py b/autoform_cli/scaffold.py index 81a536a8..0bd85682 100644 --- a/autoform_cli/scaffold.py +++ b/autoform_cli/scaffold.py @@ -355,10 +355,14 @@ def _committed_scaffold_entries( def _git_blob_id(content: bytes, length: int) -> str | None: - algorithm = {40: hashlib.sha1, 64: hashlib.sha256}.get(length) + algorithm = {40: "sha1", 64: "sha256"}.get(length) if algorithm is None: return None - digest = algorithm(f"blob {len(content)}\0".encode("ascii"), usedforsecurity=False) + try: + digest = hashlib.new(algorithm, usedforsecurity=False) + except ValueError: + return None + digest.update(f"blob {len(content)}\0".encode("ascii")) digest.update(content) return digest.hexdigest() diff --git a/tests/test_scaffold.py b/tests/test_scaffold.py index f1ec0420..add40df5 100644 --- a/tests/test_scaffold.py +++ b/tests/test_scaffold.py @@ -950,6 +950,19 @@ def test_an_installed_plugin_pins_from_the_marketplace_checkout( ) +def test_plugin_pin_is_empty_when_git_digests_are_unavailable( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + def unavailable(*_args: object, **_kwargs: object) -> None: + raise ValueError("unsupported hash type") + + _fake_plugin_install(tmp_path, monkeypatch) + digests = SimpleNamespace(new=unavailable, sha1=unavailable, sha256=unavailable) + monkeypatch.setattr(scaffold_module, "hashlib", digests) + + assert scaffold_module.plugin_pin() == ("", "") + + def test_plugin_pin_omits_a_local_commit_no_remote_tracking_ref_contains( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: