vipune 0.19.1

A minimal memory layer for AI agents
Documentation
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
//! Contention tests for the FTS5 drop-and-recreate migration (issue #255).
//!
//! Sibling of `fts_tests.rs` (both wired from `sqlite/tests.rs`), split out to
//! keep each file under the project's 500-line cap. These are the cases that
//! drive `Database::initialize_fts` against a real `BEGIN EXCLUSIVE` writer,
//! plus the classifier/context contract of the error it returns — a coherent
//! seam separate from the BM25/desync coverage next door.
//!
//! Issue #278 added the two `delete`-mode precheck cases and the
//! absent-FTS-table case, and stopped these tests leaking their `TempDir`
//! (`std::mem::forget(dir)` was what filled the operator's temp quota).

/// The context string the `with_context` cases below prepend — the same one
/// `initialize_fts`'s rebuild arm uses, so those cases exercise the real call
/// shape rather than a paraphrase of it.
const MIGRATION_CONTEXT: &str = "FTS5 schema migration failed";

use crate::sqlite::Database;
use tempfile::TempDir;

/// Bring a store into the state the drop-and-recreate branch handles: the
/// FTS table exists but was built without the `project_id` column, and
/// `user_version` is at the latest so `Database::open` runs no migration of
/// its own (a pending migration takes `BEGIN EXCLUSIVE` on open and would
/// fail earlier, testing the open rather than the rebuild).
fn put_store_in_need_of_fts_rebuild(path: &std::path::Path, wal: bool) {
    let conn = rusqlite::Connection::open(path).unwrap();
    if wal {
        // vipune never sets `journal_mode`, so the default is `delete`; a
        // WAL store is what an operator who set it externally (or a future
        // change) produces. The two modes differ under contention — see
        // `a_lock_before_the_fts_rebuild_now_errors_in_delete_journal_mode`.
        conn.execute_batch("PRAGMA journal_mode=WAL;").unwrap();
    }
    conn.execute_batch("DROP TABLE IF EXISTS memories_fts;")
        .unwrap();
    conn.execute_batch(
        "CREATE VIRTUAL TABLE memories_fts USING fts5(
            content,
            tokenize='porter unicode61',
            content_rowid='rowid',
            content='memories'
         );",
    )
    .unwrap();
    conn.pragma_update(None, "user_version", 6).unwrap();
}

/// Everything a contended test needs: the store handle, the connection holding
/// the write lock, and the temp dir that owns the file both handles point at.
struct Contended {
    /// Owned so the directory outlives both handles and is removed when the
    /// test returns — the property #278 is about, and the reason this is a
    /// field rather than a local the seeding helper drops on the way out.
    /// Read by `Drop`, never by code, hence the underscore: the lib target
    /// denies warnings and would otherwise reject a field nothing reads.
    _dir: TempDir,
    /// Handle over the store, ready for the contended statement.
    db: Database,
    /// The connection holding the write lock; release it with `release_writer`.
    lock_conn: rusqlite::Connection,
}

/// Release the `BEGIN EXCLUSIVE` writer. Deliberately *not* folded into the
/// assertion helper: when an assertion there fires the panic unwinds, and a
/// temp dir dropped under a live writer is a second, unrelated failure that
/// buries the real one. Every caller calls this before it asserts.
fn release_writer(contended: &Contended) {
    contended
        .lock_conn
        .execute("ROLLBACK", [])
        .expect("release the writer");
}

/// How many `memories_fts` columns are named `project_id` — zero means the
/// drop-and-recreate never completed, which is the evidence a precheck failure
/// has to be told apart from a rebuild that hit the lock partway.
fn project_id_columns(db: &Database) -> i64 {
    db.conn()
        .query_row(
            "SELECT COUNT(*) FROM pragma_table_info('memories_fts') WHERE name = 'project_id'",
            [],
            |r| r.get(0),
        )
        .expect("read the memories_fts column roster after releasing the writer")
}

/// Seed a store that needs the FTS rebuild, then put it under a real
/// `BEGIN EXCLUSIVE` writer that holds the write lock (the shape a running MCP
/// server produces), and return everything the caller needs to contend
/// `initialize_fts` against it and to gather evidence afterwards.
///
/// The lock is *live* on return: the contended statement is what the caller
/// runs, and `release_writer` has to run before anything reads the file (a read
/// through the lock reports the lock, not the schema shape).
///
/// The WAL test and the two `delete`-mode tests shared this setup byte for
/// byte; what differs between them is why each one exists (see their own
/// comments), so the mechanics live here and the history stays with the tests.
fn seed_store_under_exclusive_writer(filename: &str, wal: bool) -> Contended {
    let dir = TempDir::new().unwrap();
    let path = dir.path().join(filename);

    // Open once to create the schema, then drop the handle so the only handle
    // into the store is the contended one below.
    {
        let db = Database::open(&path).unwrap();
        db.insert(
            "proj1",
            "needs the rebuild",
            &vec![0.1f32; 384],
            None,
            "fact",
            "active",
        )
        .unwrap();
    }
    put_store_in_need_of_fts_rebuild(&path, wal);

    let db = Database::open(&path).unwrap();
    let lock_conn = rusqlite::Connection::open(&path).unwrap();
    lock_conn.execute("BEGIN EXCLUSIVE", []).unwrap();
    // A bare `BEGIN EXCLUSIVE` takes no lock until the writer's first
    // statement; the write is what makes the lock real.
    lock_conn
        .execute(
            "INSERT INTO memories_fts(rowid, content) VALUES (9999, 'hold')",
            [],
        )
        .unwrap();
    // Zero timeout: without it SQLite waits out the default 5s busy handler
    // and the test measures latency rather than the verdict.
    db.set_busy_timeout(std::time::Duration::ZERO).unwrap();

    Contended {
        _dir: dir,
        db,
        lock_conn,
    }
}

/// Run the contended `initialize_fts`, then release the writer, and hand back
/// the error. Split into release-then-assert (rather than one helper doing
/// both) so a failing assertion can never leave the dir dropped under an
/// unreleased writer.
fn contend_and_release(contended: &Contended, why: &str) -> crate::sqlite::Error {
    let err = contended.db.initialize_fts().expect_err(why);
    release_writer(contended);
    err
}

#[test]
fn a_lock_during_the_fts_schema_migration_stays_the_locked_verdict() {
    // The rebuild's `execute_batch` used to be flattened with
    // `format!("…: {e}")`, which erased the SQLite result code — so a
    // `SQLITE_BUSY` came back as `Error::Database` carrying busy text with
    // `is_locked() == false`, the one case `errors.rs` says must never
    // happen. This drives a real `BEGIN EXCLUSIVE` writer (the shape a
    // running MCP server produces) rather than a hand-built variant.
    //
    // WAL is required for the rebuild to be the statement that notices the
    // lock: in the default `delete` journal mode the read that precedes it
    // fails first (pinned by
    // `a_lock_before_the_fts_rebuild_now_errors_in_delete_journal_mode`),
    // which would test the precheck rather than the classifier. After #278
    // that earlier read *errors* rather than returning Ok, so this test still
    // needs WAL to reach the rebuild at all — do not "simplify" it away.
    let contended = seed_store_under_exclusive_writer("locked-migration.db", true);

    let err = contend_and_release(&contended, "the rebuild cannot take the write lock");
    assert!(
        matches!(err, crate::sqlite::Error::Locked),
        "a busy rebuild must keep the storage Locked verdict, got {err:?}",
    );
    let public: crate::errors::Error = err.into();
    assert!(
        public.is_locked(),
        "the verdict must survive the public conversion, got {public:?}",
    );
    // The rebuild, not the precheck, is the contended statement here, so the
    // same zero from the precheck tests carries the opposite meaning: the
    // migration was reached and the writer kept it from completing.
    assert_eq!(
        project_id_columns(&contended.db),
        0,
        "the rebuild was reached and blocked — it did not complete",
    );
    // `contended._dir` is dropped here rather than forgotten (issue #278): the lock
    // has been released and both handles close with the test.
}

#[test]
fn a_lock_before_the_fts_rebuild_now_errors_in_delete_journal_mode() {
    // This is the test PR #273 left behind to *pin the swallow*: in vipune's
    // default `delete` mode a `BEGIN EXCLUSIVE` writer blocked the
    // `sqlite_master` read that opens `initialize_fts`, that read was
    // `unwrap_or(false)`, and so a lock looked like "no FTS table at all" —
    // `initialize_fts` returned `Ok(())` on a store that still needed the
    // migration. Issue #278 propagates the precheck, so the assertion is
    // inverted: the same setup must now produce the locked verdict. Kept as
    // its own test rather than folded into the WAL case above, so the history
    // of the pinned behaviour stays in the file and the WAL test keeps its
    // motivation visible — without WAL the rebuild is never the contended
    // statement, and that test would pass on a store that was never contended.
    //
    // The premise this test rests on is vipune's *default* journal mode, so it
    // is read from the store rather than taken on faith from the `wal: false`
    // argument (and read before contending, since after the writer steps every
    // read reports the lock instead).
    let contended = seed_store_under_exclusive_writer("locked-migration-delete.db", false);
    // vipune never sets `journal_mode`, so `delete` is both SQLite's default and
    // the premise of this test; read it from the store rather than trusting the
    // seeding helper's `wal: false` argument, and read it *before* contending
    // (after the writer steps, every read reports the lock instead of the mode).
    let mode: String = contended
        .db
        .conn()
        .query_row("PRAGMA journal_mode;", [], |r| r.get(0))
        .expect("read the journal-mode premise");
    assert_eq!(
        mode, "delete",
        "the premise of this test is vipune's default journal mode",
    );

    // The observable fix: where the pinned version of this test expected
    // `Ok(())`, the locked precheck now returns the locked verdict.
    let err = contend_and_release(
        &contended,
        "#278: a locked sqlite_master read must surface as an error, not 'no FTS table'",
    );
    // Two things are asserted, because "an error came back" alone could be true
    // for the wrong reason (a lock noticed later, or a rebuild that half-ran):
    // the verdict *is* the lock, and the rebuild did *not* run.
    assert!(
        matches!(err, crate::sqlite::Error::Locked),
        "a busy precheck must keep the storage Locked verdict, got {err:?}",
    );
    let public: crate::errors::Error = err.into();
    assert!(
        public.is_locked(),
        "the verdict must survive the public conversion, got {public:?}",
    );
    assert_eq!(
        project_id_columns(&contended.db),
        0,
        "the rebuild never ran — the error came from the precheck, not from a \
         half-completed migration",
    );
}

#[test]
fn initialize_fts_reports_ok_when_the_fts_table_legitimately_does_not_exist() {
    // The counterpart to the fix: propagating the precheck must not turn the
    // ordinary "no `memories_fts` table" case into an error. `Database::open`
    // always creates the table, so the absence is built by hand here — the
    // table *and* its triggers dropped, and `user_version` pinned to the
    // latest so nothing is pending: a reopen would otherwise run a migration,
    // which takes `BEGIN EXCLUSIVE` and would make this measure the open
    // rather than the precheck.
    //
    // With nothing to migrate, `initialize_fts` takes the `fts_exists == false`
    // branch and returns `Ok(())` without touching the rebuild.
    //
    // The table is dropped through a raw connection rather than
    // `Database::open`: `open` re-runs `create_schema`, whose
    // `CREATE VIRTUAL TABLE IF NOT EXISTS` resurrects whatever this test just
    // removed (`put_store_in_need_of_fts_rebuild` relies on the same
    // asymmetry). The triggers have to go with the table — an INSERT trigger
    // pointing at a dropped FTS table makes the next write to `memories` fail
    // outright — so they are enumerated from `sqlite_master` rather than named
    // by hand: a trigger added to `create_schema` or to the rebuild batch is
    // dropped here too, which keeps the completeness claim mechanical rather
    // than prose. `memories` keeps one row so the empty precheck is reached
    // with real data behind it.
    let dir = TempDir::new().unwrap();
    let path = dir.path().join("no-fts.db");
    {
        let db = Database::open(&path).unwrap();
        db.insert(
            "proj1",
            "a row with no fts index",
            &vec![0.1f32; 384],
            None,
            "fact",
            "active",
        )
        .unwrap();
    }
    let conn = rusqlite::Connection::open(&path).unwrap();
    conn.execute_batch("DROP TABLE IF EXISTS memories_fts;")
        .unwrap();
    let fts_triggers: Vec<String> = conn
        .prepare(
            "SELECT name FROM sqlite_master WHERE type='trigger' AND name LIKE 'memories_fts_%'",
        )
        .unwrap()
        .query_map([], |r| r.get(0))
        .unwrap()
        .collect::<rusqlite::Result<Vec<String>>>()
        .unwrap();
    assert!(
        !fts_triggers.is_empty(),
        "the premise: the store has FTS triggers to drop, got {fts_triggers:?}",
    );
    for name in &fts_triggers {
        // `name` came from `sqlite_master`, so it is an identifier this store
        // itself created; the interpolation cannot widen the statement.
        conn.execute_batch(&format!("DROP TRIGGER IF EXISTS {name};"))
            .unwrap();
    }
    conn.pragma_update(None, "user_version", 6).unwrap();

    // The precheck has to run on a handle that did not re-bootstrap the
    // schema: every `Database::open` re-runs `create_schema`, whose
    // `CREATE VIRTUAL TABLE IF NOT EXISTS` resurrects the table this test just
    // removed. `Database::from_conn` wraps the very connection that did the
    // drop, so the assertion is about `initialize_fts` and not about which
    // handle type the test happened to use.
    let db = Database::from_conn(conn);
    let absent: i64 = db
        .conn()
        .query_row(
            "SELECT COUNT(*) FROM sqlite_master WHERE type='table' AND name='memories_fts'",
            [],
            |r| r.get(0),
        )
        .unwrap();
    assert_eq!(absent, 0, "the premise: the FTS table is genuinely absent");

    let leftover_triggers: i64 = db
        .conn()
        .query_row(
            "SELECT COUNT(*) FROM sqlite_master WHERE type='trigger' AND name LIKE 'memories_fts_%'",
            [],
            |r| r.get(0),
        )
        .unwrap();
    assert_eq!(
        leftover_triggers, 0,
        "no trigger may survive pointing at the dropped FTS table — the next write \
         to `memories` would fail outright rather than reach the precheck",
    );

    db.initialize_fts()
        .expect("a genuinely absent FTS table is Ok, not an error");
}

#[test]
fn an_unclassified_fts_migration_failure_keeps_its_context_and_its_message() {
    // The other half of `with_context`: the context is prepended after
    // classification, so a genuine (non-lock) failure still reads as the
    // operator saw it before, and a non-`Sqlite` verdict is never
    // flattened to make room for prose.
    let plain = crate::sqlite::Error::Sqlite("no such table: nope".to_string())
        .with_context(MIGRATION_CONTEXT);
    match plain {
        crate::sqlite::Error::Sqlite(msg) => {
            assert_eq!(msg, "FTS5 schema migration failed: no such table: nope");
        }
        other => panic!("expected Sqlite with the context prepended, got {other:?}"),
    }

    // A verdict has no message to grow, so the context must not swallow it.
    let locked = crate::sqlite::Error::Locked.with_context(MIGRATION_CONTEXT);
    assert!(
        matches!(locked, crate::sqlite::Error::Locked),
        "context must never flatten the locked verdict, got {locked:?}"
    );
}

#[test]
fn with_context_leaves_every_non_sqlite_variant_untouched() {
    // The arms `with_context` must NOT touch. An earlier revision mapped the
    // three structured decoding variants to `Error::Locked` here, which turned
    // a corrupt BLOB into a retryable lock and dropped the `id`/`expected`/
    // `actual` a caller acts on — so each is pinned individually, plus the
    // verdict arm in the test above, rather than covered by a blanket wildcard.
    // Each case pairs a storage error with what its public form must be; the
    // `fn` pointer is factored out because the pair type trips
    // `clippy::type_complexity` when written inline.
    type Kept = (crate::sqlite::Error, fn(&crate::sqlite::Error) -> bool);
    let cases: Vec<Kept> = vec![
        (
            crate::sqlite::Error::CorruptEmbedding {
                id: "mem-1".to_string(),
                reason: "not enough bytes".to_string(),
            },
            |e| {
                matches!(e, crate::sqlite::Error::CorruptEmbedding { id, reason }
                if id == "mem-1" && reason == "not enough bytes")
            },
        ),
        (
            crate::sqlite::Error::InvalidBlobSize {
                expected: crate::embedding::EMBEDDING_DIMS * 4,
                actual: 4,
            },
            |e| {
                matches!(
                    e,
                    crate::sqlite::Error::InvalidBlobSize {
                        expected,
                        actual: 4
                    } if *expected == crate::embedding::EMBEDDING_DIMS * 4
                )
            },
        ),
        (
            crate::sqlite::Error::MismatchedDimensions {
                expected: 384,
                actual: 768,
            },
            |e| {
                matches!(
                    e,
                    crate::sqlite::Error::MismatchedDimensions {
                        expected: 384,
                        actual: 768
                    }
                )
            },
        ),
        (crate::sqlite::Error::Locked, |e| {
            matches!(e, crate::sqlite::Error::Locked)
        }),
    ];
    for (err, is_kept) in cases {
        let expected = err.to_string();
        let after = err.with_context(MIGRATION_CONTEXT);
        assert!(
            is_kept(&after),
            "a non-Sqlite variant must survive with_context with its payload, got {after:?}"
        );
        assert_eq!(
            after.to_string(),
            expected,
            "with_context must not rewrite a non-Sqlite variant's rendering"
        );
    }
}