test_committed_at_raw.py
file-level
1
files
1
commits
0
hotspots
0
🧊 dead
0
💥 blast risk
| 1 | """TDD — musehub#195 narrow fix: committed_at_raw preserves commit identity |
| 2 | across the Postgres timestamptz UTC-normalization boundary. |
| 3 | |
| 4 | Root cause: a commit's content-addressed id is a hash whose preimage |
| 5 | includes the exact committed_at ISO string the client used |
| 6 | (muse/core/ids.py::hash_commit). musehub_commits.timestamp is a Postgres |
| 7 | timestamptz, which normalizes any non-UTC offset to UTC on write. The wire- |
| 8 | serve path (_to_wire_commit) previously reconstructed committed_at via |
| 9 | `timestamp.isoformat()` — for a commit originally authored with a non-UTC |
| 10 | offset, this produces a DIFFERENT string than the one baked into commit_id, |
| 11 | so the client's own hash check correctly rejects the downloaded commit. |
| 12 | |
| 13 | Fix: an additive `committed_at_raw` column stores the exact original string. |
| 14 | The serve path prefers it verbatim; when absent (legacy rows), it falls |
| 15 | back to `timestamp.isoformat()` unchanged from prior behavior. |
| 16 | |
| 17 | verify_commit_identity_or_raise (a standalone, explicitly-callable helper -- |
| 18 | NOT wired automatically into the serve path) recomputes a commit's id from |
| 19 | its wire representation and raises ObjectHashMismatch on mismatch. It is |
| 20 | deliberately NOT called automatically by _commit_to_wire_s3/wire_fetch_mpack: |
| 21 | confirmed experimentally that at least two legitimate write sites |
| 22 | (musehub_repository.py's repo-init synthetic commit, musehub_sync.py's |
| 23 | synthetic content commit) never used hash_commit-derived ids in the first |
| 24 | place, and wire_repair_commit's entire purpose is repairing a commit whose |
| 25 | identity fields don't currently reproduce its id -- an automatic check on |
| 26 | every serve broke that flow outright. Available for explicit/diagnostic use |
| 27 | the same way wire_repair_commit already uses the same recompute-and-compare |
| 28 | pattern. |
| 29 | |
| 30 | Tests: |
| 31 | U1 _to_wire_commit uses committed_at_raw verbatim when present -- the |
| 32 | actual regression this closes. |
| 33 | U2 _to_wire_commit falls back to timestamp.isoformat() when raw is |
| 34 | absent -- unchanged behavior for legacy rows, no regression. |
| 35 | U3 verify_commit_identity_or_raise passes for a self-consistent commit. |
| 36 | U4 verify_commit_identity_or_raise raises ObjectHashMismatch for a |
| 37 | mismatched commit (available for explicit/diagnostic use). |
| 38 | E1 wire_fetch_mpack serves the exact original non-UTC string when |
| 39 | committed_at_raw is present. |
| 40 | E3 wire_fetch_mpack still serves fine for a legacy row with |
| 41 | committed_at_raw=NULL when the original was already UTC (the common |
| 42 | case — reconstruction matches, no regression for existing data). |
| 43 | """ |
| 44 | from __future__ import annotations |
| 45 | |
| 46 | from datetime import datetime, timezone |
| 47 | |
| 48 | import pytest |
| 49 | from sqlalchemy.dialects.postgresql import insert as pg_insert |
| 50 | from sqlalchemy.ext.asyncio import AsyncSession |
| 51 | |
| 52 | from muse.core.mpack import parse_wire_mpack |
| 53 | from muse.core.types import blob_id |
| 54 | from musehub.db import musehub_repo_models as db |
| 55 | from musehub.muse_cli.snapshot import compute_commit_id as hub_compute_commit_id |
| 56 | from musehub.services.musehub_wire_shared import ObjectHashMismatch, _to_wire_commit |
| 57 | from tests.factories import create_repo |
| 58 | |
| 59 | _NON_UTC_TS = "2026-03-18T12:00:00-07:00" |
| 60 | _UTC_TS = "2026-03-18T19:00:00+00:00" # same instant as _NON_UTC_TS |
| 61 | |
| 62 | |
| 63 | def _commit_row( |
| 64 | *, |
| 65 | commit_id: str, |
| 66 | committed_at_iso: str, |
| 67 | committed_at_raw: str | None, |
| 68 | snapshot_id: str = "", |
| 69 | ) -> db.MusehubCommit: |
| 70 | return db.MusehubCommit( |
| 71 | commit_id=commit_id, |
| 72 | branch="main", |
| 73 | parent_ids=[], |
| 74 | message="m", |
| 75 | author="gabriel", |
| 76 | timestamp=datetime.fromisoformat(committed_at_iso), |
| 77 | committed_at_raw=committed_at_raw, |
| 78 | snapshot_id=snapshot_id or None, |
| 79 | ) |
| 80 | |
| 81 | |
| 82 | # --------------------------------------------------------------------------- |
| 83 | # U1/U2 — _to_wire_commit |
| 84 | # --------------------------------------------------------------------------- |
| 85 | |
| 86 | |
| 87 | class TestToWireCommitPrefersRaw: |
| 88 | def test_uses_committed_at_raw_verbatim_when_present(self) -> None: |
| 89 | # timestamp simulates what Postgres normalized the row to (UTC) -- |
| 90 | # deliberately DIFFERENT from committed_at_raw, so this test actually |
| 91 | # discriminates "prefers raw" from "always reconstructs from timestamp". |
| 92 | row = _commit_row( |
| 93 | commit_id="sha256:" + "a" * 64, |
| 94 | committed_at_iso=_UTC_TS, |
| 95 | committed_at_raw=_NON_UTC_TS, |
| 96 | ) |
| 97 | wire = _to_wire_commit(row) |
| 98 | assert wire.committed_at == _NON_UTC_TS |
| 99 | assert wire.committed_at != row.timestamp.isoformat() |
| 100 | |
| 101 | def test_falls_back_to_timestamp_isoformat_when_raw_absent(self) -> None: |
| 102 | row = _commit_row( |
| 103 | commit_id="sha256:" + "b" * 64, |
| 104 | committed_at_iso=_UTC_TS, |
| 105 | committed_at_raw=None, |
| 106 | ) |
| 107 | wire = _to_wire_commit(row) |
| 108 | assert wire.committed_at == row.timestamp.isoformat() |
| 109 | |
| 110 | |
| 111 | # --------------------------------------------------------------------------- |
| 112 | # U3/U4 — fail-closed verification helper |
| 113 | # --------------------------------------------------------------------------- |
| 114 | |
| 115 | |
| 116 | class TestVerifyCommitIdentityOrRaise: |
| 117 | def test_passes_for_self_consistent_commit(self) -> None: |
| 118 | from musehub.services.musehub_wire_shared import verify_commit_identity_or_raise |
| 119 | |
| 120 | snap_id = "sha256:" + "0" * 64 |
| 121 | commit_id = hub_compute_commit_id( |
| 122 | parent_ids=[], snapshot_id=snap_id, message="m", |
| 123 | committed_at_iso=_NON_UTC_TS, author="gabriel", signer_public_key="", |
| 124 | ) |
| 125 | row = _commit_row( |
| 126 | commit_id=commit_id, |
| 127 | committed_at_iso=_NON_UTC_TS, |
| 128 | committed_at_raw=_NON_UTC_TS, |
| 129 | snapshot_id=snap_id, |
| 130 | ) |
| 131 | wire = _to_wire_commit(row) |
| 132 | verify_commit_identity_or_raise(row, wire) # must not raise |
| 133 | |
| 134 | def test_raises_for_mismatched_commit(self) -> None: |
| 135 | from musehub.services.musehub_wire_shared import verify_commit_identity_or_raise |
| 136 | |
| 137 | snap_id = "sha256:" + "0" * 64 |
| 138 | commit_id = hub_compute_commit_id( |
| 139 | parent_ids=[], snapshot_id=snap_id, message="m", |
| 140 | committed_at_iso=_NON_UTC_TS, author="gabriel", signer_public_key="", |
| 141 | ) |
| 142 | # Row's timestamp reconstructs to the UTC-normalized string, not the |
| 143 | # original non-UTC one baked into commit_id -- and raw is absent. |
| 144 | row = _commit_row( |
| 145 | commit_id=commit_id, |
| 146 | committed_at_iso=_UTC_TS, |
| 147 | committed_at_raw=None, |
| 148 | snapshot_id=snap_id, |
| 149 | ) |
| 150 | wire = _to_wire_commit(row) |
| 151 | with pytest.raises(ObjectHashMismatch): |
| 152 | verify_commit_identity_or_raise(row, wire) |
| 153 | |
| 154 | |
| 155 | # --------------------------------------------------------------------------- |
| 156 | # E1-E3 — wire_fetch_mpack end-to-end |
| 157 | # --------------------------------------------------------------------------- |
| 158 | |
| 159 | |
| 160 | def _stub_backend(monkeypatch: pytest.MonkeyPatch) -> dict[str, bytes]: |
| 161 | from unittest.mock import AsyncMock |
| 162 | |
| 163 | store: dict[str, bytes] = {} |
| 164 | |
| 165 | async def _put(oid: str, data: bytes, **_: object) -> str: |
| 166 | store[oid] = data |
| 167 | return f"mem://{oid}" |
| 168 | |
| 169 | async def _get(oid: str) -> bytes | None: |
| 170 | return store.get(oid) |
| 171 | |
| 172 | async def _exists(oid: str, **_: object) -> bool: |
| 173 | return oid in store |
| 174 | |
| 175 | async def _presign_get(oid: str, ttl: int) -> str: |
| 176 | return f"https://minio.test/{oid}?ttl={ttl}" |
| 177 | |
| 178 | async def _get_mpack(mpack_id: str) -> bytes | None: |
| 179 | return store.get(mpack_id) |
| 180 | |
| 181 | async def _put_mpack(mpack_id: str, data: bytes) -> str: |
| 182 | store[mpack_id] = data |
| 183 | return f"mem://mpacks/{mpack_id}" |
| 184 | |
| 185 | async def _presign_mpack_get(mpack_id: str, ttl: int) -> str: |
| 186 | return f"https://minio.test/mpacks/{mpack_id}?ttl={ttl}" |
| 187 | |
| 188 | backend = AsyncMock() |
| 189 | backend.put = _put |
| 190 | backend.get = _get |
| 191 | backend.exists = _exists |
| 192 | backend.presign_get = _presign_get |
| 193 | backend.get_mpack = _get_mpack |
| 194 | backend.put_mpack = _put_mpack |
| 195 | backend.presign_mpack_get = _presign_mpack_get |
| 196 | backend.supports_presign = True |
| 197 | monkeypatch.setattr("musehub.services.musehub_wire.get_backend", lambda: backend) |
| 198 | monkeypatch.setattr("musehub.services.musehub_wire_fetch.get_backend", lambda: backend) |
| 199 | monkeypatch.setattr("musehub.services.musehub_wire_shared.get_backend", lambda: backend) |
| 200 | return store |
| 201 | |
| 202 | |
| 203 | async def _seed_commit( |
| 204 | session: AsyncSession, |
| 205 | repo_id: str, |
| 206 | *, |
| 207 | committed_at_iso: str, |
| 208 | committed_at_raw: str | None, |
| 209 | seed: str, |
| 210 | ) -> str: |
| 211 | import msgpack |
| 212 | |
| 213 | from muse.core.ids import hash_snapshot as _hash_snapshot |
| 214 | |
| 215 | manifest: dict[str, str] = {} |
| 216 | snap_id = _hash_snapshot(manifest, []) |
| 217 | await session.execute( |
| 218 | pg_insert(db.MusehubSnapshot) |
| 219 | .values( |
| 220 | snapshot_id=snap_id, |
| 221 | directories=[], |
| 222 | entry_count=0, |
| 223 | manifest_blob=msgpack.packb(manifest, use_bin_type=True), |
| 224 | created_at=datetime.now(timezone.utc), |
| 225 | ) |
| 226 | .on_conflict_do_nothing(index_elements=["snapshot_id"]) |
| 227 | ) |
| 228 | await session.execute( |
| 229 | pg_insert(db.MusehubSnapshotRef) |
| 230 | .values(repo_id=repo_id, snapshot_id=snap_id) |
| 231 | .on_conflict_do_nothing() |
| 232 | ) |
| 233 | |
| 234 | message = f"commit {seed}" |
| 235 | commit_id = hub_compute_commit_id( |
| 236 | parent_ids=[], snapshot_id=snap_id, message=message, |
| 237 | committed_at_iso=committed_at_iso, author="gabriel", signer_public_key="", |
| 238 | ) |
| 239 | session.add(db.MusehubCommit( |
| 240 | commit_id=commit_id, |
| 241 | branch="main", |
| 242 | parent_ids=[], |
| 243 | message=message, |
| 244 | author="gabriel", |
| 245 | timestamp=datetime.fromisoformat(committed_at_iso), |
| 246 | committed_at_raw=committed_at_raw, |
| 247 | snapshot_id=snap_id, |
| 248 | )) |
| 249 | await session.execute( |
| 250 | pg_insert(db.MusehubCommitRef) |
| 251 | .values(repo_id=repo_id, commit_id=commit_id) |
| 252 | .on_conflict_do_nothing() |
| 253 | ) |
| 254 | await session.execute( |
| 255 | pg_insert(db.MusehubCommitGraph) |
| 256 | .values(commit_id=commit_id, parent_ids=[], generation=0, snapshot_id=snap_id) |
| 257 | .on_conflict_do_nothing() |
| 258 | ) |
| 259 | await session.commit() |
| 260 | # Force a real re-read from Postgres -- without this, SQLAlchemy's |
| 261 | # identity map keeps serving the in-memory datetime we constructed (with |
| 262 | # its original, un-normalized offset) instead of what the DB actually |
| 263 | # stored, masking the exact bug this test exists to catch. |
| 264 | from sqlalchemy import select as _select |
| 265 | await session.execute( |
| 266 | _select(db.MusehubCommit) |
| 267 | .where(db.MusehubCommit.commit_id == commit_id) |
| 268 | .execution_options(populate_existing=True) |
| 269 | ) |
| 270 | return commit_id |
| 271 | |
| 272 | |
| 273 | @pytest.mark.asyncio |
| 274 | async def test_e1_serves_exact_non_utc_string_when_raw_present( |
| 275 | db_session: AsyncSession, monkeypatch: pytest.MonkeyPatch |
| 276 | ) -> None: |
| 277 | from musehub.services.musehub_wire import wire_fetch_mpack |
| 278 | |
| 279 | store = _stub_backend(monkeypatch) |
| 280 | repo = await create_repo(db_session, owner="gabriel", visibility="public") |
| 281 | commit_id = await _seed_commit( |
| 282 | db_session, repo.repo_id, |
| 283 | committed_at_iso=_NON_UTC_TS, committed_at_raw=_NON_UTC_TS, seed="e1", |
| 284 | ) |
| 285 | |
| 286 | result = await wire_fetch_mpack( |
| 287 | db_session, repo.repo_id, want=[commit_id], have=[], force_build=True |
| 288 | ) |
| 289 | mpack = parse_wire_mpack(store[result["mpack_id"]]) |
| 290 | served = next(c for c in mpack["commits"] if c["commit_id"] == commit_id) |
| 291 | assert served["committed_at"] == _NON_UTC_TS |
| 292 | |
| 293 | |
| 294 | @pytest.mark.asyncio |
| 295 | async def test_e2_legacy_non_utc_row_missing_raw_is_unchanged_not_worsened( |
| 296 | db_session: AsyncSession, monkeypatch: pytest.MonkeyPatch |
| 297 | ) -> None: |
| 298 | """Explicit scope boundary: a pre-migration row authored non-UTC, with |
| 299 | no committed_at_raw to fall back on, still serves a mismatched |
| 300 | committed_at exactly as it did before this fix -- NOT newly rejected. |
| 301 | |
| 302 | This is deliberate, not an oversight: an automatic fail-closed check |
| 303 | was tried and reverted after it broke wire_repair_commit (whose entire |
| 304 | purpose is repairing an already-mismatched commit) and at least two |
| 305 | legitimate write sites that never used hash_commit-derived ids at all |
| 306 | (repo-init, sync's synthetic content commit). verify_commit_identity_or_raise |
| 307 | still exists and is correct (see TestVerifyCommitIdentityOrRaise) for |
| 308 | explicit/diagnostic use -- it is just not an automatic gate on every serve. |
| 309 | Fixing already-affected legacy data is out of scope for this narrow fix. |
| 310 | """ |
| 311 | from musehub.services.musehub_wire import wire_fetch_mpack |
| 312 | |
| 313 | store = _stub_backend(monkeypatch) |
| 314 | repo = await create_repo(db_session, owner="gabriel", visibility="public") |
| 315 | commit_id = await _seed_commit( |
| 316 | db_session, repo.repo_id, |
| 317 | committed_at_iso=_NON_UTC_TS, committed_at_raw=None, seed="e2", |
| 318 | ) |
| 319 | |
| 320 | result = await wire_fetch_mpack( |
| 321 | db_session, repo.repo_id, want=[commit_id], have=[], force_build=True |
| 322 | ) |
| 323 | mpack = parse_wire_mpack(store[result["mpack_id"]]) |
| 324 | served = next(c for c in mpack["commits"] if c["commit_id"] == commit_id) |
| 325 | # Reconstructed from the UTC-normalized `timestamp`, not the original -- |
| 326 | # same lossy behavior as before this fix, for pre-existing data only. |
| 327 | assert served["committed_at"] != _NON_UTC_TS |
| 328 | |
| 329 | |
| 330 | @pytest.mark.asyncio |
| 331 | async def test_e3_legacy_utc_row_still_serves_fine( |
| 332 | db_session: AsyncSession, monkeypatch: pytest.MonkeyPatch |
| 333 | ) -> None: |
| 334 | """No regression for the common case: a legacy row with no |
| 335 | committed_at_raw, originally authored in UTC, still serves normally -- |
| 336 | reconstruction matches, nothing to fail closed on.""" |
| 337 | from musehub.services.musehub_wire import wire_fetch_mpack |
| 338 | |
| 339 | store = _stub_backend(monkeypatch) |
| 340 | repo = await create_repo(db_session, owner="gabriel", visibility="public") |
| 341 | commit_id = await _seed_commit( |
| 342 | db_session, repo.repo_id, |
| 343 | committed_at_iso=_UTC_TS, committed_at_raw=None, seed="e3", |
| 344 | ) |
| 345 | |
| 346 | result = await wire_fetch_mpack( |
| 347 | db_session, repo.repo_id, want=[commit_id], have=[], force_build=True |
| 348 | ) |
| 349 | mpack = parse_wire_mpack(store[result["mpack_id"]]) |
| 350 | served = next(c for c in mpack["commits"] if c["commit_id"] == commit_id) |
| 351 | assert served["committed_at"] == _UTC_TS |