From 70e55e1e6cdc21adb6f66c2a61bcae02ae6e5090 Mon Sep 17 00:00:00 2001 From: Pengfei Hu Date: Fri, 7 Aug 2026 18:23:36 -0700 Subject: [PATCH 1/3] Normalize effective worktree verification diffs --- docs/verification-reproducibility.md | 7 + src/agents_shipgate/cli/verification.py | 52 +++- src/agents_shipgate/cli/verify/git.py | 231 ++++++++++++++---- .../cli/verify/orchestrator.py | 89 +++++-- src/agents_shipgate/core/agent_boundary.py | 27 +- src/agents_shipgate/core/static_inputs.py | 12 + .../core/verification_identity.py | 27 +- tests/test_adapter_static_only.py | 4 +- tests/test_agent_boundary.py | 31 +++ tests/test_verification_git_snapshot.py | 66 +++++ tests/test_verify_orchestrator.py | 122 +++++++++ 11 files changed, 577 insertions(+), 91 deletions(-) diff --git a/docs/verification-reproducibility.md b/docs/verification-reproducibility.md index d48f9cfd..4be70d86 100644 --- a/docs/verification-reproducibility.md +++ b/docs/verification-reproducibility.md @@ -42,6 +42,13 @@ Committed snapshots are materialized from Git objects with `git ls-tree` and `export-ignore` and `export-subst` cannot change the evaluated bytes. Symlinks and submodules fail closed for archived verification inputs. +Worktree verification evaluates one merge-base-to-effective-worktree diff, +including staged and unstaged changes, instead of concatenating a committed +range with a HEAD-relative overlay. The request separately binds the exact +HEAD-relative overlay path set and each path's presence, content hash, and Git +file mode. A path changed in both layers therefore has one policy-evaluation +record while the terminal receipt still identifies the complete overlay. + `attempt_id` is diagnostic and deliberately excluded from `receipt_id`. Changing an authoritative input, result, decision, or artifact changes the corresponding content ID. Reusing a base-scan cache does not change the public diff --git a/src/agents_shipgate/cli/verification.py b/src/agents_shipgate/cli/verification.py index 56bd54c1..60a34f78 100644 --- a/src/agents_shipgate/cli/verification.py +++ b/src/agents_shipgate/cli/verification.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +import stat import tempfile from pathlib import Path from typing import Any @@ -18,6 +19,7 @@ merge_base_sha, ref_exists, repository_identity, + require_merge_base_sha, resolve_source_head_identity, tree_sha, validate_source_head_identity, @@ -77,11 +79,23 @@ def prepare( head_ref = head or "HEAD" if not ref_exists(root, head_ref): raise typer.BadParameter(f"head ref is unavailable locally: {head_ref}") - changed, diff_text = ( - diff_context(root, base, head_ref) - if base - else working_tree_context(root, reject_index_hidden=True) - ) + worktree_overlay_paths: list[str] | None = None + if head is not None: + changed, diff_text = diff_context(root, base, head_ref) + else: + worktree_overlay_paths, overlay_diff = working_tree_context( + root, + reject_index_hidden=True, + ) + if base: + effective_base = require_merge_base_sha(root, base, head_ref) + changed, diff_text = working_tree_context( + root, + comparison_ref=effective_base, + reject_index_hidden=True, + ) + else: + changed, diff_text = worktree_overlay_paths, overlay_diff resolved_date = evaluation_date or commit_date(root, head_ref) config_relative = _under(root, config).relative_to(root) policy_paths = [_under(root, path) for path in policy_packs or []] @@ -112,6 +126,7 @@ def prepare( "plugins_enabled": not no_plugins, }, plugins_enabled=False if no_plugins else None, + worktree_overlay_paths=worktree_overlay_paths, ) else: source_identity = resolve_source_head_identity( @@ -512,16 +527,37 @@ def _validate_git_subject(plan: VerificationPlan, workspace: Path) -> None: raise ValueError("worktree-overlay plan cannot carry source-head authority") if commit_sha(root, "HEAD") != subject.head_commit_sha: raise ValueError("worker HEAD does not match the worktree-overlay plan") + declared_overlay_paths = plan.inputs.options.get("worktree_overlay_paths") + if declared_overlay_paths is None: + # Compatibility reader for plans produced before issue #336 split + # effective paths from HEAD-relative overlay paths. + overlay_paths = plan.inputs.changed_paths + elif not isinstance(declared_overlay_paths, list) or not all( + isinstance(path, str) for path in declared_overlay_paths + ): + raise ValueError("plan worktree_overlay_paths must be a string list") + elif declared_overlay_paths != sorted(set(declared_overlay_paths)): + raise ValueError("plan worktree_overlay_paths must be sorted and unique") + else: + overlay_paths = declared_overlay_paths rows: list[dict[str, Any]] = [] - for relative in plan.inputs.changed_paths: + for relative in overlay_paths: candidate = (root / relative).resolve() if candidate != root and root not in candidate.parents: raise ValueError(f"plan changed path escapes workspace: {relative}") + present = candidate.is_file() rows.append( { "path": relative, - "status": "present" if candidate.is_file() else "deleted", - "sha256": sha256_file(candidate) if candidate.is_file() else None, + "status": "present" if present else "deleted", + "sha256": sha256_file(candidate) if present else None, + "git_mode": ( + "100755" + if present and stat.S_IMODE(candidate.stat().st_mode) & 0o111 + else "100644" + if present + else None + ), } ) actual_overlay = content_id(rows) if rows else None diff --git a/src/agents_shipgate/cli/verify/git.py b/src/agents_shipgate/cli/verify/git.py index 2d4903e3..14d3d412 100644 --- a/src/agents_shipgate/cli/verify/git.py +++ b/src/agents_shipgate/cli/verify/git.py @@ -225,6 +225,11 @@ class _UnavailableRevisionError(ConfigError): "-c", "diff.renameLimit=32767", ] +# Tree-to-worktree comparisons must retain Git's executable-bit semantics. +# The generic config disables file-mode reads for portable committed-tree +# comparisons; the later override is intentional and scoped to local overlay +# collection, whose exact mode is receipt-bound. +_SAFE_WORKTREE_DIFF_CONFIG = [*_SAFE_DIFF_CONFIG, "-c", "core.fileMode=true"] _DETERMINISTIC_DIFF_OPTIONS = [ "--no-ext-diff", "--no-textconv", @@ -516,6 +521,59 @@ def merge_base_sha(workspace: Path, base: str, head: str) -> str | None: return result.stdout.strip() or None +def require_merge_base_sha(workspace: Path, base: str, head: str) -> str: + """Resolve one merge base or raise a typed diff-input failure. + + Worktree verification needs only this ancestry identity before comparing + the merge-base tree directly with the effective worktree. Reading the + intermediate committed diff would make canceled or superseded branch + content an accidental input again. + """ + + base_commit = commit_sha(workspace, base) + head_commit = commit_sha(workspace, head) + if base_commit is None or head_commit is None: + missing = base if base_commit is None else head + raise DiffInputError( + DiffContext( + completeness="unavailable", + reason="refs_missing", + detail=f"Git ref {missing!r} is not available locally.", + ) + ) + result = _run_git( + workspace, + ["merge-base", "--", base_commit, head_commit], + check=False, + ) + resolved = result.stdout.strip() + if result.returncode == 0 and _GIT_OBJECT_RE.fullmatch(resolved): + return resolved + detail = _redact_local_paths(result.stderr.strip(), workspace) + if result.returncode == 1: + truncated = _history_is_truncated(workspace) + reason: DiffInputReason = ( + "merge_base_missing" + if truncated is True + else "unrelated_histories" + if truncated is False + else "git_failed" + ) + else: + reason, detail = _classify_diff_failure( + _BoundedGitResult(payload=None, stderr=detail), + limit_reason="metadata_limit_exceeded", + workspace=workspace, + ) + raise DiffInputError( + DiffContext( + completeness="unavailable", + reason=reason, + detail=detail or "Git could not resolve one merge base.", + ) + ) + + def commit_date(workspace: Path, ref: str) -> str: commit = commit_sha(workspace, ref) if commit is None: @@ -1092,28 +1150,141 @@ def removes_a_yaml_file(workspace: Path, base: str | None, head: str) -> bool | def working_tree_context( workspace: Path, *, + comparison_ref: str = "HEAD", exclude: Path | None = None, reject_index_hidden: bool = False, ) -> tuple[list[str], str]: - """Return uncommitted changed paths and tracked-file diff text. + """Return effective changed paths and tracked-file diff text. + + ``git diff `` compares one committed tree with the current + index/worktree, so staged and unstaged tracked changes are represented in + one record per effective path. Untracked file paths are included for + trigger/check context, but their contents are intentionally not read into + the diff body. + + The verifier passes the merge-base commit as ``comparison_ref`` when a + worktree sits on top of committed branch changes. This is deliberately one + comparison, rather than concatenating ``base...HEAD`` and ``HEAD`` diffs: + concatenation makes a valid overlapping edit look structurally ambiguous. + """ + + _reject_unbound_diff_configuration(workspace) + _reject_executable_worktree_filters(workspace) + comparison_commit = commit_sha(workspace, comparison_ref) + if comparison_commit is None: + raise DiffInputError( + DiffContext( + completeness="unavailable", + reason="refs_missing", + detail=f"Git ref {comparison_ref!r} is not available locally.", + ) + ) + pathspec = _worktree_pathspec(workspace, exclude) + if reject_index_hidden: + _reject_index_hidden_capability_paths(workspace, pathspec=pathspec) + paths = _working_tree_paths( + workspace, + comparison_commit=comparison_commit, + pathspec=pathspec, + ) + body = _run_git_bounded_result( + workspace, + [ + *_SAFE_WORKTREE_DIFF_CONFIG, + "diff", + *_DETERMINISTIC_DIFF_OPTIONS, + comparison_commit, + "--", + *pathspec, + ], + max_output_bytes=_DIFF_BODY_LIMIT, + ) + if body.payload is None: + reason, detail = _classify_diff_failure( + body, limit_reason="body_limit_exceeded", workspace=workspace + ) + raise DiffInputError( + DiffContext( + changed_files=tuple(paths), + completeness="partial", + reason=reason, + detail=detail, + ) + ) + diff_text = _decode_diff_body(body.payload) + try: + _reject_binary_capability_paths( + workspace, + comparison_commit, + pathspec=pathspec, + ) + except BinaryCapabilityDiffError as exc: + exc.changed_paths = tuple(paths) + exc.diff_text = diff_text + raise + except DiffInputError as exc: + # The binary-hiding guard could not run, so the body is not proven to + # cover every capability path. Carry what was read: a caller that can + # act on partial evidence should not have to re-collect it. + raise DiffInputError( + DiffContext( + changed_files=tuple(paths), + diff_text=diff_text, + completeness="partial", + reason=exc.context.reason, + detail=exc.context.detail, + ) + ) from exc + return paths, diff_text - ``git diff HEAD`` includes staged and unstaged tracked changes. Untracked - file paths are included for trigger/check context, but their contents are - intentionally not read into the diff body. + +def working_tree_paths( + workspace: Path, + *, + comparison_ref: str = "HEAD", + exclude: Path | None = None, + reject_index_hidden: bool = False, +) -> list[str]: + """Return the bounded path inventory for one worktree comparison. + + This metadata-only form is used to bind the HEAD-relative overlay identity + independently from the merge-base-relative diff evaluated by policy. """ _reject_unbound_diff_configuration(workspace) _reject_executable_worktree_filters(workspace) + comparison_commit = commit_sha(workspace, comparison_ref) + if comparison_commit is None: + raise DiffInputError( + DiffContext( + completeness="unavailable", + reason="refs_missing", + detail=f"Git ref {comparison_ref!r} is not available locally.", + ) + ) pathspec = _worktree_pathspec(workspace, exclude) if reject_index_hidden: _reject_index_hidden_capability_paths(workspace, pathspec=pathspec) + return _working_tree_paths( + workspace, + comparison_commit=comparison_commit, + pathspec=pathspec, + ) + + +def _working_tree_paths( + workspace: Path, + *, + comparison_commit: str, + pathspec: list[str], +) -> list[str]: names = _run_git_bounded_result( workspace, [ - *_SAFE_DIFF_CONFIG, + *_SAFE_WORKTREE_DIFF_CONFIG, "diff", *_DETERMINISTIC_DIFF_OPTIONS, - "HEAD", + comparison_commit, "--name-status", "-z", "--", @@ -1156,51 +1327,7 @@ def working_tree_context( path = os.fsdecode(raw_path) if path not in paths: paths.append(path) - body = _run_git_bounded_result( - workspace, - [ - *_SAFE_DIFF_CONFIG, - "diff", - *_DETERMINISTIC_DIFF_OPTIONS, - "HEAD", - "--", - *pathspec, - ], - max_output_bytes=_DIFF_BODY_LIMIT, - ) - if body.payload is None: - reason, detail = _classify_diff_failure( - body, limit_reason="body_limit_exceeded", workspace=workspace - ) - raise DiffInputError( - DiffContext( - changed_files=tuple(paths), - completeness="partial", - reason=reason, - detail=detail, - ) - ) - diff_text = _decode_diff_body(body.payload) - try: - _reject_binary_capability_paths(workspace, "HEAD", pathspec=pathspec) - except BinaryCapabilityDiffError as exc: - exc.changed_paths = tuple(paths) - exc.diff_text = diff_text - raise - except DiffInputError as exc: - # The binary-hiding guard could not run, so the body is not proven to - # cover every capability path. Carry what was read: a caller that can - # act on partial evidence should not have to re-collect it. - raise DiffInputError( - DiffContext( - changed_files=tuple(paths), - diff_text=diff_text, - completeness="partial", - reason=exc.context.reason, - detail=exc.context.detail, - ) - ) from exc - return paths, diff_text + return sorted(paths) def _worktree_pathspec(workspace: Path, exclude: Path | None) -> list[str]: @@ -1989,6 +2116,7 @@ def staged_paths_under(workspace: Path, subdir: str) -> list[str]: "merge_base_sha", "read_file_at_ref", "repository_identity", + "require_merge_base_sha", "resolve_tree_path_identity", "resolve_git_push_endpoint", "resolve_source_head_identity", @@ -1998,4 +2126,5 @@ def staged_paths_under(workspace: Path, subdir: str) -> list[str]: "tree_sha", "validate_source_head_identity", "working_tree_context", + "working_tree_paths", ] diff --git a/src/agents_shipgate/cli/verify/orchestrator.py b/src/agents_shipgate/cli/verify/orchestrator.py index b39eee60..0ac76e86 100644 --- a/src/agents_shipgate/cli/verify/orchestrator.py +++ b/src/agents_shipgate/cli/verify/orchestrator.py @@ -124,10 +124,12 @@ ref_exists, removes_a_yaml_file, repository_identity, + require_merge_base_sha, resolve_source_head_identity, resolve_tree_path_identity, tree_sha, working_tree_context, + working_tree_paths, ) HEAD_FORMATS = ["markdown", "json", "sarif"] @@ -343,6 +345,7 @@ def run_verify( return verifier, None, 2 changed_files: list[str] = [] + worktree_overlay_paths: list[str] = [] diff_text = "" base_status: VerifierBaseStatus = "not_requested" base_tree: str | None = None @@ -455,24 +458,43 @@ def run_verify( ) base_exists = False + effective_worktree_ref = head + committed_diff_complete = False if base: base_exists = ref_exists(git_root, base) if base_exists: - collected = _collect_diff(git_root, base, head) - changed_files = list(collected.changed_files) - diff_text = collected.diff_text - if collected.completeness != "complete": - # The refs resolved, so the shortfall is about history depth, - # object availability, or Git itself — each of which has a - # different repair. Report which one instead of a single - # "could not be read". - diff_unavailable = True - base_status = "archive_failed" - diff_failures.append((collected, f"{base}...{head}")) - base_notes.append( - f"Could not collect the {base}...{head} diff in full. " - f"{collected.note}" - ) + if archive_head: + collected = _collect_diff(git_root, base, head) + changed_files = list(collected.changed_files) + diff_text = collected.diff_text + if collected.completeness != "complete": + # The refs resolved, so the shortfall is about history depth, + # object availability, or Git itself — each of which has a + # different repair. Report which one instead of a single + # "could not be read". + diff_unavailable = True + base_status = "archive_failed" + diff_failures.append((collected, f"{base}...{head}")) + base_notes.append( + f"Could not collect the {base}...{head} diff in full. " + f"{collected.note}" + ) + else: + try: + effective_worktree_ref = require_merge_base_sha( + git_root, + base, + head, + ) + committed_diff_complete = True + except DiffInputError as exc: + diff_unavailable = True + base_status = "archive_failed" + diff_failures.append((exc.context, f"{base}...{head}")) + base_notes.append( + "Could not resolve the merge base needed for one " + f"effective-head diff. {exc.context.note}" + ) else: diff_unavailable = True base_status = "ref_missing" @@ -495,11 +517,32 @@ def run_verify( try: worktree_paths, worktree_diff = working_tree_context( git_root, + comparison_ref=effective_worktree_ref, exclude=out_dir, reject_index_hidden=True, ) - changed_files = _dedupe_paths([*changed_files, *worktree_paths]) - diff_text = _join_diff_text(diff_text, worktree_diff) + if committed_diff_complete: + # This is the single merge-base-to-effective-worktree diff. + # Do not append the committed range: overlapping paths must be + # represented exactly once at their effective content. + changed_files = worktree_paths + diff_text = worktree_diff + else: + # The committed side was unavailable or partial. Preserve all + # evidence collected before the mandatory fail-closed exit. + changed_files = _dedupe_paths([*changed_files, *worktree_paths]) + diff_text = _join_diff_text(diff_text, worktree_diff) + head_commit = commit_sha(git_root, head) + worktree_overlay_paths = ( + worktree_paths + if head_commit == commit_sha(git_root, effective_worktree_ref) + else working_tree_paths( + git_root, + comparison_ref=head, + exclude=out_dir, + reject_index_hidden=True, + ) + ) changed_files = _bind_worktree_config_to_head( git_root=git_root, head=head, @@ -507,6 +550,13 @@ def run_verify( worktree_text=worktree_manifest_text, changed_files=changed_files, ) + worktree_overlay_paths = _bind_worktree_config_to_head( + git_root=git_root, + head=head, + config_relative=config_relative, + worktree_text=worktree_manifest_text, + changed_files=worktree_overlay_paths, + ) except Exception as exc: # noqa: BLE001 - local context degrades only. diff_unavailable = True worktree_failure = _as_diff_context(exc) @@ -689,7 +739,7 @@ def capture_capability_lock(lock: CapabilityLockFileV1) -> None: config_path, worktree_manifest_text.encode("utf-8"), ) - for relative in changed_files: + for relative in _dedupe_paths([*changed_files, *worktree_overlay_paths]): candidate = Path( os.path.abspath( os.path.normpath(os.fspath(git_root / relative)) @@ -911,6 +961,7 @@ def capture_capability_lock(lock: CapabilityLockFileV1) -> None: os.getenv("EVENT_NAME") or os.getenv("GITHUB_EVENT_NAME") or None ), }, + worktree_overlay_paths=worktree_overlay_paths, evaluation_date=verification_date, ) except Exception: @@ -2379,6 +2430,7 @@ def _write_artifacts( diff_from_path: Path | None = None, authorization_path: Path | None = None, verification_options: dict[str, Any] | None = None, + worktree_overlay_paths: list[str] | None = None, evaluation_date: str | None = None, ) -> None: verifier_path.parent.mkdir(parents=True, exist_ok=True) @@ -2569,6 +2621,7 @@ def _write_artifacts( **resolved_options, }, plugins_enabled=plugins_enabled, + worktree_overlay_paths=worktree_overlay_paths, external_input_root=external_input_root, captured_input_paths=captured_input_paths, ) diff --git a/src/agents_shipgate/core/agent_boundary.py b/src/agents_shipgate/core/agent_boundary.py index d4234819..e6a4a630 100644 --- a/src/agents_shipgate/core/agent_boundary.py +++ b/src/agents_shipgate/core/agent_boundary.py @@ -1294,6 +1294,7 @@ def _structural_diff_issues( ) -> list[BoundaryInputIssue]: issues: list[BoundaryInputIssue] = [] seen_paths: set[str] = set() + duplicate_paths_seen: set[str] = set() if diff_text.strip() and not diff_files: issues.append( BoundaryInputIssue( @@ -1309,17 +1310,7 @@ def _structural_diff_issues( duplicate_paths = sorted(record_paths.intersection(seen_paths)) seen_paths.update(record_paths) if duplicate_paths: - issues.extend( - BoundaryInputIssue( - code="boundary_diff_shape_invalid", - path=path, - message=( - "The supplied diff contains more than one file record " - "for the same path; one coherent record per path is required." - ), - ) - for path in duplicate_paths - ) + duplicate_paths_seen.update(duplicate_paths) continue shape_errors = [ *( @@ -1403,6 +1394,20 @@ def _structural_diff_issues( ), ) ) + if duplicate_paths_seen: + shown = ", ".join(sorted(duplicate_paths_seen)[:3]) + suffix = "" if len(duplicate_paths_seen) <= 3 else ", …" + issues.append( + BoundaryInputIssue( + code="boundary_diff_shape_invalid", + path="", + message=( + "The supplied diff contains multiple file records for " + f"{len(duplicate_paths_seen)} path(s) ({shown}{suffix}); " + "one coherent record per path is required." + ), + ) + ) return issues diff --git a/src/agents_shipgate/core/static_inputs.py b/src/agents_shipgate/core/static_inputs.py index 87bd8b3f..23ec4fbc 100644 --- a/src/agents_shipgate/core/static_inputs.py +++ b/src/agents_shipgate/core/static_inputs.py @@ -46,6 +46,7 @@ def __init__( for path in excluded_paths } self._entries: dict[Path, bytes] = {} + self._modes: dict[Path, int] = {} self._total_bytes = 0 self._budget = IdentityReadBudget( max_entries=max_files * 32, @@ -94,6 +95,13 @@ def has(self, path: Path) -> bool: key = Path(os.path.abspath(os.path.normpath(os.fspath(raw)))) return key in self._entries + def mode(self, path: Path) -> int | None: + """Return the captured permission bits for one snapshotted file.""" + + raw = path if path.is_absolute() else self.root / path + key = Path(os.path.abspath(os.path.normpath(os.fspath(raw)))) + return self._modes.get(key) + def paths_under(self, path: Path) -> list[Path]: raw = path if path.is_absolute() else self.root / path directory = Path(os.path.abspath(os.path.normpath(os.fspath(raw)))) @@ -191,7 +199,11 @@ def _record(self, key: Path, data: bytes) -> None: "static input snapshot exceeds the " f"{self.max_total_bytes}-byte aggregate limit" ) + metadata = key.lstat() + if not stat.S_ISREG(metadata.st_mode): + raise ValueError(f"static input is not a regular file: {key}") self._entries[key] = data + self._modes[key] = stat.S_IMODE(metadata.st_mode) self._total_bytes = next_total diff --git a/src/agents_shipgate/core/verification_identity.py b/src/agents_shipgate/core/verification_identity.py index 887bd214..a0241127 100644 --- a/src/agents_shipgate/core/verification_identity.py +++ b/src/agents_shipgate/core/verification_identity.py @@ -180,6 +180,7 @@ def build_verification_plan( evaluation_date: str, options: dict[str, Any], plugins_enabled: bool | None, + worktree_overlay_paths: list[str] | None = None, diff_logical_path: str = "verification-input.diff", external_input_root: Path | None = None, captured_input_paths: list[Path] | None = None, @@ -187,7 +188,17 @@ def build_verification_plan( effective_plugins_enabled = _plugins_enabled(plugins_enabled) normalized_options = dict(options) normalized_options["plugins_enabled"] = effective_plugins_enabled - overlay = _worktree_overlay(git_root, changed_files) if not archived_head else [] + overlay_paths = sorted( + set(changed_files if worktree_overlay_paths is None else worktree_overlay_paths) + ) + if not archived_head: + # The evaluated change set is merge-base-relative, while the overlay is + # HEAD-relative. Keeping the latter path set inside the content- + # addressed inputs makes cancellations and overlapping edits + # reproducible without polluting policy evaluation with non-effective + # paths. + normalized_options["worktree_overlay_paths"] = overlay_paths + overlay = _worktree_overlay(git_root, overlay_paths) if not archived_head else [] overlay_hash = content_id(overlay) if overlay else None if not archived_head and source_head_commit_sha is not None: raise ValueError("worktree-overlay plans cannot declare a source head commit") @@ -944,11 +955,25 @@ def _worktree_overlay(root: Path, paths: list[str]) -> list[dict[str, Any]]: if snapshot is not None and snapshot.contains(candidate) else candidate.is_file() ) + captured_mode = ( + snapshot.mode(candidate) + if present and snapshot is not None and snapshot.contains(candidate) + else stat.S_IMODE(candidate.stat().st_mode) + if present + else None + ) rows.append( { "path": relative, "status": "present" if present else "deleted", "sha256": sha256_file(candidate) if present else None, + "git_mode": ( + "100755" + if captured_mode is not None and captured_mode & 0o111 + else "100644" + if present + else None + ), } ) return rows diff --git a/tests/test_adapter_static_only.py b/tests/test_adapter_static_only.py index 1c71ded1..7ec3d93e 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=1682, + line=1809, snippet=( "subprocess.Popen(cmd, env=env, stderr=subprocess.PIPE, " "stdin=subprocess.PIPE if input is not None else " @@ -276,7 +276,7 @@ class AllowedException: AllowedException( relative_path="cli/verify/git.py", surface="attr_call:subprocess.run", - line=1926, + line=2053, snippet=( "subprocess.run(cmd, capture_output=capture_output, check=check, " "env=env, input=input, stderr=stderr, stdin=stdin, stdout=stdout, " diff --git a/tests/test_agent_boundary.py b/tests/test_agent_boundary.py index 6de3728e..51d274dd 100644 --- a/tests/test_agent_boundary.py +++ b/tests/test_agent_boundary.py @@ -192,6 +192,37 @@ def test_contradictory_new_file_headers_fail_closed( ) +def test_duplicate_diff_records_are_aggregated_as_one_structural_root_cause( + tmp_path: Path, +) -> None: + diff = "".join( + [ + _change_diff("AGENTS.md", "base", "first"), + _change_diff("AGENTS.md", "first", "second"), + _change_diff(".codex/config.toml", "model = 'a'", "model = 'b'"), + _change_diff(".codex/config.toml", "model = 'b'", "model = 'c'"), + ] + ) + + result = _build(tmp_path, diff) + + structural = [ + item + for item in result.violated_rules + if item.evidence.get("kind") == "boundary_input_unresolved" + and item.evidence.get("code") == "boundary_diff_shape_invalid" + ] + assert len(structural) == 1 + assert structural[0].path == "" + structural_diagnostics = [ + item + for item in result.diagnostics + if item.code == "boundary_diff_shape_invalid" + ] + assert len(structural_diagnostics) == 1 + assert "2 path(s)" in structural_diagnostics[0].message + + def test_safe_untracked_boundary_file_is_read_and_evaluated(tmp_path: Path) -> None: _init_repo(tmp_path) target = tmp_path / ".claude" / "settings.json" diff --git a/tests/test_verification_git_snapshot.py b/tests/test_verification_git_snapshot.py index f676e9d1..039309ab 100644 --- a/tests/test_verification_git_snapshot.py +++ b/tests/test_verification_git_snapshot.py @@ -10,6 +10,7 @@ archive_tree, diff_revspec_context, repository_identity, + working_tree_context, ) from agents_shipgate.core.errors import ConfigError @@ -118,3 +119,68 @@ def test_repository_identity_normalizes_ssh_and_https_remotes(tmp_path: Path) -> "https://token@example.test/org/repo.git?credential=secret", ) assert repository_identity(root) == "example.test/org/repo" + + +def test_effective_worktree_diff_coalesces_committed_staged_and_unstaged_edits( + tmp_path: Path, +) -> None: + root = _repo(tmp_path) + target = root / "AGENTS.md" + target.write_text("base\n", encoding="utf-8") + _git(root, "add", "AGENTS.md") + _git(root, "commit", "-m", "base") + + target.write_text("committed\n", encoding="utf-8") + _git(root, "add", "AGENTS.md") + _git(root, "commit", "-m", "issue fix") + target.write_text("staged review\n", encoding="utf-8") + _git(root, "add", "AGENTS.md") + target.write_text("unstaged review\n", encoding="utf-8") + + changed, diff_text = working_tree_context( + root, + comparison_ref="HEAD~1", + reject_index_hidden=True, + ) + + assert changed == ["AGENTS.md"] + assert diff_text.count("diff --git a/AGENTS.md b/AGENTS.md") == 1 + assert "unstaged review" in diff_text + assert "committed" not in diff_text + assert "+staged review" not in diff_text + + +def test_effective_worktree_diff_preserves_rename_and_mode_semantics( + tmp_path: Path, +) -> None: + root = _repo(tmp_path) + source = root / "AGENTS.md" + source.write_text( + "shared rule\nshared scope\nbase instructions\n", + encoding="utf-8", + ) + _git(root, "add", "AGENTS.md") + _git(root, "commit", "-m", "base") + + destination = root / "CLAUDE.md" + source.rename(destination) + _git(root, "add", "-A") + _git(root, "commit", "-m", "rename instructions") + destination.write_text( + "shared rule\nshared scope\nreviewed instructions\n", + encoding="utf-8", + ) + destination.chmod(0o755) + + changed, diff_text = working_tree_context( + root, + comparison_ref="HEAD~1", + reject_index_hidden=True, + ) + + assert changed == ["AGENTS.md", "CLAUDE.md"] + assert diff_text.count("diff --git ") == 1 + assert "rename from AGENTS.md" in diff_text + assert "rename to CLAUDE.md" in diff_text + assert "old mode 100644" in diff_text + assert "new mode 100755" in diff_text diff --git a/tests/test_verify_orchestrator.py b/tests/test_verify_orchestrator.py index 86cc2f01..508452f4 100644 --- a/tests/test_verify_orchestrator.py +++ b/tests/test_verify_orchestrator.py @@ -563,6 +563,128 @@ def test_verify_threads_uncommitted_worktree_files_into_head_scan(tmp_path): ) +def test_verify_normalizes_overlapping_commit_and_worktree_into_one_diff_record( + tmp_path: Path, +) -> None: + """Issue #336: review follow-ups may overlap the committed issue fix.""" + + repo = tmp_path / "repo" + sample_dst = repo / "samples" / "support_refund_agent" + sample_dst.parent.mkdir(parents=True) + shutil.copytree(REPO_ROOT / "samples" / "support_refund_agent", sample_dst) + target = repo / "AGENTS.md" + target.write_text("base instructions\n", encoding="utf-8") + + _git(repo, "init") + _git(repo, "config", "user.email", "test@example.test") + _git(repo, "config", "user.name", "Test User") + _git(repo, "add", ".") + _git(repo, "commit", "-m", "base") + + target.write_text("committed issue fix\n", encoding="utf-8") + _git(repo, "add", "AGENTS.md") + _git(repo, "commit", "-m", "issue fix") + target.write_text("uncommitted review follow-up\n", encoding="utf-8") + + out_dir = repo / "agents-shipgate-reports" + verifier, report, _exit_code = run_verify( + workspace=repo, + config=Path("samples/support_refund_agent/shipgate.yaml"), + base="HEAD~1", + head="HEAD", + archive_head=False, + out=out_dir, + ci_mode="advisory", + fail_on=None, + baseline=None, + baseline_mode="new-findings", + diff_from=None, + policy_packs=None, + plugins_enabled=False, + strict_plugins=False, + suggest_patches=False, + no_heuristics=False, + verbose=False, + ) + + assert report is not None + assert not any( + finding.check_id == "SHIP-AGENT-BOUNDARY-INPUT-INCOMPLETE" + and finding.evidence.get("code") == "boundary_diff_shape_invalid" + for finding in report.findings + ) + diff_text = (out_dir / "verification-input.diff").read_text(encoding="utf-8") + assert diff_text.count("diff --git a/AGENTS.md b/AGENTS.md") == 1 + assert "uncommitted review follow-up" in diff_text + assert "committed issue fix" not in diff_text + assert verifier.changed_files == ["AGENTS.md"] + + plan = VerificationPlan.model_validate_json( + (out_dir / "verification-plan.json").read_text(encoding="utf-8") + ) + assert plan.inputs.changed_paths == ["AGENTS.md"] + assert plan.inputs.options["worktree_overlay_paths"] == ["AGENTS.md"] + assert plan.subject.git.worktree_overlay_sha256 is not None + receipt = VerificationReceipt.model_validate_json( + (out_dir / "verification-receipt.json").read_text(encoding="utf-8") + ) + assert receipt.subject_id == plan.subject.subject_id + assert receipt.request_id == plan.request_id + + +def test_worktree_overlay_identity_preserves_an_effectively_cancelled_path( + tmp_path: Path, +) -> None: + """A worktree cancellation is absent from policy diff but present in identity.""" + + repo = tmp_path / "repo" + sample_dst = repo / "samples" / "support_refund_agent" + sample_dst.parent.mkdir(parents=True) + shutil.copytree(REPO_ROOT / "samples" / "support_refund_agent", sample_dst) + _git(repo, "init") + _git(repo, "config", "user.email", "test@example.test") + _git(repo, "config", "user.name", "Test User") + _git(repo, "add", ".") + _git(repo, "commit", "-m", "base") + + cancelled = repo / "CLAUDE.md" + cancelled.write_text("committed instructions\n", encoding="utf-8") + _git(repo, "add", "CLAUDE.md") + _git(repo, "commit", "-m", "add instructions") + cancelled.unlink() + + out_dir = repo / "agents-shipgate-reports" + verifier, report, _exit_code = run_verify( + workspace=repo, + config=Path("samples/support_refund_agent/shipgate.yaml"), + base="HEAD~1", + head="HEAD", + archive_head=False, + out=out_dir, + ci_mode="advisory", + fail_on=None, + baseline=None, + baseline_mode="new-findings", + diff_from=None, + policy_packs=None, + plugins_enabled=False, + strict_plugins=False, + suggest_patches=False, + no_heuristics=False, + verbose=False, + ) + + assert report is not None + assert verifier.changed_files == [] + assert (out_dir / "verification-input.diff").read_text(encoding="utf-8") == "" + plan = VerificationPlan.model_validate_json( + (out_dir / "verification-plan.json").read_text(encoding="utf-8") + ) + assert plan.inputs.changed_paths == [] + assert plan.inputs.options["worktree_overlay_paths"] == ["CLAUDE.md"] + assert plan.subject.git.worktree_overlay_sha256 is not None + + def test_verify_fails_closed_when_worktree_diff_cannot_be_collected(monkeypatch, tmp_path): repo = tmp_path / "repo" sample_dst = repo / "samples" / "support_refund_agent" From 6ad43a70d267e56c5e46a82df7f3457ab7853283 Mon Sep 17 00:00:00 2001 From: Pengfei Hu Date: Fri, 7 Aug 2026 22:00:49 -0700 Subject: [PATCH 2/3] Address verification diff review feedback --- CHANGELOG.md | 11 +++ src/agents_shipgate/cli/verification.py | 29 ++++---- src/agents_shipgate/cli/verify/git.py | 37 +++++----- .../cli/verify/orchestrator.py | 12 ++++ .../core/verification_identity.py | 2 +- tests/test_adapter_static_only.py | 4 +- tests/test_verification_git_snapshot.py | 72 +++++++++++++++++++ tests/test_verify_orchestrator.py | 63 +++++++++++++++- 8 files changed, 193 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 981918fb..d2a443f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,17 @@ ## Unreleased +- **Local verification now evaluates committed and uncommitted edits as one + effective worktree diff.** When a branch change and a review follow-up touch + the same path, `verify` compares the merge base directly with the current + worktree instead of concatenating overlapping diff records. The verification + plan separately binds the exact HEAD-relative overlay paths together with + their presence, content hashes, and Git file modes, so cancellations remain + reproducible even when they disappear from policy evaluation. Canceled + committed changes are called out in `base_notes`, worktree mode handling + honors repositories that set `core.fileMode=false`, and legacy plans that + predate mode-bound overlay identity fail with an explicit re-prepare action. + - **An unreadable PR diff is no longer reported as "nothing here is agent-related."** `verify --preview` collapsed every diff-acquisition failure into one message, then evaluated the trigger catalog against the empty inputs diff --git a/src/agents_shipgate/cli/verification.py b/src/agents_shipgate/cli/verification.py index 60a34f78..871017f5 100644 --- a/src/agents_shipgate/cli/verification.py +++ b/src/agents_shipgate/cli/verification.py @@ -24,6 +24,7 @@ tree_sha, validate_source_head_identity, working_tree_context, + working_tree_paths, ) from agents_shipgate.core.agent_handoff import build_agent_handoff from agents_shipgate.core.errors import ConfigError, InputParseError @@ -82,20 +83,20 @@ def prepare( worktree_overlay_paths: list[str] | None = None if head is not None: changed, diff_text = diff_context(root, base, head_ref) + elif base: + worktree_overlay_paths = working_tree_paths(root) + effective_base = require_merge_base_sha(root, base, head_ref) + changed, diff_text = working_tree_context( + root, + comparison_ref=effective_base, + reject_index_hidden=True, + ) else: worktree_overlay_paths, overlay_diff = working_tree_context( root, reject_index_hidden=True, ) - if base: - effective_base = require_merge_base_sha(root, base, head_ref) - changed, diff_text = working_tree_context( - root, - comparison_ref=effective_base, - reject_index_hidden=True, - ) - else: - changed, diff_text = worktree_overlay_paths, overlay_diff + changed, diff_text = worktree_overlay_paths, overlay_diff resolved_date = evaluation_date or commit_date(root, head_ref) config_relative = _under(root, config).relative_to(root) policy_paths = [_under(root, path) for path in policy_packs or []] @@ -529,9 +530,10 @@ def _validate_git_subject(plan: VerificationPlan, workspace: Path) -> None: raise ValueError("worker HEAD does not match the worktree-overlay plan") declared_overlay_paths = plan.inputs.options.get("worktree_overlay_paths") if declared_overlay_paths is None: - # Compatibility reader for plans produced before issue #336 split - # effective paths from HEAD-relative overlay paths. - overlay_paths = plan.inputs.changed_paths + raise ValueError( + "worktree-overlay plan predates overlay mode binding; " + "re-run `agents-shipgate verification prepare`" + ) elif not isinstance(declared_overlay_paths, list) or not all( isinstance(path, str) for path in declared_overlay_paths ): @@ -553,7 +555,8 @@ def _validate_git_subject(plan: VerificationPlan, workspace: Path) -> None: "sha256": sha256_file(candidate) if present else None, "git_mode": ( "100755" - if present and stat.S_IMODE(candidate.stat().st_mode) & 0o111 + if present + and stat.S_IMODE(candidate.stat().st_mode) & stat.S_IXUSR else "100644" if present else None diff --git a/src/agents_shipgate/cli/verify/git.py b/src/agents_shipgate/cli/verify/git.py index 14d3d412..6a9b2c80 100644 --- a/src/agents_shipgate/cli/verify/git.py +++ b/src/agents_shipgate/cli/verify/git.py @@ -197,39 +197,33 @@ class _UnavailableRevisionError(ConfigError): """A revision expression that names refs this checkout does not have.""" -_SAFE_DIFF_CONFIG = [ - "-c", +_SAFE_DIFF_SETTINGS = [ "core.fsmonitor=false", - "-c", "core.autocrlf=false", - "-c", "core.safecrlf=false", - "-c", "core.eol=lf", - "-c", "core.bigFileThreshold=32m", - "-c", "core.fileMode=false", - "-c", "core.precomposeUnicode=false", - "-c", "submodule.recurse=false", - "-c", "core.quotePath=false", - "-c", f"core.attributesFile={os.devnull}", - "-c", f"diff.orderFile={os.devnull}", - "-c", "diff.suppressBlankEmpty=false", - "-c", "diff.renameLimit=32767", ] -# Tree-to-worktree comparisons must retain Git's executable-bit semantics. -# The generic config disables file-mode reads for portable committed-tree -# comparisons; the later override is intentional and scoped to local overlay -# collection, whose exact mode is receipt-bound. -_SAFE_WORKTREE_DIFF_CONFIG = [*_SAFE_DIFF_CONFIG, "-c", "core.fileMode=true"] +_SAFE_DIFF_CONFIG = [ + argument for setting in _SAFE_DIFF_SETTINGS for argument in ("-c", setting) +] +# Worktree comparisons honor the repository's effective core.fileMode setting. +# Git writes it as false on filesystems that cannot preserve executable bits; +# forcing true there turns ordinary checkouts into whole-repository mode diffs. +_SAFE_WORKTREE_DIFF_CONFIG = [ + argument + for setting in _SAFE_DIFF_SETTINGS + if not setting.casefold().startswith("core.filemode=") + for argument in ("-c", setting) +] _DETERMINISTIC_DIFF_OPTIONS = [ "--no-ext-diff", "--no-textconv", @@ -811,13 +805,15 @@ def _reject_binary_capability_paths( revspec: str, *, pathspec: list[str] | None = None, + diff_config: list[str] | None = None, ) -> None: """Fail closed when a source-like path is hidden behind a binary marker.""" + effective_config = _SAFE_DIFF_CONFIG if diff_config is None else diff_config result = _run_git_bounded_result( workspace, [ - *_SAFE_DIFF_CONFIG, + *effective_config, "diff", *_DETERMINISTIC_DIFF_OPTIONS, "--no-renames", @@ -1217,6 +1213,7 @@ def working_tree_context( workspace, comparison_commit, pathspec=pathspec, + diff_config=_SAFE_WORKTREE_DIFF_CONFIG, ) except BinaryCapabilityDiffError as exc: exc.changed_paths = tuple(paths) diff --git a/src/agents_shipgate/cli/verify/orchestrator.py b/src/agents_shipgate/cli/verify/orchestrator.py index 0ac76e86..e79d5fc8 100644 --- a/src/agents_shipgate/cli/verify/orchestrator.py +++ b/src/agents_shipgate/cli/verify/orchestrator.py @@ -543,6 +543,18 @@ def run_verify( reject_index_hidden=True, ) ) + if committed_diff_complete: + cancelled_committed_paths = sorted( + set(worktree_overlay_paths) - set(worktree_paths) + ) + if cancelled_committed_paths: + count = len(cancelled_committed_paths) + noun = "change" if count == 1 else "changes" + verb = "is" if count == 1 else "are" + base_notes.append( + f"{count} committed {noun} {verb} canceled by uncommitted " + "worktree edits; the committed branch has not been verified." + ) changed_files = _bind_worktree_config_to_head( git_root=git_root, head=head, diff --git a/src/agents_shipgate/core/verification_identity.py b/src/agents_shipgate/core/verification_identity.py index a0241127..30d06b83 100644 --- a/src/agents_shipgate/core/verification_identity.py +++ b/src/agents_shipgate/core/verification_identity.py @@ -969,7 +969,7 @@ def _worktree_overlay(root: Path, paths: list[str]) -> list[dict[str, Any]]: "sha256": sha256_file(candidate) if present else None, "git_mode": ( "100755" - if captured_mode is not None and captured_mode & 0o111 + if captured_mode is not None and captured_mode & stat.S_IXUSR else "100644" if present else None diff --git a/tests/test_adapter_static_only.py b/tests/test_adapter_static_only.py index 7ec3d93e..d8467f2e 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=1809, + line=1806, snippet=( "subprocess.Popen(cmd, env=env, stderr=subprocess.PIPE, " "stdin=subprocess.PIPE if input is not None else " @@ -276,7 +276,7 @@ class AllowedException: AllowedException( relative_path="cli/verify/git.py", surface="attr_call:subprocess.run", - line=2053, + line=2050, snippet=( "subprocess.run(cmd, capture_output=capture_output, check=check, " "env=env, input=input, stderr=stderr, stdin=stdin, stdout=stdout, " diff --git a/tests/test_verification_git_snapshot.py b/tests/test_verification_git_snapshot.py index 039309ab..0f0e5401 100644 --- a/tests/test_verification_git_snapshot.py +++ b/tests/test_verification_git_snapshot.py @@ -13,6 +13,7 @@ working_tree_context, ) from agents_shipgate.core.errors import ConfigError +from agents_shipgate.core.verification_identity import _worktree_overlay def _git(root: Path, *args: str) -> None: @@ -184,3 +185,74 @@ def test_effective_worktree_diff_preserves_rename_and_mode_semantics( assert "rename to CLAUDE.md" in diff_text assert "old mode 100644" in diff_text assert "new mode 100755" in diff_text + + +def test_effective_worktree_diff_honors_repository_filemode_false( + tmp_path: Path, +) -> None: + root = _repo(tmp_path) + target = root / "lib" / "util.py" + target.parent.mkdir() + target.write_text("base\n", encoding="utf-8") + manifest = root / "shipgate.yaml" + manifest.write_text("version: '0.1'\n", encoding="utf-8") + _git(root, "add", ".") + _git(root, "commit", "-m", "base") + + _git(root, "config", "core.fileMode", "false") + target.write_text("committed change\n", encoding="utf-8") + _git(root, "add", "lib/util.py") + _git(root, "commit", "-m", "change utility") + target.chmod(0o755) + manifest.chmod(0o755) + + changed, diff_text = working_tree_context( + root, + comparison_ref="HEAD~1", + reject_index_hidden=True, + ) + + assert changed == ["lib/util.py"] + assert "shipgate.yaml" not in diff_text + assert "old mode 100644" not in diff_text + assert "new mode 100755" not in diff_text + + +def test_effective_worktree_diff_retains_merge_base_relative_untracked_paths( + tmp_path: Path, +) -> None: + root = _repo(tmp_path) + tracked = root / "agent.py" + tracked.write_text("base\n", encoding="utf-8") + _git(root, "add", "agent.py") + _git(root, "commit", "-m", "base") + + tracked.write_text("committed\n", encoding="utf-8") + _git(root, "add", "agent.py") + _git(root, "commit", "-m", "change agent") + untracked = root / "new_agent.py" + untracked.write_text("untracked capability\n", encoding="utf-8") + + changed, diff_text = working_tree_context( + root, + comparison_ref="HEAD~1", + reject_index_hidden=True, + ) + + assert changed == ["agent.py", "new_agent.py"] + assert "diff --git a/agent.py b/agent.py" in diff_text + assert "new_agent.py" not in diff_text + + +def test_worktree_overlay_mode_uses_git_owner_execute_semantics(tmp_path: Path) -> None: + root = _repo(tmp_path) + target = root / "agent.py" + target.write_text("capability\n", encoding="utf-8") + target.chmod(0o654) + + [group_executable] = _worktree_overlay(root, ["agent.py"]) + assert group_executable["git_mode"] == "100644" + + target.chmod(0o754) + [owner_executable] = _worktree_overlay(root, ["agent.py"]) + assert owner_executable["git_mode"] == "100755" diff --git a/tests/test_verify_orchestrator.py b/tests/test_verify_orchestrator.py index 508452f4..bab87ba4 100644 --- a/tests/test_verify_orchestrator.py +++ b/tests/test_verify_orchestrator.py @@ -10,7 +10,7 @@ import pytest from agents_shipgate.cli.scan import writing as scan_writing -from agents_shipgate.cli.verification import assemble, worker +from agents_shipgate.cli.verification import _validate_git_subject, assemble, worker from agents_shipgate.cli.verify import orchestrator as verify_orchestrator from agents_shipgate.cli.verify.git import commit_date from agents_shipgate.cli.verify.orchestrator import run_verify @@ -683,6 +683,67 @@ def test_worktree_overlay_identity_preserves_an_effectively_cancelled_path( assert plan.inputs.changed_paths == [] assert plan.inputs.options["worktree_overlay_paths"] == ["CLAUDE.md"] assert plan.subject.git.worktree_overlay_sha256 is not None + assert any( + "1 committed change is canceled by uncommitted worktree edits" in note + for note in verifier.base_notes + ) + + legacy_options = dict(plan.inputs.options) + legacy_options.pop("worktree_overlay_paths") + legacy_plan = plan.model_copy( + update={ + "inputs": plan.inputs.model_copy(update={"options": legacy_options}) + } + ) + with pytest.raises(ValueError, match="predates overlay mode binding"): + _validate_git_subject(legacy_plan, repo) + + +def test_verify_fail_closes_on_merge_base_relative_untracked_trust_root( + tmp_path: Path, +) -> None: + repo = tmp_path / "repo" + sample_dst = repo / "samples" / "support_refund_agent" + sample_dst.parent.mkdir(parents=True) + shutil.copytree(REPO_ROOT / "samples" / "support_refund_agent", sample_dst) + _git(repo, "init") + _git(repo, "config", "user.email", "test@example.test") + _git(repo, "config", "user.name", "Test User") + _git(repo, "add", ".") + _git(repo, "commit", "-m", "base") + + (repo / "notes.txt").write_text("committed branch change\n", encoding="utf-8") + _git(repo, "add", "notes.txt") + _git(repo, "commit", "-m", "branch change") + (repo / "AGENTS.md").write_text("untracked instructions\n", encoding="utf-8") + + verifier, report, _exit_code = run_verify( + workspace=repo, + config=Path("samples/support_refund_agent/shipgate.yaml"), + base="HEAD~1", + head="HEAD", + archive_head=False, + out=repo / "agents-shipgate-reports", + ci_mode="advisory", + fail_on=None, + baseline=None, + baseline_mode="new-findings", + diff_from=None, + policy_packs=None, + plugins_enabled=False, + strict_plugins=False, + suggest_patches=False, + no_heuristics=False, + verbose=False, + ) + + assert report is not None + assert "AGENTS.md" in verifier.changed_files + assert any( + finding.check_id == "SHIP-VERIFY-TRUST-ROOT-TOUCHED" + and finding.evidence.get("changed_file") == "AGENTS.md" + for finding in report.findings + ) def test_verify_fails_closed_when_worktree_diff_cannot_be_collected(monkeypatch, tmp_path): From 2e882ee858f65755d7682c3dc0960a0537597c15 Mon Sep 17 00:00:00 2001 From: Pengfei Hu Date: Sat, 8 Aug 2026 10:07:03 -0700 Subject: [PATCH 3/3] test: cover a cancelled overlay path that is still present MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The merge with #332 exposed a semantic conflict the suite could not see. #332 makes the static input snapshot report a path it contains but never read as absent, and this branch's overlay set is HEAD-relative while the change set is merge-base-relative — so a cancelled-but-present path is bound by neither unless both binding sites take the union. The existing cancellation test cancels by deleting the file, so the producer records "deleted" either way and the corruption is invisible. This one cancels by restoring merge-base content, then calls _validate_git_subject: it fails with "worker worktree overlay does not match the plan" if either binding site drops back to changed_files. --- tests/test_verify_orchestrator.py | 63 +++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/tests/test_verify_orchestrator.py b/tests/test_verify_orchestrator.py index bab87ba4..3de97776 100644 --- a/tests/test_verify_orchestrator.py +++ b/tests/test_verify_orchestrator.py @@ -699,6 +699,69 @@ def test_worktree_overlay_identity_preserves_an_effectively_cancelled_path( _validate_git_subject(legacy_plan, repo) +def test_worktree_overlay_binds_a_cancelled_path_that_is_still_present( + tmp_path: Path, +) -> None: + """A canceled path leaves the policy diff but stays bound at its real bytes. + + The overlay set is HEAD-relative while the change set is merge-base- + relative, so a canceled path exists only in the former. Plan construction + runs under the static input snapshot, which reports a path it contains but + never read as absent — binding only the change set would attest a present + file as ``deleted`` and make the worker reject the plan the producer just + wrote. The sibling test above cancels by deleting, which cannot catch this. + """ + + repo = tmp_path / "repo" + sample_dst = repo / "samples" / "support_refund_agent" + sample_dst.parent.mkdir(parents=True) + shutil.copytree(REPO_ROOT / "samples" / "support_refund_agent", sample_dst) + cancelled = repo / "KEEP.md" + cancelled.write_text("original\n", encoding="utf-8") + _git(repo, "init") + _git(repo, "config", "user.email", "test@example.test") + _git(repo, "config", "user.name", "Test User") + _git(repo, "add", ".") + _git(repo, "commit", "-m", "base") + + cancelled.write_text("committed change\n", encoding="utf-8") + _git(repo, "add", "KEEP.md") + _git(repo, "commit", "-m", "change keep") + cancelled.write_text("original\n", encoding="utf-8") + + out_dir = repo / "agents-shipgate-reports" + verifier, report, _exit_code = run_verify( + workspace=repo, + config=Path("samples/support_refund_agent/shipgate.yaml"), + base="HEAD~1", + head="HEAD", + archive_head=False, + out=out_dir, + ci_mode="advisory", + fail_on=None, + baseline=None, + baseline_mode="new-findings", + diff_from=None, + policy_packs=None, + plugins_enabled=False, + strict_plugins=False, + suggest_patches=False, + no_heuristics=False, + verbose=False, + ) + + assert report is not None + assert verifier.changed_files == [] + plan = VerificationPlan.model_validate_json( + (out_dir / "verification-plan.json").read_text(encoding="utf-8") + ) + assert plan.inputs.changed_paths == [] + assert plan.inputs.options["worktree_overlay_paths"] == ["KEEP.md"] + # Raises unless the producer recorded the still-present canceled file at its + # real content, exactly as the worker recomputes it from the filesystem. + _validate_git_subject(plan, repo) + + def test_verify_fail_closes_on_merge_base_relative_untracked_trust_root( tmp_path: Path, ) -> None: