From af5bc5cc3c49de7ad0ed19cdade4acd646c5af05 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 21:36:42 +0000 Subject: [PATCH 1/9] refactor(sqlite-shared): remove redundant code - Trim the barrels to what consumers import; everything else was re-exported but used only inside the package. - Drop the SortField/SORT_FIELDS aliases of core's NativeSortField and NATIVE_SORT_FIELDS. - record-logic: one recordNotFound(), currentVersion(), readForMutation() and hydrate() instead of repeated inline copies. - lock: acquireLock no longer checks existsSync before readLock(), which already treats a missing lock as absent. - query: compute the native sort field once. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DsiVnSgkrjGjpzpqaSr5xg --- .changeset/sqlite-shared-dedupe.md | 5 ++ packages/sqlite-shared/src/cursor.ts | 20 ++---- packages/sqlite-shared/src/index.ts | 2 +- packages/sqlite-shared/src/lock.ts | 2 +- packages/sqlite-shared/src/query.ts | 16 ++--- packages/sqlite-shared/src/record-logic.ts | 72 ++++++++++++--------- packages/sqlite-shared/src/record.ts | 30 +-------- packages/sqlite-shared/tests/cursor.test.ts | 11 +--- 8 files changed, 63 insertions(+), 95 deletions(-) create mode 100644 .changeset/sqlite-shared-dedupe.md diff --git a/.changeset/sqlite-shared-dedupe.md b/.changeset/sqlite-shared-dedupe.md new file mode 100644 index 00000000..5243c20c --- /dev/null +++ b/.changeset/sqlite-shared-dedupe.md @@ -0,0 +1,5 @@ +--- +'@haverstack/record-adapter-sqlite': patch +--- + +Remove redundant code from the bundled SQLite record logic: unused barrel exports, local aliases of core's native sort fields, and repeated not-found/version-read/row-hydration code. diff --git a/packages/sqlite-shared/src/cursor.ts b/packages/sqlite-shared/src/cursor.ts index db9de669..f7cebd76 100644 --- a/packages/sqlite-shared/src/cursor.ts +++ b/packages/sqlite-shared/src/cursor.ts @@ -11,16 +11,6 @@ import { NATIVE_SORT_FIELDS, StackBadRequestError } from '@haverstack/core'; import { contentSortEntry } from '@haverstack/core/adapter'; import type { StackRecord, NativeSortField, QuerySort, ScalarFieldKind } from '@haverstack/core'; -export type SortField = NativeSortField; - -/** - * Re-exported under this package's own name rather than re-spelled: the - * set a cursor's `native` field is narrowed against is the same set core - * narrows a wire request's sort field to, and a fourth column added there - * must not need finding here too. - */ -export const SORT_FIELDS: readonly SortField[] = NATIVE_SORT_FIELDS; - /** * Which partition of the ordering a cursor sits in: a native column or a * content field's number, text, or absent value. The three content cases @@ -31,7 +21,7 @@ export const SORT_FIELDS: readonly SortField[] = NATIVE_SORT_FIELDS; export type CursorKind = 'native' | 'num' | 'text' | 'absent'; export type DecodedCursor = - | { kind: 'native'; field: SortField; value: number; id: string } + | { kind: 'native'; field: NativeSortField; value: number; id: string } | { kind: 'num'; field: string; value: number; id: string } | { kind: 'text'; field: string; value: string; id: string } | { kind: 'absent'; field: string; id: string }; @@ -82,20 +72,20 @@ export const decodeCursor = (cursor: string): DecodedCursor => { throw new StackBadRequestError(`Invalid cursor: non-numeric sort value`); } if (k === 'native') { - if (!SORT_FIELDS.includes(f as SortField)) { + if (!NATIVE_SORT_FIELDS.includes(f as NativeSortField)) { throw new StackBadRequestError(`Invalid cursor: unknown sort field "${f}"`); } - return { kind: 'native', field: f as SortField, value: v, id: i }; + return { kind: 'native', field: f as NativeSortField, value: v, id: i }; } return { kind: 'num', field: f, value: v, id: i }; } throw malformed(); }; -export const getSortField = (query: { sort?: QuerySort }): SortField => +export const getSortField = (query: { sort?: QuerySort }): NativeSortField => query.sort?.field ?? 'createdAt'; -export const getSortColumn = (field: SortField): string => +export const getSortColumn = (field: NativeSortField): string => field === 'createdAt' ? 'created_at' : field === 'updatedAt' ? 'updated_at' : 'version'; /** diff --git a/packages/sqlite-shared/src/index.ts b/packages/sqlite-shared/src/index.ts index 90323da4..8ba970f9 100644 --- a/packages/sqlite-shared/src/index.ts +++ b/packages/sqlite-shared/src/index.ts @@ -8,4 +8,4 @@ export * from './record.js'; export { TOKENS_SCHEMA_SQL } from './schema.js'; export { acquireLock, releaseLock } from './lock.js'; -export { SharedTokenLogic, type SharedTokenLogicDeps } from './token-logic.js'; +export { SharedTokenLogic } from './token-logic.js'; diff --git a/packages/sqlite-shared/src/lock.ts b/packages/sqlite-shared/src/lock.ts index 4adebe25..1c32ce81 100644 --- a/packages/sqlite-shared/src/lock.ts +++ b/packages/sqlite-shared/src/lock.ts @@ -32,7 +32,7 @@ const isProcessAlive = (pid: number): boolean => { */ export const acquireLock = (dbPath: string, force?: boolean): void => { const lockPath = lockPathFor(dbPath); - const info = existsSync(lockPath) ? readLock(lockPath) : null; + const info = readLock(lockPath); if (info) { const ownedBySelf = info.pid === process.pid; if (!ownedBySelf && !force && isProcessAlive(info.pid)) { diff --git a/packages/sqlite-shared/src/query.ts b/packages/sqlite-shared/src/query.ts index 04e28a08..caa36ddc 100644 --- a/packages/sqlite-shared/src/query.ts +++ b/packages/sqlite-shared/src/query.ts @@ -15,13 +15,8 @@ import { StackBadRequestError, type StackQuery } from '@haverstack/core'; import { contentSortKey, parseContentFilterKey } from '@haverstack/core/adapter'; -import { - decodeCursor, - getSortColumn, - getSortField, - type DecodedCursor, - type SortField, -} from './cursor.js'; +import type { NativeSortField } from '@haverstack/core'; +import { decodeCursor, getSortColumn, getSortField, type DecodedCursor } from './cursor.js'; import { sanitizeFts5Query } from './fts5.js'; /** One statement plus the parameters it binds, in textual order. */ @@ -135,7 +130,7 @@ const cursorMismatch = (cursor: DecodedCursor, field: string): StackBadRequestEr `Cursor sort field "${cursor.field}" does not match query sort field "${field}"`, ); -const nativeCursor = (cursor: DecodedCursor, field: SortField) => { +const nativeCursor = (cursor: DecodedCursor, field: NativeSortField) => { if (cursor.kind !== 'native' || cursor.field !== field) throw cursorMismatch(cursor, field); return cursor; }; @@ -417,9 +412,10 @@ export const buildQueryPlan = (query: StackQuery): QueryStatement[] => { if (contentField === undefined) { const { conditions, params } = recordConditions(query); - const col = getSortColumn(getSortField(query)); + const field = getSortField(query); + const col = getSortColumn(field); if (decoded) { - const cursor = nativeCursor(decoded, getSortField(query)); + const cursor = nativeCursor(decoded, field); conditions.push(`(r.${col} ${op} ? OR (r.${col} = ? AND r.id ${op} ?))`); params.push(cursor.value, cursor.value, cursor.id); } diff --git a/packages/sqlite-shared/src/record-logic.ts b/packages/sqlite-shared/src/record-logic.ts index 6afbf334..e30d8ffc 100644 --- a/packages/sqlite-shared/src/record-logic.ts +++ b/packages/sqlite-shared/src/record-logic.ts @@ -90,6 +90,9 @@ const versionConflict = (id: string, expected: number, actual: number): StackVer actual, ); +const recordNotFound = (id: string): StackNotFoundError => + new StackNotFoundError(`Record not found: "${id}"`); + /** * Every `versions` column beyond the (record_id, version) key, and the * values for them in the same order — one list, so the INSERT that writes a @@ -174,11 +177,26 @@ export class SharedSqlRecordLogic { * the actual current version for the caller to act on). */ private throwVersionConflict(id: string, expectedVersion: number | undefined): never { - const row = this.exec.get<{ version: number }>('SELECT version FROM records WHERE id = ?', [ - id, - ]); - if (!row) throw new StackNotFoundError(`Record not found: "${id}"`); - throw versionConflict(id, expectedVersion as number, row.version); + const version = this.currentVersion(id); + if (version === undefined) throw recordNotFound(id); + throw versionConflict(id, expectedVersion as number, version); + } + + private currentVersion(id: string): number | undefined { + return this.exec.get<{ version: number }>('SELECT version FROM records WHERE id = ?', [id]) + ?.version; + } + + /** + * The read a content-rewriting mutation makes before its transaction, so + * the precondition settles before fts5Strategy.remove() runs — see + * checkExpectedVersion(). + */ + private async readForMutation(id: string, ifVersion: number | undefined): Promise { + const existing = await this.getRecord(id); + if (!existing) throw recordNotFound(id); + this.checkExpectedVersion(existing, ifVersion); + return existing; } /** @@ -312,9 +330,12 @@ export class SharedSqlRecordLogic { */ private readRecord(id: string): StackRecord | null { const row = this.exec.get>('SELECT * FROM records WHERE id = ?', [id]); - if (!row) return null; - const associations = this.getAssociationsForRecord(id); - return rowToRecord(row, associations); + return row ? this.hydrate(row) : null; + } + + /** A records row with its association set attached. */ + private hydrate(row: Record): StackRecord { + return rowToRecord(row, this.getAssociationsForRecord(row.id as string)); } /** @@ -344,9 +365,7 @@ export class SharedSqlRecordLogic { changes: RecordChangeSet, opts: MutateOptions & BumpVersionOptions = {}, ): Promise { - const existing = await this.getRecord(id); - if (!existing) throw new StackNotFoundError(`Record not found: "${id}"`); - this.checkExpectedVersion(existing, opts.ifVersion); + const existing = await this.readForMutation(id, opts.ifVersion); const now = toMs(new Date()); @@ -372,7 +391,7 @@ export class SharedSqlRecordLogic { if (opts.bumpsVersion === false) { this.exec.transaction(() => { const current = this.readRecord(id); - if (!current) throw new StackNotFoundError(`Record not found: "${id}"`); + if (!current) throw recordNotFound(id); this.checkExpectedVersion(current, opts.ifVersion); if (sets.length > 0) { this.exec.run(`UPDATE records SET ${sets.join(', ')} WHERE id = ?`, [...values, id]); @@ -445,7 +464,7 @@ export class SharedSqlRecordLogic { // A CAS against a record that isn't there is a failed precondition, // not the "nothing to delete" that an unconditional purge // reports by returning null. - if (ifVersion !== undefined) throw new StackNotFoundError(`Record not found: "${id}"`); + if (ifVersion !== undefined) throw recordNotFound(id); return null; } this.checkExpectedVersion(purged, ifVersion); @@ -476,9 +495,7 @@ export class SharedSqlRecordLogic { version: number, opts: MutateOptions = {}, ): Promise { - const existing = await this.getRecord(id); - if (!existing) throw new StackNotFoundError(`Record not found: "${id}"`); - this.checkExpectedVersion(existing, opts.ifVersion); + await this.readForMutation(id, opts.ifVersion); const target = await this.getVersion(id, version); if (!target) throw new StackNotFoundError(`Version not found: "${id}"@${version}`); @@ -507,9 +524,7 @@ export class SharedSqlRecordLogic { // work starts. rewriteContent()'s UPDATE still carries the guard, so a // writer slipping in between the two is caught rather than overwritten // — same shape as mutateRecord() and restoreVersion(). - const existing = await this.getRecord(id); - if (!existing) throw new StackNotFoundError(`Record not found: "${id}"`); - this.checkExpectedVersion(existing, opts.ifVersion); + await this.readForMutation(id, opts.ifVersion); this.exec.transaction(() => { if (opts.snapshot) this.snapshotBeforeMutation(id, opts.snapshot); @@ -568,10 +583,7 @@ export class SharedSqlRecordLogic { const hasMore = rows.length > limit; const page = hasMore ? rows.slice(0, limit) : rows; - const records = page.map((row) => { - const associations = this.getAssociationsForRecord(row.id as string); - return rowToRecord(row, associations); - }); + const records = page.map((row) => this.hydrate(row)); const lastRecord = records[records.length - 1]; const cursor = @@ -716,10 +728,7 @@ export class SharedSqlRecordLogic { } catch (err) { if (!(err instanceof StackConflictError)) throw err; - const row = this.exec.get<{ version: number }>('SELECT version FROM records WHERE id = ?', [ - id, - ]); - if (!row || row.version !== version.version) throw err; + if (this.currentVersion(id) !== version.version) throw err; this.overwriteVersionRow(id, version); } @@ -751,7 +760,7 @@ export class SharedSqlRecordLogic { type_id: string; parent_id: string | null; }>('SELECT version, type_id, parent_id FROM records WHERE id = ?', [recordId]); - if (!row) throw new StackNotFoundError(`Record not found: "${recordId}"`); + if (!row) throw recordNotFound(recordId); this.exec.run( `INSERT INTO journal @@ -783,7 +792,7 @@ export class SharedSqlRecordLogic { // that isn't there cannot be spelled that way. A purged record is gone, // and gets the same refusal. See docs/spec/journal.md § Reading it. if (!this.exec.get('SELECT 1 FROM records WHERE id = ?', [id])) { - throw new StackNotFoundError(`Record not found: "${id}"`); + throw recordNotFound(id); } const conditions = ['record_id = ?']; const values: unknown[] = [id]; @@ -853,8 +862,7 @@ export class SharedSqlRecordLogic { ): Promise { assertOneSurface(changes); this.exec.transaction(() => { - if (!this.readRecord(recordId)) - throw new StackNotFoundError(`Record not found: "${recordId}"`); + if (!this.readRecord(recordId)) throw recordNotFound(recordId); for (const change of changes) { if (change.op !== 'remove') continue; const { association } = change; @@ -946,7 +954,7 @@ export class SharedSqlRecordLogic { ); } catch (err) { if (isForeignKeyViolation(err)) { - throw new StackNotFoundError(`Record not found: "${recordId}"`); + throw recordNotFound(recordId); } throw err; } diff --git a/packages/sqlite-shared/src/record.ts b/packages/sqlite-shared/src/record.ts index b9f6d226..20c4056a 100644 --- a/packages/sqlite-shared/src/record.ts +++ b/packages/sqlite-shared/src/record.ts @@ -16,37 +16,13 @@ * for lock.ts's plain functions — so avoiding the import (not just the * unused export) has to happen at this file's level. */ -export { - RECORD_SCHEMA_SQL, - FTS5_SCHEMA_SQL, - PRAGMA_FOREIGN_KEYS_ON, - PRAGMA_JOURNAL_MODE_WAL, - applyRecordSchema, - type RecordSchemaOptions, -} from './schema.js'; -export { buildQueryPlan, atBudget, type QueryStatement } from './query.js'; -export { - encodeCursor, - decodeCursor, - makeCursor, - getSortField, - getSortColumn, - SORT_FIELDS, - type SortField, - type DecodedCursor, -} from './cursor.js'; -export { rowToRecord, rowToAssociation, rowToType, rowToVersion, toMs, fromMs } from './mappers.js'; -export { sanitizeFts5Query, fts5Strategy } from './fts5.js'; -export { - type SqlExecutor, - isForeignKeyViolation, - isUniqueConstraintViolation, -} from './executor.js'; +export { PRAGMA_JOURNAL_MODE_WAL, applyRecordSchema } from './schema.js'; +export { sanitizeFts5Query } from './fts5.js'; +export { type SqlExecutor } from './executor.js'; export { insertConfigRecord, readStackConfig, tryReadStackConfig, type StackConfig, } from './config.js'; -export { SharedSqlRecordLogic, type SharedSqlRecordLogicDeps } from './record-logic.js'; export { SharedSqlRecordAdapter, SQLITE_RECORD_CAPABILITIES } from './record-adapter.js'; diff --git a/packages/sqlite-shared/tests/cursor.test.ts b/packages/sqlite-shared/tests/cursor.test.ts index d2b9e269..b0cfde8e 100644 --- a/packages/sqlite-shared/tests/cursor.test.ts +++ b/packages/sqlite-shared/tests/cursor.test.ts @@ -1,6 +1,6 @@ import { describe, test, expect } from 'vitest'; import { NATIVE_SORT_FIELDS, StackBadRequestError } from '@haverstack/core'; -import { encodeCursor, decodeCursor, makeCursor, SORT_FIELDS } from '../src/cursor.js'; +import { encodeCursor, decodeCursor, makeCursor } from '../src/cursor.js'; import type { ScalarFieldKind, StackRecord } from '@haverstack/core'; const makeRecord = (overrides: Partial = {}): StackRecord => ({ @@ -121,14 +121,7 @@ describe('makeCursor', () => { }); }); -describe('SORT_FIELDS', () => { - // This package's name for the set, not a second copy of it: a cursor's - // `native` field is narrowed against exactly what core admits as a sort - // field, and a fourth column added there must not need finding here too. - test('is exactly the set core admits as a sort field', () => { - expect(SORT_FIELDS).toEqual([...NATIVE_SORT_FIELDS]); - }); - +describe('native sort fields', () => { test('round-trips a cursor on every field it names', () => { const record = makeRecord(); for (const field of NATIVE_SORT_FIELDS) { From d8ef10d10ca670f5a26323c369b1966556091a09 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 12:05:21 +0000 Subject: [PATCH 2/9] fix(sqlite-shared): keep NOT inside a search phrase The bare-NOT strip ran over the whole search string, so a NOT at the start of a phrase or after a "(" inside one was deleted from the text the user searched for. Phrases are now matched first and kept whole. Output outside phrases is unchanged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DsiVnSgkrjGjpzpqaSr5xg --- .changeset/fts5-not-in-phrase.md | 5 +++++ packages/sqlite-shared/src/fts5.ts | 7 +++++-- packages/sqlite-shared/tests/fts5.test.ts | 6 ++++++ 3 files changed, 16 insertions(+), 2 deletions(-) create mode 100644 .changeset/fts5-not-in-phrase.md diff --git a/.changeset/fts5-not-in-phrase.md b/.changeset/fts5-not-in-phrase.md new file mode 100644 index 00000000..4ecaec48 --- /dev/null +++ b/.changeset/fts5-not-in-phrase.md @@ -0,0 +1,5 @@ +--- +'@haverstack/record-adapter-sqlite': patch +--- + +`filter.search` no longer drops a `NOT` written inside a quoted phrase: `"cats (not dogs"` searches for the phrase as typed. diff --git a/packages/sqlite-shared/src/fts5.ts b/packages/sqlite-shared/src/fts5.ts index ccad28d7..3f0a0b1b 100644 --- a/packages/sqlite-shared/src/fts5.ts +++ b/packages/sqlite-shared/src/fts5.ts @@ -125,8 +125,11 @@ export const sanitizeFts5Query = (query: string, maxDepth = 2): string => { // this is where the long runs come from. clean = collapseWhitespace(clean); - // Strip bare NOT with no left operand — FTS5 requires "term NOT term", not "NOT term" - clean = clean.replace(/(?:^|\(\s*)NOT\s+/gi, (m) => m.replace(/NOT\s+/i, '')); + // Strip bare NOT with no left operand — FTS5 requires "term NOT term", not + // "NOT term". A phrase is matched first and kept whole, so a NOT inside one + // stays the word it is; the closing quote is optional because an odd one + // is only closed further down. + clean = clean.replace(/"[^"]*"?|(^|\(\s*)NOT\s+/gi, (m, lead?: string) => lead ?? m); // Enforce max nesting depth; replace excess ( with a space and discard // unmatched ). Parens inside a phrase are literal text to FTS5, so the diff --git a/packages/sqlite-shared/tests/fts5.test.ts b/packages/sqlite-shared/tests/fts5.test.ts index f8ca80b4..daae63f5 100644 --- a/packages/sqlite-shared/tests/fts5.test.ts +++ b/packages/sqlite-shared/tests/fts5.test.ts @@ -150,6 +150,12 @@ describe('sanitizeFts5Query', () => { expect(sanitizeFts5Query('"cats AND"')).toBe('"cats AND"'); }); + test('leaves a NOT inside a phrase alone, even after a paren', () => { + expect(sanitizeFts5Query('"NOT cats"')).toBe('"NOT cats"'); + expect(sanitizeFts5Query('"cats (not dogs"')).toBe('"cats (not dogs"'); + expect(sanitizeFts5Query('x "a (NOT b')).toBe('x "a (NOT b"'); + }); + // The mask a phrase leaves behind is an operand, so neither of these // operators is dangling. test('a phrase counts as an operand on either side', () => { From c427f435620c5766121723a54516f5a68df63561 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 12:09:47 +0000 Subject: [PATCH 3/9] feat(core): export the association authority partition isAuthorityAssociation joins the root entry point: an app undoing a journal entry has to route each edit to amendAssociations() or amendAccess(), and the spec's own example called a predicate no package exported. partitionAssociations joins @haverstack/core/adapter for engines that store both halves in one table. sqlite-shared and MemoryAdapter now use these instead of their own copies of the kind check. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DsiVnSgkrjGjpzpqaSr5xg --- .changeset/export-association-partition.md | 5 +++++ docs/spec/journal.md | 6 ++++-- packages/core/src/adapter/index.ts | 1 + packages/core/src/index.ts | 2 +- packages/core/src/testing.ts | 8 ++------ packages/sqlite-shared/src/mappers.ts | 22 +++------------------- 6 files changed, 16 insertions(+), 28 deletions(-) create mode 100644 .changeset/export-association-partition.md diff --git a/.changeset/export-association-partition.md b/.changeset/export-association-partition.md new file mode 100644 index 00000000..5017e2da --- /dev/null +++ b/.changeset/export-association-partition.md @@ -0,0 +1,5 @@ +--- +'@haverstack/core': minor +--- + +Export `isAuthorityAssociation` from `@haverstack/core`, for apps that split a journal entry's association changes between `amendAssociations()` and `amendAccess()`, and `partitionAssociations` from `@haverstack/core/adapter`, for storage engines that read both halves from one table. diff --git a/docs/spec/journal.md b/docs/spec/journal.md index b1437c49..2cbf61c3 100644 --- a/docs/spec/journal.md +++ b/docs/spec/journal.md @@ -48,14 +48,16 @@ type AssociationEdit = Exclude; Undoing one entry's association changes is at most two calls, one per half of [the partition](./access-control.md#storage-unifies-the-api-does-not), passing each element's inverse to `amendAssociations()` / `amendAccess()`: ```ts +import { isAuthorityAssociation } from '@haverstack/core'; + const inverse = (change: AssociationChange): AssociationEdit => change.op === 'add' ? { op: 'remove', association: change.association } : { op: 'add', association: change.op === 'repoint' ? change.previous : change.association }; const edits = (entry.associations ?? []).map(inverse); -const data = edits.filter((e) => !isAuthority(e.association)); -const authority = edits.filter((e) => isAuthority(e.association)); +const data = edits.filter((e) => !isAuthorityAssociation(e.association)); +const authority = edits.filter((e) => isAuthorityAssociation(e.association)); if (data.length) await stack.amendAssociations(recordId, data); if (authority.length) await stack.amendAccess(recordId, authority); ``` diff --git a/packages/core/src/adapter/index.ts b/packages/core/src/adapter/index.ts index 37320f70..aba0239b 100644 --- a/packages/core/src/adapter/index.ts +++ b/packages/core/src/adapter/index.ts @@ -33,3 +33,4 @@ export { } from '../query-validation.js'; export { contentSortEntry, contentSortKey, compareSortEntries } from './sort.js'; export type { SortEntry } from './sort.js'; +export { partitionAssociations } from '../associations/identity.js'; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 0333413f..c19e47d8 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -162,4 +162,4 @@ export { hashSchema, isCompatible } from './schema.js'; export type { SchemaDriftViolation } from './schema.js'; export type { ValidationError } from './validate.js'; export { applyMergePatch } from './merge.js'; -export { assertOneSurface } from './associations/identity.js'; +export { assertOneSurface, isAuthorityAssociation } from './associations/identity.js'; diff --git a/packages/core/src/testing.ts b/packages/core/src/testing.ts index 9d0fdf5c..4028f9f9 100644 --- a/packages/core/src/testing.ts +++ b/packages/core/src/testing.ts @@ -52,6 +52,7 @@ import { applyAssociationEdits, assertOneSurface, associationEqual, + partitionAssociations, } from './associations/identity.js'; /** Core resolves the default direction; an adapter only ever sees an explicit one. */ @@ -712,10 +713,6 @@ export class IncapableMemoryAdapter extends MemoryAdapter { } } -/** Whether an association carries authority — mirrors core's own predicate. */ -const isAuthority = (a: Association): a is AuthorityAssociation => - a.kind === 'permission' || a.kind === 'anyone'; - /** * A record's whole association table, both projections rejoined — what * amendAssociations() acts on, since storage keys all kinds alike. @@ -739,8 +736,7 @@ function withAssociationSet(record: StackRecord, associations: Association[]): S [], ); const { associations: _dropA, permissions: _dropP, ...rest } = record; - const data = keyed.filter((a): a is DataAssociation => !isAuthority(a)); - const authority = keyed.filter(isAuthority); + const { associations: data, permissions: authority } = partitionAssociations(keyed); return { ...(rest as StackRecord), ...(data.length && { associations: data }), diff --git a/packages/sqlite-shared/src/mappers.ts b/packages/sqlite-shared/src/mappers.ts index 44fa6ea0..a922999b 100644 --- a/packages/sqlite-shared/src/mappers.ts +++ b/packages/sqlite-shared/src/mappers.ts @@ -6,8 +6,6 @@ import type { Actor, - AuthorityAssociation, - DataAssociation, GroupRole, Grantee, RecordJournalEntry, @@ -17,6 +15,7 @@ import type { Association, RelationshipTarget, } from '@haverstack/core'; +import { partitionAssociations } from '@haverstack/core/adapter'; export const toMs = (d: Date): number => d.getTime(); @@ -33,23 +32,6 @@ const actorFromColumns = (subject: unknown, principal: unknown): Actor | undefin }; export const fromMs = (ms: number): Date => new Date(ms); -/** - * A record's stored edges, split into the two fields it presents. - * Partitioned by kind rather than stored apart: one table, one delta, one - * durability tier, and an app editing tags never sees the authority half. - * See docs/spec/access-control.md § Record-level permissions. - */ -export const partitionAssociations = ( - associations: Association[], -): { associations: DataAssociation[]; permissions: AuthorityAssociation[] } => ({ - associations: associations.filter( - (a): a is DataAssociation => a.kind !== 'permission' && a.kind !== 'anyone', - ), - permissions: associations.filter( - (a): a is AuthorityAssociation => a.kind === 'permission' || a.kind === 'anyone', - ), -}); - export const rowToRecord = ( row: Record, associations: Association[], @@ -75,6 +57,8 @@ export const rowToRecord = ( if (updatedBy) record.updatedBy = updatedBy; if (row.deleted_at != null) record.deletedAt = fromMs(row.deleted_at as number); if (row.unlisted_at != null) record.unlistedAt = fromMs(row.unlisted_at as number); + // One table, partitioned on read. See docs/spec/access-control.md + // § Record-level permissions. const partitioned = partitionAssociations(associations); if (partitioned.associations.length) record.associations = partitioned.associations; if (partitioned.permissions.length) record.permissions = partitioned.permissions; From 2d9da432994d7cbf7c9cb09057d8b88a8636b655 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 12:18:40 +0000 Subject: [PATCH 4/9] refactor(sqlite-shared): move the derived indexes into indexes.ts record-logic.ts carried the content_index subsystem (its per-type field cache, sync, and kind lookup) inline, and fts5.ts mixed the search-text sanitizer with the FTS5 write strategy. Both indexes are rebuildable from records and are maintained the same way, so they now live together in indexes.ts: fts5Strategy unchanged, and a ContentIndex class that owns the cache and every content_index write. fts5.ts is now only the sanitizer. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DsiVnSgkrjGjpzpqaSr5xg --- .changeset/sqlite-shared-indexes.md | 5 + .../tests/record.test.ts | 2 +- packages/sqlite-shared/src/fts5.ts | 41 +----- packages/sqlite-shared/src/indexes.ts | 132 ++++++++++++++++++ packages/sqlite-shared/src/query.ts | 2 +- packages/sqlite-shared/src/record-logic.ts | 105 ++------------ packages/sqlite-shared/src/schema.ts | 2 +- 7 files changed, 155 insertions(+), 134 deletions(-) create mode 100644 .changeset/sqlite-shared-indexes.md create mode 100644 packages/sqlite-shared/src/indexes.ts diff --git a/.changeset/sqlite-shared-indexes.md b/.changeset/sqlite-shared-indexes.md new file mode 100644 index 00000000..736755e1 --- /dev/null +++ b/.changeset/sqlite-shared-indexes.md @@ -0,0 +1,5 @@ +--- +'@haverstack/record-adapter-sqlite': patch +--- + +Internal restructure of the bundled SQLite record logic: the full-text and content indexes move into their own module. No behavior change. diff --git a/packages/record-adapter-sqlite/tests/record.test.ts b/packages/record-adapter-sqlite/tests/record.test.ts index 6356c101..94bc751c 100644 --- a/packages/record-adapter-sqlite/tests/record.test.ts +++ b/packages/record-adapter-sqlite/tests/record.test.ts @@ -1396,7 +1396,7 @@ describe('file-ref indexing', () => { }); // The fields content_index covers are cached per typeId (keyed off - // saveType()) so that syncContentIndex() doesn't re-query and re-parse the + // saveType()) so that ContentIndex.sync() doesn't re-query and re-parse the // schema on every write — this pins that redefining a type reaches the // cache. test('redefining a type via saveType updates which fields are treated as file-ref', async () => { diff --git a/packages/sqlite-shared/src/fts5.ts b/packages/sqlite-shared/src/fts5.ts index 3f0a0b1b..aed5e0fa 100644 --- a/packages/sqlite-shared/src/fts5.ts +++ b/packages/sqlite-shared/src/fts5.ts @@ -1,6 +1,6 @@ /** - * FTS5 query sanitization and indexing — the full-text search half of the - * shared record logic. + * FTS5 query sanitization: turning what a person typed into a search box + * into a query FTS5 will parse. Indexing lives in indexes.ts. * * FTS5's query language supports operators (AND/OR/NOT, phrases, NEAR, * wildcards) that can be expensive or cause parse errors with untrusted @@ -42,8 +42,6 @@ * expectation and the `timeout` wire error that goes with it. */ -import type { SqlExecutor } from './executor.js'; - /** * Outside a phrase, everything but a letter, digit, mark, underscore, * whitespace, paren or quote becomes a separator. @@ -287,38 +285,3 @@ const restoreImplicitAnd = (query: string): string => (match, word: string) => (isOperator(word) ? match : `${word} AND `), ), ); - -// ------------------------------------------------------- -// Indexing strategy -// ------------------------------------------------------- - -/** - * FTS5's external-content table needs its special `('delete', rowid, - * content)` command — with the *old* content — to unindex a row; a plain - * DELETE leaves stale, still-searchable entries. remove() therefore reads - * the current rowid/content and must run before the caller's change. - */ -export const fts5Strategy = { - /** Index a record that has no FTS entry yet (used right after an INSERT). */ - insert(exec: SqlExecutor, recordId: string, content: string): void { - exec.run(`INSERT INTO records_fts(rowid, content) SELECT rowid, ? FROM records WHERE id = ?`, [ - content, - recordId, - ]); - }, - /** - * Remove a record's FTS entry. MUST be called before the records row's - * content changes or the row is deleted — see the note above. - */ - remove(exec: SqlExecutor, recordId: string): void { - const row = exec.get<{ rowid: number; content: string }>( - 'SELECT rowid, content FROM records WHERE id = ?', - [recordId], - ); - if (!row) return; - exec.run(`INSERT INTO records_fts(records_fts, rowid, content) VALUES('delete', ?, ?)`, [ - row.rowid, - row.content, - ]); - }, -}; diff --git a/packages/sqlite-shared/src/indexes.ts b/packages/sqlite-shared/src/indexes.ts new file mode 100644 index 00000000..5d534db5 --- /dev/null +++ b/packages/sqlite-shared/src/indexes.ts @@ -0,0 +1,132 @@ +/** + * The two derived indexes a records database keeps beside its rows: FTS5 + * for `filter.search`, and content_index for `sort.contentField` and the + * file-reference filters. Both can be rebuilt from `records` alone, and + * both must be rewritten in the same transaction as any write that changes + * what they cover, or a query answers from a record that no longer exists. + */ + +import type { StackType, ScalarFieldKind } from '@haverstack/core'; +import { contentSortEntry } from '@haverstack/core/adapter'; +import type { SqlExecutor } from './executor.js'; + +/** + * FTS5's external-content table needs its special `('delete', rowid, + * content)` command — with the *old* content — to unindex a row; a plain + * DELETE leaves stale, still-searchable entries. remove() therefore reads + * the current rowid/content and must run before the caller's change. + */ +export const fts5Strategy = { + /** Index a record that has no FTS entry yet (used right after an INSERT). */ + insert(exec: SqlExecutor, recordId: string, content: string): void { + exec.run(`INSERT INTO records_fts(rowid, content) SELECT rowid, ? FROM records WHERE id = ?`, [ + content, + recordId, + ]); + }, + /** + * Remove a record's FTS entry. MUST be called before the records row's + * content changes or the row is deleted — see the note above. + */ + remove(exec: SqlExecutor, recordId: string): void { + const row = exec.get<{ rowid: number; content: string }>( + 'SELECT rowid, content FROM records WHERE id = ?', + [recordId], + ); + if (!row) return; + exec.run(`INSERT INTO records_fts(records_fts, rowid, content) VALUES('delete', ?, ?)`, [ + row.rowid, + row.content, + ]); + }, +}; + +/** + * Top-level scalar fields, by name and declared kind — what content_index + * covers. Arrays and objects are left out: a value inside one has no + * single position to order its record by, and no file reference the + * attachment filter can see. + */ +const indexedFieldKinds = ( + schema: Record, +): Map => { + const kinds = new Map(); + for (const [field, def] of Object.entries(schema)) { + if (!def.kind || def.kind === 'array' || def.kind === 'object') continue; + kinds.set(field, def.kind as ScalarFieldKind); + } + return kinds; +}; + +/** + * One row per top-level scalar field a record holds a value at. Caches each + * type's indexed fields so a write doesn't re-read and re-parse the schema. + */ +export class ContentIndex { + private readonly fieldsByType = new Map>(); + + constructor(private readonly exec: SqlExecutor) {} + + /** Prime the cache from a type as it is saved. */ + rememberType(type: StackType): void { + this.fieldsByType.set( + type.id, + indexedFieldKinds(type.schema as Record), + ); + } + + /** + * Replace a record's rows with what its content currently holds, on every + * write that can change content or typeId, so neither the ordering nor the + * file-reference lookup can drift from the record. A field holding no + * orderable value gets no row at all, which is what puts the record at the + * end of a sort on that field. + */ + sync(recordId: string, typeId: string, content: Record): void { + this.remove(recordId); + + for (const [field, kind] of this.fieldsOf(typeId)) { + const entry = contentSortEntry(kind, content[field]); + if (!entry) continue; + const sql = `INSERT OR IGNORE INTO content_index + (record_id, field, value_rank, num_value, text_key, text_value, file_id) + VALUES (?, ?, ?, ?, ?, ?, ?)`; + if (entry.kind === 'num') { + this.exec.run(sql, [recordId, field, 0, entry.num, null, null, null]); + } else { + // A file-ref field's value is its file id, so the row it already + // needs for ordering also answers the attachment filter — one + // field is one row, whatever a query reads it for. + const fileId = kind === 'file-ref' ? entry.text : null; + this.exec.run(sql, [recordId, field, 1, null, entry.key, entry.text, fileId]); + } + } + } + + remove(recordId: string): void { + this.exec.run('DELETE FROM content_index WHERE record_id = ?', [recordId]); + } + + /** The declared kind of a type's top-level `field`, if it has one. */ + kindOf(typeId: string, field: string): ScalarFieldKind | undefined { + return this.fieldsOf(typeId).get(field); + } + + /** + * From the cache, falling back to a `types` lookup only for a typeId this + * process hasn't seen via rememberType() yet, e.g. right after open(). + */ + private fieldsOf(typeId: string): Map { + const cached = this.fieldsByType.get(typeId); + if (cached) return cached; + + const typeRow = this.exec.get<{ schema: string }>('SELECT schema FROM types WHERE id = ?', [ + typeId, + ]); + const fields = typeRow + ? indexedFieldKinds(JSON.parse(typeRow.schema) as Record) + : new Map(); + this.fieldsByType.set(typeId, fields); + return fields; + } +} diff --git a/packages/sqlite-shared/src/query.ts b/packages/sqlite-shared/src/query.ts index caa36ddc..6727bb8b 100644 --- a/packages/sqlite-shared/src/query.ts +++ b/packages/sqlite-shared/src/query.ts @@ -2,7 +2,7 @@ * StackQuery -> SQL. Engine-independent — shared verbatim between * SQLite-backed record adapters. The one caveat is full-text search: * `f.search` here assumes a `records_fts` virtual table with a MATCH-able - * `content` column (see fts5.ts). + * `content` column (see FTS5_SCHEMA_SQL in schema.ts). * * A query compiles to a plan rather than to a single statement, because a * content-field sort is two orderings end to end: the records that hold a diff --git a/packages/sqlite-shared/src/record-logic.ts b/packages/sqlite-shared/src/record-logic.ts index e30d8ffc..ae5d8fdf 100644 --- a/packages/sqlite-shared/src/record-logic.ts +++ b/packages/sqlite-shared/src/record-logic.ts @@ -32,7 +32,6 @@ import type { AssociationEdit, ActorOptions, IfVersionOptions, - ScalarFieldKind, } from '@haverstack/core'; import type { BumpVersionOptions, @@ -40,11 +39,10 @@ import type { JournalOptions, MutateOptions, } from '@haverstack/core/adapter'; -import { contentSortEntry } from '@haverstack/core/adapter'; import type { SqlExecutor } from './executor.js'; import { isForeignKeyViolation, isUniqueConstraintViolation } from './executor.js'; import { buildQueryPlan, atBudget } from './query.js'; -import { fts5Strategy } from './fts5.js'; +import { ContentIndex, fts5Strategy } from './indexes.js'; import { rowToRecord, rowToAssociation, @@ -123,33 +121,12 @@ export type SharedSqlRecordLogicDeps = { exec: SqlExecutor; }; -/** - * Top-level scalar fields, by name and declared kind — what content_index - * covers. Arrays and objects are left out: a value inside one has no - * single position to order its record by, and no file reference the - * attachment filter can see. - */ -const indexedFieldKinds = ( - schema: Record, -): Map => { - const kinds = new Map(); - for (const [field, def] of Object.entries(schema)) { - if (!def.kind || def.kind === 'array' || def.kind === 'object') continue; - kinds.set(field, def.kind as ScalarFieldKind); - } - return kinds; -}; - export class SharedSqlRecordLogic { - /** - * Per-typeId cache of the fields content_index covers and their declared - * kinds, so syncContentIndex() doesn't hit the `types` table and - * re-parse schema JSON on every write. Populated eagerly in saveType() - * and lazily via getIndexedFields(). - */ - private readonly indexedFieldsByType = new Map>(); + private readonly contentIndex: ContentIndex; - constructor(private readonly deps: SharedSqlRecordLogicDeps) {} + constructor(private readonly deps: SharedSqlRecordLogicDeps) { + this.contentIndex = new ContentIndex(deps.exec); + } private get exec(): SqlExecutor { return this.deps.exec; @@ -303,7 +280,7 @@ export class SharedSqlRecordLogic { if (edges.length) this.insertAssociations(record.id, edges); fts5Strategy.insert(this.exec, record.id, JSON.stringify(record.content)); - this.syncContentIndex(record.id, record.typeId, record.content); + this.contentIndex.sync(record.id, record.typeId, record.content); this.appendJournal(record.id, opts.journal); }); @@ -420,7 +397,7 @@ export class SharedSqlRecordLogic { if (merged !== undefined) { fts5Strategy.insert(this.exec, id, JSON.stringify(merged)); - this.syncContentIndex(id, existing.typeId, merged); + this.contentIndex.sync(id, existing.typeId, merged); } this.appendJournal(id, opts.journal); }); @@ -475,7 +452,7 @@ export class SharedSqlRecordLogic { // version history: a log naming what a destroyed record once held is // exactly the residue this verb exists to leave nothing of. this.exec.run('DELETE FROM journal WHERE record_id = ?', [id]); - this.exec.run('DELETE FROM content_index WHERE record_id = ?', [id]); + this.contentIndex.remove(id); this.exec.run('DELETE FROM records WHERE id = ?', [id]); return purged; } @@ -555,7 +532,7 @@ export class SharedSqlRecordLogic { fts5Strategy.remove(this.exec, id); this.versionedUpdate(id, ['type_id = ?', 'content = ?'], [typeId, json], opts); fts5Strategy.insert(this.exec, id, json); - this.syncContentIndex(id, typeId, content); + this.contentIndex.sync(id, typeId, content); } async queryRecords(query: StackQuery): Promise { @@ -588,7 +565,9 @@ export class SharedSqlRecordLogic { const lastRecord = records[records.length - 1]; const cursor = hasMore && lastRecord - ? makeCursor(lastRecord, query.sort, (record, field) => this.sortFieldKind(record, field)) + ? makeCursor(lastRecord, query.sort, (record, field) => + this.contentIndex.kindOf(record.typeId, field), + ) : null; return { records, cursor }; @@ -829,8 +808,7 @@ export class SharedSqlRecordLogic { toMs(type.createdAt), ], ); - const schema = type.schema as Record; - this.indexedFieldsByType.set(type.id, indexedFieldKinds(schema)); + this.contentIndex.rememberType(type); } async getType(id: TypeId): Promise { @@ -968,61 +946,4 @@ export class SharedSqlRecordLogic { ); return rows.map(rowToAssociation); } - - /** - * The fields content_index covers for typeId, from the in-memory cache - * — falling back to a `types` lookup (and populating the cache) only for - * a typeId this process hasn't seen via saveType() yet, e.g. right after - * open(). - */ - private getIndexedFields(typeId: string): Map { - const cached = this.indexedFieldsByType.get(typeId); - if (cached) return cached; - - const typeRow = this.exec.get<{ schema: string }>('SELECT schema FROM types WHERE id = ?', [ - typeId, - ]); - const fields = typeRow - ? indexedFieldKinds(JSON.parse(typeRow.schema) as Record) - : new Map(); - this.indexedFieldsByType.set(typeId, fields); - return fields; - } - - /** - * Replace a record's content_index rows with what its content currently - * holds, on every write that can change content or typeId, so neither - * the ordering nor the file-reference lookup can drift from the record. - * A field holding no orderable value gets no row at all, which is what - * puts the record at the end of a sort on that field. - */ - private syncContentIndex( - recordId: string, - typeId: string, - content: Record, - ): void { - this.exec.run('DELETE FROM content_index WHERE record_id = ?', [recordId]); - - for (const [field, kind] of this.getIndexedFields(typeId)) { - const entry = contentSortEntry(kind, content[field]); - if (!entry) continue; - const sql = `INSERT OR IGNORE INTO content_index - (record_id, field, value_rank, num_value, text_key, text_value, file_id) - VALUES (?, ?, ?, ?, ?, ?, ?)`; - if (entry.kind === 'num') { - this.exec.run(sql, [recordId, field, 0, entry.num, null, null, null]); - } else { - // A file-ref field's value is its file id, so the row it already - // needs for ordering also answers the attachment filter — one - // field is one row, whatever a query reads it for. - const fileId = kind === 'file-ref' ? entry.text : null; - this.exec.run(sql, [recordId, field, 1, null, entry.key, entry.text, fileId]); - } - } - } - - /** The declared kind of a record's top-level `field`, if it has one. */ - private sortFieldKind(record: StackRecord, field: string): ScalarFieldKind | undefined { - return this.getIndexedFields(record.typeId).get(field); - } } diff --git a/packages/sqlite-shared/src/schema.ts b/packages/sqlite-shared/src/schema.ts index e9af1338..fb54b727 100644 --- a/packages/sqlite-shared/src/schema.ts +++ b/packages/sqlite-shared/src/schema.ts @@ -192,7 +192,7 @@ export const TOKENS_SCHEMA_SQL = ` `; /** - * The full-text index behind `StackQuery.filter.search` (see fts5.ts). + * The full-text index behind `StackQuery.filter.search` (see indexes.ts). * What `columnsize=0` costs a reader is in docs/spec/adapters.md * § Adapter backends. `detail` stays at its default: sanitizeFts5Query * passes phrase queries through, and a phrase needs the token positions From cc64114b6bd51febea30d00d051a6687b78dd171 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 12:48:40 +0000 Subject: [PATCH 5/9] docs(sqlite-shared): bring code comments in line with AGENTS.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every comment block in the package is now at most five lines, outside file-top module comments. Where the reasoning is spec material the comment links to the spec instead; the sanitizer's regex-backtracking rationale, which is not, moves into fts5.ts's module comment. Also removes comments describing earlier implementations (a test's before/after timings, purgeRecord's reference to a write() call), fixes a doc comment attached to the wrong declaration in fts5.ts, and corrects a version-conflict comment that miscounted its call sites. Spec: the SQLite-specific prose in adapters.md sat under § Conformance; it now has its own § SQLite-backed adapters heading, and the two references that meant it point there. data-model.md's allow-list paragraph no longer narrates the strip-list it replaced. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DsiVnSgkrjGjpzpqaSr5xg --- docs/spec/adapters.md | 2 + docs/spec/data-model.md | 2 +- docs/spec/events.md | 2 +- packages/sqlite-shared/src/executor.ts | 12 +- packages/sqlite-shared/src/fts5.ts | 150 ++++++---------- packages/sqlite-shared/src/lock.ts | 11 +- packages/sqlite-shared/src/query.ts | 42 ++--- packages/sqlite-shared/src/record-logic.ts | 198 ++++++--------------- packages/sqlite-shared/src/schema.ts | 53 +++--- packages/sqlite-shared/src/token-logic.ts | 2 +- packages/sqlite-shared/tests/fts5.test.ts | 11 +- 11 files changed, 162 insertions(+), 323 deletions(-) diff --git a/docs/spec/adapters.md b/docs/spec/adapters.md index bfdc267a..cc9dbfad 100644 --- a/docs/spec/adapters.md +++ b/docs/spec/adapters.md @@ -117,6 +117,8 @@ An adapter that holds a stack's identity has exactly one entry point, an async s The invariants above are easy to state in prose and easy to violate silently — FTS5's external-content index going stale, a content path matching an unfiltered superset, a cursor replayed under the wrong sort. **`@haverstack/adapter-conformance`** is the runnable form of this document: `runRecordAdapterConformance()` and `runBlobAdapterConformance()` take an `open()` for your adapter and its declared `capabilities`, and register a vitest suite checking exactly the invariants named here, gated so a capability you don't declare is never penalized for its absence. `record-adapter-sqlite` and `blob-adapter-disk` both run it against themselves (`tests/conformance.test.ts`), alongside `@haverstack/core`'s own `MemoryAdapter` — three independent implementations passing the same suite is the check that the suite describes the contract rather than one adapter's accidents. +## SQLite-backed adapters + **`@haverstack/sqlite-shared`** is an internal, non-public package holding everything a SQLite-backed record adapter needs that isn't specific to one binding — schema DDL and the function that applies it, `WHERE`/`ORDER` building, the cursor codec, row mappers, the FTS5 sanitizer and indexing strategy, the storage-ownership lock, the `_config` record's reader and writer, the capability declaration every engine running this query builder makes, and (via a small `SqlExecutor` interface normalizing a binding's call convention) the actual CRUD/query/version/type/association/token logic itself. `SharedSqlRecordAdapter` is where the last of those meet: an abstract base implementing all of `StackRecordAdapter` by forwarding to that logic, so an adapter supplies only what's genuinely engine-specific — constructing the database handle, transaction semantics, and its own lifecycle (`flush`/`close`, the two members the base leaves abstract). `record-adapter-sqlite` and `record-adapter-do-sqlite` both consume it; the split exists so a second (and third) SQLite engine inherits the behavior rather than reimplementing it, and so a cursor minted by one is decodable by another. **It exposes two entry points.** The full barrel (`.`) includes the token-store logic (`SharedTokenLogic`, `TOKENS_SCHEMA_SQL`) and the file-lock helpers, both Node-specific (`node:crypto`, `node:fs`) — fine for `record-adapter-sqlite`, but a bare `import` of either survives tree-shaking as a dead-but-still-imported module in a bundle, which throws at load time in a Workers runtime without `nodejs_compat`. A record-only consumer with no token store and no lock file — `record-adapter-do-sqlite`, where the platform's single-writer-per-id model already is the lock — imports the `./record` subpath instead, which never reaches either. diff --git a/docs/spec/data-model.md b/docs/spec/data-model.md index 96dc5b6d..ac569eaa 100644 --- a/docs/spec/data-model.md +++ b/docs/spec/data-model.md @@ -677,7 +677,7 @@ A `search` that sanitizes to nothing (a bare `*`, punctuation-only input) is tre **Search text is repaired, not rejected.** `filter.search` is the one filter carrying a query language, and it holds what a person typed into a box — where an unbalanced quote (`5" nails`), a trailing operator (`cats AND`) or a leading one is ordinary input on the way to a longer query, not a malformed request. The FTS sanitizers close an odd trailing quote, drop operators left without an operand, and reduce everything outside a phrase to letters, digits, marks, whitespace, parens and quotes — so the search runs against the terms actually present rather than failing. Text inside a phrase is left alone: `"cats AND dogs"` is literal to the engine, and rewriting inside it would change what was asked for. -**The reduction is an allow-list, not a list of characters to strip.** FTS5's column-filter syntax is wider than `colname:term` — `-name` and `{a b}` filter columns with no colon in them — so a strip-list written against the colon form still let `-cats` and even the hyphen in `cats-dogs` reach the engine as column names, answered with `no such column`. Naming what may pass means the next piece of syntax nobody anticipated arrives as a separator rather than as an error, and nothing is lost: the default tokenizer already splits terms on those characters, so `cats-dogs` searches for the two tokens it was indexed as. Two grammar rules the same reduction has to respect: a group needs an explicit operator beside it (`cats (dogs)` is an error where `cats AND (dogs)` is not, so the implied `AND` is written back in), and a control character ends the string early for SQLite's C API, so control characters are dropped inside a phrase as well as outside. +**The reduction is an allow-list, not a list of characters to strip.** FTS5's column-filter syntax is wider than `colname:term` — `-name` and `{a b}` filter columns with no colon in them — so a strip-list written against the colon form would let `-cats` and even the hyphen in `cats-dogs` reach the engine as column names, answered with `no such column`. Naming what may pass means the next piece of syntax nobody anticipated arrives as a separator rather than as an error, and nothing is lost: the default tokenizer already splits terms on those characters, so `cats-dogs` searches for the two tokens it was indexed as. Two grammar rules the same reduction has to respect: a group needs an explicit operator beside it (`cats (dogs)` is an error where `cats AND (dogs)` is not, so the implied `AND` is written back in), and a control character ends the string early for SQLite's C API, so control characters are dropped inside a phrase as well as outside. The repair claims no completeness against the engine's grammar, so adapters back it with a rule that does: **whatever reaches the engine as search text and fails to parse surfaces as `StackBadRequestError`** (`bad_request`/400), never as a raw engine error. Membership in the [error taxonomy](./wire-format.md#the-taxonomy-root) is what gives a server a status to answer with — an error outside it has no mapping, and ordinary search input would land as a 500. The rule is scoped to `filter.search` deliberately: every other clause is built from bound parameters, so a parse failure in one is the adapter's own bug and must not be relabelled as the caller's. diff --git a/docs/spec/events.md b/docs/spec/events.md index 41dd18a3..cedbd889 100644 --- a/docs/spec/events.md +++ b/docs/spec/events.md @@ -159,7 +159,7 @@ There is no `previous`. Prior state is already a first-class, addressable thing: **An `async` handler is permitted; it is simply not awaited.** The handler type is `(change: RecordChange) => void`, and a `void` return means the value is ignored, not that the function must be synchronous. Passing an `async` function is the normal way to defer work: it runs to its first `await`, yields, and the emitter moves on. What it does not buy is ordering or completion. -**What "never delays a write" does and does not promise.** It is a guarantee about the emitter: no handler's returned promise is awaited, so a subscriber cannot extend a write by deferring work. It is not a guarantee against a handler's _synchronous_ body. Handlers run inline on the caller's thread, and a record adapter backed by `node:sqlite` has just run its write synchronously on that same thread ([Adapters § Adapter backends](./adapters.md#adapter-backends)), so a handler doing heavy synchronous work extends that occupied window. Handlers should return promptly and defer anything substantial. +**What "never delays a write" does and does not promise.** It is a guarantee about the emitter: no handler's returned promise is awaited, so a subscriber cannot extend a write by deferring work. It is not a guarantee against a handler's _synchronous_ body. Handlers run inline on the caller's thread, and a record adapter backed by `node:sqlite` has just run its write synchronously on that same thread ([Adapters § SQLite-backed adapters](./adapters.md#sqlite-backed-adapters)), so a handler doing heavy synchronous work extends that occupied window. Handlers should return promptly and defer anything substantial. An indexer that must not miss a write does not get that from a blocking hook — it gets it from a resumable cursor and idempotent application. A blocking hook buys a world where every subscriber is a latency and failure dependency of every write. diff --git a/packages/sqlite-shared/src/executor.ts b/packages/sqlite-shared/src/executor.ts index 3e20f396..7c0a7481 100644 --- a/packages/sqlite-shared/src/executor.ts +++ b/packages/sqlite-shared/src/executor.ts @@ -20,14 +20,10 @@ export interface SqlExecutor { /** Run a parameterized statement and return all result rows. */ all>(sql: string, params?: readonly unknown[]): T[]; /** - * Run `fn` atomically: every statement it issues commits together, or - * none do if it throws. `fn` must be synchronous and call back into - * this executor only — not every engine's transaction primitive can - * straddle a suspended call. A binding with a real multi-statement - * transaction (BEGIN/COMMIT/ROLLBACK) implements this with one; a - * binding whose transaction primitive is itself a callback wrapper - * (e.g. a Durable Object's storage.transactionSync) can pass `fn` - * straight through to it. + * Run `fn` atomically: every statement it issues commits, or none do if it + * throws. A callback rather than BEGIN/COMMIT text because some engines + * only offer one (a Durable Object's storage.transactionSync); for the + * same reason `fn` must be synchronous and use only this executor. */ transaction(fn: () => T): T; } diff --git a/packages/sqlite-shared/src/fts5.ts b/packages/sqlite-shared/src/fts5.ts index aed5e0fa..b89f688a 100644 --- a/packages/sqlite-shared/src/fts5.ts +++ b/packages/sqlite-shared/src/fts5.ts @@ -40,42 +40,41 @@ * Bounding execution time belongs to whoever drives the engine under load * — see docs/spec/wire-format.md § Bounding query cost for the server-side * expectation and the `timeout` wire error that goes with it. + * + * Backtracking. The input is untrusted text, so every regex here is + * written not to backtrack quadratically on it: + * + * - Whitespace runs are collapsed before any rule that spells "beside an + * operator or paren" as `\s*`; a `\s*` facing a long run re-tries the + * whole run per starting position. The rewrite is still a little + * worse than linear overall, so bounding a search's length belongs to + * whoever accepts it. + * - The NEAR rule has no `\s*` after its paren (its `[^,()]*` already + * takes a space, and two quantifiers matching the same character give + * the engine an ambiguous split), and neither of its runs crosses a + * `(`, so an unclosed `NEAR(NEAR(NEAR(` doesn't re-scan to the end once + * per occurrence. + * - The no-right-operand rule has no leading `\s*`, for the same reason + * as the collapse. + * - The operand-before-paren rule's lookbehind pins each match to the + * start of a word; without it every position inside a long word is a + * start, each re-scanning the rest of it. + * - Empty phrases and empty paren pairs are removed by one rule, so a run + * of them becomes one space rather than one per construct. */ /** * Outside a phrase, everything but a letter, digit, mark, underscore, - * whitespace, paren or quote becomes a separator. - * - * An allow-list rather than a list of metacharacters to strip, because - * FTS5's column-filter syntax is wider than it looks and an enumeration - * kept missing parts of it: `-name` and `{a b}` filter columns with no - * colon in sight, so `-cats` and even the hyphen inside `cats-dogs` were - * read as column names and answered "no such column". `+` is a bare - * syntax error. Listing what may pass means the next piece of syntax - * nobody here anticipated arrives as a separator instead of as an error. - * - * Nothing is lost by it: FTS5's default tokenizer already splits terms on - * these characters, so `cats-dogs` searches for the same two tokens it was - * indexed as. + * whitespace, paren or quote becomes a separator. An allow-list because + * FTS5's column-filter syntax is wider than any list of metacharacters. + * See docs/spec/data-model.md § Capability-gated filters. */ const OUTSIDE_PHRASE_DISALLOWED = /[^\p{L}\p{N}\p{M}_\s()"]/gu; /** - * Collapse every whitespace run to a single space. - * - * Every rule below that reasons about what sits beside an operator or a - * paren spells "beside" as `\s*`, and a `\s*` with a long run to chew on - * re-tries the whole run once per starting position — quadratic in the - * run, on text a caller types into a box. Keeping runs collapsed bounds - * what any of them can backtrack over to a single character. What it does - * not buy is linearity overall: the rewrite still costs a little more - * than proportionally as the text grows, which is why bounding a search's - * length belongs to whoever accepts it. - * - * Nothing is lost by it: FTS5's tokenizer splits on whitespace, so a - * collapsed run tokenizes exactly as the original did — inside a phrase - * as well as outside, since a phrase matches a sequence of tokens rather - * than the spacing between them. + * Bounds what any `\s*` can backtrack over (see Backtracking above). Safe + * inside a phrase too: FTS5 matches a phrase as a sequence of tokens, not + * the spacing between them. */ const collapseWhitespace = (query: string): string => query.replace(/\s+/g, ' '); @@ -90,21 +89,9 @@ export const sanitizeFts5Query = (query: string, maxDepth = 2): string => { clean = clean.replace(/\*/g, ''); - // Replace NEAR(terms, distance) with just its terms — see note above. - // - // Two things here are about what the engine does when the closing paren - // never arrives, on text a caller types into a box. - // - // No `\s*` after the paren, though the terms it captures may open with - // whitespace: `[^,()]*` already matches a space, so a second quantifier - // that also does gives the engine an ambiguous split to backtrack - // through. The captured space is collapsed with all the rest on the way - // out. - // - // And neither run crosses a `(`, which bounds where a failed match gives - // up: without it, `NEAR(NEAR(NEAR(` re-scans to the end of the string - // once per occurrence. A NEAR's terms are words and phrases and its - // distance a number, so neither holds a paren FTS5 would accept anyway. + // NEAR(terms, distance) keeps its terms — see the note above. The + // pattern's shape is about backtracking (see Backtracking); excluding `(` + // costs nothing, since a NEAR's terms and distance never hold one. clean = clean.replace(/\bNEAR\s*\(([^,()]*)(?:,[^()]*)?\)/gi, '$1'); // A NEAR( with no closing paren isn't the function-call form the rule @@ -112,15 +99,12 @@ export const sanitizeFts5Query = (query: string, maxDepth = 2): string => { // call it never meant to write. Keep the grouping, drop the keyword. clean = clean.replace(/\bNEAR\s*\(/gi, '('); - // Reduce to the characters that carry meaning outside a phrase — see note - // above. Everything else becomes a separator, which is what FTS5's own - // tokenizer would do with it anyway. + // See OUTSIDE_PHRASE_DISALLOWED. clean = clean.replace(/"[^"]*"|[^"]+/g, (span) => span.startsWith('"') ? span : span.replace(OUTSIDE_PHRASE_DISALLOWED, ' '), ); - // The reduction above turns every disallowed character into a space, so - // this is where the long runs come from. + // The reduction above is where long whitespace runs come from. clean = collapseWhitespace(clean); // Strip bare NOT with no left operand — FTS5 requires "term NOT term", not @@ -162,20 +146,15 @@ export const sanitizeFts5Query = (query: string, maxDepth = 2): string => { if (currentDepth > 0) result += ')'.repeat(currentDepth); - // Remove empty paren pairs left behind by NEAR/NOT stripping, and drop - // operators left without an operand. Looped together because each can - // create work for the other: removing an operator can empty a paren pair - // ("(a AND)" -> "(a)" -> ...), and removing a pair can strand the - // operator that pointed at it ("x AND ()" -> "x AND"). + // Looped to a fixpoint because each rule can create work for another: + // dropping an operator can empty a pair ("(a AND)"), dropping a pair can + // strand an operator ("x AND ()"), and an empty phrase's quotes can + // re-pair once text is inserted beside it. let prev: string; do { prev = result; - // Empty phrases and empty paren pairs, each of which matches nothing, - // in one rule so that a run of them collapses to a single space - // rather than to one space apiece — a run of spaces is what the rules - // below would have to backtrack over. An empty phrase is also the one - // construct whose quotes can re-pair differently once a rule inserts - // text beside it, which is why this repeats to a fixpoint. + // Empty phrases and empty paren pairs match nothing. One rule for both + // (see Backtracking). result = result.replace(/(?:""|\(\s*\))+/g, ' '); result = dropDanglingOperators(result); result = restoreImplicitAnd(result); @@ -189,23 +168,17 @@ const OPERATOR = String.raw`(?:AND|OR|NOT)`; /** * Stands in for a phrase while operators are rewritten. A private-use - * character is not a word character, so `\b` still sees the operator - * beside it, while `^`, `$` and the paren anchors see an operand — which - * is what a phrase is. Nothing reaches here holding one: `clean` is - * rewritten from the caller's string, and a literal U+E000 in it would be - * a term character FTS5 tokenizes away regardless. + * character is not a word character, so `\b` still sees an operator beside + * it, while the anchors see an operand. A literal U+E000 in the input is + * one FTS5 tokenizes away regardless. */ const PHRASE_HOLE = '\u{E000}'; /** - * Drop operators left without an operand — the shape a person types on - * their way to a longer query ("cats AND") or lands on once the rules - * above have removed a term. FTS5 rejects each of them outright, so what - * would otherwise be a syntax error becomes the search for the terms that - * are actually there. - * - * Phrases are masked out first: `"cats AND dogs"` is literal text to FTS5, - * and rewriting inside it would change what the user asked for. + * Drop operators left without an operand — typed on the way to a longer + * query ("cats AND"), or left behind once a rule above removed a term. + * FTS5 rejects each outright. Phrases are masked: inside one, an operator + * is a word the user searched for. */ const dropDanglingOperators = (query: string): string => withPhrasesMasked(query, (masked) => { @@ -221,9 +194,8 @@ const dropDanglingOperators = (query: string): string => // No left operand: at the start of the query or straight after "(". out = out.replace(new RegExp(String.raw`(^|\()\s*\b${OPERATOR}\b\s*`, 'gi'), '$1'); // No right operand: at the end of the query or straight before ")". - // No leading `\s*` — whitespace before the operator is left for the - // collapse on the way out, and a `\s*` here would re-scan a run of - // spaces once per position in it before failing to find an operator. + // No leading `\s*` (see Backtracking); the collapse on the way out + // takes that whitespace. out = out.replace(new RegExp(String.raw`\b${OPERATOR}\b\s*($|\))`, 'gi'), '$1'); } while (out !== prev); return out; @@ -245,24 +217,17 @@ const withPhrasesMasked = (query: string, rewrite: (masked: string) => string): return rewrite(masked).replace(new RegExp(PHRASE_HOLE, 'g'), () => phrases[i++]); }; -/** - * Put back the operator FTS5 requires around a group. Two terms side by - * side are an implicit AND (`cats dogs`), but a group is not: `cats (dogs)` - * is a syntax error where `cats AND (dogs)` is fine. Grouping survives a - * rewrite that strips the operator beside it, so this - * names the AND that was always meant rather than dropping the parens. - * - * Both rules test a *whole* word against the operator list. Matching a - * prefix instead reads the `D` of a preceding `AND` as a term and inserts - * a second one beside it. - * - * Phrases are masked for the same reason as elsewhere — a phrase is an - * operand, so `"a b" (c)` needs the operator too. - */ const OPERAND_CHAR = String.raw`[\p{L}\p{N}_${PHRASE_HOLE}]`; +/** A whole word, never a prefix: the `D` of `AND` is not a term. */ const isOperator = (word: string): boolean => /^(?:AND|OR|NOT)$/i.test(word); +/** + * Put back the operator FTS5 requires beside a group: `cats dogs` is an + * implicit AND, but `cats (dogs)` is a syntax error. A rewrite that strips + * the operator beside a group keeps the grouping, so this writes back the + * AND that was meant. A masked phrase is an operand like any other. + */ const restoreImplicitAnd = (query: string): string => withPhrasesMasked(query, (masked) => masked @@ -275,11 +240,8 @@ const restoreImplicitAnd = (query: string): string => return next && isOperator(next[1]) ? match : ') AND '; }, ) - // An operand then "(", unless the operand is an operator. - // The lookbehind pins the match to the start of the operand run. - // Without it every position inside a run is a start position, and - // each one re-scans the rest of the run before the lookahead - // rejects it — quadratic in a long word before a paren. + // An operand then "(", unless the operand is an operator. The + // lookbehind is load-bearing (see Backtracking). .replace( new RegExp(String.raw`(? (isOperator(word) ? match : `${word} AND `), diff --git a/packages/sqlite-shared/src/lock.ts b/packages/sqlite-shared/src/lock.ts index 1c32ce81..cdb14c14 100644 --- a/packages/sqlite-shared/src/lock.ts +++ b/packages/sqlite-shared/src/lock.ts @@ -47,13 +47,10 @@ export const acquireLock = (dbPath: string, force?: boolean): void => { }; /** - * Read the lock, treating an unreadable one as absent. A lock file is - * written non-atomically, so a crash mid-write leaves a torn one — and a - * lock naming no live process is exactly what acquireLock() reclaims. The - * alternative is a database that cannot be opened again by any means the - * error message mentions, which is the worse failure: the lock exists to - * prevent a second writer, not to outrank the data it guards. - * releaseLock() takes the same view. + * Read the lock, treating an unreadable one as absent. A crash mid-write + * leaves a torn lock naming no live process, which is what acquireLock() + * reclaims anyway; refusing it would lock the data out for good. The lock + * exists to stop a second writer, not to outrank the data it guards. */ const readLock = (lockPath: string): LockInfo | null => { try { diff --git a/packages/sqlite-shared/src/query.ts b/packages/sqlite-shared/src/query.ts index 6727bb8b..63d7e125 100644 --- a/packages/sqlite-shared/src/query.ts +++ b/packages/sqlite-shared/src/query.ts @@ -35,13 +35,10 @@ export const atBudget = (page: QueryStatement, rows: number): QueryStatement => }); /** - * Reduce `sort.direction` to a keyword this module may interpolate into - * SQL. Core's assertValidSort() already rejects anything else at the - * invariant layer, but this builder writes the value straight into - * `ORDER BY` and the cursor comparison, so it re-checks rather than trust - * an upstream that a future refactor could bypass: an unvalidated direction - * here is a SQL-injection sink. A sort without a direction is refused: core - * resolves the default before any adapter sees the query. + * Reduce `sort.direction` to a keyword safe to interpolate. Core's + * assertValidSort() already checks it, but this is where it reaches SQL + * text, so an unchecked value here would be an injection sink. Core + * resolves the default direction before any adapter sees the query. */ const sqlDirection = (query: StackQuery): 'ASC' | 'DESC' => { // No sort at all is the unsorted default; a sort always carries its direction. @@ -207,13 +204,9 @@ const recordConditions = (query: StackQuery): { conditions: string[]; params: un rangeFilter('updated_at', f.updatedAt); // Every association filter below is a semi-join rather than a correlated - // EXISTS, so the planner drives from the association side — reading the - // matching record ids straight out of idx_assoc_kind_label / - // idx_assoc_kind_file_id / idx_assoc_related / idx_content_index_file, - // each of which ends with record_id and so needs no table read at all — - // instead of scanning every record and probing for each. The work is - // proportional to how many records match, not to how many the stack - // holds. + // EXISTS, so the planner reads matching record ids straight out of an + // index ending in record_id instead of probing once per record: the work + // scales with the matches, not the stack. /** * The semi-join itself. `kind` is one of this module's own literals, never @@ -256,14 +249,9 @@ const recordConditions = (query: StackQuery): { conditions: string[]; params: un params.push(f.referencesFileId, f.referencesFileId); } - // Relationship filter — each clause is an optional pattern, so a bare - // label matches every target under it and an external target with no - // `id` matches its whole namespace (docs/spec/data-model.md § Filter). - // - // A record or entity target always stores '' in related_ns (see - // associationKeyColumns). Saying so turns idx_assoc_related's - // (related_scope, related_ns, related_id) prefix into a contiguous - // equality run for those kinds instead of a seek on related_scope alone. + // Each clause is an optional pattern (docs/spec/data-model.md § Filter). + // A record or entity target always stores '' in related_ns; saying so + // keeps idx_assoc_related's prefix a contiguous equality run. if (f.relatedTo) { const clauses: string[] = []; const target = f.relatedTo.target; @@ -357,12 +345,10 @@ const recordConditions = (query: StackQuery): { conditions: string[]; params: un }; /** - * Everything ordered after the cursor's record within the partition that - * holds a value: a later value rank, or the same rank past the cursor's - * value and id. `op` runs the comparison the same way the ORDER BY does, - * so one form covers both directions — in the direction where the - * cursor's rank is the last one, the leading disjunct is simply never - * true. + * Everything after the cursor's record within the partition that holds a + * value. `op` compares the way the ORDER BY does, so one form covers both + * directions; where the cursor's rank is the last one, the leading + * disjunct is simply never true. */ const presentCursorCondition = ( cursor: Extract, diff --git a/packages/sqlite-shared/src/record-logic.ts b/packages/sqlite-shared/src/record-logic.ts index ae5d8fdf..c2b4e2c9 100644 --- a/packages/sqlite-shared/src/record-logic.ts +++ b/packages/sqlite-shared/src/record-logic.ts @@ -55,15 +55,10 @@ import { import { makeCursor } from './cursor.js'; /** - * Run a query statement, restating an engine parse error as the wire - * taxonomy's `bad_request` rather than letting it escape as a raw engine - * error (which a server has no code to map, so it becomes a 500). - * - * Scoped to the one filter that carries a query language: `filter.search` - * is FTS5 source text, and sanitizeFts5Query() repairs the common shapes - * but claims no completeness against the grammar. Every other clause here - * is built from parameters, so a parse error in one is this module's bug — - * rethrown untouched, where it belongs. + * Restate an engine parse error as `bad_request` when the query carries + * search text, the one filter with a query language the sanitizer can't + * fully repair. Any other clause is built from parameters, so a parse + * error there is this module's bug and is rethrown untouched. */ const asStackBadRequestError = (query: StackQuery, run: () => T): T => { try { @@ -78,7 +73,7 @@ const asStackBadRequestError = (query: StackQuery, run: () => T): T => { /** * One spelling of the optimistic-concurrency failure, so the message a - * caller sees never depends on which of the three places noticed it. + * caller sees never depends on which check noticed it. */ const versionConflict = (id: string, expected: number, actual: number): StackVersionConflictError => new StackVersionConflictError( @@ -133,14 +128,10 @@ export class SharedSqlRecordLogic { } /** - * Precondition check for a mutation that has already read the record — - * either because it can't fold the check into its own statement - * (purgeRecord: children reference the row, so the DELETE can't - * carry it), or because it has to settle the precondition *before* - * fts5Strategy.remove() runs, which needs the records row still holding - * the old content. In the latter case versionedUpdate() re-checks it - * inside the statement, and that is the enforcement; this is what lets - * the work before it be skipped. + * Check `ifVersion` against a record already read: for a purge, whose + * DELETE can't carry the guard, and before fts5Strategy.remove(), which + * must run first. In the second case versionedUpdate()'s WHERE clause is + * the enforcement; this only lets the work before it be skipped. */ private checkExpectedVersion(record: StackRecord, expectedVersion: number | undefined): void { if (expectedVersion === undefined || record.version === expectedVersion) return; @@ -177,15 +168,10 @@ export class SharedSqlRecordLogic { } /** - * The one shape every mutating UPDATE in this class has: whatever columns - * the caller is changing, then the version bump and the actor/timestamp - * stamp that every mutation owes, gated on the opt-in `ifVersion` - * precondition and failing loudly when nothing matched. - * - * The guard rides in the WHERE clause even where the caller has already - * checked the version against a record it read: that read is outside this - * statement, so a writer slipping in between the two is caught here - * rather than silently overwritten. + * Every version-bumping UPDATE. The `ifVersion` guard rides in the WHERE + * clause even when the caller already checked a record it read: that read + * is outside this statement, so a writer slipping in between is caught + * here rather than overwritten. */ private versionedUpdate( id: string, @@ -211,15 +197,9 @@ export class SharedSqlRecordLogic { } /** - * Read a record back after its own mutation committed, for the caller to - * return. Its absence is not a "not found" for the caller to handle — the - * mutation just succeeded against it — so it fails as the invariant - * breach it would be. - * - * Synchronous, via readRecord() rather than getRecord(), so the read - * cannot be separated from the commit by anything: an async caller - * returning this still hands back a record read in the same tick its - * transaction committed. See readRecord() for why that matters. + * Read a record back after its own mutation committed. Absence here is an + * invariant breach, not a "not found". Synchronous, so nothing can run + * between the commit and the read — see readRecord(). */ private reread(id: string, verb: string): StackRecord { const updated = this.readRecord(id); @@ -232,16 +212,10 @@ export class SharedSqlRecordLogic { // ------------------------------------------------------- /** - * A duplicate id hits the `records` PK — mapped to StackConflictError - * instead of surfacing the raw engine exception, mirroring saveVersion's - * collision mapping below. - * - * One transaction, because a record and the four tables that describe it - * are one fact. A half-applied create is worse than a failed one: the - * call raises, so the caller believes nothing landed, while the records - * row survives to fail their retry on the PK — and the row it leaves is - * invisible to `filter.search` and mis-ordered by `sort.contentField` - * until something writes it again. + * One transaction, because a record and the tables that describe it are + * one fact: a half-applied create raises, so the caller believes nothing + * landed, while the surviving row fails their retry on the PK and is + * missing from both indexes. A duplicate id maps to StackConflictError. */ async createRecord(record: StackRecord, opts: JournalOptions = {}): Promise { this.exec.transaction(() => { @@ -292,18 +266,10 @@ export class SharedSqlRecordLogic { } /** - * The synchronous read behind getRecord(). Callers inside exec.transaction() - * use this one: an `await` inside that callback yields the microtask queue - * mid-transaction, and the next operation to run would try to open one of - * its own — transaction() requires a synchronous callback for exactly this - * reason (see SqlExecutor.transaction). - * - * The same synchrony is what lets the post-commit reads below report the - * version their own mutation produced: reread() calls this directly, so - * nothing interleaves between the commit and the read even though the - * method returning it is async. A backend that made these reads genuinely - * asynchronous would open that window, and a concurrent mutation could be - * reported under this one's verb. + * The synchronous read behind getRecord(), for use inside + * exec.transaction(), whose callback can't await. reread() relies on the + * same synchrony: nothing interleaves between a commit and its read, so a + * concurrent mutation is never reported under this one's verb. */ private readRecord(id: string): StackRecord | null { const row = this.exec.get>('SELECT * FROM records WHERE id = ?', [id]); @@ -316,26 +282,10 @@ export class SharedSqlRecordLogic { } /** - * Apply a change set in one transaction: one UPDATE over the record row - * with only the columns the change set names, the association set - * replaced when it carries one, and the snapshot written alongside — so - * one call is one version whatever it moved. - * - * `opts.bumpsVersion: false` is a change set that touches only - * `associations`, `parentId` and/or `unlisted` — Stack computes this from - * which ops the set actually moves. Such a write still lands its columns, - * but leaves `version`, `updatedAt` and the actor stamps exactly as they - * stood and takes no snapshot. `ifVersion`, if given, is re-checked - * synchronously inside the transaction — the CAS guard the bumping path - * gets from its UPDATE's WHERE clause, restated as a read because the - * UPDATE here carries no version predicate. See - * docs/spec/versioning.md § Version history. - * - * The ifVersion check for a bumping write is standalone rather than - * folded into the UPDATE alone, because a content change needs - * fts.remove() to run *before* the records-table content changes. The - * guard is still in the UPDATE's WHERE, so a writer that slipped in - * between the read and the write is caught there rather than overwriting. + * One transaction, so one call is one version whatever it moved. A + * non-bumping write (see docs/spec/versioning.md § Version history) has + * no versioned UPDATE to carry `ifVersion`, so it re-checks it by reading + * inside the transaction instead. */ async mutateRecord( id: string, @@ -346,11 +296,9 @@ export class SharedSqlRecordLogic { const now = toMs(new Date()); - // Assembled rather than written out per aspect: every aspect is the - // same column-and-value pair on one row, and a fixed statement per - // combination would be 2^4 of them. Built before the no-bump branch - // because both paths write the same columns; only `content` is the - // bumping path's alone, since a patch always bumps. + // Assembled, since a fixed statement per combination of aspects would + // be 2^4 of them. Shared by both paths; only `content` is the bumping + // path's alone, since a patch always bumps. const sets: string[] = []; const values: unknown[] = []; if (changes.parentId !== undefined) { @@ -427,13 +375,9 @@ export class SharedSqlRecordLogic { } /** - * Deletes a record's FTS entry, associations, versions, file-refs, sort - * index rows, and row, returning the record as it stood — the last copy that will ever - * exist, read here so that nothing can overtake it between the read and - * the deletes. Null when there was no such record. No write() call — - * callers batch it. ifVersion is checked first so a lost CAS race - * leaves nothing touched (children reference the row, so the check can't - * fold into the final DELETE). + * Delete a record and everything that refers to it, returning the record + * as it stood (null if there was none). Read here, inside the caller's + * transaction, so nothing can overtake the last copy that will ever exist. */ private purgeRecord(id: string, ifVersion?: number): StackRecord | null { const purged = this.readRecord(id); @@ -495,12 +439,6 @@ export class SharedSqlRecordLogic { content: Record, opts: MutateOptions = {}, ): Promise { - // Checked against a read record first, rather than left to the - // UPDATE's WHERE clause alone: fts5Strategy.remove() has to run before - // the content changes, so the precondition has to settle before that - // work starts. rewriteContent()'s UPDATE still carries the guard, so a - // writer slipping in between the two is caught rather than overwritten - // — same shape as mutateRecord() and restoreVersion(). await this.readForMutation(id, opts.ifVersion); this.exec.transaction(() => { @@ -513,14 +451,9 @@ export class SharedSqlRecordLogic { } /** - * Replace a record's type and content wholesale — what restoreVersion and - * commitMigration each do, differing only in where the content came from. - * - * The order is the constraint: fts5Strategy.remove() has to read the old - * content off the records row, so it runs before the UPDATE, and the - * re-index runs after. Every caller must already sit inside - * exec.transaction() — a failure between the two halves would otherwise - * leave the record unindexed. + * Replace a record's type and content wholesale. Unindex, write, re-index, + * in that order (docs/spec/adapters.md § The search index is unindexed + * before the row moves); callers must be inside exec.transaction(). */ private rewriteContent( id: string, @@ -540,11 +473,8 @@ export class SharedSqlRecordLogic { // One extra row, to learn whether a further page follows. const fetch = limit + 1; - // A content-field sort answers from two statements, the records that - // hold a value at the field and then the records that hold none. They - // read in order, each asking only for the rows the ones before it left - // of the page, and the second is skipped once the first has filled it — - // the common case, since a page rarely straddles the boundary. + // A content-field sort is two statements (see buildQueryPlan). Each asks + // only for what the page still lacks, so the second usually never runs. const rows: Record[] = []; for (const page of buildQueryPlan(query)) { const budget = fetch - rows.length; @@ -574,11 +504,8 @@ export class SharedSqlRecordLogic { } /** - * Atomically verify fileId is unreferenced, then purge its - * metadata records across every typeId in `metadataTypeIds` (see - * deleteUnreferencedAttachmentRecords on the adapter contract). A real - * SQL transaction of synchronous calls — no `await` between the check - * and the deletes, so nothing interleaves. + * The reference check and the purges share one synchronous transaction, + * so nothing can add a reference between the two. */ async deleteUnreferencedAttachmentRecords( fileId: FileId, @@ -718,22 +645,16 @@ export class SharedSqlRecordLogic { // ------------------------------------------------------- /** - * Append one entry, stamping the record-derived half off the row as it - * now stands — so every caller must already sit inside the same - * transaction as the write it describes, after that write has landed. - * - * `seq` is allocated here, from the log's own max, rather than computed - * by the caller. That is what spares this table the collision healing - * versions needs: nothing outside this statement ever holds a seq it - * expects to be free. See docs/spec/journal.md § Ordering. + * Append one entry, stamped from the row as it now stands, so callers run + * this inside their write's transaction, after the write. `seq` is + * allocated in the INSERT itself, so no caller ever holds one it expects + * to be free. See docs/spec/journal.md § Ordering. */ private appendJournal(recordId: string, entry: JournalEntryInput | undefined): void { if (!entry) return; - // Three columns, not readRecord(): the stamp needs version/typeId/ - // parentId and nothing else, and this runs inside every mutating - // write — a full row plus its association set, per write, to read - // three values. `parent_id` is used raw so the NULL/''/id spelling - // reaches the journal exactly as the records table holds it. + // Not readRecord(): this runs on every write and needs three columns, + // not the row plus its associations. parent_id stays raw so its + // NULL/''/id spelling reaches the journal unchanged. const row = this.exec.get<{ version: number; type_id: string; @@ -829,9 +750,8 @@ export class SharedSqlRecordLogic { /** * Never bumps `version`/`updatedAt` and never snapshots — see - * StackRecordAdapter.amendAssociations(). The records row is untouched - * entirely; only the associations table changes, in one transaction: - * removes first, then adds. + * StackRecordAdapter.amendAssociations(). Removes apply before adds, so + * removing and re-adding one identity in a call leaves it present. */ async amendAssociations( recordId: string, @@ -871,11 +791,8 @@ export class SharedSqlRecordLogic { private static readonly AUTHORITY_KINDS = ['permission', 'anyone']; /** - * Apply a change set's two association keys, each replacing only within - * its own half of the partition. That partition is the whole reason a - * tag write cannot drop an ACL: the `associations` key deletes data - * kinds alone, so the authority rows beside them are never in scope. - * Caller-transactional, like every write below. + * Each key replaces only within its own half of the partition, which is + * why a tag write cannot drop an ACL. Caller-transactional. * See docs/spec/access-control.md § Record-level permissions. */ private replaceAssociationHalves(recordId: string, changes: RecordChangeSet): void { @@ -904,14 +821,9 @@ export class SharedSqlRecordLogic { } /** - * Upserts on association identity: the primary key decides which row a - * write lands on, and `attachment_record_id` — outside it — is whatever - * the incoming association says, so re-pointing one costs no second row. - * - * FK enforcement (PRAGMA_FOREIGN_KEYS_ON) means inserting an - * association against a record that doesn't exist throws — mapped - * here to StackNotFoundError so amendAssociations() on a nonexistent record - * fails loudly instead of silently creating an orphan row. + * Upserts on association identity, so re-pointing `attachment_record_id` + * (outside the key) costs no second row. A missing record trips the + * foreign key, reported as StackNotFoundError rather than an orphan row. */ private insertAssociations(recordId: string, associations: Association[]): void { for (const assoc of associations) { diff --git a/packages/sqlite-shared/src/schema.ts b/packages/sqlite-shared/src/schema.ts index fb54b727..4508dbcf 100644 --- a/packages/sqlite-shared/src/schema.ts +++ b/packages/sqlite-shared/src/schema.ts @@ -27,39 +27,32 @@ export const RECORD_SCHEMA_SQL = ` ) STRICT; -- Every edge a record carries, authority and data alike: a permission - -- entry has element identity and a delta the same shape as a tag's, so - -- it earns the same table and the same durability tier. The permissions - -- and associations fields are projections over it, partitioned by - -- kind. See docs/spec/access-control.md § Record-level permissions. - -- - -- A relationship's target is (related_scope, related_id) plus one - -- qualifier: related_stack for a record in another stack, related_ns for - -- a foreign namespace. All four are in the primary key, so two targets - -- differing only by namespace are two associations. A permission's - -- grantee reuses them — related_scope 'entity' or 'group', related_id the - -- DID or group id — plus related_role, which only a group grantee sets - -- and which is in the key because member and admin name two different - -- sets of people. - -- - -- attachment_record_id stays out of the primary key: it annotates a - -- reference rather than naming one, so re-pointing it updates the row an - -- an amendAssociations() add lands on instead of writing a second reference to the same - -- file. See docs/spec/attachments.md § Naming the upload a reference came from. - -- - -- The primary key leads with record_id, so its implicit index already - -- serves every by-record read and delete; the indexes below exist only - -- for the filter directions it cannot answer. + -- has element identity and a delta shaped like a tag's, so it earns the + -- same table. The permissions and associations fields are projections + -- over it, partitioned by kind. + -- See docs/spec/access-control.md § Record-level permissions. CREATE TABLE IF NOT EXISTS associations ( record_id TEXT NOT NULL REFERENCES records(id), kind TEXT NOT NULL CHECK (kind IN ('tag', 'attachment', 'relationship', 'permission', 'anyone')), label TEXT NOT NULL, file_id TEXT NOT NULL DEFAULT '', + -- A relationship target is (related_scope, related_id) qualified by + -- related_ns (a foreign namespace) or related_stack (another stack); a + -- permission's grantee reuses scope and id. All are in the key, so + -- targets differing only by a qualifier are distinct associations. related_scope TEXT NOT NULL DEFAULT '' CHECK (related_scope IN ('', 'record', 'entity', 'external', 'group')), related_id TEXT NOT NULL DEFAULT '', related_ns TEXT NOT NULL DEFAULT '', related_stack TEXT NOT NULL DEFAULT '', + -- Set only by a group grantee; in the key because member and admin name + -- two different sets of people. related_role TEXT NOT NULL DEFAULT '' CHECK (related_role IN ('', 'member', 'admin')), + -- Out of the key: it annotates a reference rather than naming one, so + -- re-pointing it updates the existing row. See docs/spec/attachments.md + -- § Naming the upload a reference came from. attachment_record_id TEXT NOT NULL DEFAULT '', + -- Leads with record_id, so its implicit index serves every by-record + -- read and delete; the indexes below cover only the filter directions. PRIMARY KEY (record_id, kind, label, file_id, related_scope, related_id, related_ns, related_stack, related_role) ) STRICT; @@ -122,7 +115,6 @@ export const RECORD_SCHEMA_SQL = ` -- One row per top-level scalar content field a record holds a value at: -- what it orders as, and the file it references. See - -- docs/spec/adapters.md § Adapter backends and -- docs/spec/data-model.md § Sorting by a content field. -- -- value_rank is 0 for a numeric value and 1 for a text one — the same @@ -139,14 +131,11 @@ export const RECORD_SCHEMA_SQL = ` PRIMARY KEY (record_id, field) ) STRICT; - -- Indexes. Each filter column continues into (created_at, id) — the - -- default ordering and its tiebreak — because these columns are - -- low-cardinality by design, and a narrow index on one leaves the - -- planner sorting the whole matching set before it can honor LIMIT. - -- - -- deleted_at and unlisted_at deliberately have none: nothing searches - -- for the rows they mark, and an index on a column every query tests - -- for IS NULL only lures the planner off the one answering ORDER BY. + -- Each filter column continues into (created_at, id), the default order + -- and its tiebreak: these columns are low-cardinality, and a narrow index + -- leaves the planner sorting every match before it can honor LIMIT. + -- deleted_at and unlisted_at get none: an index on a column every query + -- tests IS NULL only lures the planner off the one answering ORDER BY. CREATE INDEX IF NOT EXISTS idx_records_type_id ON records(type_id, created_at, id); CREATE INDEX IF NOT EXISTS idx_records_parent_id ON records(parent_id, created_at, id); CREATE INDEX IF NOT EXISTS idx_records_entity_id ON records(entity_id, created_at, id); @@ -194,7 +183,7 @@ export const TOKENS_SCHEMA_SQL = ` /** * The full-text index behind `StackQuery.filter.search` (see indexes.ts). * What `columnsize=0` costs a reader is in docs/spec/adapters.md - * § Adapter backends. `detail` stays at its default: sanitizeFts5Query + * § SQLite-backed adapters. `detail` stays at its default: sanitizeFts5Query * passes phrase queries through, and a phrase needs the token positions * only that setting records. */ diff --git a/packages/sqlite-shared/src/token-logic.ts b/packages/sqlite-shared/src/token-logic.ts index f515b61d..d013f72a 100644 --- a/packages/sqlite-shared/src/token-logic.ts +++ b/packages/sqlite-shared/src/token-logic.ts @@ -1,6 +1,6 @@ /** * The actual StackTokenStore logic (createToken/lookupToken/listTokens/ - * revokeToken), parametrized over a SqlExecutor. Currently consumed by + * revokeToken), parametrized over a SqlExecutor. Consumed by * record-adapter-sqlite's NativeTokenStore, which points it at a file * separate from records — see TOKENS_SCHEMA_SQL in schema.ts. */ diff --git a/packages/sqlite-shared/tests/fts5.test.ts b/packages/sqlite-shared/tests/fts5.test.ts index daae63f5..41335e3f 100644 --- a/packages/sqlite-shared/tests/fts5.test.ts +++ b/packages/sqlite-shared/tests/fts5.test.ts @@ -179,14 +179,9 @@ describe('sanitizeFts5Query', () => { }); /** - * Search text is whatever a caller typed, so no rule may re-scan the - * same run of it once per starting position inside that run. Each shape - * below did: at this size the cost ran from seven seconds to four - * minutes, against the tens of milliseconds each takes now. - * - * The bound is against that collapse, not a linearity claim — cost - * still grows a little faster than length here, as it does on ordinary - * input. + * No rule may re-scan a run of the input once per starting position in + * it; at this size a quadratic rule takes seconds to minutes. The bound + * catches that, not non-linearity in general. See Backtracking in fts5.ts. */ describe('adversarial input does not blow up', () => { test.each([ From dd7cba03facde763d23819a92fa0873861775c91 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 13:08:27 +0000 Subject: [PATCH 6/9] fix(sqlite-shared): read and check inside the write transaction mutateRecord read the record, awaited, then wrote: a content patch was applied to that earlier read, so two concurrent unguarded patches lost one of them, and content_index could be rebuilt under a stale typeId. mutateRecord, restoreVersion and commitMigration now take a narrow read (version, type, content) inside their transaction and build from it. The pre-transaction readForMutation() is gone: the search-index removal it used to precede is rolled back with the transaction on any failure, so there is no work for an early check to save. amendAssociations' existence check reads the version instead of hydrating the record. Tests that simulated a writer in the old await gap now pin the same guarantees from the outside: racing guarded writes, one wins and the other conflicts; racing unguarded patches, both land. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DsiVnSgkrjGjpzpqaSr5xg --- .changeset/mutate-fresh-read.md | 6 + .../tests/record.test.ts | 110 ++++++------------ packages/sqlite-shared/src/record-logic.ts | 101 ++++++++-------- 3 files changed, 93 insertions(+), 124 deletions(-) create mode 100644 .changeset/mutate-fresh-read.md diff --git a/.changeset/mutate-fresh-read.md b/.changeset/mutate-fresh-read.md new file mode 100644 index 00000000..1871a2ea --- /dev/null +++ b/.changeset/mutate-fresh-read.md @@ -0,0 +1,6 @@ +--- +'@haverstack/record-adapter-sqlite': patch +'@haverstack/record-adapter-do-sqlite': patch +--- + +Concurrent writes to one record no longer lose updates: `mutateRecord`, `restoreVersion` and `commitMigration` read and check the record inside the transaction that writes it, so a content patch applies to the content as it stands rather than to an earlier read. diff --git a/packages/record-adapter-sqlite/tests/record.test.ts b/packages/record-adapter-sqlite/tests/record.test.ts index 94bc751c..fd306d65 100644 --- a/packages/record-adapter-sqlite/tests/record.test.ts +++ b/packages/record-adapter-sqlite/tests/record.test.ts @@ -598,37 +598,33 @@ describe('ifVersion', () => { expect((await adapter.getRecord(record.id))?.version).toBe(1); }); - // mutateRecord's non-bumping path (opts.bumpsVersion: false — what a - // caller passes for an associations-only change set) re-checks - // ifVersion against a fresh read inside its own transaction, - // since there is no version-bumping UPDATE for the guard to ride in. - // That read can come back null if a concurrent purge lands - // between mutateRecord's own read and this re-check; the adapter must - // report that as StackNotFoundError like every other write path here, - // not crash on it. - test('mutateRecord with bumpsVersion: false reports StackNotFoundError, not a crash, for a concurrently deleted record', async () => { + test('a non-bumping mutateRecord racing a purge reports StackNotFoundError', async () => { const adapter = await initAdapter(); const record = await adapter.createRecord(makeRecord()); - const logic = ( - adapter as unknown as { - record: { getRecord: (id: string) => Promise }; - } - ).record; - const originalGetRecord = logic.getRecord.bind(logic); - vi.spyOn(logic, 'getRecord').mockImplementationOnce(async (id: string) => { - const found = await originalGetRecord(id); - await adapter.deleteRecord(id, { purge: true }); // races mutateRecord's own no-bump re-read - return found; - }); - - await expect( + const [, mutated] = await Promise.allSettled([ + adapter.deleteRecord(record.id, { purge: true }), adapter.mutateRecord( record.id, { associations: [{ kind: 'tag', label: 'x' }] }, { bumpsVersion: false, ifVersion: 1 }, ), - ).rejects.toBeInstanceOf(StackNotFoundError); + ]); + expect(mutated.status === 'rejected' && mutated.reason).toBeInstanceOf(StackNotFoundError); + }); + + test('concurrent unguarded patches both land', async () => { + const adapter = await initAdapter(); + const record = await adapter.createRecord(makeRecord({ content: { a: 0, b: 0 } })); + + await Promise.all([ + adapter.mutateRecord(record.id, { contentPatch: { a: 1 } }), + adapter.mutateRecord(record.id, { contentPatch: { b: 1 } }), + ]); + + const after = await adapter.getRecord(record.id); + expect(after?.content).toEqual({ a: 1, b: 1 }); + expect(after?.version).toBe(3); }); test('a permissions change set enforces ifVersion', async () => { @@ -682,37 +678,15 @@ describe('ifVersion', () => { }); // ------------------------------------------------------- -// ifVersion — enforced against the row actually written +// ifVersion — checked inside the transaction that writes // ------------------------------------------------------- /** - * restoreVersion() and commitMigration() settle their precondition against - * a record read before the transaction opens, because fts5Strategy.remove() - * needs the old content still on the row. That read is not the enforcement: - * the UPDATE carries the guard too, so a writer landing in the window loses - * rather than being silently overwritten. - * - * The window is reachable from a test because both methods `await` that - * read: calling one runs its body up to the await and hands control back - * here with the precondition already satisfied, so the next synchronous - * statement below lands squarely between the check and the write. + * Two calls guarded on the same version: whichever commits first wins, and + * the other sees the version it moved to. */ -describe('ifVersion is re-checked by the statement that writes', () => { - /** - * A second writer, on its own connection — the adapter holds a lock - * *file*, which only initialize()/open() consult, so this is the real - * race rather than a stand-in for one. - */ - const bumpVersionBehindTheAdapter = (id: string): void => { - const other = new DatabaseSync(dbPath); - try { - other.prepare('UPDATE records SET version = version + 1 WHERE id = ?').run(id); - } finally { - other.close(); - } - }; - - test('restoreVersion loses to a writer that lands after the precondition check', async () => { +describe('ifVersion holds between concurrent writes', () => { + test('restoreVersion: one of two racing restores wins', async () => { const adapter = await initAdapter(); const record = await adapter.createRecord(makeRecord({ content: { text: 'original' } })); await adapter.saveVersion(record.id, { @@ -723,39 +697,31 @@ describe('ifVersion is re-checked by the statement that writes', () => { }); await adapter.mutateRecord(record.id, { contentPatch: { text: 'v2' } }); // -> v2 - const restoring = adapter.restoreVersion(record.id, 1, { ifVersion: 2 }); - bumpVersionBehindTheAdapter(record.id); // -> v3, after the check, before the write - - const err = await restoring.catch((e: unknown) => e); + const results = await Promise.allSettled([ + adapter.restoreVersion(record.id, 1, { ifVersion: 2 }), + adapter.restoreVersion(record.id, 1, { ifVersion: 2 }), + ]); + expect(results.map((r) => r.status)).toEqual(['fulfilled', 'rejected']); + const err = (results[1] as PromiseRejectedResult).reason; expect(err).toBeInstanceOf(StackVersionConflictError); expect((err as StackVersionConflictError).actualVersion).toBe(3); - - // And the restore left nothing behind: the content the other writer - // found is the content still there. - const after = await adapter.getRecord(record.id); - expect(after?.content).toEqual({ text: 'v2' }); - expect(after?.version).toBe(3); }); - test('commitMigration loses to a writer that lands after the precondition check', async () => { + test('commitMigration: one of two racing migrations wins', async () => { const adapter = await initAdapter(); const record = await adapter.createRecord(makeRecord({ content: { text: 'original' } })); - const migrating = adapter.commitMigration( - record.id, - 'com.example/note@2', - { text: 'migrated' }, - { ifVersion: 1 }, - ); - bumpVersionBehindTheAdapter(record.id); // -> v2, after the check, before the write - - const err = await migrating.catch((e: unknown) => e); + const results = await Promise.allSettled([ + adapter.commitMigration(record.id, 'com.example/note@2', { text: 'a' }, { ifVersion: 1 }), + adapter.commitMigration(record.id, 'com.example/note@2', { text: 'b' }, { ifVersion: 1 }), + ]); + expect(results.map((r) => r.status)).toEqual(['fulfilled', 'rejected']); + const err = (results[1] as PromiseRejectedResult).reason; expect(err).toBeInstanceOf(StackVersionConflictError); expect((err as StackVersionConflictError).actualVersion).toBe(2); const after = await adapter.getRecord(record.id); - expect(after?.typeId).toBe(record.typeId); - expect(after?.content).toEqual({ text: 'original' }); + expect(after?.content).toEqual({ text: 'a' }); }); test('a soft delete stamps deleted_at and updated_at from one timestamp', async () => { diff --git a/packages/sqlite-shared/src/record-logic.ts b/packages/sqlite-shared/src/record-logic.ts index c2b4e2c9..10b636cd 100644 --- a/packages/sqlite-shared/src/record-logic.ts +++ b/packages/sqlite-shared/src/record-logic.ts @@ -127,15 +127,10 @@ export class SharedSqlRecordLogic { return this.deps.exec; } - /** - * Check `ifVersion` against a record already read: for a purge, whose - * DELETE can't carry the guard, and before fts5Strategy.remove(), which - * must run first. In the second case versionedUpdate()'s WHERE clause is - * the enforcement; this only lets the work before it be skipped. - */ - private checkExpectedVersion(record: StackRecord, expectedVersion: number | undefined): void { - if (expectedVersion === undefined || record.version === expectedVersion) return; - throw versionConflict(record.id, expectedVersion, record.version); + /** Check `ifVersion` against a version read inside the caller's transaction. */ + private checkExpectedVersion(id: string, actual: number, expected: number | undefined): void { + if (expected === undefined || actual === expected) return; + throw versionConflict(id, expected, actual); } /** @@ -156,22 +151,28 @@ export class SharedSqlRecordLogic { } /** - * The read a content-rewriting mutation makes before its transaction, so - * the precondition settles before fts5Strategy.remove() runs — see - * checkExpectedVersion(). + * What a mutation builds on, read inside its transaction so nothing can + * land between the read and the write: a patch applied to an earlier read + * would overwrite whatever changed since. Throws for a missing record or + * a failed `ifVersion`. */ - private async readForMutation(id: string, ifVersion: number | undefined): Promise { - const existing = await this.getRecord(id); - if (!existing) throw recordNotFound(id); - this.checkExpectedVersion(existing, ifVersion); - return existing; + private readForWrite( + id: string, + ifVersion: number | undefined, + ): { typeId: TypeId; content: Record } { + const row = this.exec.get<{ version: number; type_id: string; content: string }>( + 'SELECT version, type_id, content FROM records WHERE id = ?', + [id], + ); + if (!row) throw recordNotFound(id); + this.checkExpectedVersion(id, row.version, ifVersion); + return { typeId: row.type_id, content: JSON.parse(row.content) }; } /** * Every version-bumping UPDATE. The `ifVersion` guard rides in the WHERE - * clause even when the caller already checked a record it read: that read - * is outside this statement, so a writer slipping in between is caught - * here rather than overwritten. + * clause, so a write that reads nothing first (a soft delete, an + * undelete) enforces it with no separate read. */ private versionedUpdate( id: string, @@ -282,18 +283,14 @@ export class SharedSqlRecordLogic { } /** - * One transaction, so one call is one version whatever it moved. A - * non-bumping write (see docs/spec/versioning.md § Version history) has - * no versioned UPDATE to carry `ifVersion`, so it re-checks it by reading - * inside the transaction instead. + * One transaction, so one call is one version whatever it moved, and the + * patch applies to the content as it stands when the write lands. */ async mutateRecord( id: string, changes: RecordChangeSet, opts: MutateOptions & BumpVersionOptions = {}, ): Promise { - const existing = await this.readForMutation(id, opts.ifVersion); - const now = toMs(new Date()); // Assembled, since a fixed statement per combination of aspects would @@ -313,29 +310,28 @@ export class SharedSqlRecordLogic { values.push(changes.unlisted ? now : null); } - if (opts.bumpsVersion === false) { - this.exec.transaction(() => { - const current = this.readRecord(id); - if (!current) throw recordNotFound(id); - this.checkExpectedVersion(current, opts.ifVersion); + this.exec.transaction(() => { + const current = this.readForWrite(id, opts.ifVersion); + + // No versioned UPDATE to carry `ifVersion`, so readForWrite() is the + // only check. See docs/spec/versioning.md § Version history. + if (opts.bumpsVersion === false) { if (sets.length > 0) { this.exec.run(`UPDATE records SET ${sets.join(', ')} WHERE id = ?`, [...values, id]); } this.replaceAssociationHalves(id, changes); this.appendJournal(id, opts.journal); - }); - return this.reread(id, 'mutateRecord'); - } + return; + } - const merged = changes.contentPatch - ? applyMergePatch(existing.content, changes.contentPatch) - : undefined; - if (merged !== undefined) { - sets.unshift('content = ?'); - values.unshift(JSON.stringify(merged)); - } + const merged = changes.contentPatch + ? applyMergePatch(current.content, changes.contentPatch) + : undefined; + if (merged !== undefined) { + sets.unshift('content = ?'); + values.unshift(JSON.stringify(merged)); + } - this.exec.transaction(() => { if (opts.snapshot) this.snapshotBeforeMutation(id, opts.snapshot); if (merged !== undefined) fts5Strategy.remove(this.exec, id); @@ -345,7 +341,7 @@ export class SharedSqlRecordLogic { if (merged !== undefined) { fts5Strategy.insert(this.exec, id, JSON.stringify(merged)); - this.contentIndex.sync(id, existing.typeId, merged); + this.contentIndex.sync(id, current.typeId, merged); } this.appendJournal(id, opts.journal); }); @@ -388,7 +384,7 @@ export class SharedSqlRecordLogic { if (ifVersion !== undefined) throw recordNotFound(id); return null; } - this.checkExpectedVersion(purged, ifVersion); + this.checkExpectedVersion(id, purged.version, ifVersion); fts5Strategy.remove(this.exec, id); this.exec.run('DELETE FROM associations WHERE record_id = ?', [id]); this.exec.run('DELETE FROM versions WHERE record_id = ?', [id]); @@ -416,15 +412,13 @@ export class SharedSqlRecordLogic { version: number, opts: MutateOptions = {}, ): Promise { - await this.readForMutation(id, opts.ifVersion); - - const target = await this.getVersion(id, version); - if (!target) throw new StackNotFoundError(`Version not found: "${id}"@${version}`); - // Only content and its type are restored — a snapshot carries nothing // else, so there is nothing here to roll containment, listing or the // associations table back to. this.exec.transaction(() => { + this.readForWrite(id, opts.ifVersion); + const target = this.readVersion(id, version); + if (!target) throw new StackNotFoundError(`Version not found: "${id}"@${version}`); if (opts.snapshot) this.snapshotBeforeMutation(id, opts.snapshot); this.rewriteContent(id, target.typeId, target.content, opts); this.appendJournal(id, opts.journal); @@ -439,9 +433,8 @@ export class SharedSqlRecordLogic { content: Record, opts: MutateOptions = {}, ): Promise { - await this.readForMutation(id, opts.ifVersion); - this.exec.transaction(() => { + this.readForWrite(id, opts.ifVersion); if (opts.snapshot) this.snapshotBeforeMutation(id, opts.snapshot); this.rewriteContent(id, toTypeId, content, opts); this.appendJournal(id, opts.journal); @@ -565,6 +558,10 @@ export class SharedSqlRecordLogic { } async getVersion(id: string, version: number): Promise { + return this.readVersion(id, version); + } + + private readVersion(id: string, version: number): RecordVersion | null { const row = this.exec.get>( 'SELECT * FROM versions WHERE record_id = ? AND version = ?', [id, version], @@ -760,7 +757,7 @@ export class SharedSqlRecordLogic { ): Promise { assertOneSurface(changes); this.exec.transaction(() => { - if (!this.readRecord(recordId)) throw recordNotFound(recordId); + if (this.currentVersion(recordId) === undefined) throw recordNotFound(recordId); for (const change of changes) { if (change.op !== 'remove') continue; const { association } = change; From b7d358daba291aa5b16312b9d5cabf4dd71aa908 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 13:10:44 +0000 Subject: [PATCH 7/9] refactor: keep the authority association kinds in one list replaceAssociationHalves deleted by its own DATA_KINDS/AUTHORITY_KINDS lists, a third spelling of the partition beside core's predicate. Core now exports AUTHORITY_ASSOCIATION_KINDS from @haverstack/core/adapter, isAuthorityAssociation reads it, and the SQL deletes the data half as "not an authority kind", so a new authority kind added to core is covered in both places at once. isAuthorityAssociation also drops its `a?.` guards: its parameter is a typed Association, and every caller passes one already validated. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DsiVnSgkrjGjpzpqaSr5xg --- .changeset/export-association-partition.md | 2 +- packages/core/src/adapter/index.ts | 2 +- packages/core/src/associations/identity.ts | 12 +++++++++++- packages/sqlite-shared/src/record-logic.ts | 19 +++++++++---------- 4 files changed, 22 insertions(+), 13 deletions(-) diff --git a/.changeset/export-association-partition.md b/.changeset/export-association-partition.md index 5017e2da..c4cec5c1 100644 --- a/.changeset/export-association-partition.md +++ b/.changeset/export-association-partition.md @@ -2,4 +2,4 @@ '@haverstack/core': minor --- -Export `isAuthorityAssociation` from `@haverstack/core`, for apps that split a journal entry's association changes between `amendAssociations()` and `amendAccess()`, and `partitionAssociations` from `@haverstack/core/adapter`, for storage engines that read both halves from one table. +Export `isAuthorityAssociation` from `@haverstack/core`, for apps that split a journal entry's association changes between `amendAssociations()` and `amendAccess()`, and `partitionAssociations` and `AUTHORITY_ASSOCIATION_KINDS` from `@haverstack/core/adapter`, for storage engines that keep both halves in one table. diff --git a/packages/core/src/adapter/index.ts b/packages/core/src/adapter/index.ts index aba0239b..99e4369d 100644 --- a/packages/core/src/adapter/index.ts +++ b/packages/core/src/adapter/index.ts @@ -33,4 +33,4 @@ export { } from '../query-validation.js'; export { contentSortEntry, contentSortKey, compareSortEntries } from './sort.js'; export type { SortEntry } from './sort.js'; -export { partitionAssociations } from '../associations/identity.js'; +export { AUTHORITY_ASSOCIATION_KINDS, partitionAssociations } from '../associations/identity.js'; diff --git a/packages/core/src/associations/identity.ts b/packages/core/src/associations/identity.ts index 984467a2..8cdbf4d2 100644 --- a/packages/core/src/associations/identity.ts +++ b/packages/core/src/associations/identity.ts @@ -92,6 +92,16 @@ function stripAssociationAnnotation(association: Association): Association { return identity as Association; } +/** + * The association kinds that carry authority. Every other kind is data. + * Storage that partitions by kind in a query reads this list rather than + * restating it, so a new authority kind can't fall into the data half. + */ +export const AUTHORITY_ASSOCIATION_KINDS: readonly AuthorityAssociation['kind'][] = [ + 'permission', + 'anyone', +]; + /** * Whether an association carries authority rather than data — the partition * `StackRecord.permissions` and `StackRecord.associations` project, and the @@ -99,7 +109,7 @@ function stripAssociationAnnotation(association: Association): Association { * See docs/spec/access-control.md § Record-level permissions. */ export function isAuthorityAssociation(a: Association): a is AuthorityAssociation { - return a?.kind === 'permission' || a?.kind === 'anyone'; + return (AUTHORITY_ASSOCIATION_KINDS as readonly string[]).includes(a.kind); } /** diff --git a/packages/sqlite-shared/src/record-logic.ts b/packages/sqlite-shared/src/record-logic.ts index 10b636cd..bbe68df7 100644 --- a/packages/sqlite-shared/src/record-logic.ts +++ b/packages/sqlite-shared/src/record-logic.ts @@ -39,6 +39,7 @@ import type { JournalOptions, MutateOptions, } from '@haverstack/core/adapter'; +import { AUTHORITY_ASSOCIATION_KINDS } from '@haverstack/core/adapter'; import type { SqlExecutor } from './executor.js'; import { isForeignKeyViolation, isUniqueConstraintViolation } from './executor.js'; import { buildQueryPlan, atBudget } from './query.js'; @@ -783,10 +784,6 @@ export class SharedSqlRecordLogic { return this.reread(recordId, 'amendAssociations'); } - /** The kinds each change-set key replaces, and nothing else. */ - private static readonly DATA_KINDS = ['tag', 'attachment', 'relationship']; - private static readonly AUTHORITY_KINDS = ['permission', 'anyone']; - /** * Each key replaces only within its own half of the partition, which is * why a tag write cannot drop an ACL. Caller-transactional. @@ -794,25 +791,27 @@ export class SharedSqlRecordLogic { */ private replaceAssociationHalves(recordId: string, changes: RecordChangeSet): void { if (changes.associations !== undefined) { - this.replaceAssociations(recordId, changes.associations, SharedSqlRecordLogic.DATA_KINDS); + this.replaceAssociations(recordId, changes.associations, 'data'); } if (changes.permissions !== undefined) { - this.replaceAssociations(recordId, changes.permissions, SharedSqlRecordLogic.AUTHORITY_KINDS); + this.replaceAssociations(recordId, changes.permissions, 'authority'); } } /** * Swap one half of a record's association set wholesale — the "replace, - * don't merge" semantics a change set's association list carries. + * don't merge" semantics a change set's association list carries. The data + * half is "not an authority kind", so core's list is the only one. */ private replaceAssociations( recordId: string, associations: Association[], - kinds: string[], + half: 'data' | 'authority', ): void { this.exec.run( - `DELETE FROM associations WHERE record_id = ? AND kind IN (${kinds.map(() => '?').join(', ')})`, - [recordId, ...kinds], + `DELETE FROM associations WHERE record_id = ? AND kind ${half === 'data' ? 'NOT IN' : 'IN'} ` + + `(${AUTHORITY_ASSOCIATION_KINDS.map(() => '?').join(', ')})`, + [recordId, ...AUTHORITY_ASSOCIATION_KINDS], ); if (associations.length) this.insertAssociations(recordId, associations); } From e68f4d49eeda8e36ef2c4820b9faafd1837a5221 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 13:13:14 +0000 Subject: [PATCH 8/9] refactor(sqlite-shared): one style for the indexes and the executor - fts5Strategy becomes a SearchIndex class holding its executor, like ContentIndex, instead of taking `exec` on every call. - SharedSqlRecordLogic and SharedTokenLogic store `exec` at construction rather than reading it through a getter, so the logic and the indexes it builds are bound to the same handle. - query.ts imports from @haverstack/core once. - PRAGMA_JOURNAL_MODE_WAL moves from the record-only barrel to the full one: its only consumer is the token store. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DsiVnSgkrjGjpzpqaSr5xg --- packages/sqlite-shared/src/index.ts | 2 +- packages/sqlite-shared/src/indexes.ts | 27 ++++++++++++---------- packages/sqlite-shared/src/query.ts | 3 +-- packages/sqlite-shared/src/record-logic.ts | 26 ++++++++++----------- packages/sqlite-shared/src/record.ts | 6 ++--- packages/sqlite-shared/src/token-logic.ts | 6 ++--- 6 files changed, 36 insertions(+), 34 deletions(-) diff --git a/packages/sqlite-shared/src/index.ts b/packages/sqlite-shared/src/index.ts index 8ba970f9..417a7da2 100644 --- a/packages/sqlite-shared/src/index.ts +++ b/packages/sqlite-shared/src/index.ts @@ -6,6 +6,6 @@ * pieces; see the note there for why that subset has to exist at all. */ export * from './record.js'; -export { TOKENS_SCHEMA_SQL } from './schema.js'; +export { TOKENS_SCHEMA_SQL, PRAGMA_JOURNAL_MODE_WAL } from './schema.js'; export { acquireLock, releaseLock } from './lock.js'; export { SharedTokenLogic } from './token-logic.js'; diff --git a/packages/sqlite-shared/src/indexes.ts b/packages/sqlite-shared/src/indexes.ts index 5d534db5..01328872 100644 --- a/packages/sqlite-shared/src/indexes.ts +++ b/packages/sqlite-shared/src/indexes.ts @@ -16,30 +16,33 @@ import type { SqlExecutor } from './executor.js'; * DELETE leaves stale, still-searchable entries. remove() therefore reads * the current rowid/content and must run before the caller's change. */ -export const fts5Strategy = { +export class SearchIndex { + constructor(private readonly exec: SqlExecutor) {} + /** Index a record that has no FTS entry yet (used right after an INSERT). */ - insert(exec: SqlExecutor, recordId: string, content: string): void { - exec.run(`INSERT INTO records_fts(rowid, content) SELECT rowid, ? FROM records WHERE id = ?`, [ - content, - recordId, - ]); - }, + insert(recordId: string, content: string): void { + this.exec.run( + `INSERT INTO records_fts(rowid, content) SELECT rowid, ? FROM records WHERE id = ?`, + [content, recordId], + ); + } + /** * Remove a record's FTS entry. MUST be called before the records row's * content changes or the row is deleted — see the note above. */ - remove(exec: SqlExecutor, recordId: string): void { - const row = exec.get<{ rowid: number; content: string }>( + remove(recordId: string): void { + const row = this.exec.get<{ rowid: number; content: string }>( 'SELECT rowid, content FROM records WHERE id = ?', [recordId], ); if (!row) return; - exec.run(`INSERT INTO records_fts(records_fts, rowid, content) VALUES('delete', ?, ?)`, [ + this.exec.run(`INSERT INTO records_fts(records_fts, rowid, content) VALUES('delete', ?, ?)`, [ row.rowid, row.content, ]); - }, -}; + } +} /** * Top-level scalar fields, by name and declared kind — what content_index diff --git a/packages/sqlite-shared/src/query.ts b/packages/sqlite-shared/src/query.ts index 63d7e125..0ebeb7dd 100644 --- a/packages/sqlite-shared/src/query.ts +++ b/packages/sqlite-shared/src/query.ts @@ -13,9 +13,8 @@ * fact, which costs a full pass over the table for every page. */ -import { StackBadRequestError, type StackQuery } from '@haverstack/core'; +import { StackBadRequestError, type NativeSortField, type StackQuery } from '@haverstack/core'; import { contentSortKey, parseContentFilterKey } from '@haverstack/core/adapter'; -import type { NativeSortField } from '@haverstack/core'; import { decodeCursor, getSortColumn, getSortField, type DecodedCursor } from './cursor.js'; import { sanitizeFts5Query } from './fts5.js'; diff --git a/packages/sqlite-shared/src/record-logic.ts b/packages/sqlite-shared/src/record-logic.ts index bbe68df7..4e3525c4 100644 --- a/packages/sqlite-shared/src/record-logic.ts +++ b/packages/sqlite-shared/src/record-logic.ts @@ -43,7 +43,7 @@ import { AUTHORITY_ASSOCIATION_KINDS } from '@haverstack/core/adapter'; import type { SqlExecutor } from './executor.js'; import { isForeignKeyViolation, isUniqueConstraintViolation } from './executor.js'; import { buildQueryPlan, atBudget } from './query.js'; -import { ContentIndex, fts5Strategy } from './indexes.js'; +import { ContentIndex, SearchIndex } from './indexes.js'; import { rowToRecord, rowToAssociation, @@ -118,14 +118,14 @@ export type SharedSqlRecordLogicDeps = { }; export class SharedSqlRecordLogic { + private readonly exec: SqlExecutor; + private readonly searchIndex: SearchIndex; private readonly contentIndex: ContentIndex; - constructor(private readonly deps: SharedSqlRecordLogicDeps) { - this.contentIndex = new ContentIndex(deps.exec); - } - - private get exec(): SqlExecutor { - return this.deps.exec; + constructor({ exec }: SharedSqlRecordLogicDeps) { + this.exec = exec; + this.searchIndex = new SearchIndex(exec); + this.contentIndex = new ContentIndex(exec); } /** Check `ifVersion` against a version read inside the caller's transaction. */ @@ -255,7 +255,7 @@ export class SharedSqlRecordLogic { const edges = [...(record.associations ?? []), ...(record.permissions ?? [])]; if (edges.length) this.insertAssociations(record.id, edges); - fts5Strategy.insert(this.exec, record.id, JSON.stringify(record.content)); + this.searchIndex.insert(record.id, JSON.stringify(record.content)); this.contentIndex.sync(record.id, record.typeId, record.content); this.appendJournal(record.id, opts.journal); }); @@ -334,14 +334,14 @@ export class SharedSqlRecordLogic { } if (opts.snapshot) this.snapshotBeforeMutation(id, opts.snapshot); - if (merged !== undefined) fts5Strategy.remove(this.exec, id); + if (merged !== undefined) this.searchIndex.remove(id); this.versionedUpdate(id, sets, values, opts, now); this.replaceAssociationHalves(id, changes); if (merged !== undefined) { - fts5Strategy.insert(this.exec, id, JSON.stringify(merged)); + this.searchIndex.insert(id, JSON.stringify(merged)); this.contentIndex.sync(id, current.typeId, merged); } this.appendJournal(id, opts.journal); @@ -386,7 +386,7 @@ export class SharedSqlRecordLogic { return null; } this.checkExpectedVersion(id, purged.version, ifVersion); - fts5Strategy.remove(this.exec, id); + this.searchIndex.remove(id); this.exec.run('DELETE FROM associations WHERE record_id = ?', [id]); this.exec.run('DELETE FROM versions WHERE record_id = ?', [id]); // A purge takes the journal with it, for the reason it takes the @@ -456,9 +456,9 @@ export class SharedSqlRecordLogic { opts: IfVersionOptions & ActorOptions, ): void { const json = JSON.stringify(content); - fts5Strategy.remove(this.exec, id); + this.searchIndex.remove(id); this.versionedUpdate(id, ['type_id = ?', 'content = ?'], [typeId, json], opts); - fts5Strategy.insert(this.exec, id, json); + this.searchIndex.insert(id, json); this.contentIndex.sync(id, typeId, content); } diff --git a/packages/sqlite-shared/src/record.ts b/packages/sqlite-shared/src/record.ts index 20c4056a..d813b4c4 100644 --- a/packages/sqlite-shared/src/record.ts +++ b/packages/sqlite-shared/src/record.ts @@ -1,8 +1,8 @@ /** * The record-only barrel, and the base the full one (index.ts) builds on: * index.ts re-exports everything here and adds the token-store pieces - * (TOKENS_SCHEMA_SQL, SharedTokenLogic) and the file-lock helpers - * (acquireLock/releaseLock). The two surfaces therefore differ by exactly + * (TOKENS_SCHEMA_SQL, the WAL pragma its file uses, SharedTokenLogic) and + * the file-lock helpers (acquireLock/releaseLock). The two surfaces therefore differ by exactly * those additions, rather than by whatever two hand-maintained lists * happen to say. * @@ -16,7 +16,7 @@ * for lock.ts's plain functions — so avoiding the import (not just the * unused export) has to happen at this file's level. */ -export { PRAGMA_JOURNAL_MODE_WAL, applyRecordSchema } from './schema.js'; +export { applyRecordSchema } from './schema.js'; export { sanitizeFts5Query } from './fts5.js'; export { type SqlExecutor } from './executor.js'; export { diff --git a/packages/sqlite-shared/src/token-logic.ts b/packages/sqlite-shared/src/token-logic.ts index d013f72a..79636b21 100644 --- a/packages/sqlite-shared/src/token-logic.ts +++ b/packages/sqlite-shared/src/token-logic.ts @@ -23,10 +23,10 @@ export type SharedTokenLogicDeps = { }; export class SharedTokenLogic { - constructor(private readonly deps: SharedTokenLogicDeps) {} + private readonly exec: SqlExecutor; - private get exec(): SqlExecutor { - return this.deps.exec; + constructor({ exec }: SharedTokenLogicDeps) { + this.exec = exec; } async createToken( From 22373532463f43b4a40087408e4a2ddd88ac9ae6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 13:15:05 +0000 Subject: [PATCH 9/9] docs: record sqlite-shared changes against both adapters that bundle it record-adapter-sqlite and record-adapter-do-sqlite each inline their own copy of sqlite-shared, so a change to it ships in both. Naming only record-adapter-sqlite left the DO adapter's changelog without the entry, even when the DO adapter was released for an unrelated reason. AGENTS.md, CONTRIBUTING.md and expand-changesets' error message now say to name both, and this branch's sqlite-shared changesets do. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DsiVnSgkrjGjpzpqaSr5xg --- .changeset/fts5-not-in-phrase.md | 1 + .changeset/sqlite-shared-dedupe.md | 1 + .changeset/sqlite-shared-indexes.md | 1 + AGENTS.md | 2 +- CONTRIBUTING.md | 2 +- scripts/expand-changesets.mjs | 5 +++-- 6 files changed, 8 insertions(+), 4 deletions(-) diff --git a/.changeset/fts5-not-in-phrase.md b/.changeset/fts5-not-in-phrase.md index 4ecaec48..a5c451b4 100644 --- a/.changeset/fts5-not-in-phrase.md +++ b/.changeset/fts5-not-in-phrase.md @@ -1,5 +1,6 @@ --- '@haverstack/record-adapter-sqlite': patch +'@haverstack/record-adapter-do-sqlite': patch --- `filter.search` no longer drops a `NOT` written inside a quoted phrase: `"cats (not dogs"` searches for the phrase as typed. diff --git a/.changeset/sqlite-shared-dedupe.md b/.changeset/sqlite-shared-dedupe.md index 5243c20c..b6f7120f 100644 --- a/.changeset/sqlite-shared-dedupe.md +++ b/.changeset/sqlite-shared-dedupe.md @@ -1,5 +1,6 @@ --- '@haverstack/record-adapter-sqlite': patch +'@haverstack/record-adapter-do-sqlite': patch --- Remove redundant code from the bundled SQLite record logic: unused barrel exports, local aliases of core's native sort fields, and repeated not-found/version-read/row-hydration code. diff --git a/.changeset/sqlite-shared-indexes.md b/.changeset/sqlite-shared-indexes.md index 736755e1..7bfe3acd 100644 --- a/.changeset/sqlite-shared-indexes.md +++ b/.changeset/sqlite-shared-indexes.md @@ -1,5 +1,6 @@ --- '@haverstack/record-adapter-sqlite': patch +'@haverstack/record-adapter-do-sqlite': patch --- Internal restructure of the bundled SQLite record logic: the full-text and content indexes move into their own module. No behavior change. diff --git a/AGENTS.md b/AGENTS.md index 86eb80e8..4d7e8216 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -92,7 +92,7 @@ pnpm changeset - **`patch`** for anything a consumer cannot observe; **`minor`** for anything they can. Every package is `0.x`, so minor _is_ the breaking slot. - **Never `major`** — that cuts a 1.0 release, which is not a PR-level decision. - **No changeset** for tests, CI, or repo tooling. -- `@haverstack/sqlite-shared` is `private` and has no version to publish. Record a change to it against `@haverstack/record-adapter-sqlite`, which bundles it — a changeset naming only private packages is rejected. If a template hands you a list of every package (changeset-bot's does), trim it to what you changed. +- `@haverstack/sqlite-shared` is `private` and has no version to publish. Record a change to it against both packages that bundle it, `@haverstack/record-adapter-sqlite` and `@haverstack/record-adapter-do-sqlite` — each ships its own copy, so each changelog owes the entry. A changeset naming only private packages is rejected. If a template hands you a list of every package (changeset-bot's does), trim it to what you changed. A `minor` on a package minor-bumps everything that depends on it, transitively; a `patch` bumps nothing else. Preview with `node scripts/expand-changesets.mjs --dry-run`. Never hand-edit a `version` field or a `CHANGELOG.md` — CI owns both. See [CONTRIBUTING.md § Releasing](./CONTRIBUTING.md#releasing). diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7ad5e9b3..7f81a836 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -81,7 +81,7 @@ Every package is on `0.x`, where semver puts breaking changes in the **minor** s **Don't select `major`.** It cuts a 1.0 release, which is a decision about the install base (see [No backward compatibility yet](#no-backward-compatibility-yet)), not something a single PR makes. -`@haverstack/sqlite-shared` is `private` and has no version to publish. Record a change to it against `@haverstack/record-adapter-sqlite`, which bundles it. Naming a private package beside a public one makes Changesets refuse the whole release plan, so `scripts/expand-changesets.mjs` drops the line first — changeset-bot's template is safe to commit unedited. A changeset naming _only_ private packages fails: it would release nowhere. +`@haverstack/sqlite-shared` is `private` and has no version to publish. Record a change to it against both packages that bundle it, `@haverstack/record-adapter-sqlite` and `@haverstack/record-adapter-do-sqlite`: each inlines its own copy at build time, so a change to sqlite-shared ships in both and belongs in both changelogs. Naming a private package beside a public one makes Changesets refuse the whole release plan, so `scripts/expand-changesets.mjs` drops the line first — changeset-bot's template is safe to commit unedited. A changeset naming _only_ private packages fails: it would release nowhere. ### The ripple rule diff --git a/scripts/expand-changesets.mjs b/scripts/expand-changesets.mjs index 5d5d7541..fd63c2e9 100644 --- a/scripts/expand-changesets.mjs +++ b/scripts/expand-changesets.mjs @@ -115,8 +115,9 @@ for (const file of readdirSync(changesetDir)) { if (releasable.length === 0) { console.error( `${file} names only packages that are never published (${dropped.join(', ')}).\n` + - 'Name the package that ships the change instead — for sqlite-shared that is ' + - '@haverstack/record-adapter-sqlite, which bundles it.', + 'Name the packages that ship the change instead — for sqlite-shared that is ' + + '@haverstack/record-adapter-sqlite and @haverstack/record-adapter-do-sqlite, ' + + 'which both bundle it.', ); process.exit(1); }