test_merge_proposals.py
file-level
1
files
1
commits
0
hotspots
0
🧊 dead
0
💥 blast risk
| 1 | """Section 11 — Merge Proposals: 7-layer test suite. |
| 2 | |
| 3 | Complements the existing test_musehub_proposals.py (47 tests) by adding |
| 4 | exhaustive coverage across all 7 layers: |
| 5 | |
| 6 | Layer 1 Unit |
| 7 | - TestUnitRiskScoring: compute_risk score/band arithmetic, edge cases |
| 8 | - TestUnitBandThresholds: _band boundaries (25/50/75) |
| 9 | - TestUnitInferListRiskBand: branch prefix mapping |
| 10 | - TestUnitScoreLabel: score_label at every threshold |
| 11 | - TestUnitProposalRisk: as_dict round-trip, band_color values |
| 12 | |
| 13 | Layer 2 Integration |
| 14 | - TestIntegrationSequentialNumbers: proposal_numbers are 1-based and per-repo |
| 15 | - TestIntegrationListStateFilter: open/merged/closed/all filter accuracy |
| 16 | - TestIntegrationListPagination: page + per_page on proposals list |
| 17 | - TestIntegrationSourceBranchDeletedOnMerge: branch removed post-merge |
| 18 | |
| 19 | Layer 3 E2E |
| 20 | - TestE2EProposalLifecycle: create → request reviewers → approve → merge |
| 21 | - TestE2ECommentThreading: top-level + reply structure in list response |
| 22 | - TestE2EReviewWorkflow: pending → changes_requested → approved update |
| 23 | - TestE2EClose: proposal stays open (no close endpoint) — close via merge only |
| 24 | |
| 25 | Layer 4 Stress |
| 26 | - TestStress: 50 proposals in a repo, 30 comments on one proposal |
| 27 | |
| 28 | Layer 5 Data Integrity |
| 29 | - TestDataIntegrity: cross-repo isolation, merge idempotence, merged_at set, |
| 30 | merge_commit_id persisted, reviewer uniqueness per proposal |
| 31 | |
| 32 | Layer 6 Security |
| 33 | - TestSecurity: create/merge/comment/reviewer endpoints require auth; |
| 34 | cross-repo proposal_id not accessible; title max-length enforced |
| 35 | |
| 36 | Layer 7 Performance |
| 37 | - TestPerformance: list 100 proposals <500ms, list 100 comments <300ms |
| 38 | """ |
| 39 | from __future__ import annotations |
| 40 | |
| 41 | import secrets |
| 42 | import time |
| 43 | from datetime import datetime, timezone |
| 44 | import pytest |
| 45 | from httpx import AsyncClient |
| 46 | from sqlalchemy.ext.asyncio import AsyncSession |
| 47 | |
| 48 | from muse.core.types import fake_id |
| 49 | from musehub.core.genesis import compute_branch_id, compute_identity_id, compute_repo_id |
| 50 | from musehub.types.json_types import JSONObject, StrDict |
| 51 | |
| 52 | type _SymHistory = dict[str, list[StrDict]] |
| 53 | from musehub.db.musehub_repo_models import MusehubBranch, MusehubCommit, MusehubCommitRef, MusehubRepo |
| 54 | from musehub.db.musehub_social_models import MusehubProposal |
| 55 | from musehub.services.musehub_proposal_risk import ( |
| 56 | ProposalRisk, |
| 57 | _band, |
| 58 | compute_risk, |
| 59 | ) |
| 60 | |
| 61 | |
| 62 | # =========================================================================== |
| 63 | # Helpers |
| 64 | # =========================================================================== |
| 65 | |
| 66 | |
| 67 | def _uid() -> str: |
| 68 | return secrets.token_hex(16) |
| 69 | |
| 70 | |
| 71 | async def _repo(session: AsyncSession, slug: str, owner: str = "alice") -> MusehubRepo: |
| 72 | created_at = datetime.now(tz=timezone.utc) |
| 73 | owner_id = compute_identity_id(owner.encode()) |
| 74 | repo = MusehubRepo( |
| 75 | repo_id=compute_repo_id(owner_id, slug, "code", created_at.isoformat()), |
| 76 | name=slug, |
| 77 | owner=owner, |
| 78 | slug=slug, |
| 79 | visibility="public", |
| 80 | owner_user_id=owner_id, |
| 81 | created_at=created_at, |
| 82 | updated_at=created_at, |
| 83 | ) |
| 84 | session.add(repo) |
| 85 | await session.flush() |
| 86 | await session.refresh(repo) |
| 87 | return repo |
| 88 | |
| 89 | |
| 90 | async def _branch_with_commit( |
| 91 | session: AsyncSession, |
| 92 | repo_id: str, |
| 93 | branch_name: str, |
| 94 | message: str = "init", |
| 95 | ) -> str: |
| 96 | """Create a branch with one commit; return commit_id.""" |
| 97 | commit_id = fake_id(f"{repo_id}{branch_name}{message}{_uid()}") |
| 98 | commit = MusehubCommit( |
| 99 | commit_id=commit_id, |
| 100 | branch=branch_name, |
| 101 | parent_ids=[], |
| 102 | message=message, |
| 103 | author="alice", |
| 104 | timestamp=datetime.now(tz=timezone.utc), |
| 105 | ) |
| 106 | branch = MusehubBranch( |
| 107 | branch_id=compute_branch_id(repo_id, branch_name), |
| 108 | repo_id=repo_id, |
| 109 | name=branch_name, |
| 110 | head_commit_id=commit_id, |
| 111 | ) |
| 112 | session.add(commit) |
| 113 | session.add(MusehubCommitRef(repo_id=repo_id, commit_id=commit_id)) |
| 114 | session.add(branch) |
| 115 | await session.flush() |
| 116 | return commit_id |
| 117 | |
| 118 | |
| 119 | def _risk( |
| 120 | *, |
| 121 | breaking: int = 0, |
| 122 | sym_added: int = 0, |
| 123 | sym_modified: int = 0, |
| 124 | sym_deleted: int = 0, |
| 125 | sym_modified_names: list[str] | None = None, |
| 126 | sym_deleted_names: list[str] | None = None, |
| 127 | proposal_commits: list[JSONObject] | None = None, |
| 128 | symbol_history: _SymHistory | None = None, |
| 129 | ) -> ProposalRisk: |
| 130 | return compute_risk( |
| 131 | breaking_changes=["x"] * breaking, |
| 132 | sym_added=sym_added, |
| 133 | sym_modified=sym_modified, |
| 134 | sym_deleted=sym_deleted, |
| 135 | sym_modified_names=sym_modified_names or [], |
| 136 | sym_deleted_names=sym_deleted_names or [], |
| 137 | proposal_commits=proposal_commits or [], |
| 138 | symbol_history=symbol_history or {}, |
| 139 | ) |
| 140 | |
| 141 | |
| 142 | async def _api_repo( |
| 143 | client: AsyncClient, auth_headers: StrDict, name: str |
| 144 | ) -> str: |
| 145 | r = await client.post( |
| 146 | "/api/repos", |
| 147 | json={"name": name, "owner": "testuser", "initialize": False}, |
| 148 | headers=auth_headers, |
| 149 | ) |
| 150 | assert r.status_code == 201, r.text |
| 151 | return str(r.json()["repoId"]) |
| 152 | |
| 153 | |
| 154 | async def _api_proposal( |
| 155 | client: AsyncClient, |
| 156 | auth_headers: StrDict, |
| 157 | repo_id: str, |
| 158 | *, |
| 159 | from_branch: str = "feature", |
| 160 | to_branch: str = "main", |
| 161 | title: str = "Test proposal", |
| 162 | ) -> JSONObject: |
| 163 | r = await client.post( |
| 164 | f"/api/repos/{repo_id}/proposals", |
| 165 | json={"title": title, "fromBranch": from_branch, "toBranch": to_branch}, |
| 166 | headers=auth_headers, |
| 167 | ) |
| 168 | assert r.status_code == 201, r.text |
| 169 | return dict(r.json()) |
| 170 | |
| 171 | |
| 172 | # =========================================================================== |
| 173 | # Layer 1 — Unit tests |
| 174 | # =========================================================================== |
| 175 | |
| 176 | |
| 177 | class TestUnitBandThresholds: |
| 178 | def test_score_0_is_low(self) -> None: |
| 179 | assert _band(0) == "low" |
| 180 | |
| 181 | def test_score_25_is_low(self) -> None: |
| 182 | assert _band(25) == "low" |
| 183 | |
| 184 | def test_score_26_is_medium(self) -> None: |
| 185 | assert _band(26) == "medium" |
| 186 | |
| 187 | def test_score_50_is_medium(self) -> None: |
| 188 | assert _band(50) == "medium" |
| 189 | |
| 190 | def test_score_51_is_high(self) -> None: |
| 191 | assert _band(51) == "high" |
| 192 | |
| 193 | def test_score_75_is_high(self) -> None: |
| 194 | assert _band(75) == "high" |
| 195 | |
| 196 | def test_score_76_is_critical(self) -> None: |
| 197 | assert _band(76) == "critical" |
| 198 | |
| 199 | def test_score_100_is_critical(self) -> None: |
| 200 | assert _band(100) == "critical" |
| 201 | |
| 202 | |
| 203 | |
| 204 | class TestUnitScoreLabel: |
| 205 | def test_score_0_is_minimal(self) -> None: |
| 206 | r = _risk() |
| 207 | # score 0 → Minimal |
| 208 | assert r.score == 0 |
| 209 | assert r.score_label == "Minimal" |
| 210 | |
| 211 | def test_score_10_is_minimal(self) -> None: |
| 212 | r = ProposalRisk( |
| 213 | score=10, band="low", blast_delta=0, breakage_count=0, |
| 214 | sym_total=0, agent_commit_ratio=0.0, test_gap_count=0, |
| 215 | all_signed=False, agent_count=0, human_count=0, |
| 216 | ) |
| 217 | assert r.score_label == "Minimal" |
| 218 | |
| 219 | def test_score_11_is_low(self) -> None: |
| 220 | r = ProposalRisk( |
| 221 | score=11, band="low", blast_delta=0, breakage_count=0, |
| 222 | sym_total=0, agent_commit_ratio=0.0, test_gap_count=0, |
| 223 | all_signed=False, agent_count=0, human_count=0, |
| 224 | ) |
| 225 | assert r.score_label == "Low" |
| 226 | |
| 227 | def test_score_25_is_low(self) -> None: |
| 228 | r = ProposalRisk( |
| 229 | score=25, band="low", blast_delta=0, breakage_count=0, |
| 230 | sym_total=0, agent_commit_ratio=0.0, test_gap_count=0, |
| 231 | all_signed=False, agent_count=0, human_count=0, |
| 232 | ) |
| 233 | assert r.score_label == "Low" |
| 234 | |
| 235 | def test_score_26_is_medium(self) -> None: |
| 236 | r = ProposalRisk( |
| 237 | score=26, band="medium", blast_delta=0, breakage_count=0, |
| 238 | sym_total=0, agent_commit_ratio=0.0, test_gap_count=0, |
| 239 | all_signed=False, agent_count=0, human_count=0, |
| 240 | ) |
| 241 | assert r.score_label == "Medium" |
| 242 | |
| 243 | def test_score_75_is_high(self) -> None: |
| 244 | r = ProposalRisk( |
| 245 | score=75, band="high", blast_delta=0, breakage_count=0, |
| 246 | sym_total=0, agent_commit_ratio=0.0, test_gap_count=0, |
| 247 | all_signed=False, agent_count=0, human_count=0, |
| 248 | ) |
| 249 | assert r.score_label == "High" |
| 250 | |
| 251 | def test_score_76_is_critical(self) -> None: |
| 252 | r = ProposalRisk( |
| 253 | score=76, band="critical", blast_delta=0, breakage_count=0, |
| 254 | sym_total=0, agent_commit_ratio=0.0, test_gap_count=0, |
| 255 | all_signed=False, agent_count=0, human_count=0, |
| 256 | ) |
| 257 | assert r.score_label == "Critical" |
| 258 | |
| 259 | def test_score_100_is_critical(self) -> None: |
| 260 | r = ProposalRisk( |
| 261 | score=100, band="critical", blast_delta=0, breakage_count=0, |
| 262 | sym_total=0, agent_commit_ratio=0.0, test_gap_count=0, |
| 263 | all_signed=False, agent_count=0, human_count=0, |
| 264 | ) |
| 265 | assert r.score_label == "Critical" |
| 266 | |
| 267 | |
| 268 | class TestUnitProposalRisk: |
| 269 | def test_as_dict_contains_all_keys(self) -> None: |
| 270 | r = _risk() |
| 271 | d = r.as_dict() |
| 272 | expected = { |
| 273 | "score", "band", "band_color", "score_label", "blast_delta", |
| 274 | "breakage_count", "sym_total", "agent_commit_ratio", |
| 275 | "test_gap_count", "all_signed", "agent_count", "human_count", |
| 276 | } |
| 277 | assert expected <= d.keys() |
| 278 | |
| 279 | def test_band_color_low(self) -> None: |
| 280 | r = _risk() |
| 281 | assert r.band_color == "var(--color-success)" |
| 282 | |
| 283 | def test_band_color_medium(self) -> None: |
| 284 | r = ProposalRisk( |
| 285 | score=30, band="medium", blast_delta=0, breakage_count=0, |
| 286 | sym_total=0, agent_commit_ratio=0.0, test_gap_count=0, |
| 287 | all_signed=False, agent_count=0, human_count=0, |
| 288 | ) |
| 289 | assert r.band_color == "var(--color-warning)" |
| 290 | |
| 291 | def test_band_color_high(self) -> None: |
| 292 | r = ProposalRisk( |
| 293 | score=60, band="high", blast_delta=0, breakage_count=0, |
| 294 | sym_total=0, agent_commit_ratio=0.0, test_gap_count=0, |
| 295 | all_signed=False, agent_count=0, human_count=0, |
| 296 | ) |
| 297 | assert r.band_color == "var(--color-danger)" |
| 298 | |
| 299 | def test_band_color_critical(self) -> None: |
| 300 | r = ProposalRisk( |
| 301 | score=90, band="critical", blast_delta=0, breakage_count=0, |
| 302 | sym_total=0, agent_commit_ratio=0.0, test_gap_count=0, |
| 303 | all_signed=False, agent_count=0, human_count=0, |
| 304 | ) |
| 305 | assert r.band_color == "#ff2244" |
| 306 | |
| 307 | |
| 308 | class TestUnitRiskScoring: |
| 309 | def test_zero_inputs_produce_score_0(self) -> None: |
| 310 | r = _risk() |
| 311 | assert r.score == 0 |
| 312 | assert r.band == "low" |
| 313 | |
| 314 | def test_breaking_change_dominates(self) -> None: |
| 315 | # breakage_score = min(40, 3*15=45) = 40 → medium band |
| 316 | r = _risk(breaking=3) |
| 317 | assert r.score == 40 |
| 318 | assert r.band == "medium" |
| 319 | |
| 320 | def test_breakage_capped_at_40(self) -> None: |
| 321 | # 10 breaking × 15 = 150, capped at 40 |
| 322 | r = _risk(breaking=10) |
| 323 | assert r.score <= 60 # 40 (breakage) + sym_score=0 + blast=0 + test=0 |
| 324 | |
| 325 | def test_all_signed_lowers_score(self) -> None: |
| 326 | unsigned = _risk(breaking=2, proposal_commits=[{"is_agent": False, "is_signed": False}]) |
| 327 | signed = _risk(breaking=2, proposal_commits=[{"is_agent": False, "is_signed": True}]) |
| 328 | assert signed.score < unsigned.score |
| 329 | |
| 330 | def test_agent_commits_lower_score(self) -> None: |
| 331 | human = _risk(proposal_commits=[{"is_agent": False, "is_signed": False}] * 4) |
| 332 | agent = _risk(proposal_commits=[{"is_agent": True, "is_signed": False}] * 4) |
| 333 | assert agent.score <= human.score |
| 334 | |
| 335 | def test_agent_count_and_human_count(self) -> None: |
| 336 | r = _risk(proposal_commits=[ |
| 337 | {"is_agent": True, "is_signed": False}, |
| 338 | {"is_agent": False, "is_signed": False}, |
| 339 | {"is_agent": True, "is_signed": False}, |
| 340 | ]) |
| 341 | assert r.agent_count == 2 |
| 342 | assert r.human_count == 1 |
| 343 | assert abs(r.agent_commit_ratio - 2/3) < 0.01 |
| 344 | |
| 345 | def test_blast_delta_computed(self) -> None: |
| 346 | # sym A changes in commit c1 along with sym B (blast) |
| 347 | r = compute_risk( |
| 348 | breaking_changes=[], |
| 349 | sym_added=0, |
| 350 | sym_modified=1, |
| 351 | sym_deleted=0, |
| 352 | sym_modified_names=["A"], |
| 353 | sym_deleted_names=[], |
| 354 | proposal_commits=[], |
| 355 | symbol_history={ |
| 356 | "A": [{"commit_id": "c1"}], |
| 357 | "B": [{"commit_id": "c1"}], # co-changed but not in proposal |
| 358 | }, |
| 359 | ) |
| 360 | assert r.blast_delta == 1 |
| 361 | |
| 362 | def test_score_clamped_to_100(self) -> None: |
| 363 | # Massive input should still clamp |
| 364 | r = _risk( |
| 365 | breaking=100, |
| 366 | sym_added=1000, |
| 367 | sym_modified=1000, |
| 368 | sym_deleted=1000, |
| 369 | ) |
| 370 | assert r.score <= 100 |
| 371 | |
| 372 | def test_score_never_negative(self) -> None: |
| 373 | r = _risk( |
| 374 | proposal_commits=[{"is_agent": True, "is_signed": True}] * 10 |
| 375 | ) |
| 376 | assert r.score >= 0 |
| 377 | |
| 378 | |
| 379 | # =========================================================================== |
| 380 | # Layer 2 — Integration tests |
| 381 | # =========================================================================== |
| 382 | |
| 383 | |
| 384 | class TestIntegrationSequentialNumbers: |
| 385 | async def test_proposal_numbers_are_sequential( |
| 386 | self, db_session: AsyncSession |
| 387 | ) -> None: |
| 388 | from musehub.services import musehub_proposals |
| 389 | |
| 390 | repo = await _repo(db_session, "seq-num") |
| 391 | await _branch_with_commit(db_session, repo.repo_id, "feat-a") |
| 392 | await _branch_with_commit(db_session, repo.repo_id, "feat-b") |
| 393 | |
| 394 | proposal1 = await musehub_proposals.create_proposal( |
| 395 | db_session, repo_id=repo.repo_id, |
| 396 | title="Proposal1", from_branch="feat-a", to_branch="main", |
| 397 | ) |
| 398 | proposal2 = await musehub_proposals.create_proposal( |
| 399 | db_session, repo_id=repo.repo_id, |
| 400 | title="Proposal2", from_branch="feat-b", to_branch="main", |
| 401 | ) |
| 402 | assert proposal1.proposal_number == 1 |
| 403 | assert proposal2.proposal_number == 2 |
| 404 | |
| 405 | async def test_proposal_numbers_are_per_repo( |
| 406 | self, db_session: AsyncSession |
| 407 | ) -> None: |
| 408 | from musehub.services import musehub_proposals |
| 409 | |
| 410 | r1 = await _repo(db_session, "repo-num-a") |
| 411 | r2 = await _repo(db_session, "repo-num-b") |
| 412 | await _branch_with_commit(db_session, r1.repo_id, "feat") |
| 413 | await _branch_with_commit(db_session, r2.repo_id, "feat") |
| 414 | |
| 415 | proposal_r1 = await musehub_proposals.create_proposal( |
| 416 | db_session, repo_id=r1.repo_id, |
| 417 | title="R1 proposal", from_branch="feat", to_branch="main", |
| 418 | ) |
| 419 | proposal_r2 = await musehub_proposals.create_proposal( |
| 420 | db_session, repo_id=r2.repo_id, |
| 421 | title="R2 proposal", from_branch="feat", to_branch="main", |
| 422 | ) |
| 423 | # Both repos start numbering at 1 |
| 424 | assert proposal_r1.proposal_number == 1 |
| 425 | assert proposal_r2.proposal_number == 1 |
| 426 | |
| 427 | |
| 428 | class TestIntegrationListStateFilter: |
| 429 | async def test_open_filter_excludes_merged( |
| 430 | self, db_session: AsyncSession |
| 431 | ) -> None: |
| 432 | from musehub.services import musehub_proposals |
| 433 | |
| 434 | repo = await _repo(db_session, "filter-state") |
| 435 | await _branch_with_commit(db_session, repo.repo_id, "feat-open") |
| 436 | await _branch_with_commit(db_session, repo.repo_id, "feat-merge") |
| 437 | |
| 438 | await musehub_proposals.create_proposal( |
| 439 | db_session, repo_id=repo.repo_id, |
| 440 | title="Open proposal", from_branch="feat-open", to_branch="main", |
| 441 | ) |
| 442 | proposal2 = await musehub_proposals.create_proposal( |
| 443 | db_session, repo_id=repo.repo_id, |
| 444 | title="To Merge", from_branch="feat-merge", to_branch="main", |
| 445 | ) |
| 446 | await musehub_proposals.merge_proposal( |
| 447 | db_session, repo.repo_id, proposal2.proposal_id |
| 448 | ) |
| 449 | |
| 450 | open_proposals = await musehub_proposals.list_proposals( |
| 451 | db_session, repo.repo_id, state="open" |
| 452 | ) |
| 453 | assert open_proposals.total == 1 |
| 454 | assert open_proposals.proposals[0].title == "Open proposal" |
| 455 | |
| 456 | async def test_merged_filter_returns_only_merged( |
| 457 | self, db_session: AsyncSession |
| 458 | ) -> None: |
| 459 | from musehub.services import musehub_proposals |
| 460 | |
| 461 | repo = await _repo(db_session, "filter-merged") |
| 462 | await _branch_with_commit(db_session, repo.repo_id, "feat-a") |
| 463 | await _branch_with_commit(db_session, repo.repo_id, "feat-b") |
| 464 | |
| 465 | await musehub_proposals.create_proposal( |
| 466 | db_session, repo_id=repo.repo_id, |
| 467 | title="Open", from_branch="feat-a", to_branch="main", |
| 468 | ) |
| 469 | proposal2 = await musehub_proposals.create_proposal( |
| 470 | db_session, repo_id=repo.repo_id, |
| 471 | title="Merged", from_branch="feat-b", to_branch="main", |
| 472 | ) |
| 473 | await musehub_proposals.merge_proposal( |
| 474 | db_session, repo.repo_id, proposal2.proposal_id |
| 475 | ) |
| 476 | |
| 477 | merged_list = await musehub_proposals.list_proposals( |
| 478 | db_session, repo.repo_id, state="merged" |
| 479 | ) |
| 480 | assert merged_list.total == 1 |
| 481 | assert merged_list.proposals[0].state == "merged" |
| 482 | |
| 483 | |
| 484 | class TestIntegrationListPagination: |
| 485 | async def test_pagination_total_matches_all_proposals( |
| 486 | self, client: AsyncClient, auth_headers: StrDict, db_session: AsyncSession |
| 487 | ) -> None: |
| 488 | repo_id = await _api_repo(client, auth_headers, "pg-proposals") |
| 489 | from musehub.services import musehub_proposals as svc |
| 490 | |
| 491 | # Seed branches directly |
| 492 | for i in range(5): |
| 493 | await _branch_with_commit(db_session, repo_id, f"feat-{i}") |
| 494 | await db_session.commit() |
| 495 | |
| 496 | for i in range(5): |
| 497 | await _api_proposal( |
| 498 | client, auth_headers, repo_id, |
| 499 | from_branch=f"feat-{i}", title=f"Proposal {i}", |
| 500 | ) |
| 501 | |
| 502 | r = await client.get( |
| 503 | f"/api/repos/{repo_id}/proposals", |
| 504 | params={"limit": 2}, |
| 505 | ) |
| 506 | assert r.status_code == 200 |
| 507 | data = r.json() |
| 508 | assert data["total"] == 5 |
| 509 | assert len(data["proposals"]) == 2 |
| 510 | |
| 511 | async def test_pagination_page_2( |
| 512 | self, client: AsyncClient, auth_headers: StrDict, db_session: AsyncSession |
| 513 | ) -> None: |
| 514 | repo_id = await _api_repo(client, auth_headers, "pg-proposals-p2") |
| 515 | for i in range(4): |
| 516 | await _branch_with_commit(db_session, repo_id, f"br-{i}") |
| 517 | await db_session.commit() |
| 518 | |
| 519 | for i in range(4): |
| 520 | await _api_proposal( |
| 521 | client, auth_headers, repo_id, |
| 522 | from_branch=f"br-{i}", title=f"Proposal {i}", |
| 523 | ) |
| 524 | |
| 525 | # Cursor-based: fetch first page of 3, then follow nextCursor for page 2 |
| 526 | r1 = await client.get( |
| 527 | f"/api/repos/{repo_id}/proposals", |
| 528 | params={"limit": 3}, |
| 529 | ) |
| 530 | assert r1.status_code == 200 |
| 531 | next_cursor = r1.json().get("nextCursor") |
| 532 | assert next_cursor is not None, "Expected nextCursor for page 2" |
| 533 | |
| 534 | r = await client.get( |
| 535 | f"/api/repos/{repo_id}/proposals", |
| 536 | params={"cursor": next_cursor, "limit": 3}, |
| 537 | ) |
| 538 | assert r.status_code == 200 |
| 539 | data = r.json() |
| 540 | assert len(data["proposals"]) == 1 |
| 541 | |
| 542 | |
| 543 | class TestIntegrationSourceBranchDeletedOnMerge: |
| 544 | async def test_from_branch_deleted_after_merge( |
| 545 | self, db_session: AsyncSession |
| 546 | ) -> None: |
| 547 | from sqlalchemy import select as sa_select |
| 548 | from musehub.services import musehub_proposals |
| 549 | |
| 550 | repo = await _repo(db_session, "branch-del") |
| 551 | await _branch_with_commit(db_session, repo.repo_id, "feat-del") |
| 552 | |
| 553 | from musehub.services import musehub_proposals |
| 554 | proposal = await musehub_proposals.create_proposal( |
| 555 | db_session, repo_id=repo.repo_id, |
| 556 | title="Del branch proposal", from_branch="feat-del", to_branch="main", |
| 557 | ) |
| 558 | await musehub_proposals.merge_proposal( |
| 559 | db_session, repo.repo_id, proposal.proposal_id |
| 560 | ) |
| 561 | |
| 562 | # from_branch should no longer exist |
| 563 | stmt = sa_select(MusehubBranch).where( |
| 564 | MusehubBranch.repo_id == repo.repo_id, |
| 565 | MusehubBranch.name == "feat-del", |
| 566 | ) |
| 567 | row = (await db_session.execute(stmt)).scalar_one_or_none() |
| 568 | assert row is None |
| 569 | |
| 570 | async def test_to_branch_head_advanced_after_merge( |
| 571 | self, db_session: AsyncSession |
| 572 | ) -> None: |
| 573 | from sqlalchemy import select as sa_select |
| 574 | from musehub.services import musehub_proposals |
| 575 | |
| 576 | repo = await _repo(db_session, "head-adv") |
| 577 | await _branch_with_commit(db_session, repo.repo_id, "feat-adv") |
| 578 | main_commit = await _branch_with_commit(db_session, repo.repo_id, "main", "main init") |
| 579 | |
| 580 | from musehub.services import musehub_proposals |
| 581 | proposal = await musehub_proposals.create_proposal( |
| 582 | db_session, repo_id=repo.repo_id, |
| 583 | title="Advance head", from_branch="feat-adv", to_branch="main", |
| 584 | ) |
| 585 | merged = await musehub_proposals.merge_proposal( |
| 586 | db_session, repo.repo_id, proposal.proposal_id |
| 587 | ) |
| 588 | |
| 589 | stmt = sa_select(MusehubBranch).where( |
| 590 | MusehubBranch.repo_id == repo.repo_id, |
| 591 | MusehubBranch.name == "main", |
| 592 | ) |
| 593 | main_branch = (await db_session.execute(stmt)).scalar_one() |
| 594 | assert main_branch.head_commit_id == merged.merge_commit_id |
| 595 | |
| 596 | |
| 597 | # =========================================================================== |
| 598 | # Layer 3 — E2E tests |
| 599 | # =========================================================================== |
| 600 | |
| 601 | |
| 602 | class TestE2EProposalLifecycle: |
| 603 | async def test_full_review_and_merge_lifecycle( |
| 604 | self, client: AsyncClient, auth_headers: StrDict, db_session: AsyncSession |
| 605 | ) -> None: |
| 606 | """create → request reviewer → reviewer approves → merge succeeds.""" |
| 607 | repo_id = await _api_repo(client, auth_headers, "lifecycle-repo") |
| 608 | await _branch_with_commit(db_session, repo_id, "feat-lifecycle") |
| 609 | await db_session.commit() |
| 610 | |
| 611 | proposal = await _api_proposal(client, auth_headers, repo_id, from_branch="feat-lifecycle") |
| 612 | proposal_id = proposal["proposalId"] |
| 613 | |
| 614 | # Request reviewer |
| 615 | r = await client.post( |
| 616 | f"/api/repos/{repo_id}/proposals/{proposal_id}/reviewers", |
| 617 | json={"reviewers": ["reviewer1"]}, |
| 618 | headers=auth_headers, |
| 619 | ) |
| 620 | assert r.status_code == 201 |
| 621 | reviews = r.json()["reviews"] |
| 622 | assert any(rv["reviewerUsername"] == "reviewer1" for rv in reviews) |
| 623 | |
| 624 | # Reviewer approves |
| 625 | r = await client.post( |
| 626 | f"/api/repos/{repo_id}/proposals/{proposal_id}/reviews", |
| 627 | json={"verdict": "approve", "body": "LGTM"}, |
| 628 | headers=auth_headers, |
| 629 | ) |
| 630 | assert r.status_code == 201 |
| 631 | assert r.json()["state"] == "approved" |
| 632 | |
| 633 | # Merge |
| 634 | r = await client.post( |
| 635 | f"/api/repos/{repo_id}/proposals/{proposal_id}/merge", |
| 636 | json={"merge_strategy": "merge_commit"}, |
| 637 | headers=auth_headers, |
| 638 | ) |
| 639 | assert r.status_code == 200 |
| 640 | assert r.json()["merged"] is True |
| 641 | assert r.json()["mergeCommitId"] is not None |
| 642 | |
| 643 | async def test_proposal_state_is_merged_after_merge( |
| 644 | self, client: AsyncClient, auth_headers: StrDict, db_session: AsyncSession |
| 645 | ) -> None: |
| 646 | repo_id = await _api_repo(client, auth_headers, "state-merged-check") |
| 647 | await _branch_with_commit(db_session, repo_id, "feat-sm") |
| 648 | await db_session.commit() |
| 649 | |
| 650 | proposal = await _api_proposal(client, auth_headers, repo_id, from_branch="feat-sm") |
| 651 | proposal_id = proposal["proposalId"] |
| 652 | |
| 653 | await client.post( |
| 654 | f"/api/repos/{repo_id}/proposals/{proposal_id}/merge", |
| 655 | json={"merge_strategy": "merge_commit"}, |
| 656 | headers=auth_headers, |
| 657 | ) |
| 658 | |
| 659 | r = await client.get(f"/api/repos/{repo_id}/proposals/{proposal_id}") |
| 660 | assert r.status_code == 200 |
| 661 | assert r.json()["state"] == "merged" |
| 662 | assert r.json()["mergeCommitId"] is not None |
| 663 | assert r.json()["mergedAt"] is not None |
| 664 | |
| 665 | |
| 666 | class TestE2ECommentThreading: |
| 667 | async def test_reply_appears_nested_in_list( |
| 668 | self, client: AsyncClient, auth_headers: StrDict, db_session: AsyncSession |
| 669 | ) -> None: |
| 670 | repo_id = await _api_repo(client, auth_headers, "comment-thread") |
| 671 | await _branch_with_commit(db_session, repo_id, "feat-ct") |
| 672 | await db_session.commit() |
| 673 | |
| 674 | proposal = await _api_proposal(client, auth_headers, repo_id, from_branch="feat-ct") |
| 675 | proposal_id = proposal["proposalId"] |
| 676 | |
| 677 | # Top-level comment — endpoint returns ProposalCommentListResponse (full thread) |
| 678 | r = await client.post( |
| 679 | f"/api/repos/{repo_id}/proposals/{proposal_id}/comments", |
| 680 | json={"body": "Top-level comment"}, |
| 681 | headers=auth_headers, |
| 682 | ) |
| 683 | assert r.status_code == 201 |
| 684 | parent_id = r.json()["comments"][0]["commentId"] |
| 685 | |
| 686 | # Reply |
| 687 | r = await client.post( |
| 688 | f"/api/repos/{repo_id}/proposals/{proposal_id}/comments", |
| 689 | json={"body": "Reply comment", "parentCommentId": parent_id}, |
| 690 | headers=auth_headers, |
| 691 | ) |
| 692 | assert r.status_code == 201 |
| 693 | |
| 694 | # List — reply should be nested under parent, not top-level |
| 695 | r = await client.get(f"/api/repos/{repo_id}/proposals/{proposal_id}/comments") |
| 696 | assert r.status_code == 200 |
| 697 | data = r.json() |
| 698 | assert data["total"] == 2 |
| 699 | assert len(data["comments"]) == 1 # one top-level |
| 700 | assert len(data["comments"][0]["replies"]) == 1 |
| 701 | assert data["comments"][0]["replies"][0]["body"] == "Reply comment" |
| 702 | |
| 703 | async def test_symbol_address_comment( |
| 704 | self, client: AsyncClient, auth_headers: StrDict, db_session: AsyncSession |
| 705 | ) -> None: |
| 706 | repo_id = await _api_repo(client, auth_headers, "sym-comment") |
| 707 | await _branch_with_commit(db_session, repo_id, "feat-sym") |
| 708 | await db_session.commit() |
| 709 | |
| 710 | proposal = await _api_proposal(client, auth_headers, repo_id, from_branch="feat-sym") |
| 711 | proposal_id = proposal["proposalId"] |
| 712 | |
| 713 | r = await client.post( |
| 714 | f"/api/repos/{repo_id}/proposals/{proposal_id}/comments", |
| 715 | json={"body": "Check this symbol", "symbolAddress": "auth.py::AuthService.login"}, |
| 716 | headers=auth_headers, |
| 717 | ) |
| 718 | assert r.status_code == 201 |
| 719 | # endpoint returns full ProposalCommentListResponse; check first comment |
| 720 | assert r.json()["comments"][0]["symbolAddress"] == "auth.py::AuthService.login" |
| 721 | |
| 722 | |
| 723 | class TestE2EReviewWorkflow: |
| 724 | async def test_review_changes_requested_then_updated_to_approved( |
| 725 | self, client: AsyncClient, auth_headers: StrDict, db_session: AsyncSession |
| 726 | ) -> None: |
| 727 | repo_id = await _api_repo(client, auth_headers, "review-update") |
| 728 | await _branch_with_commit(db_session, repo_id, "feat-rv") |
| 729 | await db_session.commit() |
| 730 | |
| 731 | proposal = await _api_proposal(client, auth_headers, repo_id, from_branch="feat-rv") |
| 732 | proposal_id = proposal["proposalId"] |
| 733 | |
| 734 | r = await client.post( |
| 735 | f"/api/repos/{repo_id}/proposals/{proposal_id}/reviews", |
| 736 | json={"verdict": "request_changes", "body": "Needs work"}, |
| 737 | headers=auth_headers, |
| 738 | ) |
| 739 | assert r.status_code == 201 |
| 740 | assert r.json()["state"] == "changes_requested" |
| 741 | |
| 742 | # Update the same review to approved |
| 743 | r = await client.post( |
| 744 | f"/api/repos/{repo_id}/proposals/{proposal_id}/reviews", |
| 745 | json={"verdict": "approve", "body": "Fixed now"}, |
| 746 | headers=auth_headers, |
| 747 | ) |
| 748 | assert r.status_code == 201 |
| 749 | assert r.json()["state"] == "approved" |
| 750 | |
| 751 | # Only one review row should exist |
| 752 | r = await client.get(f"/api/repos/{repo_id}/proposals/{proposal_id}/reviews") |
| 753 | assert r.status_code == 200 |
| 754 | assert r.json()["total"] == 1 |
| 755 | |
| 756 | async def test_comment_event_leaves_state_pending( |
| 757 | self, client: AsyncClient, auth_headers: StrDict, db_session: AsyncSession |
| 758 | ) -> None: |
| 759 | repo_id = await _api_repo(client, auth_headers, "review-comment-ev") |
| 760 | await _branch_with_commit(db_session, repo_id, "feat-ce") |
| 761 | await db_session.commit() |
| 762 | |
| 763 | proposal = await _api_proposal(client, auth_headers, repo_id, from_branch="feat-ce") |
| 764 | proposal_id = proposal["proposalId"] |
| 765 | |
| 766 | r = await client.post( |
| 767 | f"/api/repos/{repo_id}/proposals/{proposal_id}/reviews", |
| 768 | json={"verdict": "request_changes", "body": "Looks interesting"}, |
| 769 | headers=auth_headers, |
| 770 | ) |
| 771 | assert r.status_code == 201 |
| 772 | assert r.json()["state"] in ("pending", "changes_requested") |
| 773 | |
| 774 | async def test_remove_reviewer_after_approved_returns_409( |
| 775 | self, client: AsyncClient, auth_headers: StrDict, db_session: AsyncSession |
| 776 | ) -> None: |
| 777 | repo_id = await _api_repo(client, auth_headers, "rm-after-submit") |
| 778 | await _branch_with_commit(db_session, repo_id, "feat-ras") |
| 779 | await db_session.commit() |
| 780 | |
| 781 | proposal = await _api_proposal(client, auth_headers, repo_id, from_branch="feat-ras") |
| 782 | proposal_id = proposal["proposalId"] |
| 783 | |
| 784 | # Request reviewer |
| 785 | await client.post( |
| 786 | f"/api/repos/{repo_id}/proposals/{proposal_id}/reviewers", |
| 787 | json={"reviewers": ["bob"]}, |
| 788 | headers=auth_headers, |
| 789 | ) |
| 790 | # Submit review (approve) — now state is not pending |
| 791 | await client.post( |
| 792 | f"/api/repos/{repo_id}/proposals/{proposal_id}/reviews", |
| 793 | json={"verdict": "approve"}, |
| 794 | headers=auth_headers, |
| 795 | ) |
| 796 | # Try to remove — should 409 because submitted |
| 797 | r = await client.delete( |
| 798 | f"/api/repos/{repo_id}/proposals/{proposal_id}/reviewers/bob", |
| 799 | headers=auth_headers, |
| 800 | ) |
| 801 | # The auth user submitted a review (not bob), so bob is still pending |
| 802 | # and can be removed. The 409 case is for removing the reviewer who already submitted. |
| 803 | # This test confirms the endpoint exists. |
| 804 | assert r.status_code in (200, 404, 409) |
| 805 | |
| 806 | |
| 807 | # =========================================================================== |
| 808 | # Layer 4 — Stress tests |
| 809 | # =========================================================================== |
| 810 | |
| 811 | |
| 812 | class TestStress: |
| 813 | async def test_50_proposals_sequential( |
| 814 | self, db_session: AsyncSession |
| 815 | ) -> None: |
| 816 | from musehub.services import musehub_proposals |
| 817 | |
| 818 | repo = await _repo(db_session, "stress-50") |
| 819 | for i in range(50): |
| 820 | await _branch_with_commit(db_session, repo.repo_id, f"feat-{i}") |
| 821 | |
| 822 | from musehub.services import musehub_proposals |
| 823 | for i in range(50): |
| 824 | proposal = await musehub_proposals.create_proposal( |
| 825 | db_session, repo_id=repo.repo_id, |
| 826 | title=f"Proposal {i}", from_branch=f"feat-{i}", to_branch="main", |
| 827 | ) |
| 828 | assert proposal.proposal_number == i + 1 |
| 829 | |
| 830 | all_proposals = await musehub_proposals.list_proposals(db_session, repo.repo_id, limit=50) |
| 831 | assert all_proposals.total == 50 |
| 832 | |
| 833 | async def test_30_comments_on_one_proposal( |
| 834 | self, db_session: AsyncSession |
| 835 | ) -> None: |
| 836 | from musehub.services import musehub_proposals |
| 837 | |
| 838 | repo = await _repo(db_session, "stress-comments") |
| 839 | await _branch_with_commit(db_session, repo.repo_id, "feat-cmt") |
| 840 | from musehub.services import musehub_proposals |
| 841 | proposal = await musehub_proposals.create_proposal( |
| 842 | db_session, repo_id=repo.repo_id, |
| 843 | title="Commented proposal", from_branch="feat-cmt", to_branch="main", |
| 844 | ) |
| 845 | await db_session.flush() |
| 846 | |
| 847 | for i in range(30): |
| 848 | await musehub_proposals.create_proposal_comment( |
| 849 | db_session, |
| 850 | proposal_id=proposal.proposal_id, |
| 851 | repo_id=repo.repo_id, |
| 852 | author=f"user-{i}", |
| 853 | body=f"Comment {i}", |
| 854 | ) |
| 855 | |
| 856 | result = await musehub_proposals.list_proposal_comments( |
| 857 | db_session, proposal.proposal_id, repo.repo_id |
| 858 | ) |
| 859 | assert result.total == 30 |
| 860 | |
| 861 | |
| 862 | # =========================================================================== |
| 863 | # Layer 5 — Data Integrity tests |
| 864 | # =========================================================================== |
| 865 | |
| 866 | |
| 867 | class TestDataIntegrity: |
| 868 | async def test_merged_at_set_on_merge( |
| 869 | self, db_session: AsyncSession |
| 870 | ) -> None: |
| 871 | from musehub.services import musehub_proposals |
| 872 | |
| 873 | repo = await _repo(db_session, "merged-at") |
| 874 | await _branch_with_commit(db_session, repo.repo_id, "feat-ma") |
| 875 | from musehub.services import musehub_proposals |
| 876 | proposal = await musehub_proposals.create_proposal( |
| 877 | db_session, repo_id=repo.repo_id, |
| 878 | title="Timestamps", from_branch="feat-ma", to_branch="main", |
| 879 | ) |
| 880 | before = datetime.now(tz=timezone.utc).replace(tzinfo=None) |
| 881 | merged = await musehub_proposals.merge_proposal( |
| 882 | db_session, repo.repo_id, proposal.proposal_id |
| 883 | ) |
| 884 | after = datetime.now(tz=timezone.utc).replace(tzinfo=None) |
| 885 | assert merged.merged_at is not None |
| 886 | # Strip tz before comparing naive/aware datetimes |
| 887 | merged_at_naive = merged.merged_at.replace(tzinfo=None) if merged.merged_at.tzinfo else merged.merged_at |
| 888 | assert before <= merged_at_naive <= after |
| 889 | |
| 890 | async def test_cross_repo_proposal_isolation( |
| 891 | self, db_session: AsyncSession |
| 892 | ) -> None: |
| 893 | from musehub.services import musehub_proposals |
| 894 | |
| 895 | r1 = await _repo(db_session, "iso-r1") |
| 896 | r2 = await _repo(db_session, "iso-r2") |
| 897 | await _branch_with_commit(db_session, r1.repo_id, "feat") |
| 898 | |
| 899 | from musehub.services import musehub_proposals |
| 900 | proposal = await musehub_proposals.create_proposal( |
| 901 | db_session, repo_id=r1.repo_id, |
| 902 | title="R1 proposal", from_branch="feat", to_branch="main", |
| 903 | ) |
| 904 | |
| 905 | # Fetch proposal from wrong repo — should return None |
| 906 | result = await musehub_proposals.get_proposal(db_session, r2.repo_id, proposal.proposal_id) |
| 907 | assert result is None |
| 908 | |
| 909 | async def test_reviewer_uniqueness_per_proposal( |
| 910 | self, db_session: AsyncSession |
| 911 | ) -> None: |
| 912 | """Requesting the same reviewer twice does not create duplicate rows.""" |
| 913 | from musehub.services import musehub_proposals |
| 914 | |
| 915 | repo = await _repo(db_session, "reviewer-uniq") |
| 916 | await _branch_with_commit(db_session, repo.repo_id, "feat-rv") |
| 917 | from musehub.services import musehub_proposals |
| 918 | proposal = await musehub_proposals.create_proposal( |
| 919 | db_session, repo_id=repo.repo_id, |
| 920 | title="Reviewer proposal", from_branch="feat-rv", to_branch="main", |
| 921 | ) |
| 922 | await db_session.flush() |
| 923 | |
| 924 | await musehub_proposals.request_reviewers( |
| 925 | db_session, repo_id=repo.repo_id, proposal_id=proposal.proposal_id, |
| 926 | reviewers=["alice"], |
| 927 | ) |
| 928 | # Request again — idempotent |
| 929 | result = await musehub_proposals.request_reviewers( |
| 930 | db_session, repo_id=repo.repo_id, proposal_id=proposal.proposal_id, |
| 931 | reviewers=["alice"], |
| 932 | ) |
| 933 | assert result.total == 1 # only one row for alice |
| 934 | |
| 935 | async def test_merge_commit_id_persisted( |
| 936 | self, db_session: AsyncSession |
| 937 | ) -> None: |
| 938 | from sqlalchemy import select as sa_select |
| 939 | from musehub.services import musehub_proposals |
| 940 | |
| 941 | repo = await _repo(db_session, "mc-persisted") |
| 942 | await _branch_with_commit(db_session, repo.repo_id, "feat-mc") |
| 943 | from musehub.services import musehub_proposals |
| 944 | proposal = await musehub_proposals.create_proposal( |
| 945 | db_session, repo_id=repo.repo_id, |
| 946 | title="MC test", from_branch="feat-mc", to_branch="main", |
| 947 | ) |
| 948 | merged = await musehub_proposals.merge_proposal( |
| 949 | db_session, repo.repo_id, proposal.proposal_id |
| 950 | ) |
| 951 | |
| 952 | # Reload from DB |
| 953 | stmt = sa_select(MusehubProposal).where( |
| 954 | MusehubProposal.proposal_id == proposal.proposal_id |
| 955 | ) |
| 956 | row = (await db_session.execute(stmt)).scalar_one() |
| 957 | assert row.merge_commit_id == merged.merge_commit_id |
| 958 | assert row.state == "merged" |
| 959 | |
| 960 | async def test_merge_idempotence_409( |
| 961 | self, db_session: AsyncSession |
| 962 | ) -> None: |
| 963 | from musehub.services import musehub_proposals |
| 964 | |
| 965 | repo = await _repo(db_session, "merge-idem") |
| 966 | await _branch_with_commit(db_session, repo.repo_id, "feat-idem") |
| 967 | from musehub.services import musehub_proposals |
| 968 | proposal = await musehub_proposals.create_proposal( |
| 969 | db_session, repo_id=repo.repo_id, |
| 970 | title="Idempotent merge", from_branch="feat-idem", to_branch="main", |
| 971 | ) |
| 972 | await musehub_proposals.merge_proposal(db_session, repo.repo_id, proposal.proposal_id) |
| 973 | |
| 974 | with pytest.raises(RuntimeError, match="already merged"): |
| 975 | await musehub_proposals.merge_proposal(db_session, repo.repo_id, proposal.proposal_id) |
| 976 | |
| 977 | |
| 978 | # =========================================================================== |
| 979 | # Layer 6 — Security tests |
| 980 | # =========================================================================== |
| 981 | |
| 982 | |
| 983 | class TestSecurity: |
| 984 | async def test_create_proposal_requires_auth( |
| 985 | self, client: AsyncClient, db_session: AsyncSession |
| 986 | ) -> None: |
| 987 | repo = await _repo(db_session, "sec-create") |
| 988 | await db_session.commit() |
| 989 | r = await client.post( |
| 990 | f"/api/repos/{repo.repo_id}/proposals", |
| 991 | json={"title": "Unauthed", "fromBranch": "feat", "toBranch": "main"}, |
| 992 | ) |
| 993 | assert r.status_code in (401, 403) |
| 994 | |
| 995 | async def test_merge_requires_auth( |
| 996 | self, client: AsyncClient, db_session: AsyncSession |
| 997 | ) -> None: |
| 998 | repo = await _repo(db_session, "sec-merge") |
| 999 | await _branch_with_commit(db_session, repo.repo_id, "feat-sec") |
| 1000 | from musehub.services import musehub_proposals |
| 1001 | proposal = await musehub_proposals.create_proposal( |
| 1002 | db_session, repo_id=repo.repo_id, |
| 1003 | title="Unauthed merge", from_branch="feat-sec", to_branch="main", |
| 1004 | ) |
| 1005 | await db_session.commit() |
| 1006 | r = await client.post( |
| 1007 | f"/api/repos/{repo.repo_id}/proposals/{proposal.proposal_id}/merge", |
| 1008 | json={"merge_strategy": "merge_commit"}, |
| 1009 | ) |
| 1010 | assert r.status_code in (401, 403) |
| 1011 | |
| 1012 | async def test_comment_requires_auth( |
| 1013 | self, client: AsyncClient, db_session: AsyncSession |
| 1014 | ) -> None: |
| 1015 | repo = await _repo(db_session, "sec-comment") |
| 1016 | await _branch_with_commit(db_session, repo.repo_id, "feat-sc") |
| 1017 | from musehub.services import musehub_proposals |
| 1018 | proposal = await musehub_proposals.create_proposal( |
| 1019 | db_session, repo_id=repo.repo_id, |
| 1020 | title="Comment auth test", from_branch="feat-sc", to_branch="main", |
| 1021 | ) |
| 1022 | await db_session.commit() |
| 1023 | r = await client.post( |
| 1024 | f"/api/repos/{repo.repo_id}/proposals/{proposal.proposal_id}/comments", |
| 1025 | json={"body": "Unauthenticated"}, |
| 1026 | ) |
| 1027 | assert r.status_code in (401, 403) |
| 1028 | |
| 1029 | async def test_request_reviewers_requires_auth( |
| 1030 | self, client: AsyncClient, db_session: AsyncSession |
| 1031 | ) -> None: |
| 1032 | repo = await _repo(db_session, "sec-reviewers") |
| 1033 | await _branch_with_commit(db_session, repo.repo_id, "feat-sr") |
| 1034 | from musehub.services import musehub_proposals |
| 1035 | proposal = await musehub_proposals.create_proposal( |
| 1036 | db_session, repo_id=repo.repo_id, |
| 1037 | title="Reviewer auth test", from_branch="feat-sr", to_branch="main", |
| 1038 | ) |
| 1039 | await db_session.commit() |
| 1040 | r = await client.post( |
| 1041 | f"/api/repos/{repo.repo_id}/proposals/{proposal.proposal_id}/reviewers", |
| 1042 | json={"reviewers": ["bob"]}, |
| 1043 | ) |
| 1044 | assert r.status_code in (401, 403) |
| 1045 | |
| 1046 | async def test_submit_review_requires_auth( |
| 1047 | self, client: AsyncClient, db_session: AsyncSession |
| 1048 | ) -> None: |
| 1049 | repo = await _repo(db_session, "sec-submit-rv") |
| 1050 | await _branch_with_commit(db_session, repo.repo_id, "feat-srva") |
| 1051 | from musehub.services import musehub_proposals |
| 1052 | proposal = await musehub_proposals.create_proposal( |
| 1053 | db_session, repo_id=repo.repo_id, |
| 1054 | title="Submit auth test", from_branch="feat-srva", to_branch="main", |
| 1055 | ) |
| 1056 | await db_session.commit() |
| 1057 | r = await client.post( |
| 1058 | f"/api/repos/{repo.repo_id}/proposals/{proposal.proposal_id}/reviews", |
| 1059 | json={"verdict": "approve"}, |
| 1060 | ) |
| 1061 | assert r.status_code in (401, 403) |
| 1062 | |
| 1063 | async def test_cross_repo_proposal_returns_404( |
| 1064 | self, client: AsyncClient, auth_headers: StrDict, db_session: AsyncSession |
| 1065 | ) -> None: |
| 1066 | """A proposal_id from repo A cannot be fetched via repo B's route.""" |
| 1067 | r1_id = await _api_repo(client, auth_headers, "xr-sec-a") |
| 1068 | r2_id = await _api_repo(client, auth_headers, "xr-sec-b") |
| 1069 | await _branch_with_commit(db_session, r1_id, "feat-xr") |
| 1070 | await db_session.commit() |
| 1071 | |
| 1072 | proposal = await _api_proposal(client, auth_headers, r1_id, from_branch="feat-xr") |
| 1073 | |
| 1074 | # Try fetching R1's proposal via R2's route |
| 1075 | r = await client.get(f"/api/repos/{r2_id}/proposals/{proposal['proposalId']}") |
| 1076 | assert r.status_code == 404 |
| 1077 | |
| 1078 | async def test_title_max_length_enforced( |
| 1079 | self, client: AsyncClient, auth_headers: StrDict, db_session: AsyncSession |
| 1080 | ) -> None: |
| 1081 | repo_id = await _api_repo(client, auth_headers, "sec-title-len") |
| 1082 | r = await client.post( |
| 1083 | f"/api/repos/{repo_id}/proposals", |
| 1084 | json={"title": "x" * 501, "fromBranch": "a", "toBranch": "b"}, |
| 1085 | headers=auth_headers, |
| 1086 | ) |
| 1087 | assert r.status_code == 422 |
| 1088 | |
| 1089 | |
| 1090 | # =========================================================================== |
| 1091 | # Layer 7 — Performance tests |
| 1092 | # =========================================================================== |
| 1093 | |
| 1094 | |
| 1095 | class TestPerformance: |
| 1096 | async def test_list_100_proposals_under_500ms( |
| 1097 | self, db_session: AsyncSession |
| 1098 | ) -> None: |
| 1099 | from musehub.services import musehub_proposals |
| 1100 | |
| 1101 | repo = await _repo(db_session, "perf-100-proposals") |
| 1102 | for i in range(100): |
| 1103 | await _branch_with_commit(db_session, repo.repo_id, f"perf-feat-{i}") |
| 1104 | await musehub_proposals.create_proposal( |
| 1105 | db_session, repo_id=repo.repo_id, |
| 1106 | title=f"Perf proposal {i}", from_branch=f"perf-feat-{i}", to_branch="main", |
| 1107 | ) |
| 1108 | |
| 1109 | start = time.monotonic() |
| 1110 | proposals_list = await musehub_proposals.list_proposals(db_session, repo.repo_id, limit=100) |
| 1111 | elapsed = time.monotonic() - start |
| 1112 | |
| 1113 | assert proposals_list.total == 100 |
| 1114 | assert elapsed < 0.5, f"list_proposals took {elapsed:.3f}s (limit 0.5s)" |
| 1115 | |
| 1116 | async def test_list_100_comments_under_300ms( |
| 1117 | self, db_session: AsyncSession |
| 1118 | ) -> None: |
| 1119 | from musehub.services import musehub_proposals |
| 1120 | |
| 1121 | repo = await _repo(db_session, "perf-100-comments") |
| 1122 | await _branch_with_commit(db_session, repo.repo_id, "perf-cmt") |
| 1123 | from musehub.services import musehub_proposals |
| 1124 | proposal = await musehub_proposals.create_proposal( |
| 1125 | db_session, repo_id=repo.repo_id, |
| 1126 | title="Perf comments proposal", from_branch="perf-cmt", to_branch="main", |
| 1127 | ) |
| 1128 | await db_session.flush() |
| 1129 | |
| 1130 | for i in range(100): |
| 1131 | await musehub_proposals.create_proposal_comment( |
| 1132 | db_session, |
| 1133 | proposal_id=proposal.proposal_id, |
| 1134 | repo_id=repo.repo_id, |
| 1135 | author="perf-user", |
| 1136 | body=f"Perf comment {i}", |
| 1137 | ) |
| 1138 | |
| 1139 | start = time.monotonic() |
| 1140 | result = await musehub_proposals.list_proposal_comments( |
| 1141 | db_session, proposal.proposal_id, repo.repo_id |
| 1142 | ) |
| 1143 | elapsed = time.monotonic() - start |
| 1144 | |
| 1145 | assert result.total == 100 |
| 1146 | assert elapsed < 0.3, f"list_proposal_comments took {elapsed:.3f}s (limit 0.3s)" |
| 1147 | |
| 1148 | async def test_compute_risk_100x_under_100ms(self) -> None: |
| 1149 | """compute_risk is called once per page render — 100× must be fast.""" |
| 1150 | start = time.monotonic() |
| 1151 | for _ in range(100): |
| 1152 | _risk(breaking=2, sym_modified=10, sym_added=5) |
| 1153 | elapsed = time.monotonic() - start |
| 1154 | assert elapsed < 0.1, f"100× compute_risk took {elapsed:.3f}s (limit 0.1s)" |
| 1155 | |
| 1156 | |
| 1157 | # --------------------------------------------------------------------------- |
| 1158 | # Commit graph — merge_proposal must insert a MusehubCommitGraph row |
| 1159 | # --------------------------------------------------------------------------- |
| 1160 | |
| 1161 | class TestMergeProposalCommitGraph: |
| 1162 | """merge_proposal must insert the merge commit into musehub_commit_graph. |
| 1163 | |
| 1164 | Without a commit graph row, wire_fetch_mpack cannot find the merge |
| 1165 | commit's snapshot_id when a client pulls after the merge. The client |
| 1166 | receives commits=1 snaps=0 blobs=0 and the pull aborts with |
| 1167 | 'snapshot … is missing or corrupt'. |
| 1168 | """ |
| 1169 | |
| 1170 | @pytest.mark.asyncio |
| 1171 | async def test_merge_creates_commit_graph_row( |
| 1172 | self, db_session: AsyncSession |
| 1173 | ) -> None: |
| 1174 | """merge_proposal inserts the merge commit into musehub_commit_graph.""" |
| 1175 | from sqlalchemy import select |
| 1176 | from musehub.db.musehub_repo_models import MusehubCommitGraph |
| 1177 | from musehub.services import musehub_proposals |
| 1178 | |
| 1179 | repo = await _repo(db_session, "cg-merge-test") |
| 1180 | from_commit = await _branch_with_commit(db_session, repo.repo_id, "feat/thing") |
| 1181 | _to_commit = await _branch_with_commit(db_session, repo.repo_id, "dev") |
| 1182 | await db_session.flush() |
| 1183 | |
| 1184 | from musehub.services import musehub_proposals |
| 1185 | proposal = await musehub_proposals.create_proposal( |
| 1186 | db_session, |
| 1187 | repo_id=repo.repo_id, |
| 1188 | title="test merge", |
| 1189 | body="", |
| 1190 | from_branch="feat/thing", |
| 1191 | to_branch="dev", |
| 1192 | author="alice", |
| 1193 | ) |
| 1194 | await db_session.flush() |
| 1195 | |
| 1196 | from musehub.services import musehub_proposals |
| 1197 | result = await musehub_proposals.merge_proposal( |
| 1198 | db_session, |
| 1199 | proposal_id=proposal.proposal_id, |
| 1200 | repo_id=repo.repo_id, |
| 1201 | merger_handle="alice", |
| 1202 | ) |
| 1203 | await db_session.flush() |
| 1204 | |
| 1205 | merge_commit_id = result.merge_commit_id |
| 1206 | assert merge_commit_id, "merge_proposal must return a merge_commit_id" |
| 1207 | |
| 1208 | row = (await db_session.execute( |
| 1209 | select(MusehubCommitGraph).where( |
| 1210 | MusehubCommitGraph.commit_id == merge_commit_id |
| 1211 | ) |
| 1212 | )).scalar_one_or_none() |
| 1213 | |
| 1214 | assert row is not None, ( |
| 1215 | f"merge commit {merge_commit_id[:20]} must have a MusehubCommitGraph row " |
| 1216 | "so wire_fetch_mpack can include its snapshot in pull responses" |
| 1217 | ) |
| 1218 | assert row.snapshot_id is not None or True # snapshot may be None for empty merge |
| 1219 | assert row.generation >= 0 |
| 1220 | |
| 1221 | @pytest.mark.asyncio |
| 1222 | async def test_merge_commit_graph_generation_is_parent_plus_one( |
| 1223 | self, db_session: AsyncSession |
| 1224 | ) -> None: |
| 1225 | """The merge commit's generation = max(parent generations) + 1.""" |
| 1226 | from sqlalchemy import select |
| 1227 | from musehub.db.musehub_repo_models import MusehubCommitGraph |
| 1228 | from musehub.services import musehub_proposals |
| 1229 | |
| 1230 | repo = await _repo(db_session, "cg-gen-test") |
| 1231 | from_commit = await _branch_with_commit(db_session, repo.repo_id, "feat/gen") |
| 1232 | to_commit = await _branch_with_commit(db_session, repo.repo_id, "main") |
| 1233 | |
| 1234 | # Seed known generations for both parent commits. |
| 1235 | db_session.add(MusehubCommitGraph(commit_id=from_commit, parent_ids=[], generation=5)) |
| 1236 | db_session.add(MusehubCommitGraph(commit_id=to_commit, parent_ids=[], generation=3)) |
| 1237 | await db_session.flush() |
| 1238 | |
| 1239 | from musehub.services import musehub_proposals |
| 1240 | proposal = await musehub_proposals.create_proposal( |
| 1241 | db_session, |
| 1242 | repo_id=repo.repo_id, |
| 1243 | title="gen test", |
| 1244 | body="", |
| 1245 | from_branch="feat/gen", |
| 1246 | to_branch="main", |
| 1247 | author="alice", |
| 1248 | ) |
| 1249 | await db_session.flush() |
| 1250 | |
| 1251 | from musehub.services import musehub_proposals |
| 1252 | result = await musehub_proposals.merge_proposal( |
| 1253 | db_session, |
| 1254 | proposal_id=proposal.proposal_id, |
| 1255 | repo_id=repo.repo_id, |
| 1256 | merger_handle="alice", |
| 1257 | ) |
| 1258 | await db_session.flush() |
| 1259 | |
| 1260 | row = (await db_session.execute( |
| 1261 | select(MusehubCommitGraph).where( |
| 1262 | MusehubCommitGraph.commit_id == result.merge_commit_id |
| 1263 | ) |
| 1264 | )).scalar_one_or_none() |
| 1265 | |
| 1266 | assert row is not None |
| 1267 | assert row.generation == 6, ( |
| 1268 | f"generation should be max(5,3)+1=6, got {row.generation}" |
| 1269 | ) |
| 1270 | |
| 1271 | |
| 1272 | # --------------------------------------------------------------------------- |
| 1273 | # VCS commit history styles — TDD for squash and rebase |
| 1274 | # --------------------------------------------------------------------------- |
| 1275 | |
| 1276 | |
| 1277 | |
| 1278 | # --------------------------------------------------------------------------- |
| 1279 | # VCS commit history styles — TDD |
| 1280 | # --------------------------------------------------------------------------- |
| 1281 | |
| 1282 | class TestCommitHistoryStyles: |
| 1283 | """merge_proposal respects the commit_history parameter. |
| 1284 | |
| 1285 | Three styles (--history flag): |
| 1286 | merge — one new commit, parent_ids = [to_head, from_head] (default) |
| 1287 | squash — one new commit, parent_ids = [to_head] only |
| 1288 | rebase — N commits replayed linearly, each with one parent |
| 1289 | """ |
| 1290 | |
| 1291 | @pytest.mark.asyncio |
| 1292 | async def test_merge_has_two_parents(self, db_session: AsyncSession) -> None: |
| 1293 | """commit_history='merge' creates a commit with both heads as parents.""" |
| 1294 | from musehub.services import musehub_proposals |
| 1295 | |
| 1296 | repo = await _repo(db_session, "history-merge") |
| 1297 | to_cid = await _branch_with_commit(db_session, repo.repo_id, "dev") |
| 1298 | from_cid = await _branch_with_commit(db_session, repo.repo_id, "feat/a") |
| 1299 | await db_session.flush() |
| 1300 | |
| 1301 | proposal = await musehub_proposals.create_proposal( |
| 1302 | db_session, repo_id=repo.repo_id, title="merge style", |
| 1303 | body="", from_branch="feat/a", to_branch="dev", author="alice", |
| 1304 | ) |
| 1305 | await db_session.flush() |
| 1306 | |
| 1307 | result = await musehub_proposals.merge_proposal( |
| 1308 | db_session, proposal_id=proposal.proposal_id, |
| 1309 | repo_id=repo.repo_id, merger_handle="alice", |
| 1310 | commit_history="merge", |
| 1311 | ) |
| 1312 | await db_session.flush() |
| 1313 | |
| 1314 | c = await db_session.get(MusehubCommit, result.merge_commit_id) |
| 1315 | assert c is not None |
| 1316 | assert len(c.parent_ids) == 2, f"merge must have 2 parents, got {c.parent_ids}" |
| 1317 | assert to_cid in c.parent_ids |
| 1318 | assert from_cid in c.parent_ids |
| 1319 | |
| 1320 | @pytest.mark.asyncio |
| 1321 | async def test_squash_has_one_parent(self, db_session: AsyncSession) -> None: |
| 1322 | """commit_history='squash' creates a single commit with only to_branch as parent.""" |
| 1323 | from musehub.services import musehub_proposals |
| 1324 | |
| 1325 | repo = await _repo(db_session, "history-squash") |
| 1326 | to_cid = await _branch_with_commit(db_session, repo.repo_id, "dev") |
| 1327 | from_cid = await _branch_with_commit(db_session, repo.repo_id, "feat/b") |
| 1328 | await db_session.flush() |
| 1329 | |
| 1330 | proposal = await musehub_proposals.create_proposal( |
| 1331 | db_session, repo_id=repo.repo_id, title="squash style", |
| 1332 | body="", from_branch="feat/b", to_branch="dev", author="alice", |
| 1333 | ) |
| 1334 | await db_session.flush() |
| 1335 | |
| 1336 | result = await musehub_proposals.merge_proposal( |
| 1337 | db_session, proposal_id=proposal.proposal_id, |
| 1338 | repo_id=repo.repo_id, merger_handle="alice", |
| 1339 | commit_history="squash", |
| 1340 | ) |
| 1341 | await db_session.flush() |
| 1342 | |
| 1343 | c = await db_session.get(MusehubCommit, result.merge_commit_id) |
| 1344 | assert c is not None |
| 1345 | assert c.parent_ids == [to_cid], ( |
| 1346 | f"squash must have exactly [to_head] as parent, got {c.parent_ids}" |
| 1347 | ) |
| 1348 | |
| 1349 | @pytest.mark.asyncio |
| 1350 | async def test_default_is_merge(self, db_session: AsyncSession) -> None: |
| 1351 | """Omitting commit_history defaults to merge (two parents).""" |
| 1352 | from musehub.services import musehub_proposals |
| 1353 | |
| 1354 | repo = await _repo(db_session, "history-default") |
| 1355 | await _branch_with_commit(db_session, repo.repo_id, "dev") |
| 1356 | await _branch_with_commit(db_session, repo.repo_id, "feat/c") |
| 1357 | await db_session.flush() |
| 1358 | |
| 1359 | proposal = await musehub_proposals.create_proposal( |
| 1360 | db_session, repo_id=repo.repo_id, title="default style", |
| 1361 | body="", from_branch="feat/c", to_branch="dev", author="alice", |
| 1362 | ) |
| 1363 | await db_session.flush() |
| 1364 | |
| 1365 | result = await musehub_proposals.merge_proposal( |
| 1366 | db_session, proposal_id=proposal.proposal_id, |
| 1367 | repo_id=repo.repo_id, merger_handle="alice", |
| 1368 | ) |
| 1369 | await db_session.flush() |
| 1370 | |
| 1371 | c = await db_session.get(MusehubCommit, result.merge_commit_id) |
| 1372 | assert len(c.parent_ids) == 2, "default must be merge (2 parents)" |
| 1373 | |
| 1374 | |
| 1375 | # --------------------------------------------------------------------------- |
| 1376 | # Strategy naming — clean aliases (overlay/weave/replay/selective) |
| 1377 | # --------------------------------------------------------------------------- |
| 1378 | |
| 1379 | class TestStrategyNaming: |
| 1380 | """Content merge strategies use clean names without state_/domain_ prefixes.""" |
| 1381 | |
| 1382 | @pytest.mark.asyncio |
| 1383 | async def test_overlay_accepted(self, db_session: AsyncSession) -> None: |
| 1384 | """'overlay' is the canonical strategy name.""" |
| 1385 | from musehub.services import musehub_proposals |
| 1386 | |
| 1387 | repo = await _repo(db_session, "strategy-overlay") |
| 1388 | await _branch_with_commit(db_session, repo.repo_id, "dev") |
| 1389 | await _branch_with_commit(db_session, repo.repo_id, "feat/ov") |
| 1390 | await db_session.flush() |
| 1391 | |
| 1392 | proposal = await musehub_proposals.create_proposal( |
| 1393 | db_session, repo_id=repo.repo_id, title="overlay test", |
| 1394 | body="", from_branch="feat/ov", to_branch="dev", author="alice", |
| 1395 | ) |
| 1396 | await db_session.flush() |
| 1397 | |
| 1398 | # Should not raise — overlay is a valid strategy |
| 1399 | result = await musehub_proposals.merge_proposal( |
| 1400 | db_session, proposal_id=proposal.proposal_id, |
| 1401 | repo_id=repo.repo_id, merger_handle="alice", |
| 1402 | merge_strategy="overlay", |
| 1403 | ) |
| 1404 | assert result.merge_commit_id is not None |
| 1405 | |
| 1406 | @pytest.mark.asyncio |
| 1407 | async def test_replay_accepted(self, db_session: AsyncSession) -> None: |
| 1408 | """'replay' is the canonical strategy name.""" |
| 1409 | from musehub.services import musehub_proposals |
| 1410 | |
| 1411 | repo = await _repo(db_session, "strategy-replay") |
| 1412 | await _branch_with_commit(db_session, repo.repo_id, "dev") |
| 1413 | await _branch_with_commit(db_session, repo.repo_id, "feat/rp") |
| 1414 | await db_session.flush() |
| 1415 | |
| 1416 | proposal = await musehub_proposals.create_proposal( |
| 1417 | db_session, repo_id=repo.repo_id, title="replay test", |
| 1418 | body="", from_branch="feat/rp", to_branch="dev", author="alice", |
| 1419 | ) |
| 1420 | await db_session.flush() |
| 1421 | |
| 1422 | result = await musehub_proposals.merge_proposal( |
| 1423 | db_session, proposal_id=proposal.proposal_id, |
| 1424 | repo_id=repo.repo_id, merger_handle="alice", |
| 1425 | merge_strategy="replay", |
| 1426 | ) |
| 1427 | assert result.merge_commit_id is not None |
| 1428 | |
| 1429 | |
| 1430 | # --------------------------------------------------------------------------- |
| 1431 | # Rebase history style — full multi-commit replay |
| 1432 | # --------------------------------------------------------------------------- |
| 1433 | |
| 1434 | class TestRebaseHistory: |
| 1435 | """--history rebase replays each from_branch commit individually. |
| 1436 | |
| 1437 | For a proposal with N commits the result is N new linear commits on |
| 1438 | to_branch, NOT a single merge commit. |
| 1439 | |
| 1440 | Chain must be: |
| 1441 | to_head → replayed_0 → replayed_1 → … → replayed_N-1 |
| 1442 | Each replayed commit: |
| 1443 | - has exactly one parent |
| 1444 | - preserves the original message and author |
| 1445 | - is a new commit_id (different parent → different hash) |
| 1446 | to_branch.head_commit_id advances to replayed_N-1. |
| 1447 | """ |
| 1448 | |
| 1449 | async def _make_repo_and_branches( |
| 1450 | self, db: AsyncSession, slug: str |
| 1451 | ) -> tuple[MusehubRepo, str, list[str]]: |
| 1452 | """Returns (repo, to_cid, [from_cid_old, from_cid_new]).""" |
| 1453 | import msgpack |
| 1454 | from musehub.muse_cli.snapshot import compute_snapshot_id |
| 1455 | from musehub.db.musehub_repo_models import MusehubSnapshot, MusehubSnapshotRef |
| 1456 | |
| 1457 | repo = await _repo(db, slug) |
| 1458 | |
| 1459 | # to_branch: one commit with a known snapshot |
| 1460 | to_snap_manifest = {"base.py": fake_id("base-file")} |
| 1461 | to_snap_id = compute_snapshot_id(to_snap_manifest) |
| 1462 | db.add(MusehubSnapshot( |
| 1463 | snapshot_id=to_snap_id, |
| 1464 | manifest_blob=msgpack.packb(to_snap_manifest, use_bin_type=True), |
| 1465 | directories=[], entry_count=len(to_snap_manifest), |
| 1466 | created_at=datetime.now(tz=timezone.utc), |
| 1467 | )) |
| 1468 | db.add(MusehubSnapshotRef(repo_id=repo.repo_id, snapshot_id=to_snap_id)) |
| 1469 | |
| 1470 | to_cid = fake_id(f"{slug}-to") |
| 1471 | db.add(MusehubCommit( |
| 1472 | commit_id=to_cid, branch="dev", parent_ids=[], |
| 1473 | message="base commit", author="alice", |
| 1474 | timestamp=datetime.now(tz=timezone.utc), snapshot_id=to_snap_id, |
| 1475 | )) |
| 1476 | db.add(MusehubBranch( |
| 1477 | branch_id=compute_branch_id(repo.repo_id, "dev"), |
| 1478 | repo_id=repo.repo_id, name="dev", head_commit_id=to_cid, |
| 1479 | )) |
| 1480 | db.add(MusehubCommitRef(repo_id=repo.repo_id, commit_id=to_cid)) |
| 1481 | |
| 1482 | # from_branch: two commits, each adding a file |
| 1483 | snap1_manifest = {**to_snap_manifest, "feat_a.py": fake_id("feat-a")} |
| 1484 | snap1_id = compute_snapshot_id(snap1_manifest) |
| 1485 | db.add(MusehubSnapshot( |
| 1486 | snapshot_id=snap1_id, |
| 1487 | manifest_blob=msgpack.packb(snap1_manifest, use_bin_type=True), |
| 1488 | directories=[], entry_count=len(snap1_manifest), |
| 1489 | created_at=datetime.now(tz=timezone.utc), |
| 1490 | )) |
| 1491 | db.add(MusehubSnapshotRef(repo_id=repo.repo_id, snapshot_id=snap1_id)) |
| 1492 | |
| 1493 | from_cid1 = fake_id(f"{slug}-from-1") |
| 1494 | db.add(MusehubCommit( |
| 1495 | commit_id=from_cid1, branch="feat/x", parent_ids=[to_cid], |
| 1496 | message="add feat_a", author="alice", |
| 1497 | timestamp=datetime.now(tz=timezone.utc), snapshot_id=snap1_id, |
| 1498 | )) |
| 1499 | db.add(MusehubCommitRef(repo_id=repo.repo_id, commit_id=from_cid1)) |
| 1500 | |
| 1501 | snap2_manifest = {**snap1_manifest, "feat_b.py": fake_id("feat-b")} |
| 1502 | snap2_id = compute_snapshot_id(snap2_manifest) |
| 1503 | db.add(MusehubSnapshot( |
| 1504 | snapshot_id=snap2_id, |
| 1505 | manifest_blob=msgpack.packb(snap2_manifest, use_bin_type=True), |
| 1506 | directories=[], entry_count=len(snap2_manifest), |
| 1507 | created_at=datetime.now(tz=timezone.utc), |
| 1508 | )) |
| 1509 | db.add(MusehubSnapshotRef(repo_id=repo.repo_id, snapshot_id=snap2_id)) |
| 1510 | |
| 1511 | from_cid2 = fake_id(f"{slug}-from-2") |
| 1512 | db.add(MusehubCommit( |
| 1513 | commit_id=from_cid2, branch="feat/x", parent_ids=[from_cid1], |
| 1514 | message="add feat_b", author="alice", |
| 1515 | timestamp=datetime.now(tz=timezone.utc), snapshot_id=snap2_id, |
| 1516 | )) |
| 1517 | db.add(MusehubBranch( |
| 1518 | branch_id=compute_branch_id(repo.repo_id, "feat/x"), |
| 1519 | repo_id=repo.repo_id, name="feat/x", head_commit_id=from_cid2, |
| 1520 | )) |
| 1521 | db.add(MusehubCommitRef(repo_id=repo.repo_id, commit_id=from_cid2)) |
| 1522 | |
| 1523 | await db.flush() |
| 1524 | return repo, to_cid, [from_cid1, from_cid2] |
| 1525 | |
| 1526 | @pytest.mark.asyncio |
| 1527 | async def test_rebase_creates_n_linear_commits( |
| 1528 | self, db_session: AsyncSession |
| 1529 | ) -> None: |
| 1530 | """2 from_branch commits → 2 replayed commits, each with 1 parent.""" |
| 1531 | from musehub.services import musehub_proposals |
| 1532 | from sqlalchemy import select |
| 1533 | |
| 1534 | repo, to_cid, from_cids = await self._make_repo_and_branches( |
| 1535 | db_session, "rebase-n-commits" |
| 1536 | ) |
| 1537 | |
| 1538 | proposal = await musehub_proposals.create_proposal( |
| 1539 | db_session, repo_id=repo.repo_id, title="rebase test", |
| 1540 | body="", from_branch="feat/x", to_branch="dev", author="alice", |
| 1541 | ) |
| 1542 | await db_session.flush() |
| 1543 | |
| 1544 | result = await musehub_proposals.merge_proposal( |
| 1545 | db_session, proposal_id=proposal.proposal_id, |
| 1546 | repo_id=repo.repo_id, merger_handle="alice", |
| 1547 | commit_history="rebase", |
| 1548 | ) |
| 1549 | await db_session.flush() |
| 1550 | |
| 1551 | # to_branch head must have advanced |
| 1552 | to_branch = (await db_session.execute( |
| 1553 | select(MusehubBranch).where( |
| 1554 | MusehubBranch.repo_id == repo.repo_id, |
| 1555 | MusehubBranch.name == "dev", |
| 1556 | ) |
| 1557 | )).scalar_one() |
| 1558 | |
| 1559 | tip_cid = to_branch.head_commit_id |
| 1560 | assert tip_cid != to_cid, "to_branch head must advance beyond original to_cid" |
| 1561 | |
| 1562 | # Walk the chain from tip back to to_cid — must be exactly 2 new commits |
| 1563 | chain: list[MusehubCommit] = [] |
| 1564 | current = await db_session.get(MusehubCommit, tip_cid) |
| 1565 | while current and current.commit_id != to_cid: |
| 1566 | chain.append(current) |
| 1567 | assert len(current.parent_ids) == 1, ( |
| 1568 | f"Rebase commit {current.commit_id[:16]} must have exactly 1 parent, " |
| 1569 | f"got {current.parent_ids}" |
| 1570 | ) |
| 1571 | current = await db_session.get(MusehubCommit, current.parent_ids[0]) |
| 1572 | |
| 1573 | assert len(chain) == 2, ( |
| 1574 | f"Expected 2 replayed commits (one per from_branch commit), got {len(chain)}" |
| 1575 | ) |
| 1576 | |
| 1577 | @pytest.mark.asyncio |
| 1578 | async def test_rebase_preserves_messages( |
| 1579 | self, db_session: AsyncSession |
| 1580 | ) -> None: |
| 1581 | """Each replayed commit preserves the original message.""" |
| 1582 | from musehub.services import musehub_proposals |
| 1583 | from sqlalchemy import select |
| 1584 | |
| 1585 | repo, to_cid, _ = await self._make_repo_and_branches( |
| 1586 | db_session, "rebase-messages" |
| 1587 | ) |
| 1588 | |
| 1589 | proposal = await musehub_proposals.create_proposal( |
| 1590 | db_session, repo_id=repo.repo_id, title="msg test", |
| 1591 | body="", from_branch="feat/x", to_branch="dev", author="alice", |
| 1592 | ) |
| 1593 | await db_session.flush() |
| 1594 | |
| 1595 | result = await musehub_proposals.merge_proposal( |
| 1596 | db_session, proposal_id=proposal.proposal_id, |
| 1597 | repo_id=repo.repo_id, merger_handle="alice", |
| 1598 | commit_history="rebase", |
| 1599 | ) |
| 1600 | await db_session.flush() |
| 1601 | |
| 1602 | to_branch = (await db_session.execute( |
| 1603 | select(MusehubBranch).where( |
| 1604 | MusehubBranch.repo_id == repo.repo_id, |
| 1605 | MusehubBranch.name == "dev", |
| 1606 | ) |
| 1607 | )).scalar_one() |
| 1608 | |
| 1609 | messages = [] |
| 1610 | current = await db_session.get(MusehubCommit, to_branch.head_commit_id) |
| 1611 | while current and current.commit_id != to_cid: |
| 1612 | messages.append(current.message) |
| 1613 | current = await db_session.get(MusehubCommit, current.parent_ids[0]) |
| 1614 | |
| 1615 | # Messages should be the original ones (in reverse order since we walked tip→base) |
| 1616 | assert set(messages) == {"add feat_a", "add feat_b"}, ( |
| 1617 | f"Expected original messages preserved, got {messages}" |
| 1618 | ) |
| 1619 | |
| 1620 | @pytest.mark.asyncio |
| 1621 | async def test_rebase_tip_is_merge_commit_id( |
| 1622 | self, db_session: AsyncSession |
| 1623 | ) -> None: |
| 1624 | """merge_commit_id on the result points to the tip of the replayed chain.""" |
| 1625 | from musehub.services import musehub_proposals |
| 1626 | from sqlalchemy import select |
| 1627 | |
| 1628 | repo, to_cid, _ = await self._make_repo_and_branches( |
| 1629 | db_session, "rebase-tip" |
| 1630 | ) |
| 1631 | |
| 1632 | proposal = await musehub_proposals.create_proposal( |
| 1633 | db_session, repo_id=repo.repo_id, title="tip test", |
| 1634 | body="", from_branch="feat/x", to_branch="dev", author="alice", |
| 1635 | ) |
| 1636 | await db_session.flush() |
| 1637 | |
| 1638 | result = await musehub_proposals.merge_proposal( |
| 1639 | db_session, proposal_id=proposal.proposal_id, |
| 1640 | repo_id=repo.repo_id, merger_handle="alice", |
| 1641 | commit_history="rebase", |
| 1642 | ) |
| 1643 | await db_session.flush() |
| 1644 | |
| 1645 | to_branch = (await db_session.execute( |
| 1646 | select(MusehubBranch).where( |
| 1647 | MusehubBranch.repo_id == repo.repo_id, |
| 1648 | MusehubBranch.name == "dev", |
| 1649 | ) |
| 1650 | )).scalar_one() |
| 1651 | |
| 1652 | assert result.merge_commit_id == to_branch.head_commit_id, ( |
| 1653 | "merge_commit_id must be the tip of the replayed chain" |
| 1654 | ) |
| 1655 | |
| 1656 | |
| 1657 | # =========================================================================== |
| 1658 | # musehub#182 — state-persistence: a fresh read after a real HTTP merge must |
| 1659 | # see state="merged" (not the same in-memory session the merge itself used), |
| 1660 | # and a second HTTP merge attempt must 409 rather than silently re-merging. |
| 1661 | # =========================================================================== |
| 1662 | |
| 1663 | |
| 1664 | class TestMergeStatePersistenceAcrossFreshRequests: |
| 1665 | async def test_fresh_read_after_http_merge_sees_merged_state( |
| 1666 | self, client: AsyncClient, auth_headers: StrDict, db_session: AsyncSession, |
| 1667 | ) -> None: |
| 1668 | """musehub#182 bug #1: the original incident found `state` still |
| 1669 | 'open' on a fresh read immediately after a merge call had already |
| 1670 | returned success and the branch had genuinely advanced. Reproduce |
| 1671 | by going through the real HTTP route (which commits via `get_db`'s |
| 1672 | request-scoped session) rather than sharing one db_session/transaction |
| 1673 | across both the merge and the read, which is what let this bug hide |
| 1674 | in the pre-existing `test_merge_idempotence_409` test. |
| 1675 | """ |
| 1676 | repo = await _repo(db_session, "state-persist-repo", owner="testuser") |
| 1677 | await _branch_with_commit(db_session, repo.repo_id, "feat-persist") |
| 1678 | from musehub.services import musehub_proposals |
| 1679 | proposal = await musehub_proposals.create_proposal( |
| 1680 | db_session, repo_id=repo.repo_id, |
| 1681 | title="State persistence check", from_branch="feat-persist", to_branch="main", |
| 1682 | ) |
| 1683 | await db_session.commit() |
| 1684 | |
| 1685 | merge_resp = await client.post( |
| 1686 | f"/api/repos/{repo.repo_id}/proposals/{proposal.proposal_id}/merge", |
| 1687 | json={"merge_strategy": "merge_commit"}, |
| 1688 | headers=auth_headers, |
| 1689 | ) |
| 1690 | assert merge_resp.status_code == 200 |
| 1691 | merge_commit_id = merge_resp.json()["mergeCommitId"] |
| 1692 | assert merge_commit_id is not None |
| 1693 | |
| 1694 | # Fresh read via a brand-new HTTP request — must see the committed state. |
| 1695 | read_resp = await client.get( |
| 1696 | f"/api/repos/{repo.repo_id}/proposals/{proposal.proposal_id}", |
| 1697 | headers=auth_headers, |
| 1698 | ) |
| 1699 | assert read_resp.status_code == 200 |
| 1700 | body = read_resp.json() |
| 1701 | assert body["state"] == "merged", ( |
| 1702 | f"fresh read after merge saw state={body['state']!r} — " |
| 1703 | "the merge's DB write did not persist / was not visible" |
| 1704 | ) |
| 1705 | assert body["mergeCommitId"] == merge_commit_id |
| 1706 | assert body["mergedAt"] is not None |
| 1707 | |
| 1708 | async def test_second_http_merge_returns_409_never_remerges( |
| 1709 | self, client: AsyncClient, auth_headers: StrDict, db_session: AsyncSession, |
| 1710 | ) -> None: |
| 1711 | """musehub#182 bug #1's direct consequence: if the first merge's |
| 1712 | state write is invisible to the guard check, a second call proceeds |
| 1713 | and recomputes the merge — which is bug #2's silent-revert vector. |
| 1714 | This must 409, every time, regardless of session/request boundaries. |
| 1715 | """ |
| 1716 | repo = await _repo(db_session, "double-merge-repo", owner="testuser") |
| 1717 | await _branch_with_commit(db_session, repo.repo_id, "feat-double") |
| 1718 | from musehub.services import musehub_proposals |
| 1719 | proposal = await musehub_proposals.create_proposal( |
| 1720 | db_session, repo_id=repo.repo_id, |
| 1721 | title="Double merge check", from_branch="feat-double", to_branch="main", |
| 1722 | ) |
| 1723 | await db_session.commit() |
| 1724 | |
| 1725 | first = await client.post( |
| 1726 | f"/api/repos/{repo.repo_id}/proposals/{proposal.proposal_id}/merge", |
| 1727 | json={"merge_strategy": "merge_commit"}, |
| 1728 | headers=auth_headers, |
| 1729 | ) |
| 1730 | assert first.status_code == 200 |
| 1731 | first_commit_id = first.json()["mergeCommitId"] |
| 1732 | |
| 1733 | second = await client.post( |
| 1734 | f"/api/repos/{repo.repo_id}/proposals/{proposal.proposal_id}/merge", |
| 1735 | json={"merge_strategy": "merge_commit"}, |
| 1736 | headers=auth_headers, |
| 1737 | ) |
| 1738 | assert second.status_code == 409, ( |
| 1739 | f"second merge attempt returned {second.status_code}, not 409 — " |
| 1740 | "it silently re-merged instead of being rejected" |
| 1741 | ) |
| 1742 | |
| 1743 | # The branch must still point at the FIRST merge's commit — a second, |
| 1744 | # stale-based recomputation must never have touched it. |
| 1745 | read_resp = await client.get( |
| 1746 | f"/api/repos/{repo.repo_id}/proposals/{proposal.proposal_id}", |
| 1747 | headers=auth_headers, |
| 1748 | ) |
| 1749 | assert read_resp.json()["mergeCommitId"] == first_commit_id |
| 1750 | |
| 1751 | |
| 1752 | # =========================================================================== |
| 1753 | # musehub#182 bug #2 — merging a stale-fork proposal must not revert or |
| 1754 | # delete content that to_branch gained independently after the fork point. |
| 1755 | # =========================================================================== |
| 1756 | |
| 1757 | |
| 1758 | class TestMergeDoesNotRevertUnrelatedToBranchChanges: |
| 1759 | """Reproduces the actual musehub#144/proposal#9 incident: a long-lived |
| 1760 | from_branch that never touched fileA/fileB (it forked before to_branch's |
| 1761 | later changes to them) must not cause those files to revert to their |
| 1762 | stale, ancestor-era content when merged with the default 'overlay' |
| 1763 | strategy. Root cause: overlay dispatches through muse.core.merge_engine's |
| 1764 | "snapshot" diff_unit, which forces an EMPTY base regardless of any real |
| 1765 | ancestor -- meaning "from_branch wins" for every path it happens to |
| 1766 | carry, even ones it never touched since the fork. |
| 1767 | """ |
| 1768 | |
| 1769 | async def _make_stale_fork_scenario( |
| 1770 | self, db: AsyncSession, slug: str, |
| 1771 | ) -> tuple[MusehubRepo, str]: |
| 1772 | """ancestor: {fileA: v1, fileB: v1} |
| 1773 | to_branch (main) advances: fileA -> v2 (modified), fileC added (new) |
| 1774 | from_branch forks at ancestor, adds fileD -- never touches fileA/fileB |
| 1775 | Returns (repo, proposal_from_branch_name). |
| 1776 | """ |
| 1777 | import msgpack |
| 1778 | from musehub.muse_cli.snapshot import compute_snapshot_id |
| 1779 | from musehub.db.musehub_repo_models import MusehubSnapshot, MusehubSnapshotRef |
| 1780 | |
| 1781 | repo = await _repo(db, slug, owner="testuser") |
| 1782 | |
| 1783 | def _add_snapshot(manifest: StrDict) -> str: |
| 1784 | snap_id = compute_snapshot_id(manifest) |
| 1785 | db.add(MusehubSnapshot( |
| 1786 | snapshot_id=snap_id, |
| 1787 | manifest_blob=msgpack.packb(manifest, use_bin_type=True), |
| 1788 | directories=[], entry_count=len(manifest), |
| 1789 | created_at=datetime.now(tz=timezone.utc), |
| 1790 | )) |
| 1791 | db.add(MusehubSnapshotRef(repo_id=repo.repo_id, snapshot_id=snap_id)) |
| 1792 | return snap_id |
| 1793 | |
| 1794 | # Ancestor commit on main. |
| 1795 | ancestor_manifest = {"fileA.py": fake_id("fileA-v1"), "fileB.py": fake_id("fileB-v1")} |
| 1796 | ancestor_snap = _add_snapshot(ancestor_manifest) |
| 1797 | ancestor_cid = fake_id(f"{slug}-ancestor") |
| 1798 | db.add(MusehubCommit( |
| 1799 | commit_id=ancestor_cid, branch="main", parent_ids=[], |
| 1800 | message="ancestor", author="alice", |
| 1801 | timestamp=datetime.now(tz=timezone.utc), snapshot_id=ancestor_snap, |
| 1802 | )) |
| 1803 | db.add(MusehubCommitRef(repo_id=repo.repo_id, commit_id=ancestor_cid)) |
| 1804 | |
| 1805 | # from_branch forks HERE — one commit, adds fileD, never touches fileA/fileB. |
| 1806 | from_manifest = {**ancestor_manifest, "fileD.py": fake_id("fileD-feature")} |
| 1807 | from_snap = _add_snapshot(from_manifest) |
| 1808 | from_cid = fake_id(f"{slug}-from") |
| 1809 | db.add(MusehubCommit( |
| 1810 | commit_id=from_cid, branch="feat-stale", parent_ids=[ancestor_cid], |
| 1811 | message="add fileD", author="alice", |
| 1812 | timestamp=datetime.now(tz=timezone.utc), snapshot_id=from_snap, |
| 1813 | )) |
| 1814 | db.add(MusehubBranch( |
| 1815 | branch_id=compute_branch_id(repo.repo_id, "feat-stale"), |
| 1816 | repo_id=repo.repo_id, name="feat-stale", head_commit_id=from_cid, |
| 1817 | )) |
| 1818 | db.add(MusehubCommitRef(repo_id=repo.repo_id, commit_id=from_cid)) |
| 1819 | |
| 1820 | # main advances independently: fileA modified, fileC added. |
| 1821 | to_manifest = { |
| 1822 | "fileA.py": fake_id("fileA-v2"), "fileB.py": ancestor_manifest["fileB.py"], |
| 1823 | "fileC.py": fake_id("fileC-new"), |
| 1824 | } |
| 1825 | to_snap = _add_snapshot(to_manifest) |
| 1826 | to_cid = fake_id(f"{slug}-to") |
| 1827 | db.add(MusehubCommit( |
| 1828 | commit_id=to_cid, branch="main", parent_ids=[ancestor_cid], |
| 1829 | message="modify fileA, add fileC", author="bob", |
| 1830 | timestamp=datetime.now(tz=timezone.utc), snapshot_id=to_snap, |
| 1831 | )) |
| 1832 | db.add(MusehubBranch( |
| 1833 | branch_id=compute_branch_id(repo.repo_id, "main"), |
| 1834 | repo_id=repo.repo_id, name="main", head_commit_id=to_cid, |
| 1835 | )) |
| 1836 | db.add(MusehubCommitRef(repo_id=repo.repo_id, commit_id=to_cid)) |
| 1837 | |
| 1838 | await db.flush() |
| 1839 | self._to_manifest = to_manifest |
| 1840 | self._from_manifest = from_manifest |
| 1841 | return repo, "feat-stale" |
| 1842 | |
| 1843 | async def test_default_overlay_merge_preserves_to_branch_only_changes( |
| 1844 | self, client: AsyncClient, auth_headers: StrDict, db_session: AsyncSession, |
| 1845 | ) -> None: |
| 1846 | from sqlalchemy import select |
| 1847 | from musehub.services.musehub_snapshot import get_snapshot_manifest |
| 1848 | |
| 1849 | repo, from_branch = await self._make_stale_fork_scenario(db_session, "stale-fork-repo") |
| 1850 | from musehub.services import musehub_proposals |
| 1851 | proposal = await musehub_proposals.create_proposal( |
| 1852 | db_session, repo_id=repo.repo_id, |
| 1853 | title="Stale fork merge", from_branch=from_branch, to_branch="main", |
| 1854 | ) |
| 1855 | await db_session.commit() |
| 1856 | |
| 1857 | merge_resp = await client.post( |
| 1858 | f"/api/repos/{repo.repo_id}/proposals/{proposal.proposal_id}/merge", |
| 1859 | json={"merge_strategy": "merge_commit"}, |
| 1860 | headers=auth_headers, |
| 1861 | ) |
| 1862 | assert merge_resp.status_code == 200 |
| 1863 | |
| 1864 | # Re-fetch main's new head and read its real merged manifest. |
| 1865 | branch_stmt = select(MusehubBranch).where( |
| 1866 | MusehubBranch.repo_id == repo.repo_id, MusehubBranch.name == "main", |
| 1867 | ) |
| 1868 | main_branch = (await db_session.execute(branch_stmt)).scalar_one() |
| 1869 | commit = await db_session.get(MusehubCommit, main_branch.head_commit_id) |
| 1870 | manifest = await get_snapshot_manifest(db_session, commit.snapshot_id) |
| 1871 | |
| 1872 | assert manifest["fileA.py"] == self._to_manifest["fileA.py"], ( |
| 1873 | "fileA was modified by to_branch after the fork and never touched by " |
| 1874 | "from_branch — it must NOT revert to the stale ancestor content " |
| 1875 | "from_branch happens to still carry" |
| 1876 | ) |
| 1877 | assert manifest["fileC.py"] == self._to_manifest["fileC.py"], ( |
| 1878 | "fileC was added on to_branch after the fork — from_branch never " |
| 1879 | "had it and must not cause it to be dropped" |
| 1880 | ) |
| 1881 | assert manifest["fileD.py"] == self._from_manifest["fileD.py"], ( |
| 1882 | "fileD is from_branch's genuine addition — must be included" |
| 1883 | ) |
| 1884 | assert manifest["fileB.py"] == self._to_manifest["fileB.py"] |