diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a15e58f..7d2f1571 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,12 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). Version - **New payload binaries are linked with debug information stripped.** `-s -w` joins `ARTIFACT_LDFLAGS` in `scripts/build-agent-handoff-session-start.sh` ([#228](https://github.com/L3DigitalNet/project-standards/issues/228) lever 1), taking the launcher from 3,906,258 to 2,646,142 bytes (−32.3%). `.gopclntab` is retained, so panic traces still carry function names and line numbers; DWARF inspection of the shipped file is recovered by rebuilding from the same script. Published payload bytes stay unstripped, so a size step between a retained version and its successor is expected. Consumers running a `pre-commit` `check-added-large-files` guard still need the documented exemption for the hook path — the launcher remains far above a typical `--maxkb=1024`. +- **The provider integrity guard no longer retains the bytes it only compares, and repeated package-repository builds in one process are memoized** ([#227](https://github.com/L3DigitalNet/project-standards/issues/227)). Every provider invocation captures a whole-repository snapshot before and after the call to prove the provider changed no declared live path; those two captures read each declared target in full, held every byte in memory, and hashed each one twice. They now stream: the precondition hash is seeded from the pre-read mode and fed chunk by chunk, no bytes are retained, and the content digest is computed only when a caller asked for content. `RepositorySnapshot.capture` gained `retain_content` for that, and `assert_current` uses it too. On this repository `project-standards reconcile --check` falls from 17.5 s to 13.7 s (median of three) and peak RSS from 185 MB to 117 MB. `build_package_repository` additionally serves a repeated build of an unchanged tree from an in-process memo keyed on the size and mtime of every file under `standards/` and `catalogs/` — a cache over parse results, not an integrity guard, which is why `stat` metadata suffices there while the control plane's snapshot still reads and hashes bytes. A second build of this repository costs 0.05 s instead of 2.75 s; the fingerprint itself costs 0.05 s, which a single-build command now pays once. +- **`provider_snapshot_chain()` lets a planning pass share one integrity snapshot between consecutive providers.** Inside that window the AFTER snapshot of one provider becomes the BEFORE snapshot of the next provider declaring the identical target set, so N invocations cost N+1 captures instead of 2N (measured on this repository: 201 captures to 102, and 12.5 s to 10.4 s in-process). Every AFTER capture is still a fresh full read, so a provider that changes a declared live path is still refused with `CP-PROVIDER-INTEGRITY`. The window is opt-in because it carries an obligation: the caller promises that nothing but the providers themselves touches a declared path between two invocations, so publication, recovery, or an operator edit between two plans in a long-lived process is never misattributed to the next provider. + +- **The provider integrity guard no longer retains the bytes it only compares, and repeated package-repository builds in one process are memoized** ([#227](https://github.com/L3DigitalNet/project-standards/issues/227)). Every provider invocation captures a whole-repository snapshot before and after the call to prove the provider changed no declared live path; those two captures read each declared target in full, held every byte in memory, and hashed each one twice. They now stream: the precondition hash is seeded from the pre-read mode and fed chunk by chunk, no bytes are retained, and the content digest is computed only when a caller asked for content. `RepositorySnapshot.capture` gained `retain_content` for that, and `assert_current` uses it too. Combined with the snapshot chain below, `project-standards reconcile --check` falls from 17.5 s to 9.5 s on this repository (median of three) and peak RSS from 185 MB to 117 MB. `build_package_repository` additionally serves a repeated build of an unchanged tree from an in-process memo keyed on the size and mtime of every file under `standards/` and `catalogs/` — a cache over parse results, not an integrity guard, which is why `stat` metadata suffices there while the control plane's snapshot still reads and hashes bytes. A second build of this repository costs 0.05 s instead of 2.75 s; the fingerprint itself costs 0.05 s, which a single-build command now pays once. +- **`plan_reconciliation` now shares one integrity snapshot between consecutive providers.** Inside the new `provider_snapshot_chain()` window the AFTER snapshot of one provider becomes the BEFORE snapshot of the next provider declaring the identical target set, so N invocations cost N+1 captures instead of 2N — 201 captures to 102 on this repository. Every AFTER capture is still a fresh full read, so a provider that changes a declared live path is still refused with `CP-PROVIDER-INTEGRITY`. The window is opt-in and planning is the only pass that enters it: it invokes every provider and writes nothing itself, so between two invocations only a provider could have touched a declared path. Publication and the executor's post-publication verification providers stay outside, because a window spanning a write would report the control plane's own bytes as the next provider's violation. + ### Fixed - **`Agent Handoff 1.17` stops an untrusted checkout from executing a command during session start.** The `session-start` launcher ran its Git reads with the full inherited process environment and no configuration isolation, so a repository-local or ancestor `core.fsmonitor` setting named a hook that Git ran — unconditionally, before the operator had seen anything — as soon as that checkout was opened as a session-start target ([#235](https://github.com/L3DigitalNet/project-standards/issues/235)). Every read now runs with an explicit minimal environment (`PATH` and `HOME` only, so no `GIT_DIR`, `GIT_WORK_TREE`, or `GIT_CONFIG_*` value from the harness can redirect it) and passes `-c core.fsmonitor=`, which outranks every configuration file, plus `--no-optional-locks` so the read cannot race a concurrent write. The injected session context is byte-identical to 1.16; reconcile replaces the installed hook because its digest moved. Catalog 5 promotes `agent-handoff@1.17` and retains 1.16. diff --git a/src/project_standards/control_plane/planner.py b/src/project_standards/control_plane/planner.py index 9bb93f8c..06b196fd 100644 --- a/src/project_standards/control_plane/planner.py +++ b/src/project_standards/control_plane/planner.py @@ -77,6 +77,7 @@ ProviderInvocation, ProviderResult, invoke_provider, + provider_snapshot_chain, resolve_referenced_inputs, ) from project_standards.control_plane.resolution import ( @@ -2894,6 +2895,19 @@ def _alias_analysis( def plan_reconciliation(request: PlannerRequest) -> ReconciliationPlan: """Build one deterministic, complete, and read-only reconciliation plan.""" + # Planning is the one pass that satisfies the snapshot chain's obligation: + # it invokes every render, validate, and transform provider in turn and + # writes nothing itself, so between two invocations only a provider could + # have touched a declared path — which is exactly what the chained BEFORE + # snapshot then proves it did not. Publication and the executor's + # post-publication verification providers stay outside the window; a window + # spanning a write would report the control plane's own bytes as a + # CP-PROVIDER-INTEGRITY violation by the next provider. + with provider_snapshot_chain(): + return _plan_reconciliation(request) + + +def _plan_reconciliation(request: PlannerRequest) -> ReconciliationPlan: original_request = request resolution = resolve_packages(request.resolution) payloads = _payload_map(request.payloads) diff --git a/src/project_standards/control_plane/providers.py b/src/project_standards/control_plane/providers.py index e15f5176..2cc7f93a 100644 --- a/src/project_standards/control_plane/providers.py +++ b/src/project_standards/control_plane/providers.py @@ -10,8 +10,9 @@ import stat import sys import tempfile -from collections.abc import Iterator, Mapping, Sequence -from contextlib import redirect_stderr, redirect_stdout +import threading +from collections.abc import Generator, Iterator, Mapping, Sequence +from contextlib import contextmanager, redirect_stderr, redirect_stdout from dataclasses import dataclass, replace from pathlib import Path from types import MappingProxyType @@ -44,7 +45,10 @@ safe_failure_detail, ) from project_standards.control_plane.schemas import MutationPlanSchema, ProviderInputSchema -from project_standards.control_plane.snapshot import RepositorySnapshot +from project_standards.control_plane.snapshot import ( + RepositorySnapshot, + canonical_targets, +) from project_standards.package_contract.paths import ( PackageVersion, SafeRelativePath, @@ -488,9 +492,83 @@ def _declared_snapshot_paths(snapshots: JsonObject) -> tuple[SafeRelativePath, . raise ControlPlaneError("provider snapshot declares an invalid repository path") from exc +@dataclass(frozen=True, slots=True) +class _ChainedSnapshot: + """The AFTER snapshot of the most recent provider, offered to the next one.""" + + root: Path + targets: tuple[SafeRelativePath, ...] + snapshot: RepositorySnapshot + + +# Chaining state is per thread: two threads planning against one repository see +# each other's writes, and a slot armed by one of them describes a tree the +# other never observed. +_chain_state = threading.local() + + +def _chain_slot() -> _ChainedSnapshot | None: + return cast("_ChainedSnapshot | None", getattr(_chain_state, "slot", None)) + + +@contextmanager +def provider_snapshot_chain() -> Generator[None]: + """Allow consecutive provider invocations to share one integrity snapshot. + + Inside this window, the AFTER snapshot the CP-PROVIDER-INTEGRITY guard takes + for one provider becomes the BEFORE snapshot of the next provider that + declares the identical target set, so N invocations cost N+1 captures rather + than 2N. Every declared path is still read in full once per invocation and + every AFTER capture is still a fresh read, so a provider that changes a + declared live path is caught exactly as before. + + The window carries one obligation, and it is the reason chaining is opt-in + rather than always on: the caller promises that nothing but the providers + themselves touches a declared path between two invocations. Publication, + recovery, a spec fix that rewrites the file it just linted, or an operator + editing the tree between two plans in a long-lived process would otherwise + be attributed to the next provider as a CP-PROVIDER-INTEGRITY violation. + Open the window around one planning pass — never around a pass that writes, + and never across a repository mutation the process performed itself. + + Nested windows are permitted and share the single slot; leaving any window + discards it, so a chained snapshot never outlives the pass that vouched + for it — including the outer pass, which resumes with no slot rather than + with the reading it held before the inner pass ran. + """ + previous_active = getattr(_chain_state, "active", False) + _chain_state.active = True + _chain_state.slot = None + try: + yield + finally: + _chain_state.active = previous_active + _chain_state.slot = None + + +def _arm_snapshot_chain(after: RepositorySnapshot) -> None: + if getattr(_chain_state, "active", False): + _chain_state.slot = _ChainedSnapshot(after.root, after.targets, after) + + +def _capture_declared_paths( + root: Path, + targets: tuple[SafeRelativePath, ...], +) -> RepositorySnapshot: + """Take this invocation's BEFORE snapshot, reusing a chained one when offered.""" + ordered = canonical_targets(targets) + slot = _chain_slot() + # Consumed at most once: the reference for the next invocation is whatever + # this invocation's own AFTER capture observes, never an older reading. + _chain_state.slot = None + if slot is not None and slot.root == root and slot.targets == ordered: + return slot.snapshot + return RepositorySnapshot.capture(root, ordered, retain_content=False) + + def _assert_declared_paths_unchanged(before: RepositorySnapshot) -> None: try: - after = RepositorySnapshot.capture(before.root, before.targets) + after = RepositorySnapshot.capture(before.root, before.targets, retain_content=False) except ControlPlaneError as exc: raise ControlPlaneError( "CP-PROVIDER-INTEGRITY: provider made a declared live path unsafe" @@ -500,6 +578,9 @@ def _assert_declared_paths_unchanged(before: RepositorySnapshot) -> None: raise ControlPlaneError( f"CP-PROVIDER-INTEGRITY: provider changed live path {expected.path.original}" ) + # Armed only after the comparison succeeded: a snapshot that already failed + # the guard must never become the next invocation's reference. + _arm_snapshot_chain(after) def _output_notice(stdout: _OutputSink, stderr: _OutputSink) -> str | None: @@ -921,7 +1002,7 @@ def _invoke_command_provider( extensions=payload.manifest.extensions, ), ) - before = RepositorySnapshot.capture( + before = _capture_declared_paths( root, _declared_snapshot_paths(effective_invocation.snapshots), ) @@ -1013,7 +1094,7 @@ def invoke_provider_in_child(invocation: ProviderInvocation) -> ProviderResult: _validate_json_schema(prepared.input_schema, input_value, kind="input") frozen_input = _deep_freeze(input_value) frozen_resources = MappingProxyType(prepared.resources) - before = RepositorySnapshot.capture( + before = _capture_declared_paths( root, _declared_snapshot_paths(effective_invocation.snapshots), ) @@ -1078,7 +1159,7 @@ def invoke_provider(invocation: ProviderInvocation) -> ProviderResult: extensions=payload.manifest.extensions, ), ) - before = RepositorySnapshot.capture( + before = _capture_declared_paths( root, _declared_snapshot_paths(effective_invocation.snapshots), ) diff --git a/src/project_standards/control_plane/snapshot.py b/src/project_standards/control_plane/snapshot.py index 7c6b4951..0abd7a17 100644 --- a/src/project_standards/control_plane/snapshot.py +++ b/src/project_standards/control_plane/snapshot.py @@ -8,8 +8,8 @@ from dataclasses import dataclass from enum import StrEnum from pathlib import Path, PurePosixPath +from typing import Protocol -from project_standards.control_plane.codec import content_digest from project_standards.control_plane.containment import ( CONTAINMENT_DESTINATION_CODE, ContainmentError, @@ -27,6 +27,14 @@ _READ_SIZE = 1024 * 1024 +class _StreamingHash(Protocol): + """The `hashlib` surface a streaming capture needs, without a private typeshed name.""" + + def update(self, data: bytes, /) -> None: ... + + def hexdigest(self) -> str: ... + + class EntryKind(StrEnum): """Filesystem states relevant to planning without following links.""" @@ -39,7 +47,14 @@ class EntryKind(StrEnum): @dataclass(frozen=True, slots=True) class SnapshotEntry: - """Exact bytes and metadata observed for one declared target.""" + """Exact bytes and metadata observed for one declared target. + + A snapshot captured with ``retain_content=False`` leaves ``content`` and + ``content_digest`` unset on a regular entry: the bytes are hashed as they + stream past and never retained. ``precondition_digest`` is the only field + that mode promises, so a caller that reads bytes or compares content + digests must capture in the default full mode. + """ path: SafeRelativePath kind: EntryKind @@ -50,6 +65,16 @@ class SnapshotEntry: precondition_digest: Sha256Digest +def _precondition_hasher(kind: EntryKind, mode: str | None) -> _StreamingHash: + """Seed a precondition hash with the fixed prefix that precedes the payload.""" + digest = hashlib.sha256() + digest.update(kind.value.encode("ascii")) + digest.update(b"\0") + digest.update((mode or "").encode("ascii")) + digest.update(b"\0") + return digest + + def _precondition( kind: EntryKind, *, @@ -57,11 +82,7 @@ def _precondition( content: bytes | None = None, link_target: str | None = None, ) -> Sha256Digest: - digest = hashlib.sha256() - digest.update(kind.value.encode("ascii")) - digest.update(b"\0") - digest.update((mode or "").encode("ascii")) - digest.update(b"\0") + digest = _precondition_hasher(kind, mode) if content is not None: digest.update(content) elif link_target is not None: @@ -69,6 +90,21 @@ def _precondition( return Sha256Digest(f"sha256:{digest.hexdigest()}") +def canonical_targets(targets: tuple[SafeRelativePath, ...]) -> tuple[SafeRelativePath, ...]: + """Reject a colliding declared collection and fix the order a snapshot reads in. + + Capture applies this to its own arguments; a caller that wants to compare + two declared collections for identity — as the provider snapshot chain does + — must compare the canonical forms, because two spellings of one collection + are the same snapshot. + """ + try: + normalized = validate_path_collection(targets) + except ValueError as exc: + raise ControlPlaneError("snapshot target collection contains a collision") from exc + return tuple(sorted(normalized, key=lambda item: item.original.encode("utf-8"))) + + def safe_repository_root(repo: Path) -> Path: try: if repo.is_symlink() or not repo.is_dir(): @@ -193,6 +229,8 @@ def _regular_entry( path: SafeRelativePath, parent_descriptor: int, name: str, + *, + retain_content: bool, ) -> SnapshotEntry: try: descriptor = os.open( @@ -206,9 +244,26 @@ def _regular_entry( before = os.fstat(descriptor) if not stat.S_ISREG(before.st_mode): raise ControlPlaneError("snapshot target changed type during capture") - chunks: list[bytes] = [] + # The mode is taken from the pre-read stat because the precondition hash + # is seeded with it before the first chunk arrives; the stability check + # after the read still rejects a mode that changed mid-capture, so the + # digest can never describe a mode the file did not hold throughout. + mode = _mode(before) + precondition = _precondition_hasher(EntryKind.REGULAR, mode) + # Bytes are hashed as they stream and retained only when a caller asked + # for them: an integrity capture over a multi-megabyte declared target + # (the frozen provider binaries are ~10 MB each) otherwise holds the + # whole file for the length of a provider invocation. The content digest + # is likewise computed only when it is promised, which halves the hash + # work of a precondition-only capture. + digest = hashlib.sha256() if retain_content else None + chunks: list[bytes] | None = [] if retain_content else None while chunk := os.read(descriptor, _READ_SIZE): - chunks.append(chunk) + precondition.update(chunk) + if digest is not None: + digest.update(chunk) + if chunks is not None: + chunks.append(chunk) after = os.fstat(descriptor) except OSError as exc: raise ControlPlaneError("snapshot target could not be read") from exc @@ -217,16 +272,14 @@ def _regular_entry( stable_fields = ("st_dev", "st_ino", "st_size", "st_mtime_ns", "st_mode") if any(getattr(before, field) != getattr(after, field) for field in stable_fields): raise ControlPlaneError("snapshot target changed while being read") - content = b"".join(chunks) - mode = _mode(after) return SnapshotEntry( path=path, kind=EntryKind.REGULAR, - content=content, + content=b"".join(chunks) if chunks is not None else None, mode=mode, link_target=None, - content_digest=content_digest(content), - precondition_digest=_precondition(EntryKind.REGULAR, mode=mode, content=content), + content_digest=Sha256Digest(f"sha256:{digest.hexdigest()}") if digest is not None else None, + precondition_digest=Sha256Digest(f"sha256:{precondition.hexdigest()}"), ) @@ -294,6 +347,8 @@ def _read_entry( root: Path, root_descriptor: int, path: SafeRelativePath, + *, + retain_content: bool, ) -> SnapshotEntry: parent_descriptor = _parent_descriptor(root, root_descriptor, path.normalized.parent) if parent_descriptor is None: @@ -321,7 +376,7 @@ def _read_entry( _precondition(EntryKind.MISSING), ) if stat.S_ISREG(metadata.st_mode): - return _regular_entry(path, parent_descriptor, name) + return _regular_entry(path, parent_descriptor, name, retain_content=retain_content) if stat.S_ISLNK(metadata.st_mode): target = os.readlink(name, dir_fd=parent_descriptor) return SnapshotEntry( @@ -364,13 +419,19 @@ def capture( cls, repo: Path, targets: tuple[SafeRelativePath, ...], + *, + retain_content: bool = True, ) -> RepositorySnapshot: + """Read every declared target once, retaining bytes unless asked not to. + + `retain_content=False` yields a precondition-only snapshot: every entry + still carries the precondition digest the integrity guard compares, but + regular-file bytes are hashed as they stream and then dropped, and the + content digest is not computed. Use it for a capture that will only be + compared, never read. + """ root = safe_repository_root(repo) - try: - normalized = validate_path_collection(targets) - except ValueError as exc: - raise ControlPlaneError("snapshot target collection contains a collision") from exc - ordered = tuple(sorted(normalized, key=lambda item: item.original.encode("utf-8"))) + ordered = canonical_targets(targets) flags = os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW | os.O_CLOEXEC try: root_descriptor = os.open(root, flags) @@ -380,7 +441,10 @@ def capture( # Preflight every ancestor before the first content read: otherwise an # escape discovered late could leave earlier provider inputs observable. _preflight_ancestors(root, root_descriptor, ordered) - entries = tuple(_read_entry(root, root_descriptor, target) for target in ordered) + entries = tuple( + _read_entry(root, root_descriptor, target, retain_content=retain_content) + for target in ordered + ) finally: os.close(root_descriptor) return cls(root, ordered, entries) @@ -393,7 +457,9 @@ def entry(self, path: SafeRelativePath) -> SnapshotEntry: def assert_current(self) -> None: """Fail when any target no longer matches this snapshot's precondition.""" - current = RepositorySnapshot.capture(self.root, self.targets) + # Only precondition digests are compared, so the re-read never retains + # bytes even when this snapshot itself carries them. + current = RepositorySnapshot.capture(self.root, self.targets, retain_content=False) for expected, observed in zip(self.entries, current.entries, strict=True): if expected.precondition_digest != observed.precondition_digest: raise ControlPlaneError(f"snapshot precondition changed: {expected.path.original}") diff --git a/src/project_standards/package_contract/repository.py b/src/project_standards/package_contract/repository.py index 66f25b94..649af57d 100644 --- a/src/project_standards/package_contract/repository.py +++ b/src/project_standards/package_contract/repository.py @@ -3,6 +3,8 @@ from __future__ import annotations import json +import os +import stat from collections.abc import Iterable from dataclasses import dataclass from pathlib import Path @@ -234,13 +236,103 @@ def _load_schema_property_scopes( return tuple(scopes), findings +# The source tree a build reads: the family indexes, payload manifests, option +# schemas and payload resources under `standards/`, plus the catalogs the +# `catalog_major` argument selects. +_SOURCE_DIRECTORIES = ("standards", "catalogs") +# Bounded because a long-lived process (the MCP server) can be asked about +# several roots; the entries are large and only the most recent few are ever +# hit again. +_CACHE_LIMIT = 4 +_repository_cache: dict[ + tuple[str, int | None, tuple[str, ...] | None, tuple[tuple[str, int, int, int], ...]], + PackageRepository, +] = {} + + +def _source_content_key(root: Path) -> tuple[tuple[str, int, int, int], ...]: + """Fingerprint every file a build reads, by path, type, size and mtime. + + This keys a cache over parse results, not an integrity guard, which is why + `stat` metadata is enough here while the control plane's snapshot guard + (`control_plane/snapshot.py`) insists on reading and hashing bytes: a stale + entry here costs one stale in-process answer about the producer's own + working tree, whereas a missed change there would let a provider mutate a + declared live path undetected. Payload bytes are proven by digest inside the + build itself, so this fingerprint only has to notice that the tree moved. + """ + entries: list[tuple[str, int, int, int]] = [] + for name in _SOURCE_DIRECTORIES: + stack = [root / name] + while stack: + current = stack.pop() + try: + with os.scandir(current) as scan: + for item in scan: + if item.is_dir(follow_symlinks=False): + stack.append(Path(item.path)) + continue + metadata = item.stat(follow_symlinks=False) + entries.append( + ( + str(Path(item.path).relative_to(root)), + stat.S_IFMT(metadata.st_mode), + metadata.st_size, + metadata.st_mtime_ns, + ) + ) + except OSError: + # A directory that cannot be listed is a fact the build itself + # reports as a finding; the key simply records its absence, so a + # later repair still produces a different key. + entries.append((str(current.relative_to(root)), 0, -1, -1)) + return tuple(sorted(entries)) + + +def clear_package_repository_cache() -> None: + """Drop every memoized build; the escape hatch for a test that rewrites a tree in place.""" + _repository_cache.clear() + + def build_package_repository( root: Path, *, catalog_major: int | None = None, family_allowlist: Iterable[str] | None = None, ) -> PackageRepository: - """Load declared V2 sources without interpreting V1 manifests or unindexed trees.""" + """Load declared V2 sources without interpreting V1 manifests or unindexed trees. + + Repeated builds of an unchanged tree in one process are served from a + memo keyed on the source fingerprint above: one build reads 212 payload + manifests and 416 option schemas and hashes every payload resource + (~2.4 s on this repository), and a gate or a test module that asks three + times pays that once. The returned `PackageRepository` is immutable and is + therefore shared, not copied. + """ + allowlist_key = None if family_allowlist is None else tuple(family_allowlist) + if allowlist_key is not None: + family_allowlist = allowlist_key + key = (str(root), catalog_major, allowlist_key, _source_content_key(root)) + cached = _repository_cache.get(key) + if cached is not None: + return cached + built = _build_package_repository( + root, + catalog_major=catalog_major, + family_allowlist=family_allowlist, + ) + if len(_repository_cache) >= _CACHE_LIMIT: + _repository_cache.pop(next(iter(_repository_cache))) + _repository_cache[key] = built + return built + + +def _build_package_repository( + root: Path, + *, + catalog_major: int | None = None, + family_allowlist: Iterable[str] | None = None, +) -> PackageRepository: discovery = discover_v2_families(root, family_allowlist=family_allowlist) findings = list(discovery.findings) loaded_families: list[LoadedFamily] = [] diff --git a/tests/control_plane/test_planner.py b/tests/control_plane/test_planner.py index 35f814f9..c0087108 100644 --- a/tests/control_plane/test_planner.py +++ b/tests/control_plane/test_planner.py @@ -1,5 +1,6 @@ from __future__ import annotations +import contextlib import hashlib import json import random @@ -10,6 +11,8 @@ import pytest +import project_standards.control_plane.planner as planner_module +import project_standards.control_plane.snapshot as snapshot_module from project_standards.control_plane.adapters.toml import TomlAdapter from project_standards.control_plane.codec import parse_lock, render_lock from project_standards.control_plane.diagnostics import ( @@ -28,10 +31,15 @@ from project_standards.control_plane.providers import ( ProviderInvocation, ProviderResult, + invoke_provider, ) from project_standards.control_plane.resolution import DeclaredTransition from project_standards.control_plane.schemas import ReconciliationPlanSchema -from project_standards.package_contract.paths import PackageVersion, Sha256Digest +from project_standards.package_contract.paths import ( + PackageVersion, + SafeRelativePath, + Sha256Digest, +) from project_standards.package_contract.payload import ( ArtifactPolicy, JsonValue, @@ -45,6 +53,10 @@ resolution_request, write_payload, ) +from tests.control_plane.test_providers import ( + provider_invocation, + write_provider_payload, +) def _request( @@ -2162,3 +2174,93 @@ def test_undeclared_whole_file_relinquishment_follows_created_container( assert _action(plan, "notes.md").kind is expected assert target.read_bytes() == content assert all(artifact.path.original != "notes.md" for artifact in plan.next_lock.artifacts) + + +def _capture_counter(monkeypatch: pytest.MonkeyPatch) -> list[int]: + counted = [0] + original = snapshot_module.RepositorySnapshot.capture + + def counting( + repo: Path, + targets: tuple[SafeRelativePath, ...], + *, + retain_content: bool = True, + ) -> snapshot_module.RepositorySnapshot: + counted[0] += 1 + return original(repo, targets, retain_content=retain_content) + + monkeypatch.setattr(snapshot_module.RepositorySnapshot, "capture", counting) + return counted + + +def _chained_planner_fixture(tmp_path: Path) -> tuple[Path, PlannerRequest]: + """A plan whose two contributions each run one real, executable provider. + + The runner ignores the planner's own invocation and dispatches the + executable fixture payload instead, because chaining is only observable + across invocations that declare the identical target set — which is what + the real reconcile path does, and what an inert stub runner cannot show. + """ + repo = tmp_path / "repo" + repo.mkdir() + executable = write_provider_payload(tmp_path / "executable") + payload = write_payload( + tmp_path / "demo", + "demo", + contributions=[ + { + "id": "first", + "target": "first.txt", + "adapter": "whole-file", + "scope": "$file", + "provider": "render-tool", + }, + { + "id": "second", + "target": "second.txt", + "adapter": "whole-file", + "scope": "$file", + "provider": "render-tool", + }, + ], + render_providers=["render-tool"], + ) + + def runner(_invocation: ProviderInvocation) -> ProviderResult: + return invoke_provider(provider_invocation(repo, executable)) + + return repo, _request(repo, (payload,), provider_runner=runner) + + +def test_planning_shares_one_integrity_snapshot_between_consecutive_providers( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + repo, request = _chained_planner_fixture(tmp_path) + monkeypatch.chdir(repo) + chained = _capture_counter(monkeypatch) + + plan = plan_reconciliation(request) + + assert plan.applicable + # One capture for the plan's own target snapshot, then N+1 rather than 2N + # for the two provider invocations the contributions drive. + assert chained[0] == 4 + + +def test_planning_without_the_chain_captures_twice_per_provider( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + repo, request = _chained_planner_fixture(tmp_path) + monkeypatch.chdir(repo) + unchained = _capture_counter(monkeypatch) + + # Removing the window is the only difference from the test above, so the + # saving is pinned to the window itself rather than to any other change in + # the planner. + monkeypatch.setattr(planner_module, "provider_snapshot_chain", contextlib.nullcontext) + plan = plan_reconciliation(request) + + assert plan.applicable + assert unchained[0] == 5 diff --git a/tests/control_plane/test_providers.py b/tests/control_plane/test_providers.py index b3f000d5..c9a020da 100644 --- a/tests/control_plane/test_providers.py +++ b/tests/control_plane/test_providers.py @@ -11,11 +11,13 @@ import pytest import project_standards.control_plane.providers as provider_runtime +import project_standards.control_plane.snapshot as snapshot_module from project_standards.control_plane.diagnostics import ControlPlaneError from project_standards.control_plane.distribution import InstalledPayload from project_standards.control_plane.providers import ( ProviderInvocation, invoke_provider, + provider_snapshot_chain, resolve_referenced_inputs, ) from project_standards.control_plane.schemas import control_plane_schema_documents @@ -1280,3 +1282,94 @@ def reject_recursive_scan(_path: Path, _pattern: str) -> object: result = invoke_provider(_invocation(repo, payload)) assert result.content == b"1.2:1.2:declared-data" + + +def _count_captures(monkeypatch: pytest.MonkeyPatch) -> list[int]: + """Count every snapshot capture the guard performs during one test.""" + captured = [0] + original = snapshot_module.RepositorySnapshot.capture + + def counting( + repo: Path, + targets: tuple[SafeRelativePath, ...], + *, + retain_content: bool = True, + ) -> snapshot_module.RepositorySnapshot: + captured[0] += 1 + return original(repo, targets, retain_content=retain_content) + + monkeypatch.setattr(snapshot_module.RepositorySnapshot, "capture", counting) + return captured + + +def test_snapshot_chain_halves_captures_and_still_catches_a_provider_write( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + repo = tmp_path / "repo" + repo.mkdir() + honest = _write_provider_payload(tmp_path / "honest") + writer = _write_provider_payload(tmp_path / "writer", behavior="write") + monkeypatch.chdir(repo) + captured = _count_captures(monkeypatch) + + with provider_snapshot_chain(): + invoke_provider(_invocation(repo, honest)) + invoke_provider(_invocation(repo, honest)) + # Two invocations cost three captures, not four: the first invocation's + # AFTER snapshot is the second invocation's BEFORE. + assert captured[0] == 3 + + with pytest.raises(ControlPlaneError, match="CP-PROVIDER-INTEGRITY"): + invoke_provider(_invocation(repo, writer)) + + +def test_snapshot_chain_never_survives_its_window( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + repo = tmp_path / "repo" + repo.mkdir() + payload = _write_provider_payload(tmp_path / "payload") + monkeypatch.chdir(repo) + captured = _count_captures(monkeypatch) + + with provider_snapshot_chain(): + invoke_provider(_invocation(repo, payload)) + # A write between two windows is the caller's, not a provider's: outside the + # window the next invocation reads the tree again instead of blaming the + # provider for what the control plane itself published. + (repo / "README.md").write_bytes(b"published between two passes\n") + invoke_provider(_invocation(repo, payload)) + + assert captured[0] == 4 + + +def test_provider_guard_captures_do_not_retain_declared_bytes( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + repo = tmp_path / "repo" + repo.mkdir() + (repo / "README.md").write_bytes(b"declared bytes\n") + payload = _write_provider_payload(tmp_path / "payload") + monkeypatch.chdir(repo) + seen: list[snapshot_module.RepositorySnapshot] = [] + original = snapshot_module.RepositorySnapshot.capture + + def recording( + repo_path: Path, + targets: tuple[SafeRelativePath, ...], + *, + retain_content: bool = True, + ) -> snapshot_module.RepositorySnapshot: + result = original(repo_path, targets, retain_content=retain_content) + seen.append(result) + return result + + monkeypatch.setattr(snapshot_module.RepositorySnapshot, "capture", recording) + + invoke_provider(_invocation(repo, payload)) + + assert seen + assert all(entry.content is None for snapshot in seen for entry in snapshot.entries) diff --git a/tests/control_plane/test_snapshot.py b/tests/control_plane/test_snapshot.py new file mode 100644 index 00000000..931b09ee --- /dev/null +++ b/tests/control_plane/test_snapshot.py @@ -0,0 +1,53 @@ +from __future__ import annotations + +import hashlib +from pathlib import Path + +from project_standards.control_plane.snapshot import EntryKind, RepositorySnapshot +from project_standards.package_contract.paths import SafeRelativePath + + +def _targets(*paths: str) -> tuple[SafeRelativePath, ...]: + return tuple(SafeRelativePath.parse(path) for path in paths) + + +def _repository(tmp_path: Path) -> Path: + root = tmp_path / "repo" + (root / "nested").mkdir(parents=True) + # Deliberately larger than the 1 MiB read size so the streaming hash spans + # several chunks; a single-chunk fixture would not exercise the boundary. + (root / "large.bin").write_bytes(b"payload-byte" * 200_000) + (root / "nested/small.txt").write_bytes(b"small\n") + (root / "nested/link").symlink_to("small.txt") + return root + + +def test_precondition_only_capture_matches_a_full_capture(tmp_path: Path) -> None: + root = _repository(tmp_path) + targets = _targets("large.bin", "nested/small.txt", "nested/link", "nested", "absent.txt") + + full = RepositorySnapshot.capture(root, targets) + lean = RepositorySnapshot.capture(root, targets, retain_content=False) + + assert [entry.precondition_digest for entry in full.entries] == [ + entry.precondition_digest for entry in lean.entries + ] + assert [entry.kind for entry in full.entries] == [entry.kind for entry in lean.entries] + assert [entry.mode for entry in full.entries] == [entry.mode for entry in lean.entries] + + +def test_full_capture_keeps_exact_bytes_and_lean_capture_drops_them(tmp_path: Path) -> None: + root = _repository(tmp_path) + content = (root / "large.bin").read_bytes() + targets = _targets("large.bin") + + full = RepositorySnapshot.capture(root, targets).entries[0] + lean = RepositorySnapshot.capture(root, targets, retain_content=False).entries[0] + + assert full.kind is EntryKind.REGULAR + assert full.content == content + assert full.content_digest is not None + assert full.content_digest.value == f"sha256:{hashlib.sha256(content).hexdigest()}" + assert lean.kind is EntryKind.REGULAR + assert lean.content is None + assert lean.content_digest is None diff --git a/tests/package_contract/test_repository.py b/tests/package_contract/test_repository.py index b265276a..ad501678 100644 --- a/tests/package_contract/test_repository.py +++ b/tests/package_contract/test_repository.py @@ -113,3 +113,34 @@ def test_selected_catalog_errors_join_package_load_findings(tmp_path: Path) -> N "PC-INTEGRITY", "PC-CATALOG-INVALID", } + + +def test_repeated_build_is_memoized_until_a_manifest_changes(tmp_path: Path) -> None: + root = copy_minimal_repository(tmp_path) + + first = build_package_repository(root, catalog_major=5) + assert build_package_repository(root, catalog_major=5) is first + + # A payload manifest edit must be visible to the very next build: the memo + # is keyed on the source fingerprint, not on the root alone. + manifest = root / "standards/demo/versions/1.2/payload.toml" + manifest.write_text(manifest.read_text(encoding="utf-8") + "\n", encoding="utf-8") + + rebuilt = build_package_repository(root, catalog_major=5) + + assert rebuilt is not first + assert {finding.code for finding in rebuilt.findings} != set() + + +def test_memo_distinguishes_the_catalog_and_the_allowlist(tmp_path: Path) -> None: + root = copy_minimal_repository(tmp_path) + + with_catalog = build_package_repository(root, catalog_major=5) + without_catalog = build_package_repository(root) + narrowed = build_package_repository(root, family_allowlist=iter(("demo",))) + + assert with_catalog.catalog is not None + assert without_catalog.catalog is None + # The allowlist may arrive as a one-shot iterator; keying on it must not + # consume the iterator the build itself still needs. + assert [family.manifest.standard.id for family in narrowed.families] == ["demo"]