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 

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

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

73 } 

74 if collisions: 

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

76 return _report( 

77 "drift", 

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

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

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

81 landed_count=len(landed_set), 

82 ) 

83 if intended is not None: 

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

85 if unexpected: 

86 return _report( 

87 "out-of-scope", 

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

89 "diff did not list", 

90 unexpected=unexpected, 

91 landed_count=len(landed_set), 

92 ) 

93 return _report( 

94 "clean", 

95 "no file in this merge was changed by another pull request after this one 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 # Present on every report, so a consumer never has to distinguish 

109 # "complete" from "this key was added later". Set by the caller when a 

110 # finding is kept despite an input it could not read. 

111 "incomplete": False, 

112 } 

113 report.update(extra) 

114 return report 

115 

116 

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

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

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

120 

121 

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

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

124 lines = [ 

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

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

127 ] 

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

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

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

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

132 return "\n".join(lines)