Coverage for src/keel/mergeverify.py: 100%

1"""Did the merge apply what was reviewed? — the pure comparison (issue #561). 

2 

3`keel merge` proves a merge *succeeded*. It does not prove the merge applied the 

4diff that was reviewed, and those came apart twice in one day while shipping 

51.8.1/1.8.2: a `gh api …/update-branch` merge commit followed by a GitHub 

6squash-merge silently reverted unrelated already-merged work. Neither revert was 

7caught by CI, because the reverted state was internally consistent — old code with 

8no test for the removed behaviour — so the suite stayed green throughout. 

9 

10**Why the obvious check does not work.** The first version of this module compared 

11the PR's file set against the merge's file set, on the theory that a revert shows up 

12as the merge touching files the PR never did. Run against the actual incident 

13(#543's squash reverting #550) it reported **clean**, and the reason is the whole 

14point: `update-branch` had already pulled the reverting state into the branch, so 

15GitHub computed the PR's own diff — the thing a reviewer reads — as *including* 

16those files. The revert was inside the reviewed diff. Scope comparison cannot see it. 

17 

18**What does work** is the timing fingerprint the incident actually leaves: 

19 

20* #550 merged at 13:02, touching ``src/keel/github.py`` and three others; 

21* #543 had branched on the 8th, **before** that; 

22* #543 merged at 15:19 and its commit **removed 41 lines** from ``github.py`` — 

23 a file a "label the search input" change has no reason to touch at all. 

24 

25So the question to ask is: *did this merge write to files that some other pull 

26request changed after this one branched?* That is the only way an update-branch 

27squash can undo merged work, and it is cheap to answer. It is a **look at this** 

28signal rather than a proof — two PRs editing one file in sequence is ordinary — so 

29the report names the overtaking PR for each file and lets a human judge. 

30 

31Pure: lists in, report out. The CLI does every GitHub read. 

32""" 

33 

34from __future__ import annotations 

35 

36from collections.abc import Mapping, Sequence 

37 

38#: Report shape version, so a consumer can tell an old report from a new one. 

39SCHEMA_VERSION = "keel.merge-verify.v1" 

40 

41 

42def verify_merge( 

43 landed: Sequence[str] | None, 

44 overtaken: Mapping[str, int] | None = None, 

45 intended: Sequence[str] | None = None, 

46) -> dict: 

47 """Judge whether a merge may have silently reverted other merged work. 

48 

49 ``landed`` is what the merge commit changed. ``overtaken`` maps a path to the 

50 pull request that changed it **after this PR branched and before this PR 

51 merged** — the window in which a stale branch can carry a revert. ``intended`` 

52 is the PR's own file list, used only for the weaker secondary signal. 

53 

54 ``None`` for ``landed`` means *not observed* and yields ``unknown`` rather than 

55 a clean bill: failing to look is not evidence that nothing drifted. 

56 

57 ``status``: 

58 

59 * ``drift`` — the merge wrote to files another PR changed after this one 

60 branched. The silent-revert shape; loud, and names the overtaking PR. 

61 * ``out-of-scope`` — no overtaking, but the merge changed files the PR's own 

62 diff did not list. A different (rarer) way for a merge to do more than it said. 

63 * ``clean`` — neither. 

64 * ``unknown`` — nothing could be read. 

65 """ 

66 if landed is None: 

67 return _report("unknown", "could not read the merge commit's file list from GitHub") 

68 landed_set = {p.strip() for p in landed if p.strip()} 

69 collisions = { 

70 path: pr for path, pr in (overtaken or {}).items() 

71 if path.strip() and path.strip() in landed_set 

72 } 

73 if collisions: 

74 listed = ", ".join(f"{p} (#{pr})" for p, pr in sorted(collisions.items())) 

75 return _report( 

76 "drift", 

77 f"the merge wrote to {len(collisions)} file(s) that another pull request " 

78 f"changed after this one branched — the shape of a silent revert: {listed}", 

79 overtaken=dict(sorted(collisions.items())), 

80 landed_count=len(landed_set), 

81 ) 

82 if intended is not None: 

83 unexpected = sorted(landed_set - {p.strip() for p in intended if p.strip()}) 

84 if unexpected: 

85 return _report( 

86 "out-of-scope", 

87 f"the merge changed {len(unexpected)} file(s) the pull request's own " 

88 "diff did not list", 

89 unexpected=unexpected, 

90 landed_count=len(landed_set), 

91 ) 

92 return _report( 

93 "clean", 

94 "no file in this merge was changed by another pull request after this one " 

95 "branched", 

96 landed_count=len(landed_set), 

97 ) 

98 

99 

100def _report(status: str, reason: str, **extra) -> dict: 

101 report = { 

102 "schema_version": SCHEMA_VERSION, 

103 "status": status, 

104 "reason": reason, 

105 "overtaken": {}, 

106 "unexpected": [], 

107 "landed_count": 0, 

108 } 

109 report.update(extra) 

110 return report 

111 

112 

113def is_drift(report: dict) -> bool: 

114 """True when the report is the loud case a human must look at.""" 

115 return isinstance(report, dict) and report.get("status") == "drift" 

116 

117 

118def render(report: dict) -> str: 

119 """Human-readable one-block summary.""" 

120 lines = [ 

121 f"keel verify-merge — {report.get('status', 'unknown')}", 

122 f" {report.get('reason', '')}", 

123 ] 

124 for path, pr in (report.get("overtaken") or {}).items(): 

125 lines.append(f" {path} — also changed by #{pr} after this PR branched") 

126 for path in report.get("unexpected") or []: 

127 lines.append(f" {path} — not in the PR's own diff") 

128 return "\n".join(lines)