fix: sha256:06cdc3de1e7f8091954906cf91f22c941373474c3322392a79300adc19b33197 · gabriel/musehub

fix(#195): narrow fix — committed_at_raw preserves commit identity across UTC normalization

committed_at is identity-bearing: hash_commit's preimage includes the exact ISO-8601 string the client used. musehub_commits.timestamp is a Postgres timestamptz, which normalizes any non-UTC offset to UTC on write. The wire-serve path (_to_wire_commit) reconstructs committed_at via `timestamp.isoformat()` -- for a commit originally authored with a non-UTC offset (e.g. every bridge-imported commit carrying its original author-local timezone), this produces a DIFFERENT string than the one baked into commit_id, so the client's own hash check correctly rejects the download. Per corrected counts: 890 of 1,308 knotwation commits affected (all bridge imports); musehub itself 0 of 1,617 (native commits are always server-generated in UTC).

This is an instance of the broader problem tracked in #63 (commits are DB-only, reconstructed from DB fields rather than served as stored canonical bytes). This is the deliberately narrow, additive alternative, per gabriel's decision: still correct after #63 eventually lands (#63 would make this column redundant, not wrong).

Added: - alembic/versions/0077_add_committed_at_raw.py + matching ORM column on MusehubCommit -- additive, nullable Text column storing the exact original committed_at string. - musehub/services/musehub_wire_shared.py::_to_wire_commit -- prefers committed_at_raw verbatim when present; unchanged fallback to timestamp.isoformat() when absent (legacy rows). - verify_commit_identity_or_raise() -- a standalone helper that recomputes a commit's id from its wire representation and raises ObjectHashMismatch on mismatch, for explicit/diagnostic use (same pattern wire_repair_commit already uses internally). - committed_at_raw populated at every commit write site: wire push ingest (musehub_wire_push.py -- captures the true client-original string before CommitRecord.from_dict discards it, the only point in the ingest path where it still exists), proposal merge + rebase replay (musehub_proposals.py), repo-init and sync synthetic commits (musehub_repository.py, musehub_sync.py -- populated for consistency, though neither of these two sites uses hash_commit-derived ids at all, see below).

Deliberately NOT done -- automatic fail-closed verification on every serve: tried wiring verify_commit_identity_or_raise into _commit_to_wire_s3 (the function wire_fetch_mpack actually uses) and confirmed experimentally it breaks real things: - wire_repair_commit's entire purpose is repairing a commit whose stored identity fields don't currently reproduce its id -- an automatic check on every serve broke that flow outright (test_repair_commit_endpoint.py failed with the exact ObjectHashMismatch this ticket is about, on a commit mid-repair). - musehub_repository.py's repo-init commit and musehub_sync.py's synthetic content commit both use non-hash_commit id schemes (blob_id-based), confirmed directly: hash_commit-recomputing either one never matches its stored id, regardless of committed_at accuracy. This is a real, pre-existing, unrelated defect (filed separately, see below) that a blanket automatic check would have conflated with this ticket's actual bug. Legacy non-UTC rows with committed_at_raw=NULL therefore still serve a mismatched committed_at exactly as before this fix -- not newly broken, not silently fixed either. Fixing already-affected historical data is out of scope for this narrow fix.

Tests (TDD, red confirmed before implementation): - tests/test_committed_at_raw.py (7 tests): _to_wire_commit prefers raw when present (constructed so it actually discriminates old vs new behavior, not a self-referential no-op check) and falls back correctly when absent; verify_commit_identity_or_raise passes/raises correctly in isolation; wire_fetch_mpack end-to-end serves the exact original non-UTC string when raw is present (the actual regression closed); a legacy non-UTC row missing raw is explicitly proven unchanged (not worsened), documenting the deliberate scope boundary above.

Full regression: 345/345 across test_wire_fetch_mpack.py, test_repair_commit_endpoint.py, test_merge_commit_id_parity.py, test_musehub_repos.py, test_wire_push_external_parent_reconstruction.py, test_schema_parity.py, test_schema_check.py, test_committed_at_raw.py, test_musehub_proposals.py, test_merge_proposals.py, test_musehub_collaborators.py, test_collaborators.py -- no regressions.

Along the way: found the local dev Postgres container's musehub DB had 17 duplicate (repo_id, number) issue rows blocking migration past #184's own unique constraint (0076) -- pre-existing local dev-data cruft, cleaned up (kept one row per group) so migrations could progress; also found and fixed a missing `zstandard` package in this repo's .venv (installed) that was masking real test signal with an unrelated ModuleNotFoundError.

Related: - #63 -- the structural fix this stays correct under. - musehub_repository.py/musehub_sync.py's non-hash_commit id schemes -- new finding, filed as its own follow-up ticket.

Co-Authored-By: Claude Sonnet 5 <[email protected]>

+40

symbols added

~10

symbols modified

files changed

1075

files in snapshot

dead code introduced

Semantic Changes 50 symbols

~ alembic/versions/0077_add_committed_at_raw.py .py 11 symbols added

+ branch_labels variable variable branch_labels L41–41

+ depends_on variable variable depends_on L42–42

+ down_revision variable variable down_revision L40–40

+ downgrade function function downgrade L52–53

+ Sequence import import Sequence L32–32

+ Union import import Union L32–32

+ annotations import import annotations L30–30

+ op import import op L35–35

+ sa import import sa L34–34

+ revision variable variable revision L39–39

+ upgrade function function upgrade L45–49

~ tests/test_committed_at_raw.py .py 27 symbols added

+ TestToWireCommitPrefersRaw class class TestToWireCommitPrefersRaw L87–108

+ test_falls_back_to_timestamp_isoformat_when_raw_absent method method test_falls_back_to_timestamp_isoformat_when_raw_absent L101–108

+ test_uses_committed_at_raw_verbatim_when_present method method test_uses_committed_at_raw_verbatim_when_present L88–99

+ TestVerifyCommitIdentityOrRaise class class TestVerifyCommitIdentityOrRaise L116–152

+ test_passes_for_self_consistent_commit method method test_passes_for_self_consistent_commit L117–132

+ test_raises_for_mismatched_commit method method test_raises_for_mismatched_commit L134–152

+ _NON_UTC_TS variable variable _NON_UTC_TS L59–59

+ _UTC_TS variable variable _UTC_TS L60–60

+ _commit_row function function _commit_row L63–79

+ _seed_commit function async_function _seed_commit L203–270

+ _stub_backend function function _stub_backend L160–200

+ AsyncSession import import AsyncSession L50–50

+ ObjectHashMismatch import import ObjectHashMismatch L56–56

+ _to_wire_commit import import _to_wire_commit L56–56

+ annotations import import annotations L44–44

+ blob_id import import blob_id L53–53

+ create_repo import import create_repo L57–57

+ datetime import import datetime L46–46

+ db import import db L54–54

+ hub_compute_commit_id import import hub_compute_commit_id L55–55

+ parse_wire_mpack import import parse_wire_mpack L52–52

+ pg_insert import import pg_insert L49–49

+ pytest import import pytest L48–48

+ timezone import import timezone L46–46

+ test_e1_serves_exact_non_utc_string_when_raw_present function async_function test_e1_serves_exact_non_utc_string_when_raw_present L274–291

+ test_e2_legacy_non_utc_row_missing_raw_is_unchanged_not_worsened function async_function test_e2_legacy_non_utc_row_missing_raw_is_unchanged_not_worsened L295–327

+ test_e3_legacy_utc_row_still_serves_fine function async_function test_e3_legacy_utc_row_still_serves_fine L331–351

~ musehub/db/musehub_repo_models.py .py 1 symbol modified

~ MusehubCommit

~ musehub/services/musehub_proposals.py .py 2 symbols modified

~ _rebase_commits

~ merge_proposal

~ musehub/services/musehub_repository.py .py 1 symbol modified

~ create_repo

~ musehub/services/musehub_sync.py .py 2 symbols modified

~ commit_files_to_repo

~ ingest_push

~ musehub/services/musehub_wire_push.py .py 1 symbol modified

~ wire_push_unpack_mpack

~ musehub/services/musehub_wire_shared.py .py 2 symbols added, 3 symbols modified

+ long_id import import long_id L48–48

+ verify_commit_identity_or_raise function function verify_commit_identity_or_raise L119–138

~ _commit_to_wire_s3

~ _to_wire_commit

~ _muse_commit_identity_bytes

Files Changed

+2 ~6

1075 in snapshot

+ alembic/versions/0077_add_committed_at_raw.py .py

+ tests/test_committed_at_raw.py .py

~ musehub/db/musehub_repo_models.py .py

~ musehub/services/musehub_proposals.py .py

~ musehub/services/musehub_repository.py .py

~ musehub/services/musehub_sync.py .py

~ musehub/services/musehub_wire_push.py .py

~ musehub/services/musehub_wire_shared.py .py

← Older Oldest on fix/195-committed-at-raw

All commits

Newer → Latest on fix/195-committed-at-raw

0 comments

No comments yet. Be the first to start the discussion.

To add a comment, use the Muse CLI: muse hub commit comment sha256:06cdc3de1e7f8091954906cf91f22c941373474c3322392a79300adc19b33197 --body "your comment"