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