diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e7d018b..fd28dd55 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -209,6 +209,8 @@ - No schema, contract, member, reason code or check id is added, and host-grants stays `0.6`. See the `STABILITY.md` migration note. (#822) +- Read a materialized Git tree's blobs through a few `git cat-file --batch` processes instead of one `git cat-file blob` per file. `diff --application --scope .` materializes the whole tree on both sides, so its run time grew with the file count: measured on 2026-09-25 with #877's root-scope reach, it took 412 s on TencentCloud/CubeSandbox (3,896 files) and 203 s on dlt-hub/dlt (2,042 files), and now takes 46 s and 45 s with identical rows; materializing one side of CubeSandbox went from about 180 s to under 5 s. One `cat-file --batch-check` first types and sizes every object, so a missing object or one that is not a blob refuses with a configuration error before any content is read; the content is then read in batches of at most 64 MiB, so a large tree is never held in memory at once. Every blob is still hashed against its tree entry's object ID before it is written, and the path, containment, link-order and final digest checks are unchanged. The link texts read to resolve in-tree link targets are batched the same way. No output, schema or contract changes. (#686 cost class) + ## 1.1.0 - 2026-09-22 A legibility and presentation-correctness release on the advisory channel. diff --git a/STABILITY.md b/STABILITY.md index 8a39402f..8d3f3d13 100644 --- a/STABILITY.md +++ b/STABILITY.md @@ -3824,7 +3824,9 @@ tests on every CI run, not by convention: - **`cli/verify/git.py`** — one shared `subprocess.run` boundary invokes local Git plumbing for exact base/head and working-tree orchestration, plus `pack-objects`, `index-pack`, and `fsck` to materialize an isolated, - object-ID-validated snapshot. One shared `subprocess.Popen` boundary + object-ID-validated snapshot, whose blobs it reads through one + `cat-file --batch-check` and byte-bounded `cat-file --batch` reads rather + than a process per blob. One shared `subprocess.Popen` boundary incrementally drains fixed Git diff, changed-path, attribute, inventory, and retained-manifest reads under hard output and wall-clock bounds. The bound is fail-closed: timeout, overflow, read/write failure, or an diff --git a/src/agents_shipgate/cli/verify/git.py b/src/agents_shipgate/cli/verify/git.py index d7748e70..5391e256 100644 --- a/src/agents_shipgate/cli/verify/git.py +++ b/src/agents_shipgate/cli/verify/git.py @@ -8,7 +8,7 @@ import tempfile import threading import unicodedata -from collections.abc import Callable, Iterable, Sequence +from collections.abc import Callable, Iterable, Iterator, Sequence from dataclasses import dataclass from pathlib import Path from typing import Any, Literal @@ -2333,10 +2333,12 @@ def archive_tree( ``scope`` narrows the materialized tree to the paths a reader will actually open, plus every symlink — a linked directory is what can conceal a scoped path from the reader, so the two sides must see the - same links. Without a scope every blob is written, which costs one - ``git cat-file`` per file: 1,168 subprocesses and 25 seconds on a 26 MB - repository whose host surface is four files (#686). A scoped archive - also packs the tree rather than the commit, so no history is walked. + same links. Without a scope every blob is written; that once cost one + ``git cat-file`` per file, 1,168 subprocesses and 25 seconds on a 26 MB + repository whose host surface is four files (#686), and blobs are now + read in a few bounded ``cat-file --batch`` processes instead. A scoped + archive also packs the tree rather than the commit, so no history is + walked. Scoping changes what is *materialized*, never what is *verified*: every blob written is still checked against its object ID, and the isolated @@ -2556,12 +2558,12 @@ def _materialize_isolated_tree( object_format = _run_git_dir(git_dir, ["rev-parse", "--show-object-format"]).stdout.strip() expected_digests: dict[str, str] = {} - for mode, oid, path_text in entries: + entry_blobs = _isolated_blobs(git_dir, [(oid, path_text) for _mode, oid, path_text in entries]) + for (mode, oid, path_text), blob in zip(entries, entry_blobs, strict=True): target = (root / path_text).resolve() if target == root or root not in target.parents: raise ConfigError(f"Git tree path escapes destination: {path_text}") target.parent.mkdir(parents=True, exist_ok=True) - blob = _run_git_dir(git_dir, ["cat-file", "blob", oid], text=False).stdout if _git_object_id("blob", blob, algorithm=object_format) != oid: raise ConfigError(f"Git blob failed object-ID validation: {path_text}") target.write_bytes(blob) @@ -2570,12 +2572,11 @@ def _materialize_isolated_tree( os.chmod(target, 0o755) link_texts: dict[str, str] = {} - for oid, path_text in links: + for (oid, path_text), blob in zip(links, _isolated_blobs(git_dir, links), strict=True): target = root / path_text if not _within(root, target.parent): raise ConfigError(f"Git tree path escapes destination: {path_text}") target.parent.mkdir(parents=True, exist_ok=True) - blob = _run_git_dir(git_dir, ["cat-file", "blob", oid], text=False).stdout if _git_object_id("blob", blob, algorithm=object_format) != oid: raise ConfigError(f"Git blob failed object-ID validation: {path_text}") # The link's own text is the blob. It may point anywhere, including @@ -2603,6 +2604,103 @@ def _materialize_isolated_tree( return gitlinks +#: The most blob bytes one `cat-file --batch` read of an isolated store holds. +#: A batch is buffered whole, so this bounds memory, not the tree: a larger +#: tree takes more batches, and a single larger blob is read alone, as it was +#: when every blob had a process of its own. +_MAX_ISOLATED_BATCH_BYTES = 64 * 1024 * 1024 + + +def _isolated_blobs(git_dir: Path, wanted: Sequence[tuple[str, str]]) -> Iterator[bytes]: + """Each ``(object ID, path)`` blob's bytes from ``git_dir``, in order (#686 cost class). + + One `cat-file` process per blob made a whole-tree archive cost a process + per file: 3,485 of them and 181 seconds for one side of a 3,485-blob + repository. Here one `cat-file --batch-check` types and sizes every object + first, so a missing object, or one that is not a blob, refuses before any + content is read; the content then comes from as few `cat-file --batch` + reads as :data:`_MAX_ISOLATED_BATCH_BYTES` allows. + + Only the framing is checked here: every record must name the requested + object, as a blob of the size the check reported. The caller still hashes + each blob against its tree entry's object ID before writing it. ``path`` + only names the entry in a refusal. + """ + + if not wanted: + return + paths: dict[str, str] = {} + for oid, path_text in wanted: + # The batch protocol reads one object name per line and resolves any + # revision syntax in it, so only a full object ID may be sent. + if not _GIT_OBJECT_RE.fullmatch(oid): + raise ConfigError(f"Git tree entry has a malformed object ID: {path_text}") + paths.setdefault(oid, path_text) + checked = _run_git_dir( + git_dir, + ["cat-file", "--batch-check"], + text=False, + input=b"".join(f"{oid}\n".encode("ascii") for oid in paths), + ).stdout + records = checked.split(b"\n") + if records.pop() != b"" or len(records) != len(paths): + raise ConfigError("Git object check returned a malformed response") + sizes: dict[str, int] = {} + for (oid, path_text), record in zip(paths.items(), records, strict=True): + fields = record.split(b" ") + if fields == [oid.encode("ascii"), b"missing"]: + raise ConfigError(f"Git object is missing from the verified object graph: {path_text}") + if len(fields) != 3 or fields[0] != oid.encode("ascii") or not fields[2].isdigit(): + raise ConfigError(f"Git object check returned a malformed record: {path_text}") + if fields[1] != b"blob": + raise ConfigError( + f"Git tree entry is not a blob: {path_text} " + f"(type {fields[1].decode('ascii', errors='replace')})." + ) + sizes[oid] = int(fields[2]) + + # Consecutive runs of the requested order, each within the byte bound; + # an object repeated inside a run is read once. + runs: list[list[str]] = [[]] + distinct: set[str] = set() + run_bytes = 0 + for oid, _path_text in wanted: + if oid not in distinct: + if distinct and run_bytes + sizes[oid] > _MAX_ISOLATED_BATCH_BYTES: + runs.append([]) + distinct, run_bytes = set(), 0 + distinct.add(oid) + run_bytes += sizes[oid] + runs[-1].append(oid) + for run in runs: + requested = list(dict.fromkeys(run)) + output = _run_git_dir( + git_dir, + ["cat-file", "--batch"], + text=False, + input=b"".join(f"{oid}\n".encode("ascii") for oid in requested), + ).stdout + contents: dict[str, bytes] = {} + offset = 0 + for oid in requested: + size = sizes[oid] + header_end = output.find(b"\n", offset) + start = header_end + 1 + end = start + size + if ( + header_end < 0 + or output[offset:header_end] != f"{oid} blob {size}".encode("ascii") + or output[end : end + 1] != b"\n" + ): + raise ConfigError(f"Git object read returned a malformed record: {paths[oid]}") + contents[oid] = output[start:end] + offset = end + 1 + if offset != len(output): + raise ConfigError("Git object read returned more than was requested") + for oid in run: + yield contents[oid] + + #: How many links one resolution may pass through before it counts as unresolved. _MAX_TREE_LINK_HOPS = 8 @@ -2629,10 +2727,8 @@ def _scope_through_boundary_links( object_format = _run_git_dir(git_dir, ["rev-parse", "--show-object-format"]).stdout.strip() link_texts: dict[str, str] = {} - for mode, _object_type, oid, path_text in listed: - if mode != "120000": - continue - blob = _run_git_dir(git_dir, ["cat-file", "blob", oid], text=False).stdout + links = [(oid, path_text) for mode, _type, oid, path_text in listed if mode == "120000"] + for (oid, path_text), blob in zip(links, _isolated_blobs(git_dir, links), strict=True): if _git_object_id("blob", blob, algorithm=object_format) != oid: raise ConfigError(f"Git blob failed object-ID validation: {path_text}") link_texts[path_text] = blob.decode("utf-8", errors="strict") @@ -2832,12 +2928,14 @@ def _run_git_dir( *, check: bool = True, text: bool = True, + input: bytes | None = None, ) -> subprocess.CompletedProcess: return _run_process( ["git", "--no-replace-objects", f"--git-dir={git_dir}", *args], capture_output=True, check=check, env=_git_object_environment(), + input=input, text=text, timeout=120, ) diff --git a/tests/test_adapter_static_only.py b/tests/test_adapter_static_only.py index 7f7d86ee..ba9c90de 100644 --- a/tests/test_adapter_static_only.py +++ b/tests/test_adapter_static_only.py @@ -257,7 +257,7 @@ class AllowedException: AllowedException( relative_path="cli/verify/git.py", surface="attr_call:subprocess.Popen", - line=2897, + line=2995, snippet=( "subprocess.Popen(cmd, env=env, stderr=subprocess.PIPE, " "stdin=subprocess.PIPE if input is not None else " @@ -280,7 +280,7 @@ class AllowedException: AllowedException( relative_path="cli/verify/git.py", surface="attr_call:subprocess.run", - line=3304, + line=3402, snippet=( "subprocess.run(cmd, capture_output=capture_output, check=check, " "env=env, input=input, stderr=stderr, stdin=stdin, stdout=stdout, " @@ -289,8 +289,9 @@ class AllowedException: rationale=( "Single _run_process boundary for verify: executes Git argv " "assembled inside Shipgate (local ref/diff reads plus " - "pack-objects, index-pack, and fsck for immutable snapshots). " - "No shell, user-code execution, or fetch." + "pack-objects, index-pack, fsck, and byte-bounded cat-file " + "--batch-check/--batch reads of the isolated store for immutable " + "snapshots). No shell, user-code execution, or fetch." ), ), # core/authorization_execution.py — guarded consumer for an externally diff --git a/tests/test_scoped_base_tree.py b/tests/test_scoped_base_tree.py index 5b26c281..ed9f5260 100644 --- a/tests/test_scoped_base_tree.py +++ b/tests/test_scoped_base_tree.py @@ -154,6 +154,157 @@ def test_a_shallow_clone_is_comparable_when_scoped(self, tmp_path: Path) -> None assert (out / ".claude" / "settings.json").is_file() +def _cat_file_calls(monkeypatch: pytest.MonkeyPatch) -> list[list[str]]: + """Record every `cat-file` argv the materializer runs.""" + + from agents_shipgate.cli.verify import git + + calls: list[list[str]] = [] + original = git._run_process + + def recording(cmd: list[str], **kwargs): + if "cat-file" in cmd: + calls.append(cmd[cmd.index("cat-file") :]) + return original(cmd, **kwargs) + + monkeypatch.setattr(git, "_run_process", recording) + return calls + + +class TestBlobsAreReadInBatches: + """The #686 cost class, once the whole tree is materialized again. + + One `cat-file blob` per file was 3,485 processes and 181 seconds for one + side of a 3,485-blob repository. The reads are batched; what each blob is + checked against before it is written is not. + """ + + def test_blob_reads_do_not_scale_with_the_file_count( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + root = _host_repo(tmp_path) + (root / ".claude" / "CLAUDE.md").write_text("# a\n", encoding="utf-8") + (root / "CLAUDE.md").symlink_to(".claude/CLAUDE.md") + _commit(root) + calls = _cat_file_calls(monkeypatch) + + archive_tree(root, "HEAD", tmp_path / "whole", scope=lambda _path: True) + + assert not [call for call in calls if "blob" in call] + # Blobs, then links for the scope, then links to recreate them: one + # type-and-size check and one content read each. + assert sorted(call[1] for call in calls) == ["--batch"] * 3 + ["--batch-check"] * 3 + assert len(list((tmp_path / "whole" / "src").iterdir())) == 30 + assert (tmp_path / "whole" / "CLAUDE.md").readlink().as_posix() == ".claude/CLAUDE.md" + + def test_a_byte_bound_splits_reads_without_changing_the_tree( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Each batch is held in memory whole, so the bound is what keeps a + large tree from being held at once. Repeated content is still written + at every path that names it.""" + + from agents_shipgate.cli.verify import git + + root = _host_repo(tmp_path) + (root / "big.bin").write_bytes(bytes(range(256)) * 4) + for index in range(3): + (root / f"copy{index}.txt").write_text("same\n", encoding="utf-8") + _commit(root) + archive_tree(root, "HEAD", tmp_path / "one-batch") + calls = _cat_file_calls(monkeypatch) + monkeypatch.setattr(git, "_MAX_ISOLATED_BATCH_BYTES", 16) + + archive_tree(root, "HEAD", tmp_path / "many-batches") + + assert sum(call[1] == "--batch" for call in calls) > 1 + + def files(where: Path) -> dict[str, bytes]: + return { + path.relative_to(where).as_posix(): path.read_bytes() + for path in where.rglob("*") + if path.is_file() + } + + assert files(tmp_path / "many-batches") == files(tmp_path / "one-batch") + assert (tmp_path / "many-batches" / "big.bin").read_bytes() == bytes(range(256)) * 4 + + def test_a_blob_that_does_not_hash_to_its_entry_is_not_written( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + from agents_shipgate.cli.verify import git + + root = _host_repo(tmp_path) + _commit(root) + original = git._isolated_blobs + + def tampered(git_dir: Path, wanted): + for (_oid, path_text), blob in zip(wanted, original(git_dir, wanted), strict=True): + yield b"tampered\n" if path_text == "README.md" else blob + + monkeypatch.setattr(git, "_isolated_blobs", tampered) + out = tmp_path / "base" + + with pytest.raises(ConfigError, match="object-ID validation: README.md"): + archive_tree(root, "HEAD", out) + + assert not (out / "README.md").exists() + + def test_a_missing_object_is_refused(self, tmp_path: Path) -> None: + from agents_shipgate.cli.verify.git import _isolated_blobs + + root = _host_repo(tmp_path) + _commit(root) + + with pytest.raises(ConfigError, match="missing .*: gone.txt"): + list(_isolated_blobs(root / ".git", [("0" * 40, "gone.txt")])) + + def test_an_object_that_is_not_a_blob_is_refused(self, tmp_path: Path) -> None: + from agents_shipgate.cli.verify.git import _isolated_blobs + + root = _host_repo(tmp_path) + _commit(root) + tree = _git(root, "rev-parse", "HEAD^{tree}") + + with pytest.raises(ConfigError, match="not a blob: src .*type tree"): + list(_isolated_blobs(root / ".git", [(tree, "src")])) + + @pytest.mark.parametrize("name", ["HEAD:README.md", "HEAD", "0" * 39, f"{'0' * 40}\n{'0' * 40}"]) + def test_only_a_full_object_id_reaches_the_batch(self, tmp_path: Path, name: str) -> None: + """The batch resolves any revision expression it is given, so a name + that is not a full object ID would read something the tree never + named.""" + + from agents_shipgate.cli.verify.git import _isolated_blobs + + root = _host_repo(tmp_path) + _commit(root) + + with pytest.raises(ConfigError, match="malformed object ID: README.md"): + list(_isolated_blobs(root / ".git", [(name, "README.md")])) + + def test_a_sha256_repository_is_read_the_same_way(self, tmp_path: Path) -> None: + root = tmp_path / "sha256" + initialized = subprocess.run( + ["git", "init", "--object-format=sha256", "-q", "-b", "main", str(root)], + capture_output=True, + check=False, + text=True, + ) + if initialized.returncode != 0: + pytest.skip(f"git cannot create a SHA-256 repository: {initialized.stderr.strip()}") + _git(root, "config", "user.email", "t@example.invalid") + _git(root, "config", "user.name", "T") + (root / "a.txt").write_text("a\n", encoding="utf-8") + (root / "b.txt").write_text("b\n", encoding="utf-8") + _commit(root) + + archive_tree(root, "HEAD", tmp_path / "out") + + assert (tmp_path / "out" / "a.txt").read_text(encoding="utf-8") == "a\n" + assert (tmp_path / "out" / "b.txt").read_text(encoding="utf-8") == "b\n" + + class TestSymlinks: def test_a_symlink_outside_the_surface_does_not_refuse_the_tree( self, tmp_path: Path