diff --git a/.claude/skills/event-store-semantics/SKILL.md b/.claude/skills/event-store-semantics/SKILL.md index 57a9e9a7d0..a98f901693 100644 --- a/.claude/skills/event-store-semantics/SKILL.md +++ b/.claude/skills/event-store-semantics/SKILL.md @@ -37,7 +37,7 @@ Executable spec: the test suites in `quartz/src/commonTest/.../store/sqlite/` (`BasicTest`, `ReplaceableTest`, `AddressableTest`, `DeletionTest`, `ExpirationTest`, `RightToVanishTest`, `SearchTest`, `SearchRelevanceOrderTest`, `MergeQueryCorrectnessTest`, `TagMergeCorrectnessTest`, `QueryAssemblerTest`, -`SnapshotIdsForNegentropyTest`, `FilterMatcherTest`, …). If a rule here ever contradicts a test, +`SnapshotIdsForNegentropyTest`, `FilterMatcherTest`, `InsertOutcomeClassificationTest`, …). If a rule here ever contradicts a test, the test wins — and this file has a bug to fix. ## Kind classes (used throughout) @@ -189,6 +189,21 @@ back alone and reports `Rejected(reason)`; the rest commit. If the **outer commi entry is treated as `Rejected` (the `IEventStore.batchInsert` contract). Outcomes are returned in input order; OK frames pair by event id, not order. +**STORE-W09 — a failed row is classified against the database, not against the driver's +exception text.** `SQLiteEventStore.classifyRowError` rolls the row's savepoint back and then +asks the connection (which now shows pre-insert state): id already present → `DUPLICATE`; +a stored version that beats this one at the replaceable/addressable coordinate (the exact +complement of the supersession predicate in W01/W02) → `SUPERSEDED`; otherwise `Failed`. +Message text is only a fast path and a fallback for trigger RAISEs (`blocked:`, `not allowed`), +which leave no database-visible trace. This matters because the message is driver-specific — +the bundled JVM driver writes `UNIQUE constraint failed: event_headers.id`, Android's throws an +`android.database.SQLException` with a **null** message — so a text-only classifier answered +`OK false` on Android for events the store already held. Corollary: re-offering a stored +replaceable/addressable event **byte-for-byte** is `DUPLICATE`, not `SUPERSEDED` (it violates +both indexes and only the id answer is driver-independent); a stale *different* version is +still `SUPERSEDED`. Both carry the `duplicate:` prefix, so the relay reply is `OK true` either +way. + --- ## Deletion lifecycle — NIP-09 / NIP-62 (STORE-D) @@ -323,6 +338,11 @@ non-itemizable cases). Add one line per behavior change, newest first: `YYYY-MM-DD — what changed`. +- 2026-09-18 (pending) W09, W01/W02 — insert-failure classification now queries the database + instead of parsing the driver's exception message (Android's is null, so duplicates were + reported as `Failed`/`OK false`). A byte-for-byte re-offer of a stored replaceable/addressable + event now reports `DUPLICATE` where the JVM driver previously reported `SUPERSEDED`; both are + `duplicate:` → `OK true`, so the wire answer is unchanged. - 2026-08-04 (baseline) — rules F01–F13, W01–W08, D01–D08, C01, S01–S06, N01 written from the code at the time this skill was introduced. Changes before this date are not itemized; archaeology starts at `git log` on `nip01Core/store/`. diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SQLiteEventStore.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SQLiteEventStore.kt index a56fb390f4..e57e7c0882 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SQLiteEventStore.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SQLiteEventStore.kt @@ -519,7 +519,7 @@ class SQLiteEventStore( // ROLLBACK shouldn't mask the original cause. runCatching { db.execSQL("ROLLBACK TRANSACTION TO SAVEPOINT $sp") } runCatching { db.execSQL("RELEASE SAVEPOINT $sp") } - classifyRowError(e) + classifyRowError(e, event, db) } } @@ -531,8 +531,25 @@ class SQLiteEventStore( * I/O error, schema drift) is the store failing to write an acceptable * event: `Failed`, so a rising count is loud instead of blending into * the duplicate tally. + * + * Message text is the fast path, not the contract: which exception a + * driver throws and what it puts in `getMessage()` is the driver's + * business. Android's `SQLiteConnection` wraps constraint failures in + * an `android.database.SQLException` carrying a **null** message, + * where the bundled JVM driver spells out + * `UNIQUE constraint failed: …`. Classifying off text alone therefore + * turned every duplicate into a `Failed` on Android — an `OK false` + * the client retries forever. So [db] is asked instead: this runs after + * the savepoint rollback, so the connection shows the pre-insert state + * and the two questions that separate a duplicate from a genuine write + * failure ("is this id already here?", "does a stored version already + * beat this one?") have exact answers, on every driver. */ - private fun classifyRowError(e: Throwable): IEventStore.InsertOutcome { + private fun classifyRowError( + e: Throwable, + event: Event, + db: SQLiteConnection, + ): IEventStore.InsertOutcome { val message = e.message ?: e::class.simpleName ?: RejectionReason.INSERT_FAILED // A second copy of an event the store already holds trips the unique index on // event_headers.id. That is not a refusal of the event but a statement that it @@ -542,25 +559,108 @@ class SQLiteEventStore( if (message.contains(DUPLICATE_ID_CONSTRAINT)) { return IEventStore.InsertOutcome.Rejected(RejectionReason.DUPLICATE) } - // The replaceable / addressable unique indexes fire only when the supersession - // trigger found nothing older to delete, i.e. the stored version already wins - // (STORE-W01/W02). Same shape as a duplicate: nothing to write, `OK true`. - if (message.contains(SUPERSEDED_CONSTRAINT)) { - return IEventStore.InsertOutcome.Rejected(RejectionReason.SUPERSEDED) - } - val refusal = + // Trigger RAISEs and the immutability guards name themselves, and leave no + // database-visible trace to ask about, so they are decided by text alone. + val namedRefusal = message.contains("blocked:") || message.contains("duplicate:") || message.contains(RejectionReason.PREFIX_REPLACED) || - message.contains("not allowed") || - message.contains("constraint", ignoreCase = true) - return if (refusal) { + message.contains("not allowed") + if (namedRefusal) return IEventStore.InsertOutcome.Rejected(message) + + // Ask the database the two questions the unique indexes answer, in the + // order that makes the answer driver-independent. The id question goes + // first because *which* index a re-offered replaceable event trips is up + // to SQLite: re-inserting a stored replaceable byte-for-byte violates + // both `event_headers.id` and `replaceable_idx`, and only the id lookup + // says the same thing on every driver ("already have this event" — which + // is also the truer sentence). It costs one point lookup on an already + // open connection, on the rejected path only. + if (isAlreadyStored(event.id, db)) { + return IEventStore.InsertOutcome.Rejected(RejectionReason.DUPLICATE) + } + // The replaceable / addressable unique indexes fire only when the supersession + // trigger found nothing older to delete, i.e. the stored version already wins + // (STORE-W01/W02). Same shape as a duplicate: nothing to write, `OK true`. + if (message.contains(SUPERSEDED_CONSTRAINT) || isSupersededByStored(event, db)) { + return IEventStore.InsertOutcome.Rejected(RejectionReason.SUPERSEDED) + } + + return if (message.contains("constraint", ignoreCase = true)) { IEventStore.InsertOutcome.Rejected(message) } else { IEventStore.InsertOutcome.Failed(message) } } + /** + * Whether [id] is already in `event_headers` — the unique index on + * `event_headers.id` restated as a question, for drivers that don't say + * which index they tripped. Any failure answers "no": the point is to + * recognize a duplicate, and a connection too broken to answer is a + * write failure, which is what the caller falls through to. + */ + private fun isAlreadyStored( + id: String, + db: SQLiteConnection, + ): Boolean = + runCatching { + db.prepare("SELECT 1 FROM event_headers WHERE id = ? LIMIT 1").use { stmt -> + stmt.bindText(1, id) + stmt.step() + } + }.getOrDefault(false) + + /** + * Whether a stored version already beats [event] at its replaceable / + * addressable coordinate (STORE-W01/W02) — the exact complement of + * [displacedBy]'s predicate, so a stored row that the supersession + * trigger *would* have deleted doesn't count. That precision matters + * here: this is the fallback for unrecognized failures, and a disk + * error while inserting a winning replaceable event must stay `Failed` + * rather than turn into a silent `OK true`. An equal id is the + * duplicate case and is answered before this one. + */ + private fun isSupersededByStored( + event: Event, + db: SQLiteConnection, + ): Boolean { + val addressable = event.kind.isAddressable() && event is AddressableEvent + val sql = + when { + event.kind.isReplaceable() -> + """ + SELECT 1 FROM event_headers + WHERE kind = ? AND pubkey = ? + AND (created_at > ? OR (created_at = ? AND id < ?)) + LIMIT 1 + """.trimIndent() + + addressable -> + """ + SELECT 1 FROM event_headers + WHERE kind = ? AND pubkey = ? AND d_tag = ? + AND kind >= 30000 AND kind < 40000 + AND (created_at > ? OR (created_at = ? AND id < ?)) + LIMIT 1 + """.trimIndent() + + else -> return false + } + return runCatching { + db.prepare(sql).use { stmt -> + var i = 1 + stmt.bindLong(i++, event.kind.toLong()) + stmt.bindText(i++, event.pubKey) + if (addressable) stmt.bindText(i++, (event as AddressableEvent).dTag()) + stmt.bindLong(i++, event.createdAt) + stmt.bindLong(i++, event.createdAt) + stmt.bindText(i, event.id) + stmt.step() + } + }.getOrDefault(false) + } + inner class Transaction internal constructor( val db: SQLiteConnection, private val delta: LiveIndexDelta?, diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/InsertOutcomeClassificationTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/InsertOutcomeClassificationTest.kt new file mode 100644 index 0000000000..6507de40f4 --- /dev/null +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/InsertOutcomeClassificationTest.kt @@ -0,0 +1,133 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.quartz.nip01Core.store.sqlite + +import com.vitorpamplona.quartz.nip01Core.core.Event +import com.vitorpamplona.quartz.nip01Core.metadata.MetadataEvent +import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerSync +import com.vitorpamplona.quartz.nip01Core.store.IEventStore +import com.vitorpamplona.quartz.nip01Core.store.RejectionReason +import com.vitorpamplona.quartz.nip10Notes.TextNoteEvent +import com.vitorpamplona.quartz.nip23LongContent.LongTextNoteEvent +import com.vitorpamplona.quartz.utils.TimeUtils +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * `batchInsert` must name *why* a row didn't land, because the relay turns + * that reason into the NIP-01 answer: [RejectionReason.DUPLICATE] and + * [RejectionReason.SUPERSEDED] are `OK true` ("already covered"), everything + * else is `OK false`, which clients retry. + * + * This suite pins the classification **independently of the SQLite driver's + * exception text**. The bundled JVM driver raises + * `UNIQUE constraint failed: event_headers.id`; Android's raises an + * `android.database.SQLException` whose message is `null`. Reading the text + * was therefore enough on one target and wrong on the other — a duplicate + * came back as `error: SQLException`, an `OK false` the client re-offers + * forever. Every case below runs on both targets and so fails on either if + * the classifier ever goes back to trusting a message. + */ +class InsertOutcomeClassificationTest : BaseDBTest() { + val signer = NostrSignerSync() + + private fun assertRejected( + expectedReason: String, + outcome: IEventStore.InsertOutcome, + ) { + assertTrue(outcome is IEventStore.InsertOutcome.Rejected, "expected Rejected, got $outcome") + assertEquals(expectedReason, outcome.reason) + } + + @Test + fun duplicateIdIsRejectedAsDuplicate() = + forEachDB { db -> + val event = signer.sign(TextNoteEvent.build("hello", createdAt = TimeUtils.now())) + + assertEquals(IEventStore.InsertOutcome.Accepted, db.batchInsert(listOf(event))[0]) + assertRejected(RejectionReason.DUPLICATE, db.batchInsert(listOf(event))[0]) + } + + @Test + fun reofferingAStoredReplaceableIsRejectedAsDuplicate() = + forEachDB { db -> + // Byte-for-byte the stored version, so it violates the id index *and* + // replaceable_idx — and which one SQLite reports first is the driver's + // choice. "Already have this event" is the answer that holds on all of + // them. + val event = signer.sign(MetadataEvent.createNew("Vitor", createdAt = TimeUtils.now())) + + assertEquals(IEventStore.InsertOutcome.Accepted, db.batchInsert(listOf(event))[0]) + assertRejected(RejectionReason.DUPLICATE, db.batchInsert(listOf(event))[0]) + } + + @Test + fun olderReplaceableIsRejectedAsSuperseded() = + forEachDB { db -> + val time = TimeUtils.now() + val older = signer.sign(MetadataEvent.createNew("Vitor 1", createdAt = time)) + val newer = signer.sign(MetadataEvent.createNew("Vitor 2", createdAt = time + 1)) + + assertEquals(IEventStore.InsertOutcome.Accepted, db.batchInsert(listOf(newer))[0]) + assertRejected(RejectionReason.SUPERSEDED, db.batchInsert(listOf(older))[0]) + } + + @Test + fun sameSecondReplaceableTieLoserIsRejectedAsSuperseded() = + forEachDB { db -> + val time = TimeUtils.now() + // NIP-01 breaks a created_at tie by lowest id, so the winner is + // decided by sorting, not by insertion order. + val (winner, loser) = + listOf( + signer.sign(MetadataEvent.createNew("Vitor A", createdAt = time)), + signer.sign(MetadataEvent.createNew("Vitor B", createdAt = time)), + ).sortedBy { it.id } + + assertEquals(IEventStore.InsertOutcome.Accepted, db.batchInsert(listOf(winner))[0]) + assertRejected(RejectionReason.SUPERSEDED, db.batchInsert(listOf(loser))[0]) + } + + @Test + fun olderAddressableIsRejectedAsSuperseded() = + forEachDB { db -> + val time = TimeUtils.now() + val older = signer.sign(LongTextNoteEvent.build("v1", "title", dTag = "blog", createdAt = time)) + val newer = signer.sign(LongTextNoteEvent.build("v2", "title", dTag = "blog", createdAt = time + 1)) + + assertEquals(IEventStore.InsertOutcome.Accepted, db.batchInsert(listOf(newer))[0]) + assertRejected(RejectionReason.SUPERSEDED, db.batchInsert(listOf(older))[0]) + } + + @Test + fun aNewerVersionStillLandsAtAnOccupiedCoordinate() = + forEachDB { db -> + // The guard against the classifier over-claiming: a coordinate is + // occupied here too, but this version wins, so nothing is rejected. + val time = TimeUtils.now() + val older = signer.sign(MetadataEvent.createNew("Vitor 1", createdAt = time)) + val newer = signer.sign(MetadataEvent.createNew("Vitor 2", createdAt = time + 1)) + + assertEquals(IEventStore.InsertOutcome.Accepted, db.batchInsert(listOf(older))[0]) + assertEquals(IEventStore.InsertOutcome.Accepted, db.batchInsert(listOf(newer))[0]) + } +}