From 89e0a07b03ade71449cd20b7ad0333761c39a41f Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 02:13:18 +0000 Subject: [PATCH 1/2] feat(core): check a typed collection's write expectation in Stack A typed collection must refuse a write to a record outside its Type family, and notice a content write to a record stored at an older version. Doing either from outside Stack costs a read per write, or throws only after the write has landed. Every write verb now accepts an internal expectation under a module-private symbol key: the family, plus an exact typeId checked on a content patch. Stack checks it against the read it already makes; a family mismatch is StackNotFoundError, a version mismatch is a StoredVersionError carrying the stored record. ScopedStack checks the family ahead of its permission gate, so another family's record is not found whatever the requester may do to it, and passes the expectation through. Closes #408 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GomaLzoB7N2ipg547Kr6h4 --- .changeset/write-expectation.md | 5 + packages/core/src/scoped-stack.ts | 86 ++++++--- packages/core/src/stack.ts | 44 +++-- packages/core/src/write-expectation.ts | 69 +++++++ packages/core/tests/write-expectation.test.ts | 171 ++++++++++++++++++ 5 files changed, 341 insertions(+), 34 deletions(-) create mode 100644 .changeset/write-expectation.md create mode 100644 packages/core/src/write-expectation.ts create mode 100644 packages/core/tests/write-expectation.test.ts diff --git a/.changeset/write-expectation.md b/.changeset/write-expectation.md new file mode 100644 index 00000000..9724a08d --- /dev/null +++ b/.changeset/write-expectation.md @@ -0,0 +1,5 @@ +--- +'@haverstack/core': patch +--- + +`Stack` and `ScopedStack` accept an internal write expectation on every write verb — the record's Type family, and on a content write its exact version — checked against the read each write already makes, so a typed collection's write is refused before anything lands and costs no extra round trip. diff --git a/packages/core/src/scoped-stack.ts b/packages/core/src/scoped-stack.ts index a58f390d..e9028180 100644 --- a/packages/core/src/scoped-stack.ts +++ b/packages/core/src/scoped-stack.ts @@ -91,6 +91,8 @@ import { UNGRANTABLE_SYSTEM_TYPES, } from './grants.js'; import { bindingFieldsOf } from './identity-bindings.js'; +import { checkFamily, WRITE_EXPECTATION } from './write-expectation.js'; +import type { ExpectationOptions } from './write-expectation.js'; import { claimedFamilies, familyStanding, linkedIds, INSTALL_APP_LABEL } from './install.js'; import { assertAttachmentSize } from './limits.js'; import { validateIdTimestampSkew, validateRecordId } from './record-id.js'; @@ -621,12 +623,13 @@ export class ScopedStack implements StackClient { */ private async requireUpdatable( id: string, - opts: { mutating?: boolean } = {}, + opts: { mutating?: boolean; expect?: ExpectationOptions } = {}, ): Promise { // The `_grant` write fence applies to mutating callers only: reading a // grant Record's history is not the escalation that fence exists to stop. return this.requireVerb(id, ['update-own', 'update-any'], { fenceGrantRecord: opts.mutating ?? true, + expect: opts.expect, }); } @@ -640,10 +643,13 @@ export class ScopedStack implements StackClient { private async requireVerb( id: string, actions: GrantAction[], - opts: { fenceGrantRecord: boolean }, + opts: { fenceGrantRecord: boolean; expect?: ExpectationOptions }, ): Promise { const record = await this.stack.get(id, { includeDeleted: true }); if (!record) throw new StackNotFoundError(`Record not found: "${id}"`); + // Ahead of the gate, so another family's record is not found whatever + // this requester may do to it — the answer a typed get() gives. + checkFamily(record, opts.expect); if (opts.fenceGrantRecord) await this.requireOwnerForGrantRecord(record); if (isGroupRecord(record)) { if (!this.isGroupManager(record)) { @@ -675,10 +681,10 @@ export class ScopedStack implements StackClient { * Fetch a record the subject can reach and the principal holds `delete` on * (via permissions or a delete grant), or throw. */ - private requireDeletable(id: string): Promise { + private requireDeletable(id: string, expect?: ExpectationOptions): Promise { // No soft-delete refusal: undelete() shares this gate, and a tombstone // is exactly what it addresses. - return this.requireVerb(id, ['delete-own', 'delete-any'], { fenceGrantRecord: true }); + return this.requireVerb(id, ['delete-own', 'delete-any'], { fenceGrantRecord: true, expect }); } /** @@ -1094,7 +1100,7 @@ export class ScopedStack implements StackClient { } const id = first; const changes = second as RecordChangeSet; - const opts = (third ?? {}) as IfVersionOptions; + const opts = (third ?? {}) as IfVersionOptions & ExpectationOptions; // Ahead of every gate: a malformed change set is a validation error // for every requester, rather than one for the owner and a permission // refusal for everyone else. @@ -1129,7 +1135,9 @@ export class ScopedStack implements StackClient { // reshare keys need the record before their own gate, and a change set // carrying only those must not be held to the write gate it doesn't // need. One read either way. - const record = writes ? await this.requireUpdatable(id) : await this.requireReshareable(id); + const record = writes + ? await this.requireUpdatable(id, { expect: opts }) + : await this.requireReshareable(id, opts); // Narrowed to what the gate below actually authorized. A key the gate // read as inert is dropped rather than forwarded: `Stack` recomputes // the delta against its own read of the record, so a key left standing @@ -1213,9 +1221,10 @@ export class ScopedStack implements StackClient { * `permissions` and `unlisted` keys carry, which the write bit * deliberately does not confer. */ - private async requireReshareable(id: string): Promise { + private async requireReshareable(id: string, expect?: ExpectationOptions): Promise { const record = await this.stack.get(id, { includeDeleted: true }); if (!record) throw new StackNotFoundError(`Record not found: "${id}"`); + checkFamily(record, expect); await this.requireOwnerForGrantRecord(record); await this.requireReshareOf(record); return record; @@ -1350,21 +1359,31 @@ export class ScopedStack implements StackClient { * escalation the partition exists to stop, decided before any record is * read so it cannot depend on who is asking. */ - async associate(id: RecordId, associations: DataAssociation[]): Promise { + async associate( + id: RecordId, + associations: DataAssociation[], + opts: ExpectationOptions = {}, + ): Promise { assertAssociationList(associations, 'associate()', 'associations', 'data'); return this.amendAssociations( id, associations.map((association) => ({ op: 'add', association })), + opts, 'associate()', ); } /** See associate() — the same write gate, the same kind refusal. */ - async dissociate(id: RecordId, associations: DataAssociation[]): Promise { + async dissociate( + id: RecordId, + associations: DataAssociation[], + opts: ExpectationOptions = {}, + ): Promise { assertAssociationList(associations, 'dissociate()', 'associations', 'data'); return this.amendAssociations( id, associations.map((association) => ({ op: 'remove', association })), + opts, 'dissociate()', ); } @@ -1377,15 +1396,16 @@ export class ScopedStack implements StackClient { async amendAssociations( id: RecordId, changes: AssociationEdit[], + opts: ExpectationOptions = {}, surface = 'amendAssociations()', ): Promise { assertAssociationEdits(changes, surface, 'data'); - const record = await this.requireUpdatable(id); + const record = await this.requireUpdatable(id, { expect: opts }); for (const change of changes) { if (change.op === 'add') await this.requireAssociationAccess(record.typeId, change.association); } - return this.stack.amendAssociations(id, changes, this.actor, surface); + return this.stack.amendAssociations(id, changes, { ...opts, ...this.actor }, surface); } /** @@ -1396,21 +1416,31 @@ export class ScopedStack implements StackClient { * scoping access is for. * See docs/spec/access-control.md § Record-level permissions. */ - async grantAccess(id: RecordId, permissions: AuthorityAssociation[]): Promise { + async grantAccess( + id: RecordId, + permissions: AuthorityAssociation[], + opts: ExpectationOptions = {}, + ): Promise { assertAssociationList(permissions, 'grantAccess()', 'permissions', 'authority'); return this.amendAccess( id, permissions.map((association) => ({ op: 'add', association })), + opts, 'grantAccess()', ); } /** Withdraw elements of who reaches a record — see grantAccess(). */ - async revokeAccess(id: RecordId, permissions: AuthorityAssociation[]): Promise { + async revokeAccess( + id: RecordId, + permissions: AuthorityAssociation[], + opts: ExpectationOptions = {}, + ): Promise { assertAssociationList(permissions, 'revokeAccess()', 'permissions', 'authority'); return this.amendAccess( id, permissions.map((association) => ({ op: 'remove', association })), + opts, 'revokeAccess()', ); } @@ -1419,11 +1449,12 @@ export class ScopedStack implements StackClient { async amendAccess( id: RecordId, changes: AssociationEdit[], + opts: ExpectationOptions = {}, surface = 'amendAccess()', ): Promise { assertAssociationEdits(changes, surface, 'authority'); - await this.requireReshareable(id); - return this.stack.amendAccess(id, changes, this.actor, surface); + await this.requireReshareable(id, opts); + return this.stack.amendAccess(id, changes, { ...opts, ...this.actor }, surface); } /** @@ -1432,7 +1463,10 @@ export class ScopedStack implements StackClient { * reach it, and delegation doesn't carry it either. Everyone else is * limited to soft delete. */ - async delete(id: RecordId, opts: DeleteRecordOptions = {}): Promise { + async delete( + id: RecordId, + opts: DeleteRecordOptions & ExpectationOptions = {}, + ): Promise { const { referencedFileIds } = await this.deleteAndReturn(id, opts); return { referencedFileIds }; } @@ -1446,13 +1480,16 @@ export class ScopedStack implements StackClient { */ async deleteAndReturn( id: RecordId, - opts: DeleteRecordOptions = {}, + opts: DeleteRecordOptions & ExpectationOptions = {}, ): Promise { - await this.requireDeletable(id); + await this.requireDeletable(id, opts); if (opts.purge && !this.ownerActingAlone) { throw new StackPermissionError('Purge is owner-only'); } - return this.stack.deleteAndReturn(id, { ...opts, ...this.actor }); + // Checked against the read above. Forwarded, it would cost a purge the + // read `Stack` otherwise skips, and a family cannot change in between. + const { [WRITE_EXPECTATION]: _checked, ...rest } = opts; + return this.stack.deleteAndReturn(id, { ...rest, ...this.actor }); } /** @@ -1460,8 +1497,11 @@ export class ScopedStack implements StackClient { * inverse of soft delete, so granting one direction without the other * would be backwards. Idempotent, per Stack.undelete(). */ - async undelete(id: RecordId, opts: IfVersionOptions = {}): Promise { - await this.requireDeletable(id); + async undelete( + id: RecordId, + opts: IfVersionOptions & ExpectationOptions = {}, + ): Promise { + await this.requireDeletable(id, opts); return this.stack.undelete(id, { ...opts, ...this.actor }); } @@ -1508,9 +1548,9 @@ export class ScopedStack implements StackClient { async restoreVersion( id: RecordId, version: number, - opts: IfVersionOptions = {}, + opts: IfVersionOptions & ExpectationOptions = {}, ): Promise { - const record = await this.requireUpdatable(id); + const record = await this.requireUpdatable(id, { expect: opts }); if (!this.ownerActingAlone) { const target = await this.stack.getVersion(id, version); if (target) { diff --git a/packages/core/src/stack.ts b/packages/core/src/stack.ts index 13ec408e..1e90dde2 100644 --- a/packages/core/src/stack.ts +++ b/packages/core/src/stack.ts @@ -180,6 +180,8 @@ import { typedQuery, typedSubscribe, } from './type-handle.js'; +import { checkExpectation, WRITE_EXPECTATION } from './write-expectation.js'; +import type { ExpectationOptions } from './write-expectation.js'; import type { ContentOf, Migration, @@ -1284,7 +1286,7 @@ export class Stack implements StackClient { } const id = first; const changes = second as RecordChangeSet; - const opts = (third ?? {}) as IfVersionOptions & ActorOptions; + const opts = (third ?? {}) as IfVersionOptions & ActorOptions & ExpectationOptions; this.assertOpen(); assertNonEmptyChangeSet(changes); @@ -1292,6 +1294,7 @@ export class Stack implements StackClient { if (!existing) { throw new StackNotFoundError(`Record not found: "${id}"`); } + checkExpectation(existing, opts, changes.contentPatch !== undefined); this.refuseIfDeleted(existing); // Checked before validation, so a caller that lost the race learns its @@ -1506,7 +1509,7 @@ export class Stack implements StackClient { async associate( id: RecordId, associations: DataAssociation[], - opts: ActorOptions = {}, + opts: ActorOptions & ExpectationOptions = {}, ): Promise { assertAssociationList(associations, 'associate()', 'associations', 'data'); return this.amendAssociations( @@ -1527,7 +1530,7 @@ export class Stack implements StackClient { async dissociate( id: RecordId, associations: DataAssociation[], - opts: ActorOptions = {}, + opts: ActorOptions & ExpectationOptions = {}, ): Promise { assertAssociationList(associations, 'dissociate()', 'associations', 'data'); return this.amendAssociations( @@ -1548,12 +1551,13 @@ export class Stack implements StackClient { async amendAssociations( id: RecordId, changes: AssociationEdit[], - opts: ActorOptions = {}, + opts: ActorOptions & ExpectationOptions = {}, surface = 'amendAssociations()', ): Promise { this.assertOpen(); assertAssociationEdits(changes, surface, 'data'); const existing = await this.requireRecord(id); + checkExpectation(existing, opts, false); this.refuseIfDeleted(existing); const current = existing.associations ?? []; const next = applyAssociationEdits(current, changes) as DataAssociation[]; @@ -1588,7 +1592,7 @@ export class Stack implements StackClient { async grantAccess( id: RecordId, permissions: AuthorityAssociation[], - opts: ActorOptions = {}, + opts: ActorOptions & ExpectationOptions = {}, ): Promise { assertAssociationList(permissions, 'grantAccess()', 'permissions', 'authority'); return this.amendAccess( @@ -1607,7 +1611,7 @@ export class Stack implements StackClient { async revokeAccess( id: RecordId, permissions: AuthorityAssociation[], - opts: ActorOptions = {}, + opts: ActorOptions & ExpectationOptions = {}, ): Promise { assertAssociationList(permissions, 'revokeAccess()', 'permissions', 'authority'); return this.amendAccess( @@ -1626,12 +1630,13 @@ export class Stack implements StackClient { async amendAccess( id: RecordId, changes: AssociationEdit[], - opts: ActorOptions = {}, + opts: ActorOptions & ExpectationOptions = {}, surface = 'amendAccess()', ): Promise { this.assertOpen(); assertAssociationEdits(changes, surface, 'authority'); const existing = await this.requireRecord(id); + checkExpectation(existing, opts, false); this.refuseIfDeleted(existing); const current = existing.permissions ?? []; const next = applyAssociationEdits(current, changes) as AuthorityAssociation[]; @@ -1751,7 +1756,10 @@ export class Stack implements StackClient { * body from a separate read beforehand is exactly the race and the extra * round trip it exists to remove. */ - async delete(id: RecordId, opts: DeleteRecordOptions & ActorOptions = {}): Promise { + async delete( + id: RecordId, + opts: DeleteRecordOptions & ActorOptions & ExpectationOptions = {}, + ): Promise { const { referencedFileIds } = await this.deleteAndReturn(id, opts); return { referencedFileIds }; } @@ -1767,7 +1775,7 @@ export class Stack implements StackClient { */ async deleteAndReturn( id: RecordId, - opts: DeleteRecordOptions & ActorOptions = {}, + opts: DeleteRecordOptions & ActorOptions & ExpectationOptions = {}, ): Promise { this.assertOpen(); if (id === SYSTEM_TYPES.CONFIG) { @@ -1776,6 +1784,12 @@ export class Stack implements StackClient { ); } if (opts.purge) { + // A purge otherwise reads nothing first, so an expectation costs it + // one read. The family cannot change between the read and the write. + if (opts[WRITE_EXPECTATION]) { + const current = await this.adapter.getRecord(id); + if (current) checkExpectation(current, opts, false); + } // The adapter hands back what it destroyed, captured inside the same // write: a read here instead would race the delete, and afterwards // there is nothing left to read. Null means there was no record, so @@ -1795,6 +1809,7 @@ export class Stack implements StackClient { if (!existing) { throw new StackNotFoundError(`Record not found: "${id}"`); } + checkExpectation(existing, opts, false); this.checkIfVersion(existing, opts.ifVersion); // A tombstone's references stand — undelete() must find its // attachments intact — so nothing is stranded and nothing is reported. @@ -1834,12 +1849,16 @@ export class Stack implements StackClient { * throws StackNotFoundError for them just like any other missing record. * Snapshots and bumps version, same as delete(). */ - async undelete(id: RecordId, opts: IfVersionOptions & ActorOptions = {}): Promise { + async undelete( + id: RecordId, + opts: IfVersionOptions & ActorOptions & ExpectationOptions = {}, + ): Promise { this.assertOpen(); const existing = await this.adapter.getRecord(id); if (!existing) { throw new StackNotFoundError(`Record not found: "${id}"`); } + checkExpectation(existing, opts, false); this.checkIfVersion(existing, opts.ifVersion); if (!existing.deletedAt) return existing; @@ -1960,13 +1979,16 @@ export class Stack implements StackClient { async restoreVersion( id: RecordId, version: number, - opts: IfVersionOptions & ActorOptions = {}, + opts: IfVersionOptions & ActorOptions & ExpectationOptions = {}, ): Promise { this.assertOpen(); const existing = await this.adapter.getRecord(id); if (!existing) { throw new StackNotFoundError(`Record not found: "${id}"`); } + // A snapshot is validated against its own stored Type, never the + // record's current one, so only the family is held to the expectation. + checkExpectation(existing, opts, false); this.refuseIfDeleted(existing); this.checkIfVersion(existing, opts.ifVersion); diff --git a/packages/core/src/write-expectation.ts b/packages/core/src/write-expectation.ts new file mode 100644 index 00000000..12908e19 --- /dev/null +++ b/packages/core/src/write-expectation.ts @@ -0,0 +1,69 @@ +/** + * Stack — Write expectations + * ------------------------------------------------------- + * What a typed collection expects of the record a write addresses: its + * family on every write, and its exact version on a write whose content is + * validated against the stored Type. `Stack` checks it against the read it + * already makes before writing, so a typed write costs what an untyped one + * does and is refused before anything lands. + * + * Internal by construction: the expectation travels under a module-private + * symbol key on a write verb's options, which `StackClient` never declares + * and the wire never carries. It needs no server-side counterpart because a + * record's family never changes — `typeId` moves only through a migration, + * and a migration stays within its family — so the check cannot race. + */ + +import { StackNotFoundError } from './errors.js'; +import { baseIdOf } from './schema.js'; +import type { BaseId, StackRecord, TypeId } from './types.js'; + +export const WRITE_EXPECTATION = Symbol('haverstack.writeExpectation'); + +export type WriteExpectation = { + readonly baseId: BaseId; + /** Checked only on a write that patches content. */ + readonly typeId?: TypeId; +}; + +export type ExpectationOptions = { readonly [WRITE_EXPECTATION]?: WriteExpectation }; + +/** + * A content write addressed a record stored at another version of the + * expected family. Carries the record as read, so the collection can + * migrate it and retry without reading it again. Never reaches the wire. + */ +export class StoredVersionError extends Error { + constructor(readonly record: StackRecord) { + super(`Record "${record.id}" is stored at ${record.typeId}`); + this.name = 'StoredVersionError'; + } +} + +/** + * Refuse a record outside the expected family with the answer a typed + * `get()` of it gives: there is no such record in this family. + */ +export const checkFamily = (record: StackRecord, opts: ExpectationOptions | undefined): void => { + const expected = opts?.[WRITE_EXPECTATION]; + if (expected && baseIdOf(record.typeId) !== expected.baseId) { + throw new StackNotFoundError(`Record not found: "${record.id}"`); + } +}; + +/** + * checkFamily(), then the exact version when the write patches content: a + * patch is validated against the record's stored Type, so one written for + * another version would be checked by a schema it was not written for. + */ +export const checkExpectation = ( + record: StackRecord, + opts: ExpectationOptions | undefined, + patchesContent: boolean, +): void => { + checkFamily(record, opts); + const typeId = opts?.[WRITE_EXPECTATION]?.typeId; + if (patchesContent && typeId !== undefined && record.typeId !== typeId) { + throw new StoredVersionError(record); + } +}; diff --git a/packages/core/tests/write-expectation.test.ts b/packages/core/tests/write-expectation.test.ts new file mode 100644 index 00000000..a8e1ab2a --- /dev/null +++ b/packages/core/tests/write-expectation.test.ts @@ -0,0 +1,171 @@ +import { describe, test, expect, beforeEach, vi } from 'vitest'; +import { Stack } from '../src/stack.js'; +import { MemoryAdapter } from '../src/testing.js'; +import { typeHandle } from '../src/type-handle.js'; +import { StackNotFoundError } from '../src/errors.js'; +import { StoredVersionError, WRITE_EXPECTATION } from '../src/write-expectation.js'; +import type { WriteExpectation } from '../src/write-expectation.js'; +import type { ScopedStack } from '../src/scoped-stack.js'; +import type { AuthorityAssociation, DataAssociation, RecordId } from '../src/types.js'; + +const OWNER = 'owner-123'; +const MEMBER = 'member-456'; + +const BookV1 = typeHandle({ + id: 'com.example.reading/book@1', + name: 'Book', + schema: { title: { kind: 'string', required: true } }, +}); + +const Book = typeHandle({ + id: 'com.example.reading/book@2', + name: 'Book', + migratesFrom: BookV1, + schema: { title: { kind: 'string', required: true }, pages: { kind: 'number' } }, +}); + +const Shelf = typeHandle({ + id: 'com.example.reading/shelf@1', + name: 'Shelf', + schema: { name: { kind: 'string', required: true } }, +}); + +const TAG: DataAssociation = { kind: 'tag', label: 'favourite' }; +const SHARE: AuthorityAssociation = { + kind: 'permission', + label: 'read', + grantee: { kind: 'entity', entityId: MEMBER }, +}; + +// The options a collection passes; the verbs' public types never name the key. +const expecting = (expectation: WriteExpectation, rest: object = {}) => + ({ ...rest, [WRITE_EXPECTATION]: expectation }) as never; + +const BOOK_FAMILY: WriteExpectation = { baseId: Book.baseId, typeId: Book.id }; + +type Verb = (client: Stack | ScopedStack, id: RecordId, opts?: object) => Promise; + +// Each verb with arguments that would write had the family matched. A +// call without options passes `undefined`, as an untyped caller would. +const VERBS: Record = { + mutate: (c, id, o) => c.mutate(id, { contentPatch: { name: 'Moved' } }, o as never), + patchContent: (c, id, o) => c.patchContent(id, { name: 'Moved' }, o as never), + associate: (c, id, o) => c.associate(id, [TAG], o as never), + dissociate: (c, id, o) => c.dissociate(id, [TAG], o as never), + amendAssociations: (c, id, o) => + c.amendAssociations(id, [{ op: 'add', association: TAG }], o as never), + grantAccess: (c, id, o) => c.grantAccess(id, [SHARE], o as never), + revokeAccess: (c, id, o) => c.revokeAccess(id, [SHARE], o as never), + amendAccess: (c, id, o) => c.amendAccess(id, [{ op: 'add', association: SHARE }], o as never), + delete: (c, id, o) => c.delete(id, o as never), + purge: (c, id, o) => c.delete(id, { ...o, purge: true } as never), + undelete: (c, id, o) => c.undelete(id, o as never), + restoreVersion: (c, id, o) => c.restoreVersion(id, 1, o as never), +}; + +let adapter: MemoryAdapter; +let stack: Stack; + +beforeEach(async () => { + adapter = await MemoryAdapter.open({ ownerEntityId: OWNER, timezone: 'UTC' }); + stack = await Stack.open(adapter); + await stack.defineType(BookV1); + await stack.defineType(Book); + await stack.defineType(Shelf); +}); + +/** A shelf at version 2 carrying TAG and SHARE, so every verb has something to move. */ +const seedShelf = async (): Promise => { + const shelf = await stack.create(Shelf.id, { name: 'Kitchen' }); + await stack.patchContent(shelf.id, { name: 'Hall' }); + await stack.associate(shelf.id, [TAG]); + return shelf.id; +}; + +const clients: Record Stack | ScopedStack> = { + Stack: () => stack, + ScopedStack: () => stack.asEntity(OWNER), +}; + +describe.each(Object.keys(clients))('a write expectation on %s', (clientName) => { + const client = () => clients[clientName]!(); + + test.each(Object.keys(VERBS))( + '%s() refuses another family before writing anything', + async (verb) => { + const id = await seedShelf(); + if (verb === 'undelete') await stack.delete(id); + const before = await adapter.getRecord(id); + const journal = await stack.getJournal(id); + + await expect(VERBS[verb]!(client(), id, expecting(BOOK_FAMILY))).rejects.toThrow( + StackNotFoundError, + ); + + expect(await adapter.getRecord(id)).toEqual(before); + expect(await stack.getJournal(id)).toEqual(journal); + }, + ); + + test.each(Object.keys(VERBS))( + '%s() under an expectation reads the record no more often than without one', + async (verb) => { + // A purge on the unscoped Stack reads nothing on its own, so the + // expectation is the one read it makes. + if (verb === 'purge' && clientName === 'Stack') return; + const count = async (opts?: object): Promise => { + const { id } = await stack.create(Book.id, { title: 'Dune' }); + await stack.patchContent(id, { title: 'Dune Messiah' }); + if (verb === 'dissociate') await stack.associate(id, [TAG]); + if (verb === 'undelete') await stack.delete(id); + const spy = vi.spyOn(adapter, 'getRecord'); + await VERBS[verb]!(client(), id, opts).catch(() => {}); + const calls = spy.mock.calls.length; + spy.mockRestore(); + return calls; + }; + const plain = await count(); + expect(await count(expecting(BOOK_FAMILY))).toBe(plain); + }, + ); + + test('a content write to a record of an older version carries the stored record', async () => { + const old = await stack.create(BookV1.id, { title: 'Dune' }); + + const err = await client() + .patchContent(old.id, { pages: 412 }, expecting(BOOK_FAMILY)) + .catch((e: unknown) => e); + + expect(err).toBeInstanceOf(StoredVersionError); + expect((err as StoredVersionError).record).toEqual(old); + expect(await adapter.getRecord(old.id)).toEqual(old); + }); + + test('a write that leaves content alone holds only the family', async () => { + const old = await stack.create(BookV1.id, { title: 'Dune' }); + + const tagged = await client().associate(old.id, [TAG], expecting(BOOK_FAMILY)); + + expect(tagged.associations).toEqual([TAG]); + }); + + test('a write within the family goes through', async () => { + const book = await stack.create(Book.id, { title: 'Dune' }); + + const patched = await client().patchContent(book.id, { pages: 412 }, expecting(BOOK_FAMILY)); + + expect(patched.content).toEqual({ title: 'Dune', pages: 412 }); + }); +}); + +describe('a write expectation under delegation', () => { + test('another family is not found, even where the write itself would be refused', async () => { + const shelf = await stack.create(Shelf.id, { name: 'Kitchen' }, { permissions: [SHARE] }); + const member = stack.asEntity(MEMBER); + + await expect(member.patchContent(shelf.id, { name: 'Hall' })).rejects.toThrow('Cannot update'); + await expect( + member.patchContent(shelf.id, { name: 'Hall' }, expecting(BOOK_FAMILY)), + ).rejects.toThrow(StackNotFoundError); + }); +}); From 74c834b9e8d38a5badb209f33f07fbfec78cd21e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 12:45:51 +0000 Subject: [PATCH 2/2] fix(core): order write-expectation checks and route typed mutate through them - Stack.mutate checks the stored version after the tombstone and ifVersion refusals, so a deleted record is never handed back in a StoredVersionError and a stale caller gets a version conflict first. - ScopedStack skips its content gates when the patch targets another version, and pins ifVersion to its read so Stack must refuse it. - A purge under an expectation is pinned to the version it checked, and purges nothing when the record is missing. ScopedStack pins its delete the same way. - typedMutate passes an exact expectation instead of its own pre-read: one read per write, and no race between the check and the write. - Changeset raised to minor; seedShelf now grants SHARE as documented. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pzh2Awag1hD5tK4Y2yarfz --- .changeset/write-expectation.md | 4 +- docs/spec/data-model.md | 2 +- packages/core/src/scoped-stack.ts | 26 +++++++--- packages/core/src/stack.ts | 25 +++++---- packages/core/src/type-handle.ts | 19 ++++--- packages/core/src/write-expectation.ts | 33 ++++++++---- packages/core/tests/type-handle.test.ts | 36 +++++++++++-- packages/core/tests/write-expectation.test.ts | 51 ++++++++++++++++++- 8 files changed, 151 insertions(+), 45 deletions(-) diff --git a/.changeset/write-expectation.md b/.changeset/write-expectation.md index 9724a08d..d53445ac 100644 --- a/.changeset/write-expectation.md +++ b/.changeset/write-expectation.md @@ -1,5 +1,5 @@ --- -'@haverstack/core': patch +'@haverstack/core': minor --- -`Stack` and `ScopedStack` accept an internal write expectation on every write verb — the record's Type family, and on a content write its exact version — checked against the read each write already makes, so a typed collection's write is refused before anything lands and costs no extra round trip. +`Stack` and `ScopedStack` accept an internal write expectation on every write verb — the record's Type family, and on a content write its exact version — checked against the read each write already makes, so a typed write is refused before anything lands and costs no extra round trip. A typed `mutate()` or `patchContent()` now uses it: a record of another family is not found, and one stored at another version of the family is still refused with `StackBadRequestError`. `ScopedStack.amendAssociations()` and `amendAccess()` take their options third and `surface` fourth, as `Stack`'s do. diff --git a/docs/spec/data-model.md b/docs/spec/data-model.md index f52f0086..cf9b4cf1 100644 --- a/docs/spec/data-model.md +++ b/docs/spec/data-model.md @@ -411,7 +411,7 @@ A handle names exactly one version. A call that means the whole family takes `ha A typed `subscribe()` delivers only changes to records of exactly the handle's `id`, with `record`, when present, typed as the content. An event carries its record as stored, which a subscription cannot migrate, so other versions of the family are not delivered. A record holding an enum value the handle does not list arrives without its `record`, as it does wherever the emitter cannot supply one; the typed `get()` then says why. The change filter takes every key but `typeId` and `baseId`. -A typed `query()` matches the handle's whole family, then applies the checks above to every record. A typed write first reads the record and refuses one stored at another `typeId`, since a patch is validated against the record's own stored Type; a missing, unreadable or deleted record is left to the untyped write to refuse. +A typed `query()` matches the handle's whole family, then applies the checks above to every record. A typed write is checked against the read the write already makes, before anything lands. A record of another family is not found. A record of the family stored at another `typeId` is refused with `StackBadRequestError`, since a patch is validated against the record's own stored Type and the result could not be typed; a deleted record, or a stale `ifVersion`, is refused first, as the untyped write refuses it. The derived types are a convenience over [runtime validation](#types), which stays the guarantee. A Type known only at runtime has no static shape and stays `Record`. diff --git a/packages/core/src/scoped-stack.ts b/packages/core/src/scoped-stack.ts index e9028180..74b8d8dc 100644 --- a/packages/core/src/scoped-stack.ts +++ b/packages/core/src/scoped-stack.ts @@ -91,7 +91,7 @@ import { UNGRANTABLE_SYSTEM_TYPES, } from './grants.js'; import { bindingFieldsOf } from './identity-bindings.js'; -import { checkFamily, WRITE_EXPECTATION } from './write-expectation.js'; +import { checkFamily, storedVersionDiffers, WRITE_EXPECTATION } from './write-expectation.js'; import type { ExpectationOptions } from './write-expectation.js'; import { claimedFamilies, familyStanding, linkedIds, INSTALL_APP_LABEL } from './install.js'; import { assertAttachmentSize } from './limits.js'; @@ -1155,7 +1155,13 @@ export class ScopedStack implements StackClient { } } - if (changes.contentPatch) { + // A patch for another version is refused by `Stack`, which owes the + // tombstone and `ifVersion` refusals first; the content gates would read + // a stored Type it was not written for. Pinned to this read, `Stack` + // cannot find a record the patch lands on. + const mismatched = storedVersionDiffers(record, opts, changes.contentPatch !== undefined); + + if (changes.contentPatch && !mismatched) { const patch = changes.contentPatch; // Value-wise, not presence-wise: a client that reads a card, edits // its `name` and sends the whole content object back is not setting @@ -1188,7 +1194,8 @@ export class ScopedStack implements StackClient { ); } - return this.stack.mutate(id, authorized, { ...opts, ...this.actor }); + const ifVersion = mismatched ? (opts.ifVersion ?? record.version) : opts.ifVersion; + return this.stack.mutate(id, authorized, { ...opts, ifVersion, ...this.actor }); } async patchContent( @@ -1482,14 +1489,19 @@ export class ScopedStack implements StackClient { id: RecordId, opts: DeleteRecordOptions & ExpectationOptions = {}, ): Promise { - await this.requireDeletable(id, opts); + const record = await this.requireDeletable(id, opts); if (opts.purge && !this.ownerActingAlone) { throw new StackPermissionError('Purge is owner-only'); } - // Checked against the read above. Forwarded, it would cost a purge the - // read `Stack` otherwise skips, and a family cannot change in between. + if (!opts[WRITE_EXPECTATION]) return this.stack.deleteAndReturn(id, { ...opts, ...this.actor }); + // Checked against the read above, which the delete is pinned to. + // Forwarded, it would cost a purge the read `Stack` otherwise skips. const { [WRITE_EXPECTATION]: _checked, ...rest } = opts; - return this.stack.deleteAndReturn(id, { ...rest, ...this.actor }); + return this.stack.deleteAndReturn(id, { + ...rest, + ifVersion: opts.ifVersion ?? record.version, + ...this.actor, + }); } /** diff --git a/packages/core/src/stack.ts b/packages/core/src/stack.ts index 1e90dde2..03616cdb 100644 --- a/packages/core/src/stack.ts +++ b/packages/core/src/stack.ts @@ -180,7 +180,7 @@ import { typedQuery, typedSubscribe, } from './type-handle.js'; -import { checkExpectation, WRITE_EXPECTATION } from './write-expectation.js'; +import { checkFamily, checkStoredVersion, WRITE_EXPECTATION } from './write-expectation.js'; import type { ExpectationOptions } from './write-expectation.js'; import type { ContentOf, @@ -1294,7 +1294,7 @@ export class Stack implements StackClient { if (!existing) { throw new StackNotFoundError(`Record not found: "${id}"`); } - checkExpectation(existing, opts, changes.contentPatch !== undefined); + checkFamily(existing, opts); this.refuseIfDeleted(existing); // Checked before validation, so a caller that lost the race learns its @@ -1302,6 +1302,7 @@ export class Stack implements StackClient { // no-op short-circuit below, so a precondition is never satisfied by a // write that turned out to move nothing. if (takesIfVersion(changes)) this.checkIfVersion(existing, opts.ifVersion); + checkStoredVersion(existing, opts, changes.contentPatch !== undefined); const merged = await this.validateChangeSet(id, existing, changes); @@ -1557,7 +1558,7 @@ export class Stack implements StackClient { this.assertOpen(); assertAssociationEdits(changes, surface, 'data'); const existing = await this.requireRecord(id); - checkExpectation(existing, opts, false); + checkFamily(existing, opts); this.refuseIfDeleted(existing); const current = existing.associations ?? []; const next = applyAssociationEdits(current, changes) as DataAssociation[]; @@ -1636,7 +1637,7 @@ export class Stack implements StackClient { this.assertOpen(); assertAssociationEdits(changes, surface, 'authority'); const existing = await this.requireRecord(id); - checkExpectation(existing, opts, false); + checkFamily(existing, opts); this.refuseIfDeleted(existing); const current = existing.permissions ?? []; const next = applyAssociationEdits(current, changes) as AuthorityAssociation[]; @@ -1785,10 +1786,14 @@ export class Stack implements StackClient { } if (opts.purge) { // A purge otherwise reads nothing first, so an expectation costs it - // one read. The family cannot change between the read and the write. + // one read, and the purge is pinned to that read: a write in between + // is a conflict, never a purge of a record that was not checked. + let ifVersion = opts.ifVersion; if (opts[WRITE_EXPECTATION]) { const current = await this.adapter.getRecord(id); - if (current) checkExpectation(current, opts, false); + if (!current) return { record: null, referencedFileIds: [] }; + checkFamily(current, opts); + ifVersion ??= current.version; } // The adapter hands back what it destroyed, captured inside the same // write: a read here instead would race the delete, and afterwards @@ -1797,7 +1802,7 @@ export class Stack implements StackClient { const change = new PendingChange('purge', { actor: normalizeActor(opts.actor) }); const purged = await this.adapter.deleteRecord(id, { purge: true, - ifVersion: opts.ifVersion, + ifVersion, journal: change.journal, }); if (!purged) return { record: null, referencedFileIds: [] }; @@ -1809,7 +1814,7 @@ export class Stack implements StackClient { if (!existing) { throw new StackNotFoundError(`Record not found: "${id}"`); } - checkExpectation(existing, opts, false); + checkFamily(existing, opts); this.checkIfVersion(existing, opts.ifVersion); // A tombstone's references stand — undelete() must find its // attachments intact — so nothing is stranded and nothing is reported. @@ -1858,7 +1863,7 @@ export class Stack implements StackClient { if (!existing) { throw new StackNotFoundError(`Record not found: "${id}"`); } - checkExpectation(existing, opts, false); + checkFamily(existing, opts); this.checkIfVersion(existing, opts.ifVersion); if (!existing.deletedAt) return existing; @@ -1988,7 +1993,7 @@ export class Stack implements StackClient { } // A snapshot is validated against its own stored Type, never the // record's current one, so only the family is held to the expectation. - checkExpectation(existing, opts, false); + checkFamily(existing, opts); this.refuseIfDeleted(existing); this.checkIfVersion(existing, opts.ifVersion); diff --git a/packages/core/src/type-handle.ts b/packages/core/src/type-handle.ts index f58cb8dc..29a499af 100644 --- a/packages/core/src/type-handle.ts +++ b/packages/core/src/type-handle.ts @@ -13,6 +13,7 @@ import { StackBadRequestError, StackMigrationError, StackValidationError } from './errors.js'; import { lineageProblem, parseTypeId } from './schema.js'; +import { StoredVersionError, WRITE_EXPECTATION } from './write-expectation.js'; import type { ValidationError } from './validate.js'; import type { CreateRecordOptions, StackClient } from './stack.js'; import type { @@ -375,15 +376,17 @@ export const typedMutate = async ( changes: TypedChangeSet, opts?: IfVersionOptions, ): Promise> => { - // A patch is validated against the record's own stored Type, so one made - // through a handle for another version would be checked by a schema it - // was not written for. A missing, unreadable or deleted record is left to - // mutate(), which refuses it in its own words. - const current = await client.get(id, { includeDeleted: true }); - if (current && current.typeId !== handle.id) { - throw new StackBadRequestError(`Record "${id}" is ${current.typeId}, not ${handle.id}`); + // Checked against the read the write already makes, so nothing lands on + // a record the handle does not type. See docs/spec/data-model.md § Type handles. + const expect = { + [WRITE_EXPECTATION]: { baseId: handle.baseId, typeId: handle.id, exact: true }, + }; + try { + return narrowRecord(handle, await client.mutate(id, changes, { ...opts, ...expect } as never)); + } catch (err) { + if (!(err instanceof StoredVersionError)) throw err; + throw new StackBadRequestError(`Record "${id}" is ${err.record.typeId}, not ${handle.id}`); } - return narrowRecord(handle, await client.mutate(id, changes, opts)); }; export const typedSubscribe = ( diff --git a/packages/core/src/write-expectation.ts b/packages/core/src/write-expectation.ts index 12908e19..a8695e53 100644 --- a/packages/core/src/write-expectation.ts +++ b/packages/core/src/write-expectation.ts @@ -11,7 +11,9 @@ * symbol key on a write verb's options, which `StackClient` never declares * and the wire never carries. It needs no server-side counterpart because a * record's family never changes — `typeId` moves only through a migration, - * and a migration stays within its family — so the check cannot race. + * and a migration stays within its family. A write that does not re-read + * the record pins `ifVersion` to the read the check was made against, so a + * write landing in between is a conflict rather than an unchecked write. */ import { StackNotFoundError } from './errors.js'; @@ -22,8 +24,10 @@ export const WRITE_EXPECTATION = Symbol('haverstack.writeExpectation'); export type WriteExpectation = { readonly baseId: BaseId; - /** Checked only on a write that patches content. */ + /** Checked only on a write that patches content, unless `exact`. */ readonly typeId?: TypeId; + /** Hold every write to `typeId`, as a typed handle's write is. */ + readonly exact?: boolean; }; export type ExpectationOptions = { readonly [WRITE_EXPECTATION]?: WriteExpectation }; @@ -52,18 +56,25 @@ export const checkFamily = (record: StackRecord, opts: ExpectationOptions | unde }; /** - * checkFamily(), then the exact version when the write patches content: a - * patch is validated against the record's stored Type, so one written for - * another version would be checked by a schema it was not written for. + * The exact version, on a write that patches content: a patch is validated + * against the record's stored Type, so one written for another version + * would be checked by a schema it was not written for. Called after the + * tombstone and `ifVersion` refusals, which a caller is owed first. */ -export const checkExpectation = ( +export const checkStoredVersion = ( record: StackRecord, opts: ExpectationOptions | undefined, patchesContent: boolean, ): void => { - checkFamily(record, opts); - const typeId = opts?.[WRITE_EXPECTATION]?.typeId; - if (patchesContent && typeId !== undefined && record.typeId !== typeId) { - throw new StoredVersionError(record); - } + if (storedVersionDiffers(record, opts, patchesContent)) throw new StoredVersionError(record); +}; + +export const storedVersionDiffers = ( + record: StackRecord, + opts: ExpectationOptions | undefined, + patchesContent: boolean, +): boolean => { + const expected = opts?.[WRITE_EXPECTATION]; + if (expected?.typeId === undefined || record.typeId === expected.typeId) return false; + return patchesContent || expected.exact === true; }; diff --git a/packages/core/tests/type-handle.test.ts b/packages/core/tests/type-handle.test.ts index 122a8306..066d68dc 100644 --- a/packages/core/tests/type-handle.test.ts +++ b/packages/core/tests/type-handle.test.ts @@ -1,9 +1,14 @@ -import { describe, test, expect, expectTypeOf, beforeEach } from 'vitest'; +import { describe, test, expect, expectTypeOf, beforeEach, vi } from 'vitest'; import { Stack } from '../src/stack.js'; import { MemoryAdapter } from '../src/testing.js'; import { migration, typeHandle } from '../src/type-handle.js'; import type { ContentOf, PatchOf, TypedChange, TypedRecord } from '../src/type-handle.js'; -import { StackBadRequestError, StackMigrationError, StackValidationError } from '../src/errors.js'; +import { + StackBadRequestError, + StackMigrationError, + StackNotFoundError, + StackValidationError, +} from '../src/errors.js'; import type { StackClient } from '../src/stack.js'; import type { TypeId, TypeSchema } from '../src/types.js'; @@ -446,14 +451,37 @@ describe.each([ expect(seen[0].record).toBeUndefined(); }); - test('a typed write to a record of another Type throws before writing', async () => { + test('a typed write to a record of another family finds no record and writes nothing', async () => { const shelf = await client.create(Shelf, { name: 'Fiction' }); await expect(client.patchContent(Book, shelf.id, { title: 'x' })).rejects.toThrow( - StackBadRequestError, + StackNotFoundError, ); expect((await stack.get(shelf.id))?.version).toBe(shelf.version); }); + test('a typed write to another version of the family throws before writing', async () => { + await stack.defineType(BookV1); + const old = await client.create(BookV1, { title: 'Dune', status: 'want' }); + await expect(client.patchContent(Book, old.id, { title: 'x' })).rejects.toThrow( + `Record "${old.id}" is com.example.reading/book@1, not com.example.reading/book@2`, + ); + await expect( + client.mutate(Book, old.id, { associations: [{ kind: 'tag', label: 'x' }] }), + ).rejects.toThrow(StackBadRequestError); + expect(await adapter.getRecord(old.id)).toEqual(old); + }); + + test('a typed write reads the record no more often than an untyped one', async () => { + const book = await client.create(Book, { title: 'Dune', status: 'want' }); + const spy = vi.spyOn(adapter, 'getRecord'); + await client.patchContent(book.id, { title: 'Emma' }); + const untyped = spy.mock.calls.length; + spy.mockClear(); + await client.patchContent(Book, book.id, { title: 'Dune' }); + expect(spy.mock.calls.length).toBe(untyped); + spy.mockRestore(); + }); + test('a typed write to a tombstone is refused as an untyped one is', async () => { const book = await client.create(Book, { title: 'Dune', status: 'want' }); await client.delete(book.id); diff --git a/packages/core/tests/write-expectation.test.ts b/packages/core/tests/write-expectation.test.ts index a8e1ab2a..60a17e31 100644 --- a/packages/core/tests/write-expectation.test.ts +++ b/packages/core/tests/write-expectation.test.ts @@ -2,7 +2,11 @@ import { describe, test, expect, beforeEach, vi } from 'vitest'; import { Stack } from '../src/stack.js'; import { MemoryAdapter } from '../src/testing.js'; import { typeHandle } from '../src/type-handle.js'; -import { StackNotFoundError } from '../src/errors.js'; +import { + StackConflictError, + StackNotFoundError, + StackVersionConflictError, +} from '../src/errors.js'; import { StoredVersionError, WRITE_EXPECTATION } from '../src/write-expectation.js'; import type { WriteExpectation } from '../src/write-expectation.js'; import type { ScopedStack } from '../src/scoped-stack.js'; @@ -74,11 +78,12 @@ beforeEach(async () => { await stack.defineType(Shelf); }); -/** A shelf at version 2 carrying TAG and SHARE, so every verb has something to move. */ +/** A shelf with a version 1 to restore, carrying TAG and SHARE, so every verb has something to move. */ const seedShelf = async (): Promise => { const shelf = await stack.create(Shelf.id, { name: 'Kitchen' }); await stack.patchContent(shelf.id, { name: 'Hall' }); await stack.associate(shelf.id, [TAG]); + await stack.grantAccess(shelf.id, [SHARE]); return shelf.id; }; @@ -141,6 +146,48 @@ describe.each(Object.keys(clients))('a write expectation on %s', (clientName) => expect(await adapter.getRecord(old.id)).toEqual(old); }); + test('a deleted record is refused as deleted, whatever version it is stored at', async () => { + const old = await stack.create(BookV1.id, { title: 'Dune' }); + await stack.delete(old.id); + + const err = await client() + .patchContent(old.id, { pages: 412 }, expecting(BOOK_FAMILY)) + .catch((e: unknown) => e); + + expect(err).toBeInstanceOf(StackConflictError); + expect(err).not.toBeInstanceOf(StoredVersionError); + }); + + test('a stale ifVersion is refused ahead of the stored version', async () => { + const old = await stack.create(BookV1.id, { title: 'Dune' }); + await stack.patchContent(old.id, { title: 'Dune Messiah' }); + + await expect( + client().patchContent(old.id, { pages: 412 }, expecting(BOOK_FAMILY, { ifVersion: 1 })), + ).rejects.toThrow(StackVersionConflictError); + }); + + test('a purge under an expectation is pinned to the version it checked', async () => { + const book = await stack.create(Book.id, { title: 'Dune' }); + const getRecord = adapter.getRecord.bind(adapter); + vi.spyOn(adapter, 'getRecord').mockImplementationOnce(async (id) => { + const read = await getRecord(id); + await stack.patchContent(book.id, { pages: 412 }); + return read; + }); + + await expect(client().delete(book.id, expecting(BOOK_FAMILY, { purge: true }))).rejects.toThrow( + StackVersionConflictError, + ); + expect(await adapter.getRecord(book.id)).not.toBeNull(); + }); + + test('a purge under an expectation of a missing record purges nothing', async () => { + if (clientName === 'ScopedStack') return; + const result = await stack.deleteAndReturn('missing', expecting(BOOK_FAMILY, { purge: true })); + expect(result).toEqual({ record: null, referencedFileIds: [] }); + }); + test('a write that leaves content alone holds only the family', async () => { const old = await stack.create(BookV1.id, { title: 'Dune' });