fix(db): follow-ups of the review of PR #433

- close() on a getDb() handle runs as a "connection close" section and
  checks its generation: a handle kept from a previous profile rejects
  with DbConnectionClosedError instead of closing the current profile.
- closeUnlocked never rejects: the state is cleared and the generation
  bumped before close(), a failing close of the old pool is logged with
  logWarn, connectToProfile still opens the next profile and closeDb()
  resolves without a connection.
- The blocking alert names a getDb() call in a callback as a possible
  cause only: a long restore also waits 15 s.
- The generation check of the transaction handle is documented as a
  defence, not reachable today.
- Tests: the "behind the queue" alert (pins the reset of lockHolder),
  close() on a stale and on the current handle, a failing close during a
  switch and in closeDb().

Refs #408

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

docs/architecture.md

@@ -148,9 +148,9 @@ Index Bilan (9) — 7 ajoutés en v9 :
 
 - **Verrou strict, non réentrant (#408).** Le rappel de `withTransaction` reçoit un **handle de transaction** (typé `Database`) qui exécute directement sur la connexion. Toute autre requête attend derrière la transaction, y compris un `getDb()` appelé depuis le rappel ou depuis une fonction qu'il appelle : celui-ci attend la transaction qui l'attend, un interblocage. Règle : dans un rappel, n'utiliser **que** le handle reçu et le passer aux fonctions appelées (par exemple `exec`, obligatoire sur `findOrCreateSecurity`). Avant #408, un drapeau global laissait passer hors verrou toute requête lancée pendant un rappel, d'où qu'elle vienne.
 - **Handle révoqué.** À la fin du rappel (succès ou erreur), le handle est révoqué : un appel tardif depuis une promesse orpheline lève `TransactionHandleRevokedError` au lieu de passer hors verrou. `close()` sur le handle lève toujours `TransactionHandleCloseError`.
-- **Changement de profil dans une seule section.** `connectToProfile`, `initializeNewProfileDb` et `closeDb` sont chacun **un seul** `runExclusive` : fermeture de l'ancien pool, puis `repair_migrations` et `prepare_profile_db` (pour `connectToProfile`, appelé seulement après la fin du `close()`), puis `Database.load` et la bascule. Ils attendent donc les opérations déjà en file et une transaction en cours, et rien ne s'exécute entre la fermeture et l'ouverture. Les helpers internes (`closeUnlocked`, `loadUnlocked`) ne prennent jamais le verrou. Pendant un rattrapage long, toute la file attend : rien d'utile ne peut tourner pendant la bascule.
+- **Changement de profil dans une seule section.** `connectToProfile`, `initializeNewProfileDb` et `closeDb` sont chacun **un seul** `runExclusive` : fermeture de l'ancien pool, puis `repair_migrations` et `prepare_profile_db` (pour `connectToProfile`, appelé seulement après la fin du `close()`), puis `Database.load` et la bascule. Ils attendent donc les opérations déjà en file et une transaction en cours, et rien ne s'exécute entre la fermeture et l'ouverture. Les helpers internes (`closeUnlocked`, `loadUnlocked`) ne prennent jamais le verrou. Pendant un rattrapage long, toute la file attend : rien d'utile ne peut tourner pendant la bascule. Un `close()` de l'ancien pool qui échoue ne bloque pas la bascule : l'état est vidé et la génération incrémentée avant le `close()`, l'échec part au Journal (`logWarn`) et le profil suivant s'ouvre quand même. `closeDb()` seul se résout aussi dans ce cas, sans connexion, et tout ancien handle refuse de tourner.
-- **Génération de connexion.** Chaque ouverture et chaque fermeture incrémentent un numéro de génération. Le proxy de `getDb()` et le handle de transaction le vérifient au moment où l'opération sort de la file : une opération mise en file pour une connexion fermée entre-temps lève `DbConnectionClosedError` (« connection closed by profile switch »), au lieu de tourner sur la base du profil suivant ou sur un pool fermé. Un handle obtenu par `getDb()` avant un changement de profil ne sert donc plus après : le reprendre par `getDb()`.
+- **Génération de connexion.** Chaque ouverture et chaque fermeture incrémentent un numéro de génération. Le proxy de `getDb()` et le handle de transaction le vérifient au moment où l'opération sort de la file : une opération mise en file pour une connexion fermée entre-temps lève `DbConnectionClosedError` (« connection closed by profile switch »), au lieu de tourner sur la base du profil suivant ou sur un pool fermé. Un handle obtenu par `getDb()` avant un changement de profil ne sert donc plus après : le reprendre par `getDb()`. Son `close()` est vérifié de la même façon et lève `DbConnectionClosedError`, au lieu de fermer la connexion du profil courant. Dans le handle de transaction, la vérification est une défense, inatteignable aujourd'hui : rien ne ferme la connexion pendant qu'une transaction tient le verrou.
-- **Alerte de blocage.** Une opération qui attend le verrou depuis plus de `BLOCKED_ALERT_MS` (15 s) écrit **un** `logWarn` au Journal, qui nomme ce qui tient le verrou (`transaction`, `query`, `profile opening`, `profile initialization`, `connection close`) et depuis quand. Cause typique : un `getDb()` dans un rappel. Un rattrapage de migrations de plus de 15 s produit une alerte « profile opening », attendue. `logService` n'écrit qu'en mémoire et en `sessionStorage`, jamais en base : l'appel est sûr pendant que le verrou est tenu.
+- **Alerte de blocage.** Une opération qui attend le verrou depuis plus de `BLOCKED_ALERT_MS` (15 s) écrit **un** `logWarn` au Journal, qui nomme ce qui tient le verrou (`transaction`, `query`, `profile opening`, `profile initialization`, `connection close`) et depuis quand. L'alerte cite un `getDb()` dans un rappel comme cause possible, pas comme diagnostic : une transaction longue (grosse restauration, migration de catégories sur un gros profil) dépasse aussi 15 s. Un rattrapage de migrations de plus de 15 s produit une alerte « profile opening », attendue. `logService` n'écrit qu'en mémoire et en `sessionStorage`, jamais en base : l'appel est sûr pendant que le verrou est tenu.
 - **Tests.** Seul `src/services/db.test.ts` voit le verrou (un `Database` factice qui journalise le début et la fin de chaque opération). Les tests de services mockent `db.ts` (`withTransaction: fn => fn(await getDb())`) et ne le voient pas.
 
 ## Système de migrations

src/services/db.test.ts

@@ -322,6 +322,65 @@ describe("profile switch goes through the lock (#408)", () => {
     await expect(db.execute("AFTER")).rejects.toBeInstanceOf(DbConnectionClosedError);
   });
 
+  it("close() on a getDb() handle kept from the previous profile rejects and keeps the current one open", async () => {
+    const before = await getDb();
+    const next = makeFakeDb("next");
+    h.fakeDb = next;
+    await connectToProfile("next.db");
+    events.length = 0;
+    await expect(before.close()).rejects.toBeInstanceOf(DbConnectionClosedError);
+    expect(next.close).not.toHaveBeenCalled();
+    await (await getDb()).execute("STILL OPEN");
+    expect(next.execute).toHaveBeenCalledWith("STILL OPEN", undefined);
+  });
+
+  it("close() on the current getDb() handle closes the connection", async () => {
+    const current = h.fakeDb;
+    await (await getDb()).close();
+    expect(current.close).toHaveBeenCalledTimes(1);
+    await expect(getDb()).rejects.toThrow(/No database connection/);
+  });
+
+  it("a failing close of the old pool does not prevent the switch, and goes to the Journal", async () => {
+    vi.mocked(logWarn).mockClear();
+    const old = h.fakeDb;
+    const before = await getDb();
+    vi.mocked(old.close).mockImplementation(async () => {
+      throw new Error("close refused for /home/someone/.config/x.db");
+    });
+    const next = makeFakeDb("next");
+    h.fakeDb = next;
+    await connectToProfile("next.db");
+    expect(old.close).toHaveBeenCalledTimes(1);
+    expect(logWarn).toHaveBeenCalledWith(
+      "DB: closing the previous connection failed, continuing without it: close refused for x.db"
+    );
+    await (await getDb()).execute("ON NEXT");
+    expect(next.execute).toHaveBeenCalledWith("ON NEXT", undefined);
+    await expect(before.execute("ON OLD")).rejects.toBeInstanceOf(DbConnectionClosedError);
+    expect(old.execute).not.toHaveBeenCalled();
+  });
+
+  it("closeDb with a failing close resolves, warns, and leaves a coherent state without connection", async () => {
+    vi.mocked(logWarn).mockClear();
+    const old = h.fakeDb;
+    const before = await getDb();
+    vi.mocked(old.close).mockImplementation(async () => {
+      throw new Error("close refused");
+    });
+    await expect(closeDb()).resolves.toBeUndefined();
+    expect(logWarn).toHaveBeenCalledWith("DB: closing the previous connection failed, continuing without it: close refused");
+    // No connection, and the generation moved: the old handle refuses to run.
+    await expect(getDb()).rejects.toThrow(/No database connection/);
+    await expect(withTransaction(async () => undefined)).rejects.toThrow(/No database connection/);
+    await expect(before.execute("ON OLD")).rejects.toBeInstanceOf(DbConnectionClosedError);
+    expect(old.execute).not.toHaveBeenCalled();
+    // The old pool is no longer referenced: the next open does not close it again.
+    h.fakeDb = makeFakeDb("next");
+    await connectToProfile("next.db");
+    expect(old.close).toHaveBeenCalledTimes(1);
+  });
+
   it("initializeNewProfileDb runs its SQL inside the section, before a queued query", async () => {
     const next = makeFakeDb("new");
     h.fakeDb = next;
@@ -371,6 +430,8 @@ describe("blocking alert (#408)", () => {
       const message = String(vi.mocked(logWarn).mock.calls[0][0]);
       expect(message).toContain("a query has waited 15 s");
       expect(message).toContain(`held by a transaction since ${startedAt}`);
+      // A cause the reader may check, not a diagnosis: a long restore also waits 15 s.
+      expect(message).toContain("(possible cause: a getDb() call inside a withTransaction callback");
       // Once per operation: still one warning much later.
       await vi.advanceTimersByTimeAsync(BLOCKED_ALERT_MS * 3);
       expect(logWarn).toHaveBeenCalledTimes(1);
@@ -382,6 +443,32 @@ describe("blocking alert (#408)", () => {
     }
   });
 
+  it("says the lock is free, not who held it last, when the check lands between two operations", async () => {
+    // In the app only microtasks separate the end of one operation from the
+    // start of the next, so a timer cannot fire there; the test puts the check
+    // there to pin that the holder is cleared when an operation ends.
+    const db = await getDb();
+    let release: () => void = () => {};
+    const gate = new Promise<void>((resolve) => {
+      release = resolve;
+    });
+    vi.mocked(h.fakeDb.execute).mockImplementationOnce(async () => {
+      await gate;
+      return { rowsAffected: 0 };
+    });
+    const first = db.execute("LONG");
+    const waiting = db.select("WAITING");
+    // Registered after the queue's own reaction: runs once the first operation
+    // ended (holder cleared), before the waiting one starts.
+    const checked = first.then(() => vi.advanceTimersByTime(BLOCKED_ALERT_MS));
+    release();
+    await checked;
+    await vi.advanceTimersByTimeAsync(10);
+    await waiting;
+    expect(logWarn).toHaveBeenCalledTimes(1);
+    expect(String(vi.mocked(logWarn).mock.calls[0][0])).toBe("DB lock: a query has waited 15 s behind the queue");
+  });
+
   it("says nothing for an operation that got the lock in time", async () => {
     const db = await getDb();
     const run = db.execute("QUICK");

src/services/db.ts

@@ -41,6 +41,11 @@ let serializedInstance: Database | null = null;
 //   generation when its operation leaves the queue: an operation queued for a
 //   connection that was closed meanwhile throws `DbConnectionClosedError`
 //   rather than running on the next profile's database or on a closed pool.
+//   `close()` on a `getDb()` handle is checked the same way, so a handle kept
+//   from a previous profile cannot close the current one.
+// - A failing `close()` of the old pool never blocks a switch: the state is
+//   already cleared and the generation bumped, the failure goes to the Journal
+//   and the next profile still opens (see `closeUnlocked`).
 // ---------------------------------------------------------------------------
 
 /** An operation queued for a connection closed before it could run. */
@@ -77,10 +82,13 @@ type LockKind =
 
 /**
  * An operation still waiting for the lock after this delay writes ONE warning
- * to the Journal, naming what holds the lock and since when. The typical cause
+ * to the Journal, naming what holds the lock and since when. One possible
- * is a `getDb()` call from inside a `withTransaction` callback (deadlock). A
+ * cause is a `getDb()` call from inside a `withTransaction` callback
- * profile catch-up (`prepare_profile_db`) that takes longer also triggers it,
+ * (deadlock), but a long transaction (a large restore, a category migration on
- * naming a "profile opening": expected, nothing can run during a switch.
+ * a big profile) also exceeds the delay, so the warning names it as a possible
+ * cause only. A profile catch-up (`prepare_profile_db`) that takes longer also
+ * triggers it, naming a "profile opening": expected, nothing can run during a
+ * switch.
  * `logWarn` only writes to memory and sessionStorage, never to the DB, so it
  * is safe to call while the lock is held.
  */
@@ -102,7 +110,7 @@ function runExclusive<T>(kind: LockKind, fn: () => Promise<T>): Promise<T> {
       holder
         ? `DB lock: a ${kind} has waited ${waited} s, the lock is held by a ${holder.kind} since ${new Date(holder.since).toISOString()}` +
             (holder.kind === "transaction"
-              ? " (a getDb() call inside a withTransaction callback deadlocks: use the callback's handle)"
+              ? " (possible cause: a getDb() call inside a withTransaction callback, which deadlocks; the callback must use its own handle)"
               : "")
         : `DB lock: a ${kind} has waited ${waited} s behind the queue`
     );
@@ -157,7 +165,13 @@ function serialize(db: Database, handleGeneration: number): Database {
           });
       }
       if (prop === "close") {
-        return () => closeDb();
+        // Same check as execute/select: a handle kept from a previous profile
+        // must not close the current profile's connection (#408 review).
+        return () =>
+          runExclusive("connection close", async () => {
+            assertGeneration(handleGeneration);
+            await closeUnlocked();
+          });
       }
       const value = Reflect.get(target, prop, receiver);
       return typeof value === "function" ? value.bind(target) : value;
@@ -169,6 +183,11 @@ function serialize(db: Database, handleGeneration: number): Database {
  * The handle a `withTransaction` callback receives: runs directly on the
  * connection (the transaction already holds the lock), refuses `close()`, and
  * throws once `revoke()` ran at the end of the callback.
+ *
+ * Its generation check is a defence, not reachable today: only the exclusive
+ * profile sections change the generation, and none of them can run while a
+ * transaction holds the lock, so nothing closes the connection under a
+ * running callback. It stays in case a future path closes without the lock.
  */
 function transactionHandle(
   db: Database,
@@ -247,13 +266,29 @@ export async function withTransaction<T>(
 // Internal helpers for the profile sections below. They NEVER take the lock:
 // only call them from inside a `runExclusive` section.
 
+/**
+ * Drop the current connection, then close its pool. Never rejects: the state
+ * is cleared and the generation bumped BEFORE `close()`, so a failing close
+ * leaves a coherent "no connection" state (every older handle refuses to run)
+ * and only writes a warning to the Journal. A switch therefore still opens the
+ * next profile, and `closeDb()` resolves: the app no longer references the old
+ * pool, and keeping it would leave the app without any usable connection.
+ */
 async function closeUnlocked(): Promise<void> {
   if (rawInstance) {
     const closing = rawInstance;
     rawInstance = null;
     serializedInstance = null;
     generation += 1;
+    try {
       await closing.close();
+    } catch (e) {
+      logWarn(
+        withoutFolders(
+          `DB: closing the previous connection failed, continuing without it: ${e instanceof Error ? e.message : String(e)}`
+        )
+      );
+    }
   }
 }