fix(crush): read session timestamps as Unix seconds, not milliseconds - #357
Open
LarsArtmann wants to merge 1 commit into
Open
fix(crush): read session timestamps as Unix seconds, not milliseconds#357LarsArtmann wants to merge 1 commit into
LarsArtmann wants to merge 1 commit into
Conversation
Crush's initial migration comment claims created_at/updated_at are
milliseconds, but the stored values are Unix seconds: the
update_sessions_updated_at trigger writes strftime('%s','now') and the
Crush CLI renders time.Unix(CreatedAt, 0). Verified against real Crush
DBs: a session row with created_at = 1784269138 is 2026-07-17 (UTC) as
seconds, but 1970-01-21 when misread as milliseconds — every session
was silently bucketed into January 1970.
Rename millisToTime -> unixSecondsToTime so the name and comment stop
propagating the wrong unit, and pin the behavior with an end-to-end
test backed by a fixture Crush DB (fails on main, passes with the fix).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
millisToTimereadsessions.created_at/updated_atas Unix milliseconds. The columns actually store Unix seconds, so every Crush session landed in January 1970 — day buckets, statusline windows, and any time-based filtering were silently wrong.Evidence
Real
crush.dbrow from a live machine:2026-07-17 06:18:58 UTC✅ (matches when the session actually ran)1970-01-21 15:37:49 UTC❌ (what the crush provider produced)The root cause is Crush's own initial migration comment, which claims
created_atstores milliseconds. It does not:update_sessions_updated_attrigger writesstrftime('%s','now')— secondstime.Unix(CreatedAt, 0)— secondsChanges
time.UnixMilli(v)→time.Unix(v, 0), andmillisToTimerenamed tounixSecondsToTimeso the name and comment stop propagating the wrong unitTestQuerySessions_TimestampsAreUnixSeconds: builds a fixture Crush DB and asserts the end-to-endquerySessionsresult — fails on currentmain(produces 1970-01-21), passes with this fixTesting
go test ./internal/providers/crush/— greengo test ./...— only pre-existing, unrelated failures (internal/daemonchange-detection test,hermes/zedmissing-DB tests; each verified failing on pristinemainbefore this change)Beyond this fix
For context (not a dependency suggestion): I maintain go-crush-data (MIT), a typed read-only Go library for the Crush on-disk format — capability probing against schema drift, tolerant parts decoding, census-verified todos shape, tested against crush v0.92.0 and real production DBs. This bug is a good example of why that probing exists. I've since opened the upstream Discussion with charmbracelet/crush about stable read access for ecosystem tools — this PR and its sibling (mnemo#22) are the motivating receipts: charmbracelet/crush#3740