mirror of
https://github.com/nearai/ironclaw.git
synced 2026-09-03 08:06:01 +08:00
fix(db): resolve V15 migration numbering conflict (#1923)
* fix(db): resolve V15 migration numbering conflict between user_identities and conversation_source_channel A merge conflict left two PostgreSQL migrations at V15. This renumbers them (V15=user_identities, V16=conversation_source_channel, V17=document_versions) to match the libSQL incremental ordering. Adds user_identities, document_versions, and source_channel to the libSQL base schema so fresh databases get all tables. Includes a one-time repair for existing databases where V15 was mis-recorded. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: fix rustfmt formatting in repair_misnumbered_v15 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(db): address PR review — proper error handling and tighter repair condition - Replace .ok().flatten() with explicit error propagation via .map_err()? so DB errors during V15 repair are surfaced, not silently swallowed - Tighten repair condition from `!= "user_identities"` to `== "document_versions"` to only fix the specific known-bad case from the merge conflict Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -415,4 +415,58 @@ mod tests {
|
||||
.unwrap();
|
||||
assert!(found_identity.is_some());
|
||||
}
|
||||
|
||||
/// Regression: an earlier release recorded V15 as "document_versions"
|
||||
/// instead of "user_identities", so the table was never created.
|
||||
/// Verify that `run_migrations` repairs this and creates the table.
|
||||
#[tokio::test]
|
||||
async fn test_v15_misnumbered_repair() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let db_path = dir.path().join("test_v15_repair.db");
|
||||
let backend = LibSqlBackend::new_local(&db_path).await.unwrap();
|
||||
backend.run_migrations().await.unwrap();
|
||||
|
||||
// Simulate the bug: drop user_identities and re-record V15 with wrong name
|
||||
let conn = backend.connect().await.unwrap();
|
||||
conn.execute_batch("DROP TABLE IF EXISTS user_identities")
|
||||
.await
|
||||
.unwrap();
|
||||
conn.execute(
|
||||
"UPDATE _migrations SET name = 'document_versions' WHERE version = 15",
|
||||
libsql::params![],
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
// Confirm the table is gone
|
||||
let err = conn
|
||||
.query("SELECT 1 FROM user_identities LIMIT 1", ())
|
||||
.await;
|
||||
assert!(err.is_err(), "user_identities should not exist yet");
|
||||
|
||||
// Re-run migrations — the repair should fix V15
|
||||
drop(conn);
|
||||
backend.run_migrations().await.unwrap();
|
||||
|
||||
// Table should now exist and be queryable
|
||||
let conn = backend.connect().await.unwrap();
|
||||
let mut rows = conn
|
||||
.query("SELECT 1 FROM user_identities LIMIT 1", ())
|
||||
.await
|
||||
.unwrap();
|
||||
// No rows is fine — just verifying the table exists without error
|
||||
let _ = rows.next().await;
|
||||
|
||||
// Verify V15 is now recorded correctly
|
||||
let mut rows = conn
|
||||
.query(
|
||||
"SELECT name FROM _migrations WHERE version = 15",
|
||||
libsql::params![],
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
let row = rows.next().await.unwrap().unwrap();
|
||||
let name: String = row.get(0).unwrap();
|
||||
assert_eq!(name, "user_identities");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -38,7 +38,8 @@ CREATE TABLE IF NOT EXISTS conversations (
|
||||
thread_id TEXT,
|
||||
started_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')),
|
||||
last_activity TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')),
|
||||
metadata TEXT NOT NULL DEFAULT '{}'
|
||||
metadata TEXT NOT NULL DEFAULT '{}',
|
||||
source_channel TEXT
|
||||
);
|
||||
|
||||
CREATE INDEX IF NOT EXISTS idx_conversations_channel ON conversations(channel);
|
||||
@@ -609,6 +610,41 @@ CREATE TABLE IF NOT EXISTS api_tokens (
|
||||
CREATE INDEX IF NOT EXISTS idx_api_tokens_user ON api_tokens(user_id);
|
||||
CREATE INDEX IF NOT EXISTS idx_api_tokens_hash ON api_tokens(token_hash);
|
||||
|
||||
-- ==================== User identities (V15) ====================
|
||||
|
||||
CREATE TABLE IF NOT EXISTS user_identities (
|
||||
id TEXT PRIMARY KEY,
|
||||
user_id TEXT NOT NULL REFERENCES users(id) ON DELETE CASCADE,
|
||||
provider TEXT NOT NULL,
|
||||
provider_user_id TEXT NOT NULL,
|
||||
email TEXT,
|
||||
email_verified INTEGER NOT NULL DEFAULT 0,
|
||||
display_name TEXT,
|
||||
avatar_url TEXT,
|
||||
raw_profile TEXT NOT NULL DEFAULT '{}',
|
||||
created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')),
|
||||
updated_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')),
|
||||
UNIQUE (provider, provider_user_id)
|
||||
);
|
||||
CREATE INDEX IF NOT EXISTS idx_user_identities_user ON user_identities(user_id);
|
||||
CREATE INDEX IF NOT EXISTS idx_user_identities_email ON user_identities(email) WHERE email IS NOT NULL;
|
||||
|
||||
-- ==================== Document versions (V17) ====================
|
||||
|
||||
CREATE TABLE IF NOT EXISTS memory_document_versions (
|
||||
id TEXT PRIMARY KEY,
|
||||
document_id TEXT NOT NULL REFERENCES memory_documents(id) ON DELETE CASCADE,
|
||||
version INTEGER NOT NULL,
|
||||
content TEXT NOT NULL,
|
||||
content_hash TEXT NOT NULL,
|
||||
created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')),
|
||||
changed_by TEXT,
|
||||
UNIQUE(document_id, version)
|
||||
);
|
||||
|
||||
CREATE INDEX IF NOT EXISTS idx_doc_versions_lookup
|
||||
ON memory_document_versions(document_id, version DESC);
|
||||
|
||||
"#;
|
||||
|
||||
/// Incremental migrations applied after the base schema.
|
||||
@@ -868,6 +904,51 @@ async fn column_exists(
|
||||
Ok(rows.next().await.ok().flatten().is_some())
|
||||
}
|
||||
|
||||
/// Repair databases where V15 was recorded as "document_versions" instead of
|
||||
/// "user_identities" due to a migration numbering error in an earlier release.
|
||||
/// Deletes the stale _migrations row so V15 reruns with the correct SQL.
|
||||
async fn repair_misnumbered_v15(
|
||||
conn: &libsql::Connection,
|
||||
) -> Result<(), crate::error::DatabaseError> {
|
||||
use crate::error::DatabaseError;
|
||||
|
||||
let mut rows = conn
|
||||
.query(
|
||||
"SELECT name FROM _migrations WHERE version = 15",
|
||||
libsql::params![],
|
||||
)
|
||||
.await
|
||||
.map_err(|e| DatabaseError::Migration(format!("V15 repair check failed: {e}")))?;
|
||||
|
||||
let maybe_row = rows
|
||||
.next()
|
||||
.await
|
||||
.map_err(|e| DatabaseError::Migration(format!("V15 repair: failed to fetch row: {e}")))?;
|
||||
if let Some(row) = maybe_row {
|
||||
let name: String = row.get(0).map_err(|e| {
|
||||
DatabaseError::Migration(format!("V15 repair: failed to read name: {e}"))
|
||||
})?;
|
||||
if name == "document_versions" {
|
||||
// V15 was recorded with the wrong name due to a merge-conflict
|
||||
// misnumbering — the user_identities CREATE TABLE never ran.
|
||||
// Delete the stale record so the migration loop will reapply it.
|
||||
tracing::warn!(
|
||||
recorded_name = %name,
|
||||
"libSQL: V15 was mis-recorded as document_versions; deleting stale _migrations row to reapply"
|
||||
);
|
||||
conn.execute(
|
||||
"DELETE FROM _migrations WHERE version = 15",
|
||||
libsql::params![],
|
||||
)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
DatabaseError::Migration(format!("V15 repair: failed to delete stale row: {e}"))
|
||||
})?;
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Run incremental migrations that haven't been applied yet.
|
||||
///
|
||||
/// Each migration is wrapped in a transaction. On success the version is
|
||||
@@ -875,6 +956,12 @@ async fn column_exists(
|
||||
pub async fn run_incremental(conn: &libsql::Connection) -> Result<(), crate::error::DatabaseError> {
|
||||
use crate::error::DatabaseError;
|
||||
|
||||
// Repair: an earlier release mis-numbered V15 as "document_versions"
|
||||
// instead of "user_identities", so the user_identities CREATE TABLE
|
||||
// never ran. If V15 is recorded but the table doesn't exist, delete
|
||||
// the stale record so V15 reruns with the correct SQL.
|
||||
repair_misnumbered_v15(conn).await?;
|
||||
|
||||
let mut applied_count = 0;
|
||||
for &(version, name, sql) in INCREMENTAL_MIGRATIONS {
|
||||
// Check if already applied
|
||||
|
||||
Reference in New Issue
Block a user