fix: publish changes once the committing statement finishes

CHANGELOG.md

@@ -44,12 +44,15 @@ Databases must be registered on the Rust side with a stable key before they can
   - **`DatabaseWrapper::close()` and `remove()` now take `self`** instead of `mut self`. Source-compatible for existing callers.
   - **`ObservableWriteGuard::into_inner()` now returns the new `sqlx_sqlite_observer::UnobservedWriter` enum** (`Regular(WriteGuard)` or `Attached(AttachedWriteGuard)`) instead of unconditionally returning `WriteGuard`. Had no callers anywhere in this workspace prior to this release.
   - **`sqlx_sqlite_observer::hooks::register_hooks()` now takes a schema-alias → broker `HashMap`** instead of a single `Arc<ObservationBroker>`, to support routing attached-database writes to their owning database's broker.
-- **`sqlx_sqlite_observer::TableChange` and `sqlx_sqlite_observer::hooks::PreUpdateEvent` each gain a `pub schema: String` field** (#53) — `"main"` for the primary database, the `ATTACH` alias otherwise. See `TableChange::schema`'s own docs for why it is provenance metadata rather than a stable identifier. **Both types are now also `#[non_exhaustive]`**, so downstream struct literals and exhaustive destructuring patterns need updating (read fields, or add `..` to patterns). Only this crate constructs either type, so in practice this affects code that pattern-matches their full shape. Deliberately unlike `Error` above, which stays exhaustive: these two are expected to gain fields again, and `#[non_exhaustive]` makes the next addition a non-event rather than another major bump.
+- **`sqlx_sqlite_observer::TableChange` gains a `pub schema: String` field** (#53) — `"main"` for the primary database, the `ATTACH` alias otherwise. See `TableChange::schema`'s own docs for why it is provenance metadata rather than a stable identifier. **The type is now also `#[non_exhaustive]`**, so downstream struct literals and exhaustive destructuring patterns need updating (read fields, or add `..` to patterns). Only this crate constructs it, so in practice this affects code that pattern-matches its full shape. Deliberately unlike `Error` above, which stays exhaustive: it is expected to gain fields again, and `#[non_exhaustive]` makes the next addition a non-event rather than another major bump. (`hooks::PreUpdateEvent` gained the same field, but is no longer public - see below.)
 - **`AttachedSpec::schema_name` is now capped at 64 bytes**, failing with `sqlx_sqlite_conn_mgr::Error::InvalidSchemaName` above that (which reaches the frontend as `CONNECTION_ERROR`, or `OBSERVER_ERROR` when observation is active — see the Frontend API note above). Shape was already validated (`[A-Za-z0-9_]`, no leading digit) but length was not, and the alias is now copied into every change notification for the owning database and serialized to each of its subscribers, once per changed row.
 - **`AttachedSpec::schema_name` values of `main`/`temp` (case-insensitively) are now rejected with `Error::InvalidSchemaName`**, and two specs sharing an alias - compared case-insensitively - now fail with the new `Error::DuplicateSchemaName` instead of a raw SQLite `ATTACH` error (see Fixed below for why this also closes a stranding vector).
 - **`Error::InvalidSchemaName`'s message now lists every rule it enforces** — non-empty, `[A-Za-z0-9_]` only, no leading digit, at most 64 bytes, and not the reserved `main`/`temp`. It previously named only the character-shape rules, so a rejected `main` (or an over-long alias) produced an error whose every stated condition the input satisfied. The variant is unchanged; only the message text is, which matters for anything matching on the string rather than the variant.
 - Added **`sqlx_sqlite_conn_mgr::Error::DuplicateSchemaName`** (surfaces to the frontend the same way `InvalidSchemaName` does — `CONNECTION_ERROR`, or `OBSERVER_ERROR` when observation is active). `Error` is not `#[non_exhaustive]`, so exhaustive matches on it need a new arm.
 - Added **`sqlx_sqlite_observer::Error::BrokerAliasCollision`** (feature-gated on `conn-mgr`). Like `sqlx_sqlite_conn_mgr::Error`, the observer's `Error` is not `#[non_exhaustive]`, so exhaustive matches need a new arm. It guards the alias → broker map against being built with a colliding key; `validate_attached_specs()` now rejects every input that could reach it, so it is defence-in-depth against a future regression in `ATTACH` ordering rather than a reachable error today.
+- **`sqlx_sqlite_observer::ObservationBroker::on_preupdate()`, `on_commit()`, and `on_rollback()` are removed.** All three were `pub`. Per-transaction buffering moved from the broker to the per-connection hook context that `register_hooks()` installs, so a caller that drove a broker directly has no replacement: that hook context, not the broker, now owns every piece of per-transaction state. The broker keeps only the state it shares across connections - the observed tables, the schema information, the event conversion, and the broadcast channel.
+  - **`sqlx_sqlite_observer::hooks::PreUpdateEvent` is now crate-private.** With `on_preupdate()` gone, no public function took or returned it.
+  - **`sqlx_sqlite_observer::unregister_hooks()` does more.** It still unregisters the preupdate, commit, and rollback hooks and reclaims the hook context. It now also clears the connection's `sqlite3_trace_v2` slot, and publishes any committed change the trace hook has not published yet (a commit made during a statement reset, with no statement run since). A caller who replaced this crate's trace hook with their own loses that hook here.
 
 ### Added
 
@@ -109,6 +112,11 @@ Transaction cleanup failures propagate as errors rather than being logged and ig
   - **Attached-database observation no longer requires the main database to be observed too.** The whole observable acquisition path was gated on the *main* database's broker, so a write through an unobserved database into an observed `readWrite` attachment registered no hooks at all and reached that attachment's own subscribers not at all — silently, with every call reporting success. This was the same class of defect as #53 itself: observation depending on the wrong object. The observable path is now taken when *any* participating database is observed — main or a `readWrite` attachment — and `main` is simply omitted from the alias → broker map when the main database has no broker of its own, so its changes are dropped rather than misattributed. Nothing had covered this configuration, because the attached-observation tests enabled observation on both databases.
   - **Known limitation:** a writer holds the broker it bound at acquisition for its whole lifetime, so a `disable_observation()` + `enable_observation()` cycle *during* an open transaction leaves that transaction publishing to the previous broker. Subscribers created before the cycle still receive its commit (the hook context keeps that broker alive); subscribers created after the cycle do not, while `is_observing()`, `subscribe()`, and the commit itself all report success. Reachable as: the last window `unobserve()`s and then `observe()`s again while another caller's interruptible transaction is still open. Pinned by an `#[ignore]`d test rather than only prose; the fix (reviving the slot's previous broker, gated on a lease held for the hooks' lifetime) is follow-up work.
   - **Known limitation:** the plugin's `observe()`/`unobserve()` reference count is keyed on webview labels, so a Rust caller that enabled observation directly on a `DatabaseWrapper` is outside it. The last window releasing tears observation down database-wide and silently ends that caller's subscription; conversely a Rust `disable_observation()` breaks every window's live subscription while leaving their registrations non-zero. A plugin-level API for Rust observers is follow-up work.
+- **A change notification now means that the change is durable and readable, not only that the commit hook ran.** SQLite's commit hook runs before the commit is final. It is the hook whose non-zero return converts a `COMMIT` into a `ROLLBACK`. A subscriber that read immediately after notification previously saw pre-commit state through another connection, on most notifications. The tests `a_notification_means_the_change_is_readable_in_wal_mode` and `a_notification_means_the_change_is_readable_in_rollback_journal_mode` now guard this, one per journal mode. The commit hook now converts and holds the transaction's changes and does not publish them yet. A hook registered with `SQLITE_TRACE_PROFILE` and `SQLITE_TRACE_STMT` publishes the changes when the statement that committed finishes, or when the next statement starts. By then the commit is durable, so any connection can read it. The writer does nothing extra.
+  - **Notification order is now per connection.** Publication happens after SQLite released the write lock, so two connections that commit in one order can notify in the other. On master, the commit hook published under the lock, which kept commit order across connections. Every write through `sqlx-sqlite-conn-mgr`'s single writer, and so every write through the plugin, keeps its order.
+  - **A commit made during a statement reset notifies when the next statement starts.** `fetch_one` stops at the first row and resets the statement, and for an autocommit `INSERT ... RETURNING` the commit happens inside that reset, after SQLite's profile event. The trace hook is therefore also registered for `SQLITE_TRACE_STMT`, which fires when a statement starts, before any of its hooks. Without it, the held change waited for the next statement's profile event, and a failed next statement's rollback hook discarded it: the row was committed and no notification ever arrived. The test `a_commit_made_during_a_statement_reset_survives_a_later_failed_write` guards this. Dropping the connection also publishes it, guarded by `dropping_a_connection_publishes_a_commit_made_during_a_statement_reset`.
+  - The trace hook occupies the connection's one `sqlite3_trace_v2` slot. sqlx does not use that slot. If a consumer installs a trace hook on an observed connection, the new hook replaces this crate's hook, and notifications from that connection stop.
+  - Buffering is now per connection instead of per broker, so an abandoned transaction cannot surface as a phantom change on any other connection's commit. The mid-transaction fix above (#53) covered the writer guards of `sqlx-sqlite-conn-mgr`, which discarded the shared buffer when they came down. A connection from the plain sqlx pool (`SqliteObserver::acquire()`) had no such discard, and one shared buffer per database meant its stranded events surfaced on whichever connection committed next. The buffer now dies with the hook context on both paths.
 - **`ObservableSqliteDatabase::acquire_writer()` no longer holds the single write permit while waiting on a read-pool connection.** It warmed each observed table's cached schema info *after* taking the permit, and that warm-up awaits the read pool (six connections by default). Six tasks each holding a read connection and then waiting for the writer would deadlock against it until sqlx's 30-second acquire timeout fired, failing up to seven of them with `PoolTimedOut`. Reachable from an ordinary query-then-write pattern, and on every acquisition — not just the first — whenever an observed table doesn't exist in the schema, since that result is never cached. The attached-writer path already warmed first; both now do. **The trade this makes:** a table that joins the observed set after warming — via `subscribe()`, `subscribe_stream()`, or a later `observe()`, including *mid-transaction*, since the preupdate hook checks the observed set live when a change fires rather than snapshotting it at hook registration — has its changes published with an empty `primary_key`, and a meaningless `rowid` if the table is `WITHOUT ROWID`, converging on the next writer acquisition. Re-checking after the permit is acquired would not close that window (it ends at end-of-transaction, not at permit acquisition) and would reintroduce the deadlock, because an observed name that doesn't resolve in the schema is never cached and so would take the read pool under the permit on every acquisition.
 - **An attached-database query whose statement fails now still detaches.** `execute`, `fetch_all`, `fetch_one`, `fetch_page` and both transaction builders returned early on a statement error, or on a failed `BEGIN IMMEDIATE`, without issuing `DETACH`. (A failing `COMMIT` or `ROLLBACK` still strands the alias — a narrower case, since releasing it there means rolling back first, and the error to report becomes a judgement call. Tracked separately. This was not, however, the only remaining stranding vector - acquiring the attached connection or writer in the first place could strand an alias before a single query ever ran, see the next entry.) Neither the attached guards' `Drop` nor the pool's `after_release` hook detaches — `after_release` runs `ROLLBACK` only — so the alias stayed bound to the pooled connection. Since the write pool holds a single connection, every later attach of that alias then failed with "database ... is already in use" until the connection went genuinely idle (30 seconds by default), which a retry loop prevents indefinitely.
 - **`acquire_reader_with_attached()` and `acquire_writer_with_attached()` no longer strand an alias when a later spec in the same call is rejected or fails to `ATTACH`.** Three related gaps in the same invariant as the entry above:

README.md

@@ -50,7 +50,8 @@ without Tauri:
    * **[`sqlx-sqlite-conn-mgr`](crates/sqlx-sqlite-conn-mgr/)** — Low-level connection
      management: read pool, exclusive writer, WAL mode, attached databases
    * **[`sqlx-sqlite-observer`](crates/sqlx-sqlite-observer/)** — Reactive change
-     notifications using SQLite's native preupdate/commit/rollback hooks
+     notifications using SQLite's native `preupdate_hook`, `commit_hook`,
+     `rollback_hook`, and a `SQLITE_TRACE_PROFILE` trace hook
    * **[`sqlx-sqlite-toolkit`](crates/sqlx-sqlite-toolkit/)** — High-level API:
      `DatabaseWrapper`, builder-pattern queries, interruptible transactions, JSON
      type decoding. Optionally integrates the observer behind a feature flag.