test_push_commit_created_at.py
python
sha256:a34090cc4a394a78bd72cbbe34b08cc59525141e19135b6c0ab154f10611b9ef
debug(push/stream): instrument O-frame decode path with INF…
Sonnet 4.6
patch
121 days ago
| 1 | """TDD — musehub_commits.created_at must never be NULL after a push. |
| 2 | |
| 3 | Root cause (2026-05-07) |
| 4 | ----------------------- |
| 5 | wire_push_stream builds new_commit_rows as plain dicts and inserts them via |
| 6 | pg_insert(MusehubCommit).values(new_commit_rows). SQLAlchemy includes ALL |
| 7 | ORM model columns in the INSERT when using .values() with a list of dicts — |
| 8 | columns absent from a dict are bound as NULL, overriding any column DEFAULT |
| 9 | or server_default. |
| 10 | |
| 11 | musehub_commits.created_at is NOT NULL with DEFAULT now(). Migration 0029 |
| 12 | adds that DEFAULT at the DB level, but an explicit NULL in the INSERT still |
| 13 | violates the NOT NULL constraint. |
| 14 | |
| 15 | Fix |
| 16 | --- |
| 17 | Add "created_at": _utc_now() to the new_commit_rows dict so the value is |
| 18 | always an explicit datetime, never NULL. |
| 19 | |
| 20 | Tests |
| 21 | ----- |
| 22 | T1 Regression guard — pg_insert with explicit NULL for created_at raises |
| 23 | IntegrityError. Documents the DB constraint is enforced; always passes. |
| 24 | |
| 25 | T2 Fix driver — a full HTTP push via POST /{owner}/{slug}/push/stream |
| 26 | produces a musehub_commits row with non-null created_at. |
| 27 | RED before fix (wire_push_stream omits created_at → NULL → IntegrityError). |
| 28 | GREEN after fix (created_at=_utc_now() added to new_commit_rows). |
| 29 | |
| 30 | T3 Upsert preservation — re-pushing the same commit ID does not overwrite |
| 31 | the original created_at (created_at is absent from on_conflict_do_update |
| 32 | set_, so the original row value is preserved). |
| 33 | """ |
| 34 | from __future__ import annotations |
| 35 | |
| 36 | import zlib |
| 37 | from datetime import datetime, timezone |
| 38 | |
| 39 | import msgpack |
| 40 | import pytest |
| 41 | from httpx import AsyncClient |
| 42 | from sqlalchemy import select, text |
| 43 | from sqlalchemy.dialects.postgresql import insert as pg_insert |
| 44 | from sqlalchemy.exc import IntegrityError |
| 45 | from sqlalchemy.ext.asyncio import AsyncSession |
| 46 | |
| 47 | from muse.core.mpack import MuseWireFrameWriter |
| 48 | from muse.core.types import blob_id |
| 49 | from musehub.db import musehub_models as db |
| 50 | from musehub.models.wire import ( |
| 51 | SFRAME_COMMIT_PACK, |
| 52 | SFRAME_END, |
| 53 | SFRAME_HEADER, |
| 54 | SFRAME_OBJECT, |
| 55 | ) |
| 56 | from tests.factories import create_repo |
| 57 | |
| 58 | _fw = MuseWireFrameWriter() |
| 59 | |
| 60 | |
| 61 | # --------------------------------------------------------------------------- |
| 62 | # Helpers — identical to test_push_timeout_fix.py so each test file is |
| 63 | # self-contained and the pattern is obvious. |
| 64 | # --------------------------------------------------------------------------- |
| 65 | |
| 66 | def _utc_now() -> datetime: |
| 67 | return datetime.now(tz=timezone.utc) |
| 68 | |
| 69 | |
| 70 | def _oid(data: bytes) -> str: |
| 71 | return blob_id(data) |
| 72 | |
| 73 | |
| 74 | def _wrap(frame_type: str, payload: object) -> bytes: |
| 75 | return _fw.wrap(frame_type=frame_type, payload=msgpack.packb(payload, use_bin_type=True)) |
| 76 | |
| 77 | |
| 78 | def _header_frame(branch: str = "dev", n_objects: int = 0, n_commits: int = 1) -> bytes: |
| 79 | return _wrap(SFRAME_HEADER, { |
| 80 | "t": SFRAME_HEADER, |
| 81 | "branch": branch, |
| 82 | "force": False, |
| 83 | "have": [], |
| 84 | "head": _oid(b"head"), |
| 85 | "n_objects": n_objects, |
| 86 | "n_commits": n_commits, |
| 87 | }) |
| 88 | |
| 89 | |
| 90 | def _commit_pack_frame(commits: list, snapshots: list) -> bytes: |
| 91 | return _wrap(SFRAME_COMMIT_PACK, { |
| 92 | "t": SFRAME_COMMIT_PACK, |
| 93 | "commits": commits, |
| 94 | "snapshots": snapshots, |
| 95 | }) |
| 96 | |
| 97 | |
| 98 | def _end_frame(n_objects: int = 0, n_commits: int = 1) -> bytes: |
| 99 | return _wrap(SFRAME_END, {"t": SFRAME_END, "n_objects": n_objects, "n_commits": n_commits}) |
| 100 | |
| 101 | |
| 102 | def _make_wire_commit(snap_id: str, branch: str = "dev") -> dict: |
| 103 | return { |
| 104 | "commit_id": _oid(f"commit-{snap_id}".encode()), |
| 105 | "parent_ids": [], |
| 106 | "snapshot_id": snap_id, |
| 107 | "branch": branch, |
| 108 | "message": "fix: created_at tdd commit", |
| 109 | "author": "test-user-wire", |
| 110 | "committed_at": "2026-05-07T00:00:00+00:00", |
| 111 | "signature": "", |
| 112 | "signer_key_id": "", |
| 113 | "agent_id": "claude-code", |
| 114 | "model_id": "claude-sonnet-4-6", |
| 115 | "metadata": {}, |
| 116 | } |
| 117 | |
| 118 | |
| 119 | def _make_snapshot(snap_id: str) -> dict: |
| 120 | return {"snapshot_id": snap_id, "manifest": {}} |
| 121 | |
| 122 | |
| 123 | def _build_push_stream(snap_id: str, branch: str = "dev") -> bytes: |
| 124 | commit = _make_wire_commit(snap_id, branch) |
| 125 | snapshot = _make_snapshot(snap_id) |
| 126 | return ( |
| 127 | _header_frame(branch=branch, n_objects=0, n_commits=1) |
| 128 | + _commit_pack_frame([commit], [snapshot]) |
| 129 | + _end_frame(n_objects=0, n_commits=1) |
| 130 | ) |
| 131 | |
| 132 | |
| 133 | # --------------------------------------------------------------------------- |
| 134 | # T1 — Regression guard: explicit NULL for created_at → IntegrityError |
| 135 | # --------------------------------------------------------------------------- |
| 136 | |
| 137 | @pytest.mark.asyncio |
| 138 | async def test_t1_explicit_null_created_at_raises_integrity_error( |
| 139 | db_session: AsyncSession, |
| 140 | ) -> None: |
| 141 | """Direct SQL INSERT with NULL created_at raises a NOT NULL violation. |
| 142 | |
| 143 | This test always passes — it documents the DB constraint that prevents |
| 144 | NULL values. The fix ensures the push handler never sends NULL here. |
| 145 | """ |
| 146 | repo = await create_repo(db_session, owner="test-user-wire", name="t1-null-guard") |
| 147 | |
| 148 | with pytest.raises(Exception) as exc_info: |
| 149 | await db_session.execute(text( |
| 150 | "INSERT INTO musehub_commits " |
| 151 | "(commit_id, repo_id, branch, message, author, timestamp, parent_ids, created_at) " |
| 152 | "VALUES (:cid, :rid, 'dev', 'test', 'test-user-wire', now(), '{}', NULL)" |
| 153 | ), {"cid": _oid(b"t1-null-commit"), "rid": repo.repo_id}) |
| 154 | await db_session.flush() |
| 155 | |
| 156 | assert "created_at" in str(exc_info.value).lower() or "not null" in str(exc_info.value).lower() |
| 157 | |
| 158 | |
| 159 | # --------------------------------------------------------------------------- |
| 160 | # T2 — Fix driver: HTTP push produces commit row with non-null created_at |
| 161 | # --------------------------------------------------------------------------- |
| 162 | |
| 163 | @pytest.mark.asyncio |
| 164 | async def test_t2_push_commit_has_non_null_created_at( |
| 165 | client: AsyncClient, |
| 166 | db_session: AsyncSession, |
| 167 | wire_headers: dict, |
| 168 | ) -> None: |
| 169 | """A full push via the wire protocol must produce a commit row where |
| 170 | created_at IS NOT NULL. |
| 171 | |
| 172 | Before the fix: wire_push_stream builds new_commit_rows dicts without |
| 173 | a 'created_at' key → SQLAlchemy inserts NULL → IntegrityError / 500. |
| 174 | |
| 175 | After the fix: new_commit_rows includes 'created_at': _utc_now() → |
| 176 | INSERT succeeds → row exists with a real timestamp. |
| 177 | """ |
| 178 | repo = await create_repo( |
| 179 | db_session, |
| 180 | owner="test-user-wire", |
| 181 | name="t2-created-at-fix", |
| 182 | ) |
| 183 | |
| 184 | snap_id = _oid(b"snap-t2-created-at") |
| 185 | payload = _build_push_stream(snap_id) |
| 186 | |
| 187 | resp = await client.post( |
| 188 | f"/test-user-wire/{repo.slug}/push/stream", |
| 189 | content=payload, |
| 190 | headers=wire_headers, |
| 191 | ) |
| 192 | assert resp.status_code == 200, f"push failed: {resp.content[:500]}" |
| 193 | |
| 194 | expected_commit_id = _oid(f"commit-{snap_id}".encode()) |
| 195 | result = await db_session.execute( |
| 196 | select(db.MusehubCommit).where( |
| 197 | db.MusehubCommit.commit_id == expected_commit_id, |
| 198 | db.MusehubCommit.repo_id == repo.repo_id, |
| 199 | ) |
| 200 | ) |
| 201 | row = result.scalar_one_or_none() |
| 202 | assert row is not None, "commit row not found after push" |
| 203 | assert row.created_at is not None, ( |
| 204 | "created_at is NULL — wire_push_stream must add 'created_at': _utc_now() " |
| 205 | "to new_commit_rows so SQLAlchemy doesn't bind NULL for the column" |
| 206 | ) |
| 207 | |
| 208 | |
| 209 | # --------------------------------------------------------------------------- |
| 210 | # T3 — Upsert preservation: re-push does not overwrite original created_at |
| 211 | # --------------------------------------------------------------------------- |
| 212 | |
| 213 | @pytest.mark.asyncio |
| 214 | async def test_t3_upsert_preserves_original_created_at( |
| 215 | client: AsyncClient, |
| 216 | db_session: AsyncSession, |
| 217 | wire_headers: dict, |
| 218 | ) -> None: |
| 219 | """Re-pushing the same commit must not change the existing created_at. |
| 220 | |
| 221 | The on_conflict_do_update set_ dict deliberately excludes created_at so |
| 222 | the original server-insertion timestamp is preserved across re-pushes. |
| 223 | """ |
| 224 | repo = await create_repo( |
| 225 | db_session, |
| 226 | owner="test-user-wire", |
| 227 | name="t3-upsert-created-at", |
| 228 | ) |
| 229 | |
| 230 | snap_id = _oid(b"snap-t3-upsert") |
| 231 | payload = _build_push_stream(snap_id) |
| 232 | |
| 233 | # First push |
| 234 | resp1 = await client.post( |
| 235 | f"/test-user-wire/{repo.slug}/push/stream", |
| 236 | content=payload, |
| 237 | headers=wire_headers, |
| 238 | ) |
| 239 | assert resp1.status_code == 200, f"first push failed: {resp1.content[:500]}" |
| 240 | |
| 241 | commit_id = _oid(f"commit-{snap_id}".encode()) |
| 242 | result1 = await db_session.execute( |
| 243 | select(db.MusehubCommit).where( |
| 244 | db.MusehubCommit.commit_id == commit_id, |
| 245 | db.MusehubCommit.repo_id == repo.repo_id, |
| 246 | ) |
| 247 | ) |
| 248 | row1 = result1.scalar_one() |
| 249 | assert row1.created_at is not None |
| 250 | original_created_at = row1.created_at |
| 251 | |
| 252 | # Second push (same commit — triggers on_conflict_do_update) |
| 253 | resp2 = await client.post( |
| 254 | f"/test-user-wire/{repo.slug}/push/stream", |
| 255 | content=payload, |
| 256 | headers=wire_headers, |
| 257 | ) |
| 258 | assert resp2.status_code == 200, f"second push failed: {resp2.content[:500]}" |
| 259 | |
| 260 | result2 = await db_session.execute( |
| 261 | select(db.MusehubCommit) |
| 262 | .where( |
| 263 | db.MusehubCommit.commit_id == commit_id, |
| 264 | db.MusehubCommit.repo_id == repo.repo_id, |
| 265 | ) |
| 266 | .execution_options(populate_existing=True) |
| 267 | ) |
| 268 | row2 = result2.scalar_one() |
| 269 | assert row2.created_at == original_created_at, ( |
| 270 | f"created_at changed on re-push: " |
| 271 | f"{original_created_at} → {row2.created_at}\n" |
| 272 | "created_at must be excluded from on_conflict_do_update set_ " |
| 273 | "to preserve the original insertion timestamp." |
| 274 | ) |
File History
1 commit
sha256:a34090cc4a394a78bd72cbbe34b08cc59525141e19135b6c0ab154f10611b9ef
debug(push/stream): instrument O-frame decode path with INF…
Sonnet 4.6
patch
121 days ago