Skip to content

fix(migrations): create days_to_sync_history column read by wmiau.go - #361

Open
yanmayck wants to merge 1 commit into
asternic:mainfrom
yanmayck:fix/days-to-sync-history-migration
Open

fix(migrations): create days_to_sync_history column read by wmiau.go#361
yanmayck wants to merge 1 commit into
asternic:mainfrom
yanmayck:fix/days-to-sync-history-migration

Conversation

@yanmayck

Copy link
Copy Markdown

Problem

wmiau.go reads days_to_sync_history when deciding how much history to request on connect:

// wmiau.go:1010
query := "SELECT COALESCE(days_to_sync_history, 0) FROM users WHERE id=$1"

But no migration creates that column.

On a fresh install the query errors out, the code logs Failed to get days_to_sync_history from database and falls through to 0, so the history-sync-by-days feature silently never runs. It only works on databases where the column arrived from somewhere else (a hand-written ALTER, or a fork that added its own migration).

Because the failure path only warns and continues, this is easy to miss — there is no error, just a feature that quietly does nothing.

Fix

Adds migration 13 — the next free ID, since 9, 10 and 12 are taken — creating the column with the same guarded pattern the surrounding migrations already use:

  • PostgreSQL: DO $$ ... IF NOT EXISTS (SELECT 1 FROM information_schema.columns ...)
  • SQLite: addColumnIfNotExistsSQLite(tx, "users", "days_to_sync_history", "INTEGER DEFAULT 0")

Both are no-ops where the column already exists, so this is safe to apply to existing databases.

Testing

Reviewed against the existing migration idioms in migrations.go; the SQLite path reuses the same helper as migrations 10 and 12. I don't have a Go toolchain on this machine to run the suite — worth a CI run before merging.

wmiau.go queries days_to_sync_history to decide how much history to request
on connect:

    query := "SELECT COALESCE(days_to_sync_history, 0) FROM users WHERE id=$1"

but no migration creates that column. On a fresh install the query errors,
the code logs "Failed to get days_to_sync_history from database" and falls
through to 0, so the history-sync-by-days feature never runs. It only works
on databases where the column arrived from somewhere else.

Adds migration 13 (next free ID: 9, 10 and 12 are taken) with the same
guarded pattern used by the surrounding migrations — IF NOT EXISTS on
PostgreSQL, addColumnIfNotExistsSQLite on SQLite — so it is a no-op where
the column already exists.

Co-Authored-By: claude-flow <[email protected]>
Claude-Session: https://claude.ai/code/session_01Ep6DPMhWw9D2UhfhKtRe3s
@tecspda

tecspda commented Sep 7, 2026

Copy link
Copy Markdown

Hey, I ran into this exact same thing on a fresh SQLite install last week — nice catch, the "silent warn, feature quietly does nothing" description is spot on.

But I think the column might be the wrong half to fix. I went looking for who writes to days_to_sync_history and... nobody does. It's only ever read, in that one line:

$ grep -rn "days_to_sync_history" --include="*.go" .
./wmiau.go:1010:  query := "SELECT COALESCE(days_to_sync_history, 0) FROM users WHERE id=$1"
./wmiau.go:1014:  log.Warn()...Msg("Failed to get days_to_sync_history from database")

Everything else uses history — the migration creates it, SetHistory writes to it, GetHistory reads it back:

// handlers.go:6499 — where POST /session/history {"history": 30} actually lands
_, err = s.db.Exec("UPDATE users SET history = $1 WHERE id = $2", t.History, txtid)

So with migration 13 the query stops erroring, but it now returns 0 instead of failing, daysToSyncHistory > 0 is still false, and the sync still doesn't run. The warning goes away, the bug doesn't. Which is arguably worse, since now there's nothing in the logs pointing at it. 🙂

The one-liner that worked for me:

-             query := "SELECT COALESCE(days_to_sync_history, 0) FROM users WHERE id=$1"
+             query := "SELECT COALESCE(history, 0) FROM users WHERE id=$1"

Running it in production since this morning: set history to 30 via the API, and after the patch message_history actually filled up (~2k messages) instead of staying empty. Happy to be wrong if there's a fork or an older schema where days_to_sync_history does get written somewhere I didn't look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants