gabriel / muse public
test_anon_commit_bug.py python
342 lines 13.4 KB
Raw
sha256:51ce277f663e01a43eaffbe77509b1de7ac2d4251b55d23306304bcdeb92c90d feat(pack): delta-encode snapshots in MPackBundle wire format Sonnet 4.6 minor ⚠ breaking 121 days ago
1 """TDD — merge/revert/cherry-pick/pull/rebase commits must carry author attribution.
2
3 Bug
4 ---
5 Several commands that create new commits omit the ``author`` field from both
6 ``compute_commit_id()`` and the resulting ``CommitRecord``. The hash covers
7 ``author=""`` while the intent is to record who performed the operation. Any
8 later call to ``_verify_commit_id`` that uses the stored (empty) author field
9 stays consistent, but the data is semantically wrong — the commit is anonymous.
10
11 The fix for each command is the same:
12
13 1. Read ``user.handle`` from config (``get_config_value("user.handle", root)``).
14 2. Pass it as ``author`` to ``compute_commit_id()`` so the hash covers it.
15 3. Pass the same value to ``CommitRecord(author=...)`` so hash and stored
16 field always agree.
17
18 Affected commands
19 -----------------
20 A1 ``muse merge`` — merge commit should be authored by the merger
21 A2 ``muse revert`` — revert commit should be authored by the reverter
22 A3 ``muse cherry-pick`` — preserves original commit's author
23 A4 ``muse pull`` (merge) — auto-merge commit should be authored by the puller
24 A5 ``muse rebase --squash`` — squash commit preserves first original author
25 """
26 from __future__ import annotations
27
28 import datetime
29 import json
30 import pathlib
31
32 import pytest
33 from tests.cli_test_helper import CliRunner
34 from muse.core.paths import heads_dir, ref_path, muse_dir
35 from muse.core.types import fake_id, blob_id, Manifest
36 from muse.core.object_store import object_path
37 from muse.core.workdir import apply_manifest
38
39 cli = None
40 runner = CliRunner()
41
42 _HANDLE = "gabriel"
43
44
45 # ---------------------------------------------------------------------------
46 # Shared helpers
47 # ---------------------------------------------------------------------------
48
49 type _Env = dict[str, str]
50
51
52 def _env(root: pathlib.Path) -> _Env:
53 return {"MUSE_REPO_ROOT": str(root)}
54
55
56 def _init_repo(tmp_path: pathlib.Path) -> tuple[pathlib.Path, str]:
57 dot_muse = muse_dir(tmp_path)
58 dot_muse.mkdir()
59 repo_id = fake_id("repo")
60 (dot_muse / "repo.json").write_text(json.dumps({
61 "repo_id": repo_id,
62 "domain": "code",
63 "default_branch": "main",
64 "created_at": "2025-01-01T00:00:00+00:00",
65 }), encoding="utf-8")
66 (dot_muse / "HEAD").write_text("ref: refs/heads/main", encoding="utf-8")
67 (dot_muse / "refs" / "heads").mkdir(parents=True)
68 (dot_muse / "snapshots").mkdir()
69 (dot_muse / "commits").mkdir()
70 (dot_muse / "objects").mkdir()
71 return tmp_path, repo_id
72
73
74 _HUB_URL = "https://localhost:1337"
75
76
77 _HUB_URL = "https://localhost:1337"
78
79
80 def _set_user_handle(root: pathlib.Path, handle: str) -> None:
81 """Wire up a minimal identity so get_config_value("user.handle", root) returns handle."""
82 from muse.core.identity import save_identity
83 from muse.cli.config import set_hub_url
84
85 # Point repo config at the fake hub so get_config_value can resolve the hostname.
86 set_hub_url(_HUB_URL, root)
87
88 # Write a minimal identity entry to the (already-patched-to-tmp) identity.toml.
89 save_identity(_HUB_URL, {
90 "type": "human",
91 "handle": handle,
92 "algorithm": "ed25519",
93 "fingerprint": "0" * 64,
94 "capabilities": [],
95 "provisioned_by": "",
96 "hd_path": "",
97 "provisioned_by_fingerprint": "",
98 })
99
100
101 def _current_head_branch(root: pathlib.Path) -> str:
102 head = (muse_dir(root) / "HEAD").read_text().strip()
103 if head.startswith("ref: refs/heads/"):
104 return head[len("ref: refs/heads/"):]
105 return ""
106
107
108 def _make_commit(
109 root: pathlib.Path,
110 repo_id: str,
111 branch: str = "main",
112 message: str = "test",
113 manifest: dict[str, str] | None = None,
114 author: str = "",
115 prev_manifest: Manifest | None = None,
116 ) -> str:
117 from muse.core.store import CommitRecord, SnapshotRecord, write_commit, write_snapshot
118 from muse.core.snapshot import compute_snapshot_id, compute_commit_id
119
120 ref_file = ref_path(root, branch)
121 parent_id = ref_file.read_text().strip() if ref_file.exists() else None
122 m = manifest or {}
123 snap_id = compute_snapshot_id(m)
124 committed_at = datetime.datetime.now(datetime.timezone.utc)
125 commit_id = compute_commit_id(
126 parent_ids=[parent_id] if parent_id else [],
127 snapshot_id=snap_id,
128 message=message,
129 committed_at_iso=committed_at.isoformat(),
130 author=author,
131 )
132 write_snapshot(root, SnapshotRecord(snapshot_id=snap_id, manifest=m))
133 write_commit(root, CommitRecord(
134 commit_id=commit_id,
135 repo_id=repo_id,
136 branch=branch,
137 snapshot_id=snap_id,
138 message=message,
139 committed_at=committed_at,
140 parent_commit_id=parent_id,
141 author=author,
142 ))
143 ref_file.parent.mkdir(parents=True, exist_ok=True)
144 ref_file.write_text(commit_id, encoding="utf-8")
145 # Only sync the working tree when this commit is on the current HEAD branch.
146 # Commits on other branches must not disturb the working tree — the dirty-
147 # tree guard would otherwise fire when the command runs against HEAD.
148 if branch == _current_head_branch(root):
149 apply_manifest(root, prev_manifest if prev_manifest is not None else {}, m)
150 return commit_id
151
152
153 def _switch_branch(root: pathlib.Path, branch: str) -> None:
154 """Point HEAD at branch and sync the working tree to its tip."""
155 from muse.core.store import read_commit, read_snapshot
156 (muse_dir(root) / "HEAD").write_text(f"ref: refs/heads/{branch}", encoding="utf-8")
157 ref_file = ref_path(root, branch)
158 if not ref_file.exists():
159 return
160 commit = read_commit(root, ref_file.read_text().strip())
161 if commit is None:
162 return
163 snap = read_snapshot(root, commit.snapshot_id)
164 target: Manifest = dict(snap.manifest) if snap else {}
165 # Determine current on-disk state by reading the previous branch's snapshot.
166 apply_manifest(root, {}, target)
167
168
169 def _write_object(root: pathlib.Path, content: bytes) -> str:
170 oid = blob_id(content)
171 p = object_path(root, oid)
172 p.parent.mkdir(parents=True, exist_ok=True)
173 p.write_bytes(content)
174 return oid
175
176
177 def _head_commit(root: pathlib.Path, branch: str = "main"):
178 from muse.core.store import read_commit
179 commit_id = ref_path(root, branch).read_text().strip()
180 return read_commit(root, commit_id)
181
182
183 # ---------------------------------------------------------------------------
184 # A1 — muse merge: merge commit is authored by the current user
185 # ---------------------------------------------------------------------------
186
187 def test_a1_merge_commit_has_author(tmp_path: pathlib.Path) -> None:
188 """A1: after ``muse merge``, the new merge commit has author == user.handle.
189
190 RED: merge.py omits author from compute_commit_id() and CommitRecord.
191 GREEN: merge reads user.handle and passes it to both.
192 """
193 root, repo_id = _init_repo(tmp_path)
194 _set_user_handle(root, _HANDLE)
195
196 # Two branches diverge from a common base — forces a real merge commit.
197 obj_main = _write_object(root, b"main-only-file")
198 obj_feat = _write_object(root, b"feature-only-file")
199
200 base_id = _make_commit(root, repo_id, "main")
201 # feature branches from base, adds its own file (HEAD stays on main — no workdir change)
202 ref_path(root, "feature").parent.mkdir(parents=True, exist_ok=True)
203 ref_path(root, "feature").write_text(base_id)
204 _make_commit(root, repo_id, "feature", manifest={"feat.py": obj_feat})
205 # main also advances (diverges); workdir transitions from {} to {"main.py": obj_main}
206 _make_commit(root, repo_id, "main", manifest={"main.py": obj_main}, prev_manifest={})
207
208 result = runner.invoke(cli, ["merge", "feature"], env=_env(root), catch_exceptions=False)
209 assert result.exit_code == 0, f"stdout={result.output!r} stderr={result.stderr!r}"
210
211 commit = _head_commit(root, "main")
212 assert commit is not None
213 assert commit.parent2_commit_id is not None, "expected a merge commit with two parents"
214 assert commit.author == _HANDLE, (
215 f"merge commit has author={commit.author!r}, expected {_HANDLE!r}.\n"
216 "Fix: merge.py must read user.handle and pass it to compute_commit_id + CommitRecord."
217 )
218
219
220 # ---------------------------------------------------------------------------
221 # A2 — muse revert: revert commit is authored by the current user
222 # ---------------------------------------------------------------------------
223
224 def test_a2_revert_commit_has_author(tmp_path: pathlib.Path) -> None:
225 """A2: after ``muse revert``, the new revert commit has author == user.handle.
226
227 RED: revert.py omits author from compute_commit_id() and CommitRecord.
228 GREEN: revert reads user.handle and passes it to both.
229 """
230 root, repo_id = _init_repo(tmp_path)
231 _set_user_handle(root, _HANDLE)
232
233 _make_commit(root, repo_id, "main", message="initial")
234 target_id = _make_commit(root, repo_id, "main", message="bad change")
235
236 result = runner.invoke(cli, ["revert", target_id], env=_env(root), catch_exceptions=False)
237 assert result.exit_code == 0, f"stdout={result.output!r} stderr={result.stderr!r}"
238
239 commit = _head_commit(root, "main")
240 assert commit is not None
241 assert "revert" in commit.message.lower(), "expected a revert commit message"
242 assert commit.author == _HANDLE, (
243 f"revert commit has author={commit.author!r}, expected {_HANDLE!r}.\n"
244 "Fix: revert.py must read user.handle and pass it to compute_commit_id + CommitRecord."
245 )
246
247
248 # ---------------------------------------------------------------------------
249 # A3 — muse cherry-pick: preserves the original commit's author
250 # ---------------------------------------------------------------------------
251
252 def test_a3_cherry_pick_preserves_original_author(tmp_path: pathlib.Path) -> None:
253 """A3: cherry-pick preserves the author of the source commit.
254
255 The cherry-picker replays someone else's work — the author should reflect
256 who wrote the original code, not who ran cherry-pick.
257
258 RED: cherry_pick.py omits author entirely.
259 GREEN: cherry_pick.py passes target.author to compute_commit_id + CommitRecord.
260 """
261 root, repo_id = _init_repo(tmp_path)
262 _set_user_handle(root, _HANDLE)
263
264 original_author = "alice"
265 obj_base = _write_object(root, b"base")
266 obj_new = _write_object(root, b"new")
267 # HEAD is on main; workdir transitions from {} to {"base.py": obj_base}
268 _make_commit(root, repo_id, "main", manifest={"base.py": obj_base}, prev_manifest={})
269 # create the commit to cherry-pick on a feature branch (HEAD stays on main)
270 ref_path(root, "feature").parent.mkdir(parents=True, exist_ok=True)
271 ref_path(root, "feature").write_text(ref_path(root, "main").read_text())
272 target_id = _make_commit(
273 root, repo_id, "feature",
274 manifest={"base.py": obj_base, "new.py": obj_new},
275 message="feat: add new.py",
276 author=original_author,
277 )
278
279 result = runner.invoke(cli, ["cherry-pick", target_id], env=_env(root), catch_exceptions=False)
280 assert result.exit_code == 0, f"stdout={result.output!r} stderr={result.stderr!r}"
281
282 commit = _head_commit(root, "main")
283 assert commit is not None
284 assert commit.author == original_author, (
285 f"cherry-pick commit has author={commit.author!r}, expected {original_author!r}.\n"
286 "Fix: cherry_pick.py must pass target.author to compute_commit_id + CommitRecord."
287 )
288
289
290 # ---------------------------------------------------------------------------
291 # A4 — muse rebase --squash: squash commit preserves first original author
292 # ---------------------------------------------------------------------------
293
294 def test_a4_squash_rebase_preserves_first_commit_author(tmp_path: pathlib.Path) -> None:
295 """A4: ``muse rebase --squash`` preserves the author of the first squashed commit.
296
297 This mirrors git's squash behavior: the resulting commit carries the
298 authorship of the first commit in the squash range.
299
300 RED: rebase.py squash path omits author.
301 GREEN: rebase.py squash passes commits_to_replay[0].author to compute_commit_id + CommitRecord.
302 """
303 root, repo_id = _init_repo(tmp_path)
304 _set_user_handle(root, _HANDLE)
305
306 original_author = "bob"
307
308 obj_base = _write_object(root, b"base")
309 obj_v1 = _write_object(root, b"v1")
310 obj_v2 = _write_object(root, b"v2")
311 # HEAD is on main; workdir transitions from {} to {"base.py": obj_base}
312 base_id = _make_commit(root, repo_id, "main", manifest={"base.py": obj_base}, prev_manifest={})
313 # feature branch with two commits to squash (HEAD stays on main — no workdir change)
314 ref_path(root, "feature").parent.mkdir(parents=True, exist_ok=True)
315 ref_path(root, "feature").write_text(base_id)
316 _make_commit(
317 root, repo_id, "feature",
318 manifest={"base.py": obj_base, "feat.py": obj_v1},
319 message="feat: step 1",
320 author=original_author,
321 )
322 _make_commit(
323 root, repo_id, "feature",
324 manifest={"base.py": obj_base, "feat.py": obj_v2},
325 message="feat: step 2",
326 author=original_author,
327 )
328
329 # switch to feature — HEAD moves and workdir syncs to feature tip
330 _switch_branch(root, "feature")
331 result = runner.invoke(
332 cli, ["rebase", "--squash", "main"], env=_env(root), catch_exceptions=False
333 )
334 assert result.exit_code == 0, result.output
335
336 commit = _head_commit(root, "feature")
337 assert commit is not None
338 assert commit.author == original_author, (
339 f"squash commit has author={commit.author!r}, expected {original_author!r}.\n"
340 "Fix: rebase.py squash must pass commits_to_replay[0].author to "
341 "compute_commit_id + CommitRecord."
342 )
File History 1 commit
sha256:51ce277f663e01a43eaffbe77509b1de7ac2d4251b55d23306304bcdeb92c90d feat(pack): delta-encode snapshots in MPackBundle wire format Sonnet 4.6 minor ⚠ 121 days ago