diff --git a/CHANGELOG.md b/CHANGELOG.md index 47c14f765..f48a8622f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,13 @@ Semantic Versioning where the repository publishes a release. ### Fixed +- Keep the post-merge replay guard focused on lost protected-base evidence: a + feature-only source and its same-subject test may be removed together when + both are absent from the exact protected base, while protected-base test + deletion, test-only deletion, weakened retained tests, exact tree replay, + targeted base unmerge, and bulk regression remain fail-closed. Declare pip as + a development dependency because the materialized-lock regression invokes + its hash preflight inside the project-local uv environment. - Used the receiving repository's workflow token for same-repository scheduler Actions inventory and read calls, while retaining the established mutation credential chain. An exhausted organization-wide OpenCode App installation diff --git a/docs/doctoring/pr-head-replay-base-alignment.md b/docs/doctoring/pr-head-replay-base-alignment.md new file mode 100644 index 000000000..3774a775c --- /dev/null +++ b/docs/doctoring/pr-head-replay-base-alignment.md @@ -0,0 +1,70 @@ +# PR-head replay guard: protected-base alignment + +## Observed failure + +LineageWeave pull request 258 exact head +`6dc040c6b3ea0bfc4424bb7afb11b8afd7205d77` failed central OpenCode coverage +run `32528869351`. The replay guard found that five commits after the newest +protected-base-descended merge removed three files, including +`tests/test_lineage_contract.py`. + +The exact protected base +`ef6f5a5ffcb467bd935dc1e53acc0029669b0bd7` already omitted both that test and +its same-subject runtime module. The head also omitted both. No path had been +restored to pre-merge content, the head did not replay an earlier tree, and the +three-file delta stayed below the conservative bulk-replay threshold. The +test-regression signal nevertheless treated every post-merge test deletion as +protected-base evidence loss. + +## Decision and security boundary + +The guard now distinguishes one base-alignment case from an evidence +regression. A test absent from the exact protected base is not reported as +regressed only when a non-test file with the same logical subject and language +suffix is deleted in the same post-merge range and that source is also absent +from the protected base, with no same-subject source remaining in the exact +head. This admits removal of a feature-only source/test pair that aligns the +head with the protected branch without letting a same-named documentation or +sibling-source deletion excuse a code-test deletion. + +The following cases remain blocking: + +- a test or same-subject source present on the protected base is removed; +- a feature-only test is removed while its source remains; +- a retained test file loses declared test cases without a replacement test + file; +- the head exactly replays a pre-merge tree, restores a path to its pre-merge + content, or crosses the conservative bulk-deletion thresholds. + +This boundary follows NIST SSDF's root-cause and verification practices while +preserving GitHub's guidance that privileged pull-request workflows must treat +pull-request state as untrusted input. The decision is based only on immutable +Git objects from the validated base, merge anchor, and head; no pull-request +code executes during classification. + +## Verification + +- A new Git-history regression reproduces a feature-only source/test pair, + merges the exact protected base, removes the pair, and requires a passing + replay decision. +- Existing regressions continue to require failure for protected-base source + and test removal, test-only deletion, weakened retained tests, exact replay, + targeted unmerge, and bulk deletion. +- The patched guard passes against the observed LineageWeave base/head pair and + reports no regressed protected-base test path. +- The focused replay-guard suite, complete central Python quality suite, + docstring coverage, workflow validation, and diff hygiene run on the final + tree. +- The project-local uv environment declares pip because the existing bounded + include regression invokes `python -m pip` for its hash preflight; verification + no longer depends on an unrecorded manual venv seed. + +## References + +GitHub. (n.d.). *Secure use reference*. GitHub Docs. Retrieved August 22, 2026, +from https://docs.github.com/en/actions/reference/security/secure-use + +Souppaya, M., Scarfone, K., & Dodson, D. (2022). *Secure software development +framework (SSDF) version 1.1: Recommendations for mitigating the risk of +software vulnerabilities* (NIST Special Publication 800-218). National +Institute of Standards and Technology. https://doi.org/10.6028/NIST.SP.800-218 diff --git a/pyproject.toml b/pyproject.toml index 1954a2aaf..bfa28f150 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -6,6 +6,7 @@ dependencies = [] [dependency-groups] dev = [ + "pip>=25.0", "pytest>=8.0.0", "pytest-cov>=7.1.0", "interrogate>=1.7.0", diff --git a/scripts/ci/pr_head_replay_guard.py b/scripts/ci/pr_head_replay_guard.py index 9c2efd941..8875834e6 100644 --- a/scripts/ci/pr_head_replay_guard.py +++ b/scripts/ci/pr_head_replay_guard.py @@ -16,8 +16,10 @@ merge brought in (observed in appguardrail#297, where a stale snapshot reverted an accessibility wrapper and deleted its regression tests in a push far below the bulk thresholds); -- test regression without replacement: post-merge commits deleting test files - or reducing declared test cases while adding no replacement test file; +- test regression without replacement or protected-base alignment: post-merge + commits deleting test files or reducing declared test cases while adding no + replacement test file, except when a feature-only test and its same-subject, + same-language source are both retired to match the protected base; - the conservative bulk-regression signature: at least five tracked files and 500 lines removed, with deletions at least four times additions. @@ -91,7 +93,7 @@ def unmerges_base_work(self) -> bool: @property def suspicious_test_regression(self) -> bool: - """Return whether test cases were lost with no replacement test file.""" + """Return whether non-base-aligned tests were lost without replacement.""" return bool(self.regressed_test_paths) and self.added_test_files == 0 @property @@ -188,6 +190,20 @@ def is_test_path(path: str) -> bool: ) +def logical_test_subject(path: str) -> str: + """Return the source-like stem represented by a common test filename.""" + name = Path(path).name + for marker in (".test.", ".spec."): + if marker in name: + return name.split(marker, 1)[0] + stem = Path(name).stem + if stem.startswith("test_"): + return stem.removeprefix("test_") + if stem.endswith("_test"): + return stem.removesuffix("_test") + return stem + + def changed_paths(repo_root: Path, start: str, end: str) -> set[str]: """Return the set of paths whose content differs between two commits.""" output = git_output(repo_root, ["diff", "--name-only", start, end]) @@ -234,16 +250,64 @@ def test_case_count( return len(pattern.findall(source)) if pattern is not None else None -def test_file_changes(repo_root: Path, start: str, end: str) -> tuple[tuple[str, ...], int]: - """Return deleted or test-case-reducing paths and the added-test count.""" +def test_file_changes( + repo_root: Path, + start: str, + end: str, + protected_revision: str, +) -> tuple[tuple[str, ...], int]: + """Return non-base-aligned test regressions and the added-test count.""" + protected_paths = set( + git_output( + repo_root, + ["ls-tree", "-r", "--name-only", protected_revision], + ).splitlines() + ) + head_source_subjects: set[tuple[str, str]] = set() + for path in git_output( + repo_root, + ["ls-tree", "-r", "--name-only", end], + ).splitlines(): + if not is_test_path(path): + head_source_subjects.add( + (logical_test_subject(path), Path(path).suffix.lower()) + ) regressed: set[str] = set() added = 0 - for line in git_output(repo_root, ["diff", "--name-status", start, end]).splitlines(): + name_status_lines = git_output( + repo_root, + ["diff", "--name-status", start, end], + ).splitlines() + deleted_paths: set[str] = set() + removed_feature_sources: set[tuple[str, str]] = set() + for line in name_status_lines: + fields = line.split("\t") + if len(fields) < 2 or not fields[0].startswith("D"): + continue + deleted_paths.add(fields[-1]) + source_subject = ( + logical_test_subject(fields[-1]), + Path(fields[-1]).suffix.lower(), + ) + if ( + not is_test_path(fields[-1]) + and fields[-1] not in protected_paths + and source_subject not in head_source_subjects + ): + removed_feature_sources.add(source_subject) + for line in name_status_lines: fields = line.split("\t") if len(fields) < 2 or not is_test_path(fields[-1]): continue status = fields[0][:1] - if status == "D": + if status == "D" and ( + fields[-1] in protected_paths + or ( + logical_test_subject(fields[-1]), + Path(fields[-1]).suffix.lower(), + ) + not in removed_feature_sources + ): regressed.add(fields[-1]) elif status == "A": added += 1 @@ -252,7 +316,11 @@ def test_file_changes(repo_root: Path, start: str, end: str) -> tuple[tuple[str, if len(fields) < 3 or not fields[0].isdigit() or not fields[1].isdigit(): continue path = fields[2] - if not is_test_path(path) or int(fields[1]) <= int(fields[0]): + if ( + not is_test_path(path) + or path in deleted_paths + or int(fields[1]) <= int(fields[0]) + ): continue before_count = test_case_count(repo_root, start, path) after_count = test_case_count(repo_root, end, path) @@ -293,7 +361,12 @@ def collect_evidence(repo_root: Path, base_sha: str, head_sha: str) -> ReplayEvi head_sha, ) unmerged = unmerged_base_paths(repo_root, merge_anchor, head_sha) - regressed_tests, added_tests = test_file_changes(repo_root, merge_anchor, head_sha) + regressed_tests, added_tests = test_file_changes( + repo_root, + merge_anchor, + head_sha, + base_sha, + ) return ReplayEvidence( base_sha=base_sha, head_sha=head_sha, @@ -336,7 +409,8 @@ def format_report(evidence: ReplayEvidence) -> str: ) if evidence.suspicious_test_regression: reasons.append( - "post-merge commits deleted test files or reduced declared test cases " + "post-merge commits deleted protected-base or unpaired feature test files, " + "or reduced declared test cases " "without adding any replacement test file: " f"{summarize_paths(evidence.regressed_test_paths)}." ) diff --git a/tests/test_pr_head_replay_guard.py b/tests/test_pr_head_replay_guard.py index e5885f422..e18ba8792 100644 --- a/tests/test_pr_head_replay_guard.py +++ b/tests/test_pr_head_replay_guard.py @@ -256,6 +256,73 @@ def test_test_refactor_with_replacement_passes(tmp_path): assert not evidence.blocked +def test_removing_feature_only_test_to_match_current_base_passes(tmp_path): + """Deleting a test absent from the protected base is not a base replay.""" + repo = tmp_path / "repo" + repo.mkdir() + git(repo, "init", "-b", "main") + git(repo, "config", "user.name", "Test") + git(repo, "config", "user.email", "test@example.com") + write(repo, "README.md", "base\n") + commit(repo, "base") + + git(repo, "checkout", "-b", "feature") + write(repo, "src/legacy.py", "def legacy():\n return True\n") + write(repo, "tests/test_legacy.py", "def test_legacy():\n assert True\n") + commit(repo, "feature adds legacy code") + + git(repo, "checkout", "main") + write(repo, "base-change.txt", "current base\n") + current_base = commit(repo, "advance base without legacy code") + + git(repo, "checkout", "feature") + git(repo, "merge", "--no-ff", "--no-edit", "main") + (repo / "src/legacy.py").unlink() + (repo / "tests/test_legacy.py").unlink() + git(repo, "add", "-A") + head = commit(repo, "align feature with protected base") + + evidence = guard.collect_evidence(repo, current_base, head) + + assert evidence.regressed_test_paths == () + assert evidence.unmerged_paths == () + assert not evidence.blocked + + +def test_removing_same_named_sibling_source_does_not_excuse_test_loss(tmp_path): + """A retained same-subject source keeps its feature test protected.""" + repo = tmp_path / "repo" + repo.mkdir() + git(repo, "init", "-b", "main") + git(repo, "config", "user.name", "Test") + git(repo, "config", "user.email", "test@example.com") + write(repo, "README.md", "base\n") + commit(repo, "base") + + git(repo, "checkout", "-b", "feature") + write(repo, "src/primary/legacy.py", "def legacy():\n return True\n") + write(repo, "src/obsolete/legacy.py", "def legacy():\n return False\n") + write(repo, "tests/test_legacy.py", "def test_legacy():\n assert True\n") + commit(repo, "feature adds same-subject sources") + + git(repo, "checkout", "main") + write(repo, "base-change.txt", "current base\n") + current_base = commit(repo, "advance base") + + git(repo, "checkout", "feature") + git(repo, "merge", "--no-ff", "--no-edit", "main") + (repo / "src/obsolete/legacy.py").unlink() + (repo / "tests/test_legacy.py").unlink() + git(repo, "add", "-A") + head = commit(repo, "remove sibling and test") + + evidence = guard.collect_evidence(repo, current_base, head) + + assert evidence.regressed_test_paths == ("tests/test_legacy.py",) + assert evidence.suspicious_test_regression + assert evidence.blocked + + def test_is_test_path_covers_common_layouts(): """Test-path detection recognizes directories, prefixes, suffixes, and spec names.""" assert guard.is_test_path("tests/test_guard.py") @@ -267,6 +334,11 @@ def test_is_test_path_covers_common_layouts(): assert guard.is_test_path("tests\\test_windows.py") assert not guard.is_test_path("scripts/ci/guard.py") assert not guard.is_test_path("docs/testing.md") + assert guard.logical_test_subject("tests/test_guard.py") == "guard" + assert guard.logical_test_subject("pkg/guard_test.go") == "guard" + assert guard.logical_test_subject("app/button.test.tsx") == "button" + assert guard.logical_test_subject("app/button.spec.ts") == "button" + assert guard.logical_test_subject("tests/README.md") == "README" def test_test_case_count_fails_closed_and_supports_known_formats( @@ -343,6 +415,7 @@ def test_test_file_changes_parses_status_and_numstat(monkeypatch, tmp_path): "D\ttests/test_gone.py", "A\ttests/test_new.py", "M\ttests/test_kept.py", + "D\tdocs/gone.md", "D\tsrc/app_old.py", "badline", ] @@ -355,7 +428,14 @@ def test_test_file_changes_parses_status_and_numstat(monkeypatch, tmp_path): "2\t9\tsrc/big.py", ] ) - outputs = iter([name_status, numstat]) + outputs = iter( + [ + "tests/test_shrunk.py", + "src/kept.py", + name_status, + numstat, + ] + ) monkeypatch.setattr(guard, "git_output", lambda _root, _args: next(outputs)) monkeypatch.setattr( guard, @@ -363,7 +443,7 @@ def test_test_file_changes_parses_status_and_numstat(monkeypatch, tmp_path): lambda _root, revision, _path: 2 if revision == "a" else 1, ) - regressed, added = guard.test_file_changes(tmp_path, "a", "b") + regressed, added = guard.test_file_changes(tmp_path, "a", "b", "protected") assert regressed == ("tests/test_gone.py", "tests/test_shrunk.py") assert added == 1