From 6a359b1ac4c94c08dfa0fadd8dc02dc63d064015 Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Thu, 10 Sep 2026 13:55:34 +0000 Subject: [PATCH 01/14] Add the create/update/delete executors and the all-or-nothing batch coordinator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A batch of card writes needs to land together: several changes to several cards that either all commit or none do, under one index job and one index event. This adds the pieces that make that possible. Each base write operation becomes a pure staging function in card-operations/executors.ts: it reads the batch's pre-loaded state and returns the exact bytes every file it touches should hold, writing nothing. `stageCreate` decides a new card's URL and directory, resolves its links and serializes it — from a JSON:API document, or from a named create's lowered `of` + `fill` template. `stageUpdate` merges a patch over the stored file under the same rules the PATCH handler applies. `stageDelete` establishes that its target is there to remove. card-operations/coordinator.ts takes the realm's write lock once, resolves every `lid` up front (path math over the type a card adopts, so a create's URL is known before anything is written and a later entry can link to it), runs every executor in memory, and only then commits. A refusal therefore happens while the realm is still untouched. Realm gains `_commitBatchUnlocked`, which is the batch-write path extended with a removal leg so writes and removals commit under one index job and one index event, and `FileWriteResult.contentHash`, which is the version a caller passes back as `baseVersion`. RealmIndexUpdater gains `enqueueChanges` for a change set that mixes removals with updates; `enqueueUpdate` is that with one operation for the whole set. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EBB6JdRo5XCY5WhUkgBckS --- .../tests/card-operations-batch-test.ts | 663 +++++++++++++ .../card-operations/coordinator.ts | 420 ++++++++ .../card-operations/executors.ts | 919 ++++++++++++++++++ .../runtime-common/card-operations/index.ts | 20 + .../runtime-common/card-operations/types.ts | 6 + .../runtime-common/realm-index-updater.ts | 123 ++- packages/runtime-common/realm.ts | 251 ++++- 7 files changed, 2314 insertions(+), 88 deletions(-) create mode 100644 packages/realm-server/tests/card-operations-batch-test.ts create mode 100644 packages/runtime-common/card-operations/coordinator.ts create mode 100644 packages/runtime-common/card-operations/executors.ts diff --git a/packages/realm-server/tests/card-operations-batch-test.ts b/packages/realm-server/tests/card-operations-batch-test.ts new file mode 100644 index 00000000000..aa89c5f1c01 --- /dev/null +++ b/packages/realm-server/tests/card-operations-batch-test.ts @@ -0,0 +1,663 @@ +import QUnit from 'qunit'; +const { module, test } = QUnit; +import { basename, join } from 'path'; +import { existsSync, readFileSync } from 'fs-extra'; +import type { Test, SuperTest } from 'supertest'; +import type { DirResult } from 'tmp'; +import type { PgAdapter } from '@cardstack/postgres'; + +import { rri } from '@cardstack/runtime-common'; +import { + commitBatch, + isOperationFailure, + type BatchDocument, + type BatchEntry, +} from '@cardstack/runtime-common/card-operations'; +import type { + DBAdapter, + LooseSingleCardDocument, + Realm, +} from '@cardstack/runtime-common'; +import { APP_BOXEL_REALM_EVENT_TYPE } from '@cardstack/runtime-common/matrix-constants'; +import type { RealmHttpServer as Server } from '../server.ts'; +import { + setupPermissionedRealmCached, + setupMatrixRoom, + withRealmPath, + type RealmRequest, +} from './helpers/index.ts'; + +const testRealm = new URL('http://127.0.0.1:4445/test/'); +const testRealmHref = testRealm.href; +const PERSON = { module: rri('./person'), name: 'Person' }; + +// ============================================================================ +// The batch coordinator, driven against a real realm. +// +// What is under test is the batch's all-or-nothing property and the things +// that property is observable through: what lands on disk, how many index jobs +// the commit enqueues, how many index events it broadcasts, and what each +// entry's result reports. Those only exist against a real realm — the jobs +// table and the Matrix room are where "one job, one event" is either true or +// not — so the coordinator is called directly with the realm's own batch core +// rather than through a stub. +// ============================================================================ + +function makeFileSystem(): Record { + return { + 'person.gts': ` + import { contains, field, linksTo, CardDef, Component } from "@cardstack/base/card-api"; + import StringField from "@cardstack/base/string"; + import NumberField from "@cardstack/base/number"; + + export class Person extends CardDef { + @field firstName = contains(StringField); + @field hourlyRate = contains(NumberField); + @field friend = linksTo(() => Person, { searchable: true }); + static isolated = class Isolated extends Component { + + } + static embedded = class Embedded extends Component { + + } + static fitted = class Fitted extends Component { + + } + } + `, + 'person-a.json': { + data: { + type: 'card', + attributes: { firstName: 'Original', hourlyRate: 10 }, + meta: { adoptsFrom: PERSON }, + }, + }, + 'person-b.json': { + data: { + type: 'card', + attributes: { firstName: 'Original', hourlyRate: 10 }, + meta: { adoptsFrom: PERSON }, + }, + }, + 'person-c.json': { + data: { + type: 'card', + attributes: { firstName: 'Doomed', hourlyRate: 1 }, + meta: { adoptsFrom: PERSON }, + }, + }, + }; +} + +module(basename(import.meta.filename), function (hooks) { + let realm: Realm; + let testDbAdapter: DBAdapter; + let request: RealmRequest; + let serverRequest: SuperTest; + let testRealmHttpServer: Server; + let dir: DirResult; + + setupPermissionedRealmCached(hooks, { + mode: 'beforeEach', + realmURL: testRealm, + permissions: { + '*': ['read', 'write'], + '@node-test_realm:localhost': ['read', 'write', 'realm-owner'], + }, + subscribeToRealmEvents: true, + fileSystem: makeFileSystem(), + onRealmSetup(args) { + realm = args.testRealm; + testDbAdapter = args.dbAdapter; + request = withRealmPath(args.request, testRealm); + serverRequest = args.request; + testRealmHttpServer = args.testRealmHttpServer; + dir = args.dir; + }, + }); + + let { getMessagesSince } = setupMatrixRoom(hooks, () => ({ + testRealm: realm, + testRealmHttpServer, + request, + serverRequest, + dir, + dbAdapter: testDbAdapter as PgAdapter, + })); + + function realmFile(localPath: string): string { + return join(dir.name, 'realm_server_1', 'test', localPath); + } + + async function indexJobIds(): Promise { + let rows = (await testDbAdapter.execute( + `select id from jobs where job_type = 'incremental-index' + and concurrency_group = $1 order by id`, + { bind: [`indexing:${realm.url}`] }, + )) as { id: number | string }[]; + return rows.map((row) => Number(row.id)); + } + + async function realmEventsSince(since: number) { + let messages = await getMessagesSince(since); + return messages.filter( + (message) => message.type === APP_BOXEL_REALM_EVENT_TYPE, + ); + } + + async function commit(entries: BatchEntry[], clientRequestId?: string) { + return await commitBatch(realm.batchCore, entries, { + clientRequestId: clientRequestId ?? null, + actor: '@tester:localhost', + }); + } + + // The `links.self` a stored relationship holds, resolved against the card + // whose file carries it. Serialization is free to record a link relative to + // the card that holds it, so the absolute identity is what a test compares. + function storedLink(localPath: string, field: string): string | undefined { + let doc = JSON.parse(readFileSync(realmFile(localPath), 'utf8')); + let self = doc.data?.relationships?.[field]?.links?.self; + return self == null + ? undefined + : new URL(self, `${testRealmHref}${localPath}`).href; + } + + test('a batch creates several cards and links them by local id', async function (assert) { + let results = await commit([ + { + op: 'create', + lid: 'author', + document: { + data: { + type: 'card', + attributes: { firstName: 'Mango', hourlyRate: 100 }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + { + op: 'create', + lid: 'sidekick', + document: { + data: { + type: 'card', + attributes: { firstName: 'Van Gogh' }, + relationships: { + friend: { data: { lid: 'author', type: 'card' } }, + }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]); + + assert.strictEqual(results.length, 2, 'one result per entry, in order'); + assert.deepEqual( + results.map((result) => result && 'lid' in result && result.lid), + ['author', 'sidekick'], + 'each create echoes the local id the client named it with', + ); + assert.deepEqual( + results.map((result) => result?.id), + [`${testRealmHref}Person/author`, `${testRealmHref}Person/sidekick`], + 'a local id maps to the URL the realm minted for it', + ); + assert.ok( + existsSync(realmFile('Person/author.json')), + 'the first card is on disk', + ); + assert.ok( + existsSync(realmFile('Person/sidekick.json')), + 'the second card is on disk', + ); + assert.strictEqual( + storedLink('Person/sidekick.json', 'friend'), + `${testRealmHref}Person/author`, + 'the link resolves to the card the other entry in the batch minted', + ); + for (let result of results) { + assert.true( + (result?.meta.version ?? '').length > 0, + 'each result carries the version the file now holds', + ); + } + }); + + test('a failing entry leaves the whole batch unwritten, unindexed and unannounced', async function (assert) { + let jobsBefore = await indexJobIds(); + let since = Date.now(); + + let failure: unknown; + try { + await commit([ + { + op: 'create', + lid: 'never-written', + document: { + data: { + type: 'card', + attributes: { firstName: 'Ghost' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + { + op: 'update', + href: `${testRealmHref}does-not-exist`, + document: { + data: { + type: 'card', + attributes: { firstName: 'Nope' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]); + } catch (err: unknown) { + failure = err; + } + + assert.ok(isOperationFailure(failure), 'the batch is rejected'); + if (isOperationFailure(failure)) { + assert.strictEqual(failure.error.status, 404, "the entry's own status"); + assert.strictEqual(failure.error.code, 'target-not-found'); + assert.strictEqual( + failure.error.meta?.entry, + 1, + 'the refusal names the position of the entry that produced it', + ); + } + assert.notOk( + existsSync(realmFile('Person/never-written.json')), + 'the entry ahead of the failure is not written', + ); + assert.deepEqual( + await indexJobIds(), + jobsBefore, + 'no incremental index job is enqueued', + ); + assert.deepEqual( + await realmEventsSince(since), + [], + 'nothing is announced to the realm', + ); + }); + + test('a batch of one write and one delete commits under one index job and one index event', async function (assert) { + let jobsBefore = await indexJobIds(); + let since = Date.now(); + + let results = await commit( + [ + { + op: 'update', + href: `${testRealmHref}person-a`, + document: { + data: { + type: 'card', + attributes: { firstName: 'Renamed' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + { op: 'delete', href: `${testRealmHref}person-c` }, + ], + 'batch-1', + ); + + assert.strictEqual( + results[1], + null, + 'a delete has no state left to describe', + ); + assert.ok(results[0], 'the write reports its identity'); + assert.notOk( + existsSync(realmFile('person-c.json')), + 'the deleted card is gone from disk', + ); + assert.true( + readFileSync(realmFile('person-a.json'), 'utf8').includes('Renamed'), + 'the written card holds the patched value', + ); + + let newJobs = (await indexJobIds()).filter( + (id) => !jobsBefore.includes(id), + ); + assert.strictEqual( + newJobs.length, + 1, + `the write and the delete share one index job (got ${newJobs.length})`, + ); + + let indexEvents = (await realmEventsSince(since)).filter( + (event) => + (event.content as { eventName?: string; indexType?: string }) + .eventName === 'index' && + (event.content as { indexType?: string }).indexType === 'incremental', + ); + assert.strictEqual( + indexEvents.length, + 1, + `the batch broadcasts one index event (got ${indexEvents.length})`, + ); + assert.strictEqual( + (indexEvents[0].content as { clientRequestId?: string }).clientRequestId, + 'batch-1', + "the event carries the batch's own client request id", + ); + }); + + test('a base version is reported as matched or moved against the version the file held', async function (assert) { + let [first] = await commit([ + { + op: 'update', + href: `${testRealmHref}person-a`, + document: { + data: { + type: 'card', + attributes: { firstName: 'First' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]); + assert.ok(first, 'the first write reports a version'); + let version = first!.meta.version; + assert.strictEqual( + first!.meta.baseMatched, + undefined, + 'an unconditional write reports no base match', + ); + + let [matched] = await commit([ + { + op: 'update', + href: `${testRealmHref}person-a`, + baseVersion: version, + document: { + data: { + type: 'card', + attributes: { firstName: 'Second' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]); + assert.true( + matched!.meta.baseMatched, + 'the base the caller named is the one the file held', + ); + + let [moved] = await commit([ + { + op: 'update', + href: `${testRealmHref}person-a`, + baseVersion: version, + document: { + data: { + type: 'card', + attributes: { firstName: 'Third' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]); + assert.false( + moved!.meta.baseMatched, + 'the file has moved past the base the caller named', + ); + assert.true( + readFileSync(realmFile('person-a.json'), 'utf8').includes('Third'), + 'a moved base is reported, not refused — the write still lands', + ); + }); + + test('an update writes the same bytes a PATCH of the same document writes', async function (assert) { + let patch: BatchDocument = { + data: { + type: 'card', + attributes: { firstName: 'Paparazzi', hourlyRate: 42 }, + meta: { adoptsFrom: PERSON }, + }, + }; + + let response = await request + .patch('/person-a') + .send(patch) + .set('Accept', 'application/vnd.card+json'); + assert.strictEqual(response.status, 200, 'the PATCH is served'); + + await commit([ + { op: 'update', href: `${testRealmHref}person-b`, document: patch }, + ]); + + assert.strictEqual( + readFileSync(realmFile('person-b.json'), 'utf8'), + readFileSync(realmFile('person-a.json'), 'utf8'), + 'the two files are byte-identical', + ); + }); + + test('a patch that changes nothing leaves the file exactly as it is', async function (assert) { + let before = readFileSync(realmFile('person-a.json'), 'utf8'); + let jobsBefore = await indexJobIds(); + + let [result] = await commit([ + { + op: 'update', + href: `${testRealmHref}person-a`, + document: { + data: { + type: 'card', + attributes: { firstName: 'Original' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]); + + assert.strictEqual( + readFileSync(realmFile('person-a.json'), 'utf8'), + before, + 'the stored bytes are untouched', + ); + assert.true( + (result?.meta.version ?? '').length > 0, + 'the result still reports the version the file holds', + ); + assert.deepEqual( + await indexJobIds(), + jobsBefore, + 'nothing is queued for indexing', + ); + }); + + test('a local id claimed twice, and one nothing creates, are both refused', async function (assert) { + let duplicate: unknown; + try { + await commit([ + { + op: 'create', + lid: 'twice', + document: { + data: { + type: 'card', + attributes: { firstName: 'One' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + { + op: 'create', + lid: 'twice', + document: { + data: { + type: 'card', + attributes: { firstName: 'Two' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]); + } catch (err: unknown) { + duplicate = err; + } + assert.ok(isOperationFailure(duplicate), 'a duplicate local id is refused'); + if (isOperationFailure(duplicate)) { + assert.strictEqual(duplicate.error.status, 400); + assert.strictEqual(duplicate.error.code, 'invalid-params'); + } + + let dangling: unknown; + try { + await commit([ + { + op: 'create', + lid: 'lonely', + document: { + data: { + type: 'card', + attributes: { firstName: 'Lonely' }, + relationships: { + friend: { data: { lid: 'nobody', type: 'card' } }, + }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]); + } catch (err: unknown) { + dangling = err; + } + assert.ok( + isOperationFailure(dangling), + 'a link to a local id no entry creates is refused', + ); + if (isOperationFailure(dangling)) { + assert.strictEqual(dangling.error.status, 400); + assert.strictEqual( + dangling.error.meta?.entry, + 0, + 'the refusal names the entry that carried the link', + ); + } + assert.notOk( + existsSync(realmFile('Person/lonely.json')), + 'nothing is written for a refused batch', + ); + }); + + test('a delete of a card that is not there refuses without touching the realm', async function (assert) { + let jobsBefore = await indexJobIds(); + let failure: unknown; + try { + await commit([{ op: 'delete', href: `${testRealmHref}not-a-card` }]); + } catch (err: unknown) { + failure = err; + } + assert.ok(isOperationFailure(failure), 'the delete is refused'); + if (isOperationFailure(failure)) { + assert.strictEqual(failure.error.status, 404); + assert.strictEqual(failure.error.code, 'target-not-found'); + } + assert.deepEqual( + await indexJobIds(), + jobsBefore, + 'no index job is enqueued', + ); + }); + + test('two entries changing one card are refused rather than silently ordered', async function (assert) { + let failure: unknown; + try { + await commit([ + { + op: 'update', + href: `${testRealmHref}person-a`, + document: { + data: { + type: 'card', + attributes: { firstName: 'Left' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + { + op: 'update', + href: `${testRealmHref}person-a`, + document: { + data: { + type: 'card', + attributes: { hourlyRate: 99 }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]); + } catch (err: unknown) { + failure = err; + } + assert.ok(isOperationFailure(failure), 'the batch is refused'); + if (isOperationFailure(failure)) { + assert.strictEqual(failure.error.code, 'invalid-params'); + assert.strictEqual(failure.error.meta?.conflictsWith, 0); + } + assert.true( + readFileSync(realmFile('person-a.json'), 'utf8').includes('Original'), + 'the card is left as it was', + ); + }); + + test('a batch cannot change a card to another type', async function (assert) { + let failure: unknown; + try { + await commit([ + { + op: 'update', + href: `${testRealmHref}person-a`, + document: { + data: { + type: 'card', + attributes: { firstName: 'Shapeshifter' }, + meta: { + adoptsFrom: { + module: rri('@cardstack/base/card-api'), + name: 'CardDef', + }, + }, + }, + }, + }, + ]); + } catch (err: unknown) { + failure = err; + } + assert.ok(isOperationFailure(failure), 'the type change is refused'); + if (isOperationFailure(failure)) { + assert.strictEqual(failure.error.status, 400); + } + }); + + test('a base version on anything but an update is refused', async function (assert) { + let failure: unknown; + try { + await commit([ + { op: 'delete', href: `${testRealmHref}person-c`, baseVersion: 'abc' }, + ]); + } catch (err: unknown) { + failure = err; + } + assert.ok(isOperationFailure(failure), 'the base version is refused'); + assert.ok( + existsSync(realmFile('person-c.json')), + 'the card is left in place', + ); + }); +}); diff --git a/packages/runtime-common/card-operations/coordinator.ts b/packages/runtime-common/card-operations/coordinator.ts new file mode 100644 index 00000000000..a2eb82a1d76 --- /dev/null +++ b/packages/runtime-common/card-operations/coordinator.ts @@ -0,0 +1,420 @@ +import { RealmPaths, type LocalPath } from '../paths.ts'; +import { + createIdentity, + namesForeignRealm, + stageCreate, + stageDelete, + stagedIdentity, + stageUpdate, + type BatchEntry, + type LidIndex, + type StagedChange, + type StagedIdentity, + type StagingContext, + type StoredFile, +} from './executors.ts'; +import { + OperationFailure, + isOperationFailure, + type OperationIdentityResult, +} from './types.ts'; +import type { CodeRef } from '../code-ref.ts'; +import type { Definition } from '../definitions.ts'; +import type { LooseSingleCardDocument } from '../index.ts'; +import type { RealmResourceIdentifier } from '../realm-identifiers.ts'; + +// ============================================================================ +// The batch coordinator. +// +// A batch is all-or-nothing: several changes to several cards either all land +// or none do, under one index job and one index event. Getting that is a +// matter of ordering. The coordinator takes the realm's write lock, reads +// every file the batch touches, runs every executor in memory, and only then +// commits. Nothing is written until every entry has produced its bytes, so an +// entry that cannot be carried out is found while the realm is still +// untouched — there is no partial write to undo, no index job to cancel, and +// no event a subscriber could have already acted on. +// +// The lock is taken once, here, and never re-entered. Everything below it +// works from the state read inside it, and the commit it hands the staged +// changes to is the realm's unlocked primitive. +// ============================================================================ + +// The realm's collaborators, narrowed to what committing a batch uses. Handed +// down as plain values and bound functions rather than as a `Realm`: no +// executor resolves an identifier, opens a file, or reaches the network, since +// card modules are author-written and the realm is a trusted context. +export interface BatchCore { + realmURL: string; + // Runs `fn` holding the realm's per-realm write lock. The lock spans the + // reads the batch stages from and the commit itself, so two writers cannot + // both compute a merge over the same pre-state and have the second silently + // lose the first's changes. + withWriteLock(fn: () => Promise): Promise; + // A stored file's bytes and modification time. Called only inside the lock. + readSourceFile( + localPath: LocalPath, + ): Promise<{ content: string; lastModified: number } | undefined>; + // The write-time content fingerprints recorded for these paths, read in one + // round-trip. A caller's `baseVersion` names one of these. + contentHashes( + localPaths: LocalPath[], + ): Promise>; + // The realm's unlocked commit: writes and removals under one index job and + // one index event. Assumes the write lock is held, which it is. + commitUnlocked( + batch: { + writes?: Map; + deletes?: LocalPath[]; + }, + options?: { clientRequestId?: string | null; waitForIndex?: boolean }, + ): Promise<{ + writes: { path: string; lastModified: number; contentHash: string }[]; + generation: number | null; + }>; + serializeCard( + doc: LooseSingleCardDocument, + relativeTo: URL, + ): Promise; + codeRefKey(codeRef: CodeRef, relativeTo: URL): string; + resolveModuleId( + moduleId: RealmResourceIdentifier, + relativeTo: string, + ): RealmResourceIdentifier; + lookupDefinition( + codeRef: CodeRef, + relativeTo: URL, + ): Promise; +} + +export interface CommitBatchOptions { + // The caller's own id for this batch. Echoed on the realm's index event so a + // client can tell its own batch's event from anyone else's. + clientRequestId?: string | null; + // The invoking actor, as the identity `actor()` resolves to. It comes from + // the authenticated realm user the request's permission check verified. + actor?: string; + // Whether to return only once the batch's index job has landed. Waiting is + // the default: a caller that reports a version and a generation per entry + // needs the generation, and a caller reading the cards back needs the rows. + waitForIndex?: boolean; +} + +// One entry's answer, in the order the entries were sent. A write reports the +// card's identity and the version it now holds; a delete reports `null`, since +// there is no state left to describe. +export type BatchEntryResult = OperationIdentityResult | null; + +export async function commitBatch( + core: BatchCore, + entries: BatchEntry[], + opts: CommitBatchOptions = {}, +): Promise { + let paths = new RealmPaths(new URL(core.realmURL)); + return await core.withWriteLock(async () => { + // Every `lid` in the batch resolves to a URL before any executor runs. A + // created card's file is named after its `lid`, so its URL is path math + // over the type it adopts — no read and no write — which is what lets an + // entry link to a card a later entry mints. + let lids = indexLids(entries, paths); + let stored = await readStoredFiles(core, entries, paths); + let staged: StagedChange[] = []; + for (let [index, entry] of entries.entries()) { + staged.push( + await stageEntry(entry, index, { + realmURL: core.realmURL, + paths, + lids, + stored, + actor: opts.actor ?? '', + serializeCard: core.serializeCard, + codeRefKey: core.codeRefKey, + resolveModuleId: core.resolveModuleId, + lookupDefinition: core.lookupDefinition, + }), + ); + } + // Past this point the batch is committed. Everything above either produced + // bytes for every entry or threw, and a throw leaves the realm as it was. + return await commitStaged(core, entries, staged, stored, opts); + }); +} + +// Run one executor, and label whatever it refuses with the entry's position. +// A batch is rejected as a whole, so a caller reading one error needs to know +// which of the entries it sent produced it. +async function stageEntry( + entry: BatchEntry, + index: number, + ctx: StagingContext, +): Promise { + try { + assertVersionable(entry, index); + switch (entry.op) { + case 'create': + return await stageCreate(entry, ctx); + case 'update': + return await stageUpdate(entry, ctx); + case 'delete': + return stageDelete(entry, ctx); + default: + throw new OperationFailure({ + status: 400, + code: 'invalid-params', + title: 'Unknown entry', + detail: `entry ${index} names no staged operation`, + }); + } + } catch (err: unknown) { + throw atEntry(err, index); + } +} + +// A `baseVersion` names the state a write is computed on top of, and the +// result reports whether the target was still at it. Only an update has both: +// a create has no prior state to name, and a delete's result carries no state +// to report a match on. +function assertVersionable(entry: BatchEntry, index: number): void { + if (entry.baseVersion === undefined || entry.op === 'update') { + return; + } + throw new OperationFailure({ + status: 400, + code: 'invalid-params', + title: 'Invalid base version', + detail: `entry ${index} is a ${entry.op}, which has no base version`, + }); +} + +function atEntry(err: unknown, index: number): OperationFailure { + if (isOperationFailure(err)) { + return new OperationFailure({ + ...err.error, + meta: { ...err.error.meta, entry: index }, + }); + } + return new OperationFailure({ + status: 500, + code: 'internal-error', + title: 'Cannot stage batch', + detail: `entry ${index} could not be staged: ${ + err instanceof Error ? err.message : String(err) + }`, + meta: { entry: index }, + }); +} + +// --------------------------------------------------------------------------- +// Resolving local ids +// --------------------------------------------------------------------------- + +// Every card the batch mints, keyed by `lid`. A `lid` names one card, so two +// entries claiming the same one is a payload the realm cannot carry out: it +// says two different cards are the same card. +function indexLids(entries: BatchEntry[], paths: RealmPaths): LidIndex { + let lids = new Map(); + let claim = (lid: string, identity: StagedIdentity, position: string) => { + if (lids.has(lid)) { + throw new OperationFailure({ + status: 400, + code: 'invalid-params', + title: 'Duplicate local id', + detail: + `local id "${lid}" is claimed more than once (at ${position}); ` + + `a local id names one card`, + }); + } + lids.set(lid, identity); + }; + for (let [index, entry] of entries.entries()) { + if (entry.op === 'delete') { + continue; + } + let primary = entry.document?.data; + if (entry.op === 'create' && entry.lid) { + claim( + entry.lid, + createIdentity(entry, primary, paths, new Map()), + `entry ${index}`, + ); + } + for (let [offset, resource] of (entry.document?.included ?? []).entries()) { + // A side-loaded resource with no `lid` is not staged and nothing can + // link to it; one naming another realm is not this batch's to write. + // Neither takes an identity here, so neither can be linked to either. + if ( + typeof resource.lid !== 'string' || + namesForeignRealm(resource, paths.url) + ) { + continue; + } + claim( + resource.lid, + stagedIdentity( + resource.meta?.adoptsFrom, + resource.lid, + entry.op === 'create' ? entry.directory : undefined, + paths, + ), + `entry ${index}, included[${offset}]`, + ); + } + } + return lids; +} + +// --------------------------------------------------------------------------- +// Reading the pre-state +// --------------------------------------------------------------------------- + +// Every file the batch reads, loaded once inside the write lock: the merge base +// for each update, the existence check for each delete, and the anchoring card +// a named create reads through `instance(…)`. Loading them up front is what +// makes the executors pure — they resolve nothing and read nothing — and what +// makes the pre-write content hashes, which a `baseVersion` is compared to, a +// snapshot from inside the same critical section as the write. +async function readStoredFiles( + core: BatchCore, + entries: BatchEntry[], + paths: RealmPaths, +): Promise> { + let wanted = new Set(); + for (let entry of entries) { + // A create's href is the card it is anchored on, when it has one; an + // update's and a delete's is the card itself. + if (!entry.href) { + continue; + } + let localPath = sourcePathOf(entry.href, paths); + if (localPath) { + wanted.add(localPath); + } + } + let localPaths = [...wanted]; + let hashes = await core.contentHashes(localPaths); + let stored = new Map(); + await Promise.all( + localPaths.map(async (localPath) => { + let file = await core.readSourceFile(localPath); + if (!file) { + return; + } + stored.set(localPath, { + content: file.content, + lastModified: file.lastModified, + contentHash: hashes.get(localPath), + }); + }), + ); + return stored; +} + +// The stored-source path a target's href names, or undefined when the href is +// not a URL this realm contains. An unusable href is left for the executor to +// refuse, which has the target's own terms to refuse it in. +function sourcePathOf(href: string, paths: RealmPaths): LocalPath | undefined { + let url: URL; + try { + url = new URL(href); + } catch { + return undefined; + } + try { + return `${paths.local(url)}.json` as LocalPath; + } catch { + return undefined; + } +} + +// --------------------------------------------------------------------------- +// Committing +// --------------------------------------------------------------------------- + +async function commitStaged( + core: BatchCore, + entries: BatchEntry[], + staged: StagedChange[], + stored: Map, + opts: CommitBatchOptions, +): Promise { + let writes = new Map(); + let deletes: LocalPath[] = []; + // Which entry claimed each file. Two entries touching one file would each + // have been computed against the state the batch started from, so the second + // would silently discard the first — the very loss the write lock exists to + // prevent, reintroduced inside one batch. Refused rather than ordered: which + // change the caller meant to keep is not something the realm can infer. + let claimedBy = new Map(); + let claim = (path: LocalPath, index: number) => { + let owner = claimedBy.get(path); + if (owner !== undefined) { + throw new OperationFailure({ + status: 400, + code: 'invalid-params', + title: 'Conflicting entries', + detail: + `entries ${owner} and ${index} both change ${path}; a batch names ` + + `one change per card`, + meta: { entry: index, conflictsWith: owner }, + }); + } + claimedBy.set(path, index); + }; + for (let [index, change] of staged.entries()) { + for (let write of change.writes) { + claim(write.path, index); + writes.set(write.path, write.content); + } + for (let path of change.deletes) { + claim(path, index); + deletes.push(path); + } + } + let committed = await core.commitUnlocked( + { writes, deletes }, + { + clientRequestId: opts.clientRequestId ?? null, + waitForIndex: opts.waitForIndex ?? true, + }, + ); + let byPath = new Map(committed.writes.map((write) => [write.path, write])); + return staged.map((change, index) => { + if (!change.primaryPath) { + return null; + } + let written = byPath.get(change.primaryPath); + if (!written) { + // Every staged write is handed to the commit and every one comes back, + // so a missing result means the two no longer agree about what was + // staged. Reporting an empty version would hand the caller a token it + // could send back as a `baseVersion` that matches nothing. + throw new OperationFailure({ + id: change.id, + status: 500, + code: 'internal-error', + title: 'Missing write result', + detail: `the commit reported no result for ${change.primaryPath}`, + meta: { entry: index }, + }); + } + let { baseVersion } = entries[index]; + return { + id: change.id, + ...(change.lid ? { lid: change.lid } : {}), + meta: { + version: written.contentHash, + generation: committed.generation, + lastModified: written.lastModified, + // Compared against the fingerprint the file carried before the commit, + // read inside this same lock. A mismatch is not an error — the write + // happened, and what a moved base means is the caller's to decide. + ...(baseVersion === undefined + ? {} + : { + baseMatched: + stored.get(change.primaryPath)?.contentHash === baseVersion, + }), + }, + }; + }); +} diff --git a/packages/runtime-common/card-operations/executors.ts b/packages/runtime-common/card-operations/executors.ts new file mode 100644 index 00000000000..a8f77f4eecc --- /dev/null +++ b/packages/runtime-common/card-operations/executors.ts @@ -0,0 +1,919 @@ +import { cloneDeep, isEqual, merge, mergeWith } from 'lodash-es'; +import { v4 as uuidV4 } from 'uuid'; + +import { visitModuleDeps, type CodeRef } from '../code-ref.ts'; +import { getImmediateFieldDef, type Definition } from '../definitions.ts'; +import { getCardDirectoryName } from '../helpers/card-directory-name.ts'; +import { mergeRelationships } from '../merge-relationships.ts'; +import type { RealmPaths } from '../paths.ts'; +import { ensureTrailingSlash, type LocalPath } from '../paths.ts'; +import { normalizeRelationships } from '../relationship-utils.ts'; +import { + clearReplacedArrayFieldMeta, + isCardResource, + type CardResource, + type Relationship, +} from '../resource-types.ts'; +import { OperationFailure, type OperationDefinition } from './types.ts'; +import type { LooseSingleCardDocument } from '../index.ts'; +import type { RealmResourceIdentifier } from '../realm-identifiers.ts'; +import type { OperationTemplate } from './types.ts'; + +// ============================================================================ +// The `create`, `update` and `delete` executors. +// +// Each one is a pure staging function: it reads the batch's pre-loaded state, +// works out the exact bytes every file it touches should end up holding, and +// returns them. It writes nothing, enqueues nothing and broadcasts nothing. +// That is what makes an all-or-nothing batch possible — every entry is +// validated and resolved in memory first, so an entry that cannot be carried +// out is found while the realm is still untouched, and the batch is abandoned +// with nothing to undo. +// +// The staging is where the work is, not a formality. A create decides the new +// card's URL and directory, resolves the links it declares, and serializes the +// document against its type's definition. An update reads the stored file, +// merges the patch over it under the rules below, and serializes the result. A +// delete establishes that its target is there to remove. All three can refuse, +// and a refusal is an `OperationFailure` carrying the status the caller sees. +// ============================================================================ + +// The identity of a card the batch is creating: the URL it will answer to, and +// the file its bytes land in. Both are known before anything is written, which +// is what lets one entry link to a card another entry in the same batch mints. +export interface StagedIdentity { + id: string; + path: LocalPath; +} + +// Every card the batch mints, keyed by the `lid` the client named it with. A +// `lid` is the client's own id for a card that does not exist yet, and it is +// the only way one entry can refer to another's card, so this is the single +// place a `lid` resolves — the file path a create writes and the link another +// entry records both read the identity from here, so the two cannot disagree. +export type LidIndex = ReadonlyMap; + +// A file's stored state, read once inside the write lock before any executor +// runs. The merge base for an update is these bytes rather than the index: the +// index is downstream of the file and can lag it, so merging over indexed +// state would silently revert every field indexing has not caught up on. +export interface StoredFile { + content: string; + lastModified: number; + // The fingerprint recorded when the file was last written — the version a + // caller's `baseVersion` names. Undefined for a file written before the + // realm began recording one. + contentHash: string | undefined; +} + +// A JSON:API card document as a batch entry carries it. `included` side-loads +// cards to create alongside the primary, each linked to it by its `lid`. +export interface BatchDocument { + data: CardResource; + included?: CardResource[]; +} + +interface EntryCommon { + // The lowered operation, when the entry invokes a named operation rather + // than a plain base one. A named `create` stages its card from the + // definition's `of` and `fill` instead of from a document. + definition?: OperationDefinition; + // The payload, keyed as the definition's `params` schema declares it. + params?: Record; + // The version the caller believes it is writing on top of. Present makes + // the write conditional in the reporting sense: the result says whether the + // target was still at that version, and the write happens either way. + baseVersion?: string; +} + +export interface CreateEntry extends EntryCommon { + op: 'create'; + // The client's own id for the card being minted. It names the file the card + // lands in, and it is the key other entries in the batch link to it by. + // Absent means the realm mints an id, and nothing else in the batch can + // refer to the card. + lid?: string; + // The card the create is anchored on, for a named create whose template + // reads it through `instance(…)`. + href?: string; + // A raw JSON:API document, as a `POST` carries it. + document?: BatchDocument; + // The realm-relative directory the new card's type directory sits under. + // Absent means the realm root, which is where a `POST` to the realm itself + // creates cards. + directory?: string; +} + +export interface UpdateEntry extends EntryCommon { + op: 'update'; + // The card being patched, as an absolute URL. + href: string; + document: BatchDocument; +} + +export interface DeleteEntry extends EntryCommon { + op: 'delete'; + href: string; +} + +export type BatchEntry = CreateEntry | UpdateEntry | DeleteEntry; + +// One file the batch will write, and the exact bytes it will hold. +export interface StagedWrite { + path: LocalPath; + content: string; +} + +// What one executor stages. +export interface StagedChange { + writes: StagedWrite[]; + deletes: LocalPath[]; + // The card the entry's result reports. + id: string; + // Echoed on a create, so a client can match the URL the realm minted back to + // the `lid` it named the card with. + lid?: string; + // The file whose post-commit version and modification time the entry's + // result reports. Absent on a delete — there is no file left to version. + primaryPath?: LocalPath; +} + +// What the executors are given. Everything the realm owns arrives already +// done, as a plain value or a bound function: no executor resolves an +// identifier, reads a file, or reaches the network. Card modules are +// author-written and the realm is a trusted context, so steering a lookup is +// not something an operation gets to do. +export interface StagingContext { + realmURL: string; + paths: RealmPaths; + lids: LidIndex; + // Every target's stored file, keyed by local path, read inside the write + // lock before any executor runs. + stored: ReadonlyMap; + // The invoking actor, as the identity `actor()` resolves to. It comes from + // the authenticated realm user the request's permission check verified, and + // is supplied by the endpoint that verified it — an executor never derives + // an actor itself. + actor: string; + // A card document serialized for storage: the bytes the file holds, with + // every field resolved against the type's definition. + serializeCard( + doc: LooseSingleCardDocument, + relativeTo: URL, + ): Promise; + // A code ref's canonical key, for comparing two refs that name the same + // type through different spellings. + codeRefKey(codeRef: CodeRef, relativeTo: URL): string; + // A module identifier resolved against another, for the module refs a + // side-loaded resource carries relative to the card it was sent with. + resolveModuleId( + moduleId: RealmResourceIdentifier, + relativeTo: string, + ): RealmResourceIdentifier; + // The definition-cache entry for a type, or undefined when it cannot be + // read. + lookupDefinition( + codeRef: CodeRef, + relativeTo: URL, + ): Promise; +} + +// --------------------------------------------------------------------------- +// `create` +// --------------------------------------------------------------------------- + +export async function stageCreate( + entry: CreateEntry, + ctx: StagingContext, +): Promise { + let primary = await primaryCreateResource(entry, ctx); + if (namesForeignRealm(primary, ctx.realmURL)) { + throw new OperationFailure({ + status: 400, + code: 'invalid-params', + title: 'Invalid target', + detail: + `a create names realm ${primary.meta.realmURL}, which is not ` + + `${ctx.realmURL}; a batch commits to one realm`, + }); + } + let included = entry.document?.included ?? []; + let writes: StagedWrite[] = []; + let primaryIdentity: StagedIdentity | undefined; + // The primary first, then each side-loaded resource. A side-loaded resource + // with no `lid` is not staged: it has no id to be created under and nothing + // in the batch can link to it, so the client sent a resource the realm has + // no way to name. + for (let [index, resource] of [primary, ...included].entries()) { + if (index > 0 && typeof resource.lid !== 'string') { + continue; + } + if (namesForeignRealm(resource, ctx.realmURL)) { + continue; + } + let identity = + index === 0 + ? createIdentity(entry, primary, ctx.paths, ctx.lids) + : stagedLid(resource.lid!, ctx); + if (index === 0) { + primaryIdentity = identity; + } else { + // A side-loaded resource's module refs are written relative to the card + // it was sent with, so they are resolved against that card before the + // resource is serialized under its own URL. + visitModuleDeps(resource, (moduleId, setModuleId) => { + setModuleId(ctx.resolveModuleId(moduleId, primaryIdentity!.id)); + }); + } + promoteStagedLinks(resource, ctx); + writes.push({ + path: identity.path, + content: await serializeForStorage(resource, identity, ctx), + }); + } + return { + writes, + deletes: [], + id: primaryIdentity!.id, + ...(entry.lid ? { lid: entry.lid } : {}), + primaryPath: primaryIdentity!.path, + }; +} + +// The resource a create stages: the document the caller sent, or the one a +// named create's `of` + `fill` template describes. +async function primaryCreateResource( + entry: CreateEntry, + ctx: StagingContext, +): Promise { + if (entry.document) { + if (!isCardResource(entry.document.data)) { + throw new OperationFailure({ + status: 400, + code: 'invalid-params', + title: 'Invalid document', + detail: `a create's document is not a valid card resource`, + }); + } + return cloneDeep(entry.document.data); + } + if (entry.definition?.of) { + return await resourceFromTemplate(entry, entry.definition, ctx); + } + throw new OperationFailure({ + status: 400, + code: 'invalid-params', + title: 'Nothing to create', + detail: + `a create needs either a card document or an operation declaring the ` + + `type it mints`, + }); +} + +// --------------------------------------------------------------------------- +// `update` +// --------------------------------------------------------------------------- + +export async function stageUpdate( + entry: UpdateEntry, + ctx: StagingContext, +): Promise { + let url = targetURL(entry.href); + let localPath = localPathIn(url, ctx); + let sourcePath = `${localPath}.json` as LocalPath; + let stored = ctx.stored.get(sourcePath); + if (!stored) { + throw new OperationFailure({ + id: url.href, + status: 404, + code: 'target-not-found', + title: 'Not found', + detail: `${url.href} does not exist in realm ${ctx.realmURL}`, + }); + } + let original = storedResource(stored.content, url); + let patch = cloneDeep(entry.document.data); + if (!isCardResource(patch)) { + throw new OperationFailure({ + id: url.href, + status: 400, + code: 'invalid-params', + title: 'Invalid document', + detail: `the patch for ${url.href} is not a card document`, + }); + } + // A card's type is fixed for its life: an instance of another type is + // another card, so changing it is a create and a delete rather than a + // patch. + if ( + original.meta?.adoptsFrom && + ctx.codeRefKey(patch.meta.adoptsFrom, url) !== + ctx.codeRefKey(original.meta.adoptsFrom, url) + ) { + throw new OperationFailure({ + id: url.href, + status: 400, + code: 'invalid-params', + title: 'Cannot change type', + detail: `cannot change card instance type to ${JSON.stringify( + patch.meta.adoptsFrom, + )}`, + }); + } + let included = entry.document.included ?? []; + // Realm-managed keys never come from a patch: `realmInfo` and `realmURL` are + // stamped by the realm serving the card, `screenshots` is joined from the + // prerendered manifest at serve time, and `type` is fixed by the document + // shape. A client echoing back what it was served must not persist any of + // them into the source file. + delete (patch as { type?: unknown }).type; + delete patch.meta.realmInfo; + delete patch.meta.realmURL; + delete patch.meta.screenshots; + + promoteStagedLinks(patch, ctx); + + // A patch that fully replaces an array attribute makes that array's + // per-index field metadata stale — the polymorphic type recorded at + // `meta.fields['items.1']`, or the array-valued `meta.fields['items']` of a + // composite containsMany. The merge below overwrites arrays in `attributes` + // but deep-merges `meta.fields`, so the removed element's metadata would + // otherwise survive and be re-applied to a new entry when the array grows + // again. Dropping it from the original first lets the patch's own metadata + // win cleanly. + let merged = cloneDeep(original); + clearReplacedArrayFieldMeta(merged.meta, patch.attributes); + let primary = mergeWith(merged, patch, (_target, source: unknown) => + // A patched array replaces the original rather than merging into it — + // merging would make removing an item impossible. + Array.isArray(source) ? source : undefined, + ); + if (primary.relationships || patch.relationships) { + let mergedRelationships = mergeRelationships( + primary.relationships, + patch.relationships, + ); + if (mergedRelationships && Object.keys(mergedRelationships).length !== 0) { + primary.relationships = mergedRelationships; + } + } + + let writes: StagedWrite[] = []; + if (included.length === 0 && isEqual(primary, original)) { + // The patch makes no semantic change and side-loads nothing, so the file + // is left exactly as it is — staging the bytes it already holds is what + // says so. The commit finds them unchanged, writes nothing, leaves the + // modification time alone, and queues nothing for indexing, while the + // entry's result still reports the version the file holds. + writes.push({ path: sourcePath, content: stored.content }); + } else { + // The id lives in the file's name, not in its contents. + delete primary.id; + writes.push({ + path: sourcePath, + content: await serializeForStorage( + primary, + { id: url.href, path: sourcePath }, + ctx, + ), + }); + for (let resource of included) { + if ( + typeof resource.lid !== 'string' || + namesForeignRealm(resource, ctx.realmURL) + ) { + continue; + } + let identity = stagedLid(resource.lid, ctx); + promoteStagedLinks(resource, ctx); + visitModuleDeps(resource, (moduleId, setModuleId) => { + setModuleId(ctx.resolveModuleId(moduleId, url.href)); + }); + writes.push({ + path: identity.path, + content: await serializeForStorage(resource, identity, ctx), + }); + } + } + return { writes, deletes: [], id: url.href, primaryPath: sourcePath }; +} + +// --------------------------------------------------------------------------- +// `delete` +// --------------------------------------------------------------------------- + +export function stageDelete( + entry: DeleteEntry, + ctx: StagingContext, +): StagedChange { + let url = targetURL(entry.href); + let localPath = localPathIn(url, ctx); + let sourcePath = `${localPath}.json` as LocalPath; + // The stored file decides whether there is a card here, for the same reason + // an update merges over it: a card written a moment ago is on disk before it + // is in the index, and refusing to delete it until indexing catches up would + // make a client unable to remove what it just created. + if (!ctx.stored.has(sourcePath)) { + throw new OperationFailure({ + id: url.href, + status: 404, + code: 'target-not-found', + title: 'Not found', + detail: `${url.href} does not exist in realm ${ctx.realmURL}`, + }); + } + return { writes: [], deletes: [sourcePath], id: url.href }; +} + +// --------------------------------------------------------------------------- +// Identities and links +// --------------------------------------------------------------------------- + +// The identity a created card takes: `{TypeDirectory}/{id}.json`, under the +// realm root unless the entry names a directory. Path math only — no read and +// no serialization — which is what makes a create's URL knowable before +// anything is written. +export function stagedIdentity( + adoptsFrom: CodeRef | undefined, + id: string, + directory: string | undefined, + paths: RealmPaths, +): StagedIdentity { + let segments = [ + ...(directory ?? '').split('/'), + getCardDirectoryName(adoptsFrom, paths), + id, + ].filter(Boolean); + let url = paths.fileURL(`${segments.join('/')}.json`); + return { id: url.href.replace(/\.json$/, ''), path: paths.local(url) }; +} + +// The identity a create entry's card takes, whether the entry sent a document +// or named an operation that mints the type. The `lid` index is consulted +// first so the file a create writes and the link another entry records are +// read from one place; an entry with no `lid` cannot be linked to, so its id +// is minted here. +export function createIdentity( + entry: CreateEntry, + resource: CardResource | undefined, + paths: RealmPaths, + lids: LidIndex, +): StagedIdentity { + if (entry.lid) { + let staged = lids.get(entry.lid); + if (staged) { + return staged; + } + } + return stagedIdentity( + resource?.meta?.adoptsFrom ?? entry.definition?.of, + entry.lid ?? uuidV4(), + entry.directory, + paths, + ); +} + +// Whether a resource declares itself to belong to another realm. One batch +// commits to one realm, so a resource naming another is not this batch's to +// write. +export function namesForeignRealm( + resource: CardResource, + realmURL: string, +): boolean { + let named = resource.meta?.realmURL; + return Boolean(named) && ensureTrailingSlash(String(named)) !== realmURL; +} + +function stagedLid(lid: string, ctx: StagingContext): StagedIdentity { + let staged = ctx.lids.get(lid); + if (!staged) { + throw new OperationFailure({ + status: 400, + code: 'invalid-params', + title: 'Unknown local id', + detail: + `local id "${lid}" is referenced but no entry in the batch creates ` + + `a card under it`, + }); + } + return staged; +} + +// Rewrite every `{ lid }` reference a resource's relationships carry into a +// link to the card that `lid` names. A relationship's `data` is left as the +// client wrote it and only `links.self` is set, so the document still records +// which reference the client used. +function promoteStagedLinks(resource: CardResource, ctx: StagingContext): void { + if (!resource.relationships) { + return; + } + // Normalizing gives one flat map keyed by field name, with a plural field's + // members under `field.0`, `field.1` — the keys whose `links.self` a + // collection's edges are recorded under. The `Relationship` objects are the + // resource's own, so setting a link here sets it on the document. + let normalized = normalizeRelationships(resource.relationships); + let setSelfLink = (relationship: Relationship, lid: string) => { + relationship.links = { self: stagedLid(lid, ctx).id }; + }; + for (let [fieldName, relationship] of Object.entries(normalized)) { + let { data } = relationship; + if (Array.isArray(data)) { + for (let [index, item] of data.entries()) { + if (!('lid' in item)) { + continue; + } + let indexed = normalized[`${fieldName}.${index}`]; + if (indexed) { + setSelfLink(indexed, item.lid); + } + } + continue; + } + if (data && 'lid' in data) { + setSelfLink(relationship, data.lid); + } + } +} + +// --------------------------------------------------------------------------- +// Serialization +// --------------------------------------------------------------------------- + +// The bytes a card's file holds. The realm stamps its own URL on the resource +// before serializing, the same way it does for every card it stores, and the +// serializer resolves each field against the type's definition. +async function serializeForStorage( + resource: CardResource, + identity: StagedIdentity, + ctx: StagingContext, +): Promise { + let fileURL = ctx.paths.fileURL(identity.path); + let serialized: LooseSingleCardDocument; + try { + serialized = await ctx.serializeCard( + { data: merge(resource, { meta: { realmURL: ctx.realmURL } }) }, + fileURL, + ); + } catch (err: unknown) { + let message = err instanceof Error ? err.message : String(err); + // A field the type refuses is the caller's payload to fix; anything else + // failed inside the realm. + throw new OperationFailure({ + id: identity.id, + status: message.startsWith('field validation error') ? 400 : 500, + code: message.startsWith('field validation error') + ? 'invalid-params' + : 'internal-error', + title: 'Cannot serialize card', + detail: message, + }); + } + return JSON.stringify(serialized, null, 2); +} + +// The card resource a stored file holds. A file that is not a card document is +// reported as the realm's own fault rather than the caller's: the caller asked +// to patch a card, and what is on disk is not one. +function storedResource(content: string, url: URL): CardResource { + let resource: unknown; + try { + resource = (JSON.parse(content) as { data?: unknown }).data; + } catch (err: unknown) { + resource = undefined; + } + if (!isCardResource(resource)) { + throw new OperationFailure({ + id: url.href, + status: 500, + code: 'internal-error', + title: 'Invalid stored card', + detail: `the stored file for ${url.href} is not a valid card document`, + }); + } + let stored = cloneDeep(resource); + // Stamped from the file's own modification time when the card is served, so + // it is not part of what the merge compares or writes. + delete stored.meta.lastModified; + return stored; +} + +// --------------------------------------------------------------------------- +// Named `create` templates +// --------------------------------------------------------------------------- + +// A named create stages its card from the operation's own declaration: `of` +// names the type, and `fill` is a JSON template whose typed-reference markers +// this invocation supplies the values for. Resolving it is plain substitution +// rather than BXL — each marker becomes its value — and from there it is the +// same path a create from a document takes. +async function resourceFromTemplate( + entry: CreateEntry, + definition: OperationDefinition, + ctx: StagingContext, +): Promise { + let of = definition.of!; + let anchor = entry.href ? anchorResource(entry, ctx) : undefined; + let linkFields = await linkFieldsOf(of, entry, ctx); + let resource: CardResource = { type: 'card', meta: { adoptsFrom: of } }; + for (let [field, template] of Object.entries(definition.fill ?? {})) { + let resolved = resolveTemplate(template, { + entry, + definition, + ctx, + anchor, + field, + }); + if (resolved.value === undefined) { + continue; + } + if (resolved.isLink || linkFields.has(field)) { + setLink(resource, field, resolved.value, field); + continue; + } + resource.attributes = { ...resource.attributes, [field]: resolved.value }; + } + return resource; +} + +// The card a named create is anchored on, as `instance(…)` reads it: the +// target's stored document, never a live card instance. +function anchorResource( + entry: CreateEntry, + ctx: StagingContext, +): { id: string; resource: CardResource } { + let url = targetURL(entry.href!); + let sourcePath = `${localPathIn(url, ctx)}.json` as LocalPath; + let stored = ctx.stored.get(sourcePath); + if (!stored) { + throw new OperationFailure({ + id: url.href, + status: 404, + code: 'target-not-found', + title: 'Not found', + detail: `${url.href} does not exist in realm ${ctx.realmURL}`, + }); + } + return { id: url.href, resource: storedResource(stored.content, url) }; +} + +// The fields of the created type that hold links rather than values. A link is +// an edge in the document's `relationships`, never a member of the stored +// value, so which fields are links decides where a resolved value lands. The +// type's own definition is the authority; a `card(…)` marker and a `linkTo` +// param say the same thing about a single value, and are honored even when the +// type's entry cannot be read. +async function linkFieldsOf( + of: CodeRef, + entry: CreateEntry, + ctx: StagingContext, +): Promise> { + let relativeTo = new URL(ctx.realmURL); + let definition = await ctx.lookupDefinition(of, relativeTo); + if (!definition) { + return new Set(); + } + let links = new Set(); + for (let field of Object.keys(entry.definition?.fill ?? {})) { + let fieldDef = getImmediateFieldDef(definition, field); + if (fieldDef?.type === 'linksTo' || fieldDef?.type === 'linksToMany') { + links.add(field); + } + } + return links; +} + +// Record a link, or a collection of them, on the resource. A collection's +// edges are keyed `field.0`, `field.1` — one edge per key, which is how a +// relationship collection is stored and how it is changed one edge at a time. +function setLink( + resource: CardResource, + field: string, + value: unknown, + path: string, +): void { + resource.relationships ??= {}; + if (Array.isArray(value)) { + value.forEach((member, index) => { + resource.relationships![`${field}.${index}`] = { + links: { self: identityOf(member, `${path}[${index}]`) }, + }; + }); + return; + } + resource.relationships[field] = { + links: { self: identityOf(value, path) }, + }; +} + +// A link holds the identity of a card. Anything else in a link position would +// fail on every invocation, so it is refused with the position that names it. +function identityOf(value: unknown, path: string): string { + if (typeof value === 'string' && value.length > 0) { + return value; + } + throw new OperationFailure({ + status: 400, + code: 'invalid-params', + title: 'Invalid link', + detail: + `\`${path}\` holds a link, so it needs the identity of a card — a URL, ` + + `or the \`lid\` of one this batch creates`, + }); +} + +interface TemplateScope { + entry: CreateEntry; + definition: OperationDefinition; + ctx: StagingContext; + anchor: { id: string; resource: CardResource } | undefined; + field: string; +} + +interface ResolvedTemplate { + value: unknown; + // Whether the template itself declared this a link — a `card(…)` marker, or + // a param the schema declares as one. + isLink: boolean; +} + +function resolveTemplate( + template: OperationTemplate, + scope: TemplateScope, + path = scope.field, +): ResolvedTemplate { + if (Array.isArray(template)) { + let members = template.map((member, index) => + resolveTemplate(member, scope, `${path}[${index}]`), + ); + return { + value: members.map((member) => member.value), + isLink: members.some((member) => member.isLink), + }; + } + if (!isMarker(template)) { + if (template !== null && typeof template === 'object') { + let value: Record = {}; + let isLink = false; + for (let [key, member] of Object.entries( + template as Record, + )) { + let resolved = resolveTemplate(member, scope, `${path}.${key}`); + value[key] = resolved.value; + isLink ||= resolved.isLink; + } + return { value, isLink }; + } + return { value: template, isLink: false }; + } + return resolveMarker(template, scope, path); +} + +function resolveMarker( + marker: Record, + scope: TemplateScope, + path: string, +): ResolvedTemplate { + let { entry, definition, ctx, anchor } = scope; + switch (marker.$ref) { + case 'params': { + let key = String(marker.key); + let declared = own(definition.params, key); + let value = own(entry.params, key); + return { + // A `link` param carries a card identity: the URL of a saved card, or + // the `lid` of one this batch creates, which resolves to the URL the + // create will answer to. + value: + declared?.kind === 'link' + ? resolveLinkParam(value, ctx, path) + : value, + isLink: declared?.kind === 'link', + }; + } + case 'actor': + // The realm knows the actor by identity, which is what a link to them + // needs and the only member there is to read. + if (marker.key !== undefined && marker.key !== 'id') { + throw new OperationFailure({ + status: 400, + code: 'invalid-params', + title: 'Invalid reference', + detail: + `\`${path}\` reads actor("${String(marker.key)}"), and the realm ` + + `knows the actor by identity alone`, + }); + } + return { value: ctx.actor, isLink: false }; + case 'instance': { + if (!anchor) { + throw new OperationFailure({ + status: 400, + code: 'invalid-params', + title: 'No instance in scope', + detail: + `\`${path}\` reads the target's stored document, and this ` + + `invocation names no target`, + }); + } + if (marker.key === undefined || marker.key === 'id') { + return { value: anchor.id, isLink: false }; + } + return { + value: anchor.resource.attributes?.[String(marker.key)], + isLink: false, + }; + } + case 'card': { + // The marker says this value is a link whatever it resolves to, so a + // nested reference is resolved and the result read as an identity. + let inner = isMarker(marker.value) + ? resolveMarker(marker.value, scope, path).value + : marker.value; + return { value: resolveLinkParam(inner, ctx, path), isLink: true }; + } + default: + throw new OperationFailure({ + status: 400, + code: 'invalid-params', + title: 'Invalid reference', + detail: `\`${path}\` carries a reference the realm does not resolve: ${JSON.stringify( + marker.$ref, + )}`, + }); + } +} + +// A link's value as the caller supplied it: the URL of a saved card, or the +// `lid` of one this batch creates. +function resolveLinkParam( + value: unknown, + ctx: StagingContext, + path: string, +): unknown { + if (Array.isArray(value)) { + return value.map((member, index) => + resolveLinkParam(member, ctx, `${path}[${index}]`), + ); + } + if (isPlainRecord(value) && typeof value.lid === 'string') { + return stagedLid(value.lid, ctx).id; + } + return value; +} + +// --------------------------------------------------------------------------- +// Shared +// --------------------------------------------------------------------------- + +function targetURL(href: string): URL { + try { + return new URL(href); + } catch { + throw new OperationFailure({ + id: href, + status: 400, + code: 'invalid-params', + title: 'Invalid target', + detail: `target "${href}" is not a URL`, + }); + } +} + +function localPathIn(url: URL, ctx: StagingContext): LocalPath { + try { + return ctx.paths.local(url); + } catch { + throw new OperationFailure({ + id: url.href, + status: 404, + code: 'target-not-found', + title: 'Not found', + detail: `realm ${ctx.realmURL} does not contain ${url.href}`, + }); + } +} + +// Read a record by a key that arrived over the wire. A plain object answers +// `toString` and `constructor` with something that is not a param, and reading +// one of those as a declaration gets as far as staging a value nobody sent. +function own( + record: Record | undefined, + key: string, +): T | undefined { + if (!record || !Object.prototype.hasOwnProperty.call(record, key)) { + return undefined; + } + return record[key]; +} + +function isPlainRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +function isMarker(value: unknown): value is Record { + return ( + isPlainRecord(value) && + Object.prototype.hasOwnProperty.call(value, '$ref') && + typeof value.$ref === 'string' + ); +} diff --git a/packages/runtime-common/card-operations/index.ts b/packages/runtime-common/card-operations/index.ts index 703ac48599b..a818c363421 100644 --- a/packages/runtime-common/card-operations/index.ts +++ b/packages/runtime-common/card-operations/index.ts @@ -17,6 +17,26 @@ export type { RunOperationOptions, } from './dispatch.ts'; export { readOperation } from './read.ts'; +export { commitBatch } from './coordinator.ts'; +export type { + BatchCore, + BatchEntryResult, + CommitBatchOptions, +} from './coordinator.ts'; +export { stageCreate, stageDelete, stageUpdate } from './executors.ts'; +export type { + BatchDocument, + BatchEntry, + CreateEntry, + DeleteEntry, + LidIndex, + StagedChange, + StagedIdentity, + StagedWrite, + StagingContext, + StoredFile, + UpdateEntry, +} from './executors.ts'; export { lowerQueryOperation } from './query.ts'; export type { QueryInvocation } from './query.ts'; export { diff --git a/packages/runtime-common/card-operations/types.ts b/packages/runtime-common/card-operations/types.ts index a004319463b..2d51bfb2cd0 100644 --- a/packages/runtime-common/card-operations/types.ts +++ b/packages/runtime-common/card-operations/types.ts @@ -273,6 +273,12 @@ export interface OperationHeadResult { // nothing else. export interface OperationIdentityResult { id: string; + // Echoed by a create, so a caller can match the URL the realm minted back to + // the `lid` it named the card with. A `lid` is the caller's own id for a card + // that does not exist yet, so this is the only thing that ties the two + // together — nothing in the minted URL carries it once the realm has chosen + // one. + lid?: string; meta: { // The token a later request passes as `baseVersion`. version: string; diff --git a/packages/runtime-common/realm-index-updater.ts b/packages/runtime-common/realm-index-updater.ts index 947b0c0cd2e..c6c43aaef17 100644 --- a/packages/runtime-common/realm-index-updater.ts +++ b/packages/runtime-common/realm-index-updater.ts @@ -27,6 +27,41 @@ import type { Realm } from './realm.ts'; import { RealmPaths } from './paths.ts'; import { ignore, type Ignore } from './ignore.ts'; +// One file's place in an index job's change set: whether the file is there to +// be visited or gone. A job carries a set of these rather than one operation +// for the whole job because a single commit can write some files and remove +// others, and the invalidation fan-out for all of them has to be computed +// against one snapshot of the realm. +export interface IndexChange { + url: URL; + operation: 'update' | 'delete'; +} + +export interface IncrementalIndexOptions { + onInvalidation?: ( + invalidatedURLs: URL[], + meta: { generation?: number }, + ) => Promise; + // Runs after the worker job resolves and onInvalidation finishes, but + // before the indexing deferred is fulfilled and removed from + // #incrementalIndexingDeferreds. This is the hook callers use for work that + // must happen before `realm.incrementalIndexing()` resolves — for example, + // the post-worker invalidation broadcast on the deferred-indexing path. + // Without this, an outer `.then()` would fire after the drain returns and + // could race with test teardown. + onSettled?: () => Promise | void; + // Runs when the worker job rejects, inside the same deferred lifecycle + // as onSettled (before the quiescence deferred fulfills). A failed + // incremental job may still have persisted setup-phase error docs, so + // callers use this to run the cache-invalidation / broadcast work the + // success path routes through onInvalidation — otherwise those rows + // stay hidden behind stale caches and silent subscribers until the + // next successful swap. Best-effort: a hook failure is logged and the + // job's own rejection still propagates through `settled`. + onFailed?: (error: unknown) => Promise | void; + clientRequestId?: string | null; +} + export class RealmIndexUpdater { #realm: Realm; #realmURL: URL | undefined; @@ -212,42 +247,36 @@ export class RealmIndexUpdater { } } - // Two-phase incremental update. Returns once the job is durably enqueued - // (the queue insert into Postgres has landed), with `settled` exposing the - // promise that resolves when the worker finishes and the optional - // onInvalidation/onSettled hooks run. Pre-enqueue failures - // (getRealmOwnerUsername, queue.publish) reject from this method so the - // caller knows the work was never queued and the realm is still - // consistent. Worker-time and post-worker failures reject from `settled` - // and surface via error_doc inside the worker. - // - // `onSettled` is part of the deferred lifecycle: it runs after the worker - // and onInvalidation finish, but before the indexing deferred is - // fulfilled and removed from #incrementalIndexingDeferreds. This is the - // hook callers use for work that must happen before `realm.incrementalIndexing()` - // resolves — for example, the post-worker invalidation broadcast on the - // deferred-indexing path. Without this, an outer `.then()` would fire - // after the drain returns and could race with test teardown. + // A change set whose every URL carries the same operation: `delete: true` + // for a set of removals, otherwise a set of updates. async enqueueUpdate( urls: URL[], - opts?: { - delete?: true; - onInvalidation?: ( - invalidatedURLs: URL[], - meta: { generation?: number }, - ) => Promise; - onSettled?: () => Promise | void; - // Runs when the worker job rejects, inside the same deferred lifecycle - // as onSettled (before the quiescence deferred fulfills). A failed - // incremental job may still have persisted setup-phase error docs, so - // callers use this to run the cache-invalidation / broadcast work the - // success path routes through onInvalidation — otherwise those rows - // stay hidden behind stale caches and silent subscribers until the - // next successful swap. Best-effort: a hook failure is logged and the - // job's own rejection still propagates through `settled`. - onFailed?: (error: unknown) => Promise | void; - clientRequestId?: string | null; - }, + opts?: IncrementalIndexOptions & { delete?: true }, + ): Promise<{ settled: Promise }> { + return await this.enqueueChanges( + urls.map((url) => ({ + url, + operation: opts?.delete ? 'delete' : ('update' as const), + })), + opts, + ); + } + + // Two-phase incremental update over a change set that may mix removals with + // updates, so a batch that writes some files and removes others is indexed + // and invalidated as one unit rather than as two jobs whose fan-outs are + // computed against different snapshots of the realm. + // + // Returns once the job is durably enqueued (the queue insert into Postgres + // has landed), with `settled` exposing the promise that resolves when the + // worker finishes and the optional onInvalidation/onSettled hooks run. + // Pre-enqueue failures (getRealmOwnerUsername, queue.publish) reject from + // this method so the caller knows the work was never queued and the realm + // is still consistent. Worker-time and post-worker failures reject from + // `settled` and surface via error_doc inside the worker. + async enqueueChanges( + changes: IndexChange[], + opts?: IncrementalIndexOptions, ): Promise<{ settled: Promise }> { let indexingDeferred = new Deferred(); this.#incrementalIndexingDeferreds.add(indexingDeferred); @@ -255,9 +284,9 @@ export class RealmIndexUpdater { let job: Job; try { let args: IncrementalIndexEnqueueArgs = { - changes: urls.map((url) => ({ + changes: changes.map(({ url, operation }) => ({ url: url.href, - operation: opts?.delete ? 'delete' : 'update', + operation, })), realmURL: this.#realm.url, realmUsername: await this.#realm.getRealmOwnerUsername(), @@ -321,19 +350,25 @@ export class RealmIndexUpdater { async update( urls: URL[], - opts?: { - delete?: true; - onInvalidation?: ( - invalidatedURLs: URL[], - meta: { generation?: number }, - ) => Promise; - clientRequestId?: string | null; - }, + opts?: Pick< + IncrementalIndexOptions, + 'onInvalidation' | 'clientRequestId' + > & { delete?: true }, ): Promise { let { settled } = await this.enqueueUpdate(urls, opts); await settled; } + // The awaited form of `enqueueChanges`, for a caller that reads indexed + // state once the job has landed. + async updateChanges( + changes: IndexChange[], + opts?: Pick, + ): Promise { + let { settled } = await this.enqueueChanges(changes, opts); + await settled; + } + async copy( sourceRealmURL: URL, onInvalidation?: (invalidatedURLs: URL[]) => Promise, diff --git a/packages/runtime-common/realm.ts b/packages/runtime-common/realm.ts index 9c2b0792721..190d903d792 100644 --- a/packages/runtime-common/realm.ts +++ b/packages/runtime-common/realm.ts @@ -73,6 +73,7 @@ import { removeFileMeta, getCreatedTime, getContentMeta, + getFileMetaForPaths, } from './file-meta.ts'; import { systemError, @@ -144,6 +145,7 @@ import { type FileMetaResource, } from './index.ts'; import type { OperationCore } from './card-operations/dispatch.ts'; +import type { BatchCore } from './card-operations/coordinator.ts'; import type { FromScratchResult } from './tasks/indexer.ts'; import { isCodeRef, visitModuleDeps } from './code-ref.ts'; import { merge } from 'lodash-es'; @@ -230,7 +232,7 @@ import { AliasCache } from './cache/alias-cache.ts'; import { DirectoryViewRefresher } from './directory-view-refresher.ts'; import { fetcher } from './fetcher.ts'; import { RealmIndexQueryEngine } from './realm-index-query-engine.ts'; -import { RealmIndexUpdater } from './realm-index-updater.ts'; +import { RealmIndexUpdater, type IndexChange } from './realm-index-updater.ts'; import serialize from './file-serializer.ts'; import { fileSizeLimitFor, @@ -830,6 +832,30 @@ export interface FileWriteResult extends AdapterWriteResult { path: string; lastModified: number; created: number | null; + // A fingerprint of the bytes now at this path, and the file's version: a + // caller holding one can tell whether the file has moved on since, and can + // name the base its next write is computed against. The same value is + // persisted on the file's `realm_file_meta` row. + contentHash: string; +} + +// Everything one commit changes. Both members are optional so a caller that +// only writes, or only removes, says exactly that. +export interface CommitBatch { + writes?: Map; + deletes?: LocalPath[]; +} + +export interface CommitBatchResult { + // One result per staged write, in the order the caller staged them. A path + // whose bytes were already what the caller staged is still reported here, + // carrying the file's existing modification time and hash. + writes: FileWriteResult[]; + // The index-data generation the commit's index pass landed on, for a caller + // that reports it alongside the versions above. Null when nothing was + // indexed — either because nothing changed on disk, or because the commit + // did not wait for indexing and the generation is not known yet. + generation: number | null; } export interface WriteOptions { @@ -960,6 +986,7 @@ export class Realm { #realmIndexUpdater: RealmIndexUpdater; #realmIndexQueryEngine: RealmIndexQueryEngine; #operationCore: OperationCore | undefined; + #batchCore: BatchCore | undefined; #adapter: RealmAdapter; #router: Router; #log = logger('realm'); @@ -1701,20 +1728,18 @@ export class Realm { } private async updateIndexAndCollectInvalidations( - urls: URL[], + changes: IndexChange[], opts?: { - delete?: true; clientRequestId?: string | null; }, ): Promise<{ invalidations: string[]; generation?: number }> { - if (urls.length === 0) { + if (changes.length === 0) { return { invalidations: [] }; } let invalidations = new Set(); let generation: number | undefined; - await this.#realmIndexUpdater.update(urls, { - ...(opts?.delete ? { delete: true } : {}), + await this.#realmIndexUpdater.updateChanges(changes, { clientRequestId: opts?.clientRequestId ?? null, onInvalidation: async (invalidatedURLs: URL[], meta) => { // Drop the searchCards in-flight map: the worker's batch.done() @@ -1751,9 +1776,8 @@ export class Realm { // waits for the broadcast, which is the only way an afterEach drain can // prevent the broadcast from racing with mock-matrix teardown. private async enqueueIndexUpdateAndCollectInvalidations( - urls: URL[], + changes: IndexChange[], opts: { - delete?: true; clientRequestId?: string | null; onSettled?: ( invalidations: string[], @@ -1761,7 +1785,7 @@ export class Realm { ) => Promise | void; }, ): Promise<{ settled: Promise }> { - if (urls.length === 0) { + if (changes.length === 0) { if (opts.onSettled) { await opts.onSettled([], {}); } @@ -1770,8 +1794,7 @@ export class Realm { let invalidations = new Set(); let generation: number | undefined; - let { settled } = await this.#realmIndexUpdater.enqueueUpdate(urls, { - ...(opts?.delete ? { delete: true } : {}), + let { settled } = await this.#realmIndexUpdater.enqueueChanges(changes, { clientRequestId: opts?.clientRequestId ?? null, onInvalidation: async (invalidatedURLs: URL[], meta) => { await this.clearRealmIndexCachesAndBroadcast(); @@ -1796,7 +1819,7 @@ export class Realm { // nothing landed: subscribers re-fetch and find the rows unchanged. await this.clearRealmIndexCachesAndBroadcast(); this.broadcastIncrementalInvalidationEvent( - urls.map((url) => url.href.replace(/\.json$/, '')), + changes.map(({ url }) => url.href.replace(/\.json$/, '')), { clientRequestId: opts?.clientRequestId ?? null }, ); }, @@ -1882,7 +1905,9 @@ export class Realm { } let { invalidations, generation } = - await this.updateIndexAndCollectInvalidations(urls); + await this.updateIndexAndCollectInvalidations( + urls.map((url) => ({ url, operation: 'update' as const })), + ); this.broadcastIncrementalInvalidationEvent(invalidations, { generation }); return createResponse({ @@ -2303,6 +2328,35 @@ export class Realm { files: Map, options?: WriteOptions, ): Promise { + let { writes } = await this._commitBatchUnlocked( + { writes: files }, + options, + ); + return writes; + } + + // One commit covering every file a caller is changing — the bytes it writes + // and the paths it removes — under a single index job and a single index + // event. A caller staging several changes to several cards needs them to + // land together: two jobs would compute their invalidation fan-outs against + // different snapshots of the realm, and two events would let a subscriber + // observe the batch half-applied. + // + // Writes land before removals. A write's serialization resolves the + // definitions its instance adopts from, which the write leg's own + // module-then-instance flush is what keeps current; a removal invalidates + // rows and touches no definition, so it has nothing to contribute to that + // flush and nothing to gain from running ahead of it. + // + // Assumes the realm's write lock is held — the caller reads the pre-state it + // stages from inside the same critical section. `write`, `writeMany` and + // `delete` are the locked public entry points. + private async _commitBatchUnlocked( + batch: CommitBatch, + options?: WriteOptions, + ): Promise { + let files = batch.writes ?? new Map(); + let deletes = batch.deletes ?? []; // The /_atomic endpoint (and any other writeMany caller that opts // out of post-write indexing via waitForIndex:false) does not read // its response from the index, so it has no reason to wait for @@ -2319,7 +2373,11 @@ export class Realm { } let urls: URL[] = []; // Collect write results for all files we wrote - let results: { path: LocalPath; lastModified: number }[] = []; + let results: { + path: LocalPath; + lastModified: number; + contentHash: string; + }[] = []; let fileMetaRows: { path: LocalPath; contentHash?: string; @@ -2333,12 +2391,13 @@ export class Realm { let mintedLoaderEpoch = false; let addedFiles: LocalPath[] = []; let updatedFiles: LocalPath[] = []; + let removedFiles: LocalPath[] = []; let invalidations: Set = new Set(); let indexGeneration: number | undefined; let clientRequestId: string | null = options?.clientRequestId ?? null; - let performIndex = async () => { + let performIndex = async (changes: IndexChange[]) => { let { invalidations: workingInvalidations, generation } = - await this.updateIndexAndCollectInvalidations(urls, { + await this.updateIndexAndCollectInvalidations(changes, { clientRequestId, }); invalidations = new Set([...invalidations, ...workingInvalidations]); @@ -2382,7 +2441,7 @@ export class Realm { // modules the instances depend on and only flush when an instance // depends on a module that is part of this operation. if (lastWriteType === 'module' && currentWriteType === 'instance') { - await performIndex(); + await performIndex(asUpdates(urls)); urls = []; } @@ -2418,7 +2477,16 @@ export class Realm { this.#adapter.openFile(p), ); if (existingFile?.content === content) { - results.push({ path, lastModified: existingFile.lastModified }); + // Identical bytes: the file is left alone, so its modification time + // stands and nothing is queued for indexing. The content hash is + // still the file's own — the bytes in hand are the bytes on disk — + // so a caller reading a version off this result gets the one the + // file already holds rather than nothing. + results.push({ + path, + lastModified: existingFile.lastModified, + contentHash: computeContentHash(content), + }); fileMetaRows.push({ path }); continue; } @@ -2522,30 +2590,61 @@ export class Realm { ); } } - results.push({ path, lastModified }); + results.push({ path, lastModified, contentHash }); fileMetaRows.push({ path, contentHash, contentSize }); urls.push(url); lastWriteType = currentWriteType ?? lastWriteType; } - if (addedFiles.length > 0 || updatedFiles.length > 0) { - if ([...addedFiles, ...updatedFiles].some((f) => f === 'realm.json')) { + // The removal leg. Each path gets the same per-file treatment a write + // does — the initiation event, the own-write tracking that stops the file + // watcher from re-reporting this replica's own change, the byte-cache + // drop, and the peer notification — and then the whole batch's file + // changes are announced together below. + let deleteURLs: URL[] = []; + for (let path of deletes) { + let url = this.paths.fileURL(path); + this.sendIndexInitiationEvent(url.href); + await this.trackOwnWrite(path, { isDelete: true }); + await this.#adapter.remove(path); + this.invalidateCache(path); + await this.#notifyFileChange(path); + removedFiles.push(path); + deleteURLs.push(url); + } + + if ( + addedFiles.length > 0 || + updatedFiles.length > 0 || + removedFiles.length > 0 + ) { + if ( + [...addedFiles, ...updatedFiles, ...removedFiles].some( + (f) => f === 'realm.json', + ) + ) { this.invalidateCachedRealmInfo(); } this.broadcastRealmEvent({ eventName: 'update', ...(addedFiles.length ? { added: addedFiles } : {}), ...(updatedFiles.length ? { updated: updatedFiles } : {}), + ...(removedFiles.length ? { removed: removedFiles } : {}), realmURL: this.url, } as UpdateRealmEventContent); } // persist file meta (created_at) to DB independent of index and retrieve created let createdMap = await this.persistFileMeta(fileMetaRows); + await this.removeFileMeta(removedFiles); let waitForIndex = options?.waitForIndex !== false; - if (urls.length > 0) { + let changes: IndexChange[] = [ + ...asUpdates(urls), + ...deleteURLs.map((url) => ({ url, operation: 'delete' as const })), + ]; + if (changes.length > 0) { if (waitForIndex) { - await performIndex(); + await performIndex(changes); this.broadcastIncrementalInvalidationEvent([...invalidations], { clientRequestId, generation: indexGeneration, @@ -2570,7 +2669,7 @@ export class Realm { // them — but it's correct for the primitive in general. let priorInvalidations = [...invalidations]; let { settled } = await this.enqueueIndexUpdateAndCollectInvalidations( - urls, + changes, { clientRequestId, // Route the post-worker broadcast through onSettled so it runs @@ -2596,25 +2695,30 @@ export class Realm { // Covers worker job rejection AND post-worker realm-side work // (onInvalidation / handleExecutableInvalidations / broadcast). this.#log.error( - `Deferred indexing chain failed for ${this.url} (urls: ${urls - .map((u) => u.href) + `Deferred indexing chain failed for ${this.url} (urls: ${changes + .map(({ url }) => url.href) .join(', ')}): ${stringifyErrorForLog(err)}`, ); }); } } else { - // No urls actually written (e.g., content unchanged). Preserve the - // pre-existing always-broadcast behavior. + // Nothing changed on disk (e.g., every file's content was already what + // the caller staged). Preserve the pre-existing always-broadcast + // behavior. this.broadcastIncrementalInvalidationEvent([...invalidations], { clientRequestId, generation: indexGeneration, }); } - return results.map(({ path, lastModified }) => ({ - path, - lastModified, - created: createdMap.get(path)?.createdAt ?? null, - })); + return { + writes: results.map(({ path, lastModified, contentHash }) => ({ + path, + lastModified, + contentHash, + created: createdMap.get(path)?.createdAt ?? null, + })), + generation: indexGeneration ?? null, + }; } // persist created_at into realm_file_meta table using db adapter @@ -3075,9 +3179,9 @@ export class Realm { let waitForIndex = options?.waitForIndex !== false; if (waitForIndex) { let { invalidations, generation } = - await this.updateIndexAndCollectInvalidations([url], { - delete: true, - }); + await this.updateIndexAndCollectInvalidations([ + { url, operation: 'delete' }, + ]); this.broadcastIncrementalInvalidationEvent(invalidations, { generation }); } else { // Mirrors the write() waitForIndex:false path: await the durable @@ -3087,9 +3191,8 @@ export class Realm { // doesn't resolve before the broadcast. let enqueueStart = Date.now(); let { settled } = await this.enqueueIndexUpdateAndCollectInvalidations( - [url], + [{ url, operation: 'delete' }], { - delete: true, onSettled: (deferredInvalidations, meta) => { this.broadcastIncrementalInvalidationEvent(deferredInvalidations, { generation: meta.generation, @@ -3143,9 +3246,9 @@ export class Realm { // Remove file meta for all deleted paths await this.removeFileMeta(paths); let { invalidations, generation } = - await this.updateIndexAndCollectInvalidations(urls, { - delete: true, - }); + await this.updateIndexAndCollectInvalidations( + urls.map((url) => ({ url, operation: 'delete' as const })), + ); this.broadcastIncrementalInvalidationEvent(invalidations, { generation }); } @@ -3194,6 +3297,61 @@ export class Realm { return this.#operationCore; } + // The batch coordinator's collaborators, built the same way and for the same + // reason as the operation core's: the write-side work an operation does is + // handed down as bound functions, so no executor resolves an identifier, + // opens a file, or reaches the network. The lock and the unlocked commit are + // among them, which is what puts the coordinator in charge of taking the lock + // once and keeps it out of the business of re-entering it. + get batchCore(): BatchCore { + if (!this.#batchCore) { + this.#batchCore = { + realmURL: this.url, + withWriteLock: (fn) => this.#dbAdapter.withWriteLock(this.url, fn), + readSourceFile: async (localPath) => { + let file = await this.readFileAsText(localPath); + return file + ? { content: file.content, lastModified: file.lastModified } + : undefined; + }, + contentHashes: async (localPaths) => { + let meta = await getFileMetaForPaths( + this.#dbAdapter, + this.url, + localPaths, + ); + return new Map( + localPaths.map((localPath) => [ + localPath, + meta.get(localPath)?.contentHash, + ]), + ); + }, + commitUnlocked: (batch, options) => + this._commitBatchUnlocked(batch, options), + serializeCard: (doc, relativeTo) => + this.fileSerialization(doc, relativeTo), + codeRefKey: (codeRef, relativeTo) => + internalKeyFor(codeRef, relativeTo, this.#virtualNetwork), + resolveModuleId: (moduleId, relativeTo) => + this.#virtualNetwork.resolveRRI(moduleId, rri(relativeTo)), + lookupDefinition: async (codeRef, relativeTo) => { + let absolute = codeRefWithAbsoluteIdentifier( + codeRef, + relativeTo, + undefined, + this.#virtualNetwork, + ); + if (!isResolvedCodeRef(absolute)) { + return undefined; + } + return await this.#definitionLookup.lookupDefinition(absolute); + }, + }; + } + return this.#batchCore; + } + async reindex() { let { completed } = this.startReindex(); await completed; @@ -8755,9 +8913,9 @@ export class Realm { for (let { operation, url } of items) { this.sendIndexInitiationEvent(url.href); let { invalidations, generation } = - await this.updateIndexAndCollectInvalidations([url], { - ...(operation === 'removed' ? { delete: true } : {}), - }); + await this.updateIndexAndCollectInvalidations([ + { url, operation: operation === 'removed' ? 'delete' : 'update' }, + ]); this.broadcastIncrementalInvalidationEvent(invalidations, { generation }); } itemsDrained!(); @@ -8991,6 +9149,11 @@ export interface CardDefinitionResource { }; } +// A change set whose every URL names a file that is there to be visited. +function asUpdates(urls: URL[]): IndexChange[] { + return urls.map((url) => ({ url, operation: 'update' as const })); +} + function promoteLocalIdsToRemoteIds({ resource, realmURL, From d15fedf86ed899937f88bbeb00f3c0893f896af8 Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Thu, 10 Sep 2026 14:21:37 +0000 Subject: [PATCH 02/14] Cover the coordinator with stubbed staging tests, and drop the create loop's index arithmetic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The staging decisions — which files a batch would write, where each local id resolves, whether the batch reaches the commit at all — are checkable without a realm, and checking them against a stub is what makes "nothing is committed" an assertion about the property itself rather than a proxy for it. The realm- driven suite keeps what only exists against a realm: bytes on disk, one index job, one index event, and byte-for-byte agreement with the PATCH handler. `stageCreate` handles its primary before its side-loaded resources rather than walking one list and branching on the index, which drops two non-null assertions and lets both it and `stageUpdate` share one side-loaded staging step. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EBB6JdRo5XCY5WhUkgBckS --- .../tests/card-operations-batch-test.ts | 716 ++----------- .../tests/card-operations-commit-test.ts | 692 ++++++++++++ .../card-operations/executors.ts | 85 +- .../tests/card-operations-batch-test.ts | 994 ++++++++++++++++++ 4 files changed, 1812 insertions(+), 675 deletions(-) create mode 100644 packages/realm-server/tests/card-operations-commit-test.ts create mode 100644 packages/runtime-common/tests/card-operations-batch-test.ts diff --git a/packages/realm-server/tests/card-operations-batch-test.ts b/packages/realm-server/tests/card-operations-batch-test.ts index aa89c5f1c01..4f7210cbd8c 100644 --- a/packages/realm-server/tests/card-operations-batch-test.ts +++ b/packages/realm-server/tests/card-operations-batch-test.ts @@ -1,663 +1,113 @@ import QUnit from 'qunit'; const { module, test } = QUnit; -import { basename, join } from 'path'; -import { existsSync, readFileSync } from 'fs-extra'; -import type { Test, SuperTest } from 'supertest'; -import type { DirResult } from 'tmp'; -import type { PgAdapter } from '@cardstack/postgres'; - -import { rri } from '@cardstack/runtime-common'; -import { - commitBatch, - isOperationFailure, - type BatchDocument, - type BatchEntry, -} from '@cardstack/runtime-common/card-operations'; -import type { - DBAdapter, - LooseSingleCardDocument, - Realm, -} from '@cardstack/runtime-common'; -import { APP_BOXEL_REALM_EVENT_TYPE } from '@cardstack/runtime-common/matrix-constants'; -import type { RealmHttpServer as Server } from '../server.ts'; -import { - setupPermissionedRealmCached, - setupMatrixRoom, - withRealmPath, - type RealmRequest, -} from './helpers/index.ts'; - -const testRealm = new URL('http://127.0.0.1:4445/test/'); -const testRealmHref = testRealm.href; -const PERSON = { module: rri('./person'), name: 'Person' }; - -// ============================================================================ -// The batch coordinator, driven against a real realm. -// -// What is under test is the batch's all-or-nothing property and the things -// that property is observable through: what lands on disk, how many index jobs -// the commit enqueues, how many index events it broadcasts, and what each -// entry's result reports. Those only exist against a real realm — the jobs -// table and the Matrix room are where "one job, one event" is either true or -// not — so the coordinator is called directly with the realm's own batch core -// rather than through a stub. -// ============================================================================ - -function makeFileSystem(): Record { - return { - 'person.gts': ` - import { contains, field, linksTo, CardDef, Component } from "@cardstack/base/card-api"; - import StringField from "@cardstack/base/string"; - import NumberField from "@cardstack/base/number"; - - export class Person extends CardDef { - @field firstName = contains(StringField); - @field hourlyRate = contains(NumberField); - @field friend = linksTo(() => Person, { searchable: true }); - static isolated = class Isolated extends Component { - - } - static embedded = class Embedded extends Component { - - } - static fitted = class Fitted extends Component { - - } - } - `, - 'person-a.json': { - data: { - type: 'card', - attributes: { firstName: 'Original', hourlyRate: 10 }, - meta: { adoptsFrom: PERSON }, - }, - }, - 'person-b.json': { - data: { - type: 'card', - attributes: { firstName: 'Original', hourlyRate: 10 }, - meta: { adoptsFrom: PERSON }, - }, - }, - 'person-c.json': { - data: { - type: 'card', - attributes: { firstName: 'Doomed', hourlyRate: 1 }, - meta: { adoptsFrom: PERSON }, - }, - }, - }; -} - -module(basename(import.meta.filename), function (hooks) { - let realm: Realm; - let testDbAdapter: DBAdapter; - let request: RealmRequest; - let serverRequest: SuperTest; - let testRealmHttpServer: Server; - let dir: DirResult; - - setupPermissionedRealmCached(hooks, { - mode: 'beforeEach', - realmURL: testRealm, - permissions: { - '*': ['read', 'write'], - '@node-test_realm:localhost': ['read', 'write', 'realm-owner'], - }, - subscribeToRealmEvents: true, - fileSystem: makeFileSystem(), - onRealmSetup(args) { - realm = args.testRealm; - testDbAdapter = args.dbAdapter; - request = withRealmPath(args.request, testRealm); - serverRequest = args.request; - testRealmHttpServer = args.testRealmHttpServer; - dir = args.dir; - }, - }); - - let { getMessagesSince } = setupMatrixRoom(hooks, () => ({ - testRealm: realm, - testRealmHttpServer, - request, - serverRequest, - dir, - dbAdapter: testDbAdapter as PgAdapter, - })); - - function realmFile(localPath: string): string { - return join(dir.name, 'realm_server_1', 'test', localPath); - } - - async function indexJobIds(): Promise { - let rows = (await testDbAdapter.execute( - `select id from jobs where job_type = 'incremental-index' - and concurrency_group = $1 order by id`, - { bind: [`indexing:${realm.url}`] }, - )) as { id: number | string }[]; - return rows.map((row) => Number(row.id)); - } - - async function realmEventsSince(since: number) { - let messages = await getMessagesSince(since); - return messages.filter( - (message) => message.type === APP_BOXEL_REALM_EVENT_TYPE, - ); - } - - async function commit(entries: BatchEntry[], clientRequestId?: string) { - return await commitBatch(realm.batchCore, entries, { - clientRequestId: clientRequestId ?? null, - actor: '@tester:localhost', +import { basename } from 'path'; +import { runSharedTest } from '@cardstack/runtime-common/helpers'; +import cardOperationsBatchTests from '@cardstack/runtime-common/tests/card-operations-batch-test'; + +module(basename(import.meta.filename), function () { + module('card operations batch', function () { + test('a create is staged at the path its local id names', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); }); - } - // The `links.self` a stored relationship holds, resolved against the card - // whose file carries it. Serialization is free to record a link relative to - // the card that holds it, so the absolute identity is what a test compares. - function storedLink(localPath: string, field: string): string | undefined { - let doc = JSON.parse(readFileSync(realmFile(localPath), 'utf8')); - let self = doc.data?.relationships?.[field]?.links?.self; - return self == null - ? undefined - : new URL(self, `${testRealmHref}${localPath}`).href; - } - - test('a batch creates several cards and links them by local id', async function (assert) { - let results = await commit([ - { - op: 'create', - lid: 'author', - document: { - data: { - type: 'card', - attributes: { firstName: 'Mango', hourlyRate: 100 }, - meta: { adoptsFrom: PERSON }, - }, - }, - }, - { - op: 'create', - lid: 'sidekick', - document: { - data: { - type: 'card', - attributes: { firstName: 'Van Gogh' }, - relationships: { - friend: { data: { lid: 'author', type: 'card' } }, - }, - meta: { adoptsFrom: PERSON }, - }, - }, - }, - ]); - - assert.strictEqual(results.length, 2, 'one result per entry, in order'); - assert.deepEqual( - results.map((result) => result && 'lid' in result && result.lid), - ['author', 'sidekick'], - 'each create echoes the local id the client named it with', - ); - assert.deepEqual( - results.map((result) => result?.id), - [`${testRealmHref}Person/author`, `${testRealmHref}Person/sidekick`], - 'a local id maps to the URL the realm minted for it', - ); - assert.ok( - existsSync(realmFile('Person/author.json')), - 'the first card is on disk', - ); - assert.ok( - existsSync(realmFile('Person/sidekick.json')), - 'the second card is on disk', - ); - assert.strictEqual( - storedLink('Person/sidekick.json', 'friend'), - `${testRealmHref}Person/author`, - 'the link resolves to the card the other entry in the batch minted', - ); - for (let result of results) { - assert.true( - (result?.meta.version ?? '').length > 0, - 'each result carries the version the file now holds', - ); - } - }); + test('a later entry links to a card an earlier one mints', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); - test('a failing entry leaves the whole batch unwritten, unindexed and unannounced', async function (assert) { - let jobsBefore = await indexJobIds(); - let since = Date.now(); + test('a side-loaded resource is created alongside its primary', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); - let failure: unknown; - try { - await commit([ - { - op: 'create', - lid: 'never-written', - document: { - data: { - type: 'card', - attributes: { firstName: 'Ghost' }, - meta: { adoptsFrom: PERSON }, - }, - }, - }, - { - op: 'update', - href: `${testRealmHref}does-not-exist`, - document: { - data: { - type: 'card', - attributes: { firstName: 'Nope' }, - meta: { adoptsFrom: PERSON }, - }, - }, - }, - ]); - } catch (err: unknown) { - failure = err; - } + test('an entry that cannot be staged abandons the batch before it commits', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); - assert.ok(isOperationFailure(failure), 'the batch is rejected'); - if (isOperationFailure(failure)) { - assert.strictEqual(failure.error.status, 404, "the entry's own status"); - assert.strictEqual(failure.error.code, 'target-not-found'); - assert.strictEqual( - failure.error.meta?.entry, - 1, - 'the refusal names the position of the entry that produced it', - ); - } - assert.notOk( - existsSync(realmFile('Person/never-written.json')), - 'the entry ahead of the failure is not written', - ); - assert.deepEqual( - await indexJobIds(), - jobsBefore, - 'no incremental index job is enqueued', - ); - assert.deepEqual( - await realmEventsSince(since), - [], - 'nothing is announced to the realm', - ); - }); + test('a patch merges over the stored file, replacing arrays', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); - test('a batch of one write and one delete commits under one index job and one index event', async function (assert) { - let jobsBefore = await indexJobIds(); - let since = Date.now(); + test('a patch that changes nothing stages the bytes already on disk', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); - let results = await commit( - [ - { - op: 'update', - href: `${testRealmHref}person-a`, - document: { - data: { - type: 'card', - attributes: { firstName: 'Renamed' }, - meta: { adoptsFrom: PERSON }, - }, - }, - }, - { op: 'delete', href: `${testRealmHref}person-c` }, - ], - 'batch-1', - ); + test('a patch cannot change the type a card adopts', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); - assert.strictEqual( - results[1], - null, - 'a delete has no state left to describe', - ); - assert.ok(results[0], 'the write reports its identity'); - assert.notOk( - existsSync(realmFile('person-c.json')), - 'the deleted card is gone from disk', - ); - assert.true( - readFileSync(realmFile('person-a.json'), 'utf8').includes('Renamed'), - 'the written card holds the patched value', - ); + test('realm-managed keys in a patch never reach the file', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); - let newJobs = (await indexJobIds()).filter( - (id) => !jobsBefore.includes(id), - ); - assert.strictEqual( - newJobs.length, - 1, - `the write and the delete share one index job (got ${newJobs.length})`, - ); + test('a write and a removal reach the commit together', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); - let indexEvents = (await realmEventsSince(since)).filter( - (event) => - (event.content as { eventName?: string; indexType?: string }) - .eventName === 'index' && - (event.content as { indexType?: string }).indexType === 'incremental', - ); - assert.strictEqual( - indexEvents.length, - 1, - `the batch broadcasts one index event (got ${indexEvents.length})`, - ); - assert.strictEqual( - (indexEvents[0].content as { clientRequestId?: string }).clientRequestId, - 'batch-1', - "the event carries the batch's own client request id", - ); - }); + test('a removal needs something to remove', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); - test('a base version is reported as matched or moved against the version the file held', async function (assert) { - let [first] = await commit([ - { - op: 'update', - href: `${testRealmHref}person-a`, - document: { - data: { - type: 'card', - attributes: { firstName: 'First' }, - meta: { adoptsFrom: PERSON }, - }, - }, - }, - ]); - assert.ok(first, 'the first write reports a version'); - let version = first!.meta.version; - assert.strictEqual( - first!.meta.baseMatched, - undefined, - 'an unconditional write reports no base match', - ); + test('a base version is reported against the fingerprint the file carried', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); - let [matched] = await commit([ - { - op: 'update', - href: `${testRealmHref}person-a`, - baseVersion: version, - document: { - data: { - type: 'card', - attributes: { firstName: 'Second' }, - meta: { adoptsFrom: PERSON }, - }, - }, - }, - ]); - assert.true( - matched!.meta.baseMatched, - 'the base the caller named is the one the file held', - ); + test('an unconditional write reports no base match', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); - let [moved] = await commit([ - { - op: 'update', - href: `${testRealmHref}person-a`, - baseVersion: version, - document: { - data: { - type: 'card', - attributes: { firstName: 'Third' }, - meta: { adoptsFrom: PERSON }, - }, - }, - }, - ]); - assert.false( - moved!.meta.baseMatched, - 'the file has moved past the base the caller named', - ); - assert.true( - readFileSync(realmFile('person-a.json'), 'utf8').includes('Third'), - 'a moved base is reported, not refused — the write still lands', - ); - }); + test('a base version belongs only to an update', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); - test('an update writes the same bytes a PATCH of the same document writes', async function (assert) { - let patch: BatchDocument = { - data: { - type: 'card', - attributes: { firstName: 'Paparazzi', hourlyRate: 42 }, - meta: { adoptsFrom: PERSON }, - }, - }; + test('a local id names one card', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); - let response = await request - .patch('/person-a') - .send(patch) - .set('Accept', 'application/vnd.card+json'); - assert.strictEqual(response.status, 200, 'the PATCH is served'); + test('a link to a local id nothing creates is refused', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); - await commit([ - { op: 'update', href: `${testRealmHref}person-b`, document: patch }, - ]); + test('two entries changing one card are refused rather than ordered', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); - assert.strictEqual( - readFileSync(realmFile('person-b.json'), 'utf8'), - readFileSync(realmFile('person-a.json'), 'utf8'), - 'the two files are byte-identical', - ); - }); + test('a target outside the realm is not the batch to commit it', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); - test('a patch that changes nothing leaves the file exactly as it is', async function (assert) { - let before = readFileSync(realmFile('person-a.json'), 'utf8'); - let jobsBefore = await indexJobIds(); + test('a create naming another realm is refused', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); - let [result] = await commit([ - { - op: 'update', - href: `${testRealmHref}person-a`, - document: { - data: { - type: 'card', - attributes: { firstName: 'Original' }, - meta: { adoptsFrom: PERSON }, - }, - }, - }, - ]); + test('a field the type refuses is the payload to fix, not the realm', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); - assert.strictEqual( - readFileSync(realmFile('person-a.json'), 'utf8'), - before, - 'the stored bytes are untouched', - ); - assert.true( - (result?.meta.version ?? '').length > 0, - 'the result still reports the version the file holds', - ); - assert.deepEqual( - await indexJobIds(), - jobsBefore, - 'nothing is queued for indexing', - ); - }); + test('a stored file that is not a card document is the realm to answer for', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); - test('a local id claimed twice, and one nothing creates, are both refused', async function (assert) { - let duplicate: unknown; - try { - await commit([ - { - op: 'create', - lid: 'twice', - document: { - data: { - type: 'card', - attributes: { firstName: 'One' }, - meta: { adoptsFrom: PERSON }, - }, - }, - }, - { - op: 'create', - lid: 'twice', - document: { - data: { - type: 'card', - attributes: { firstName: 'Two' }, - meta: { adoptsFrom: PERSON }, - }, - }, - }, - ]); - } catch (err: unknown) { - duplicate = err; - } - assert.ok(isOperationFailure(duplicate), 'a duplicate local id is refused'); - if (isOperationFailure(duplicate)) { - assert.strictEqual(duplicate.error.status, 400); - assert.strictEqual(duplicate.error.code, 'invalid-params'); - } + test('the write lock is taken once for the whole batch', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); - let dangling: unknown; - try { - await commit([ - { - op: 'create', - lid: 'lonely', - document: { - data: { - type: 'card', - attributes: { firstName: 'Lonely' }, - relationships: { - friend: { data: { lid: 'nobody', type: 'card' } }, - }, - meta: { adoptsFrom: PERSON }, - }, - }, - }, - ]); - } catch (err: unknown) { - dangling = err; - } - assert.ok( - isOperationFailure(dangling), - 'a link to a local id no entry creates is refused', - ); - if (isOperationFailure(dangling)) { - assert.strictEqual(dangling.error.status, 400); - assert.strictEqual( - dangling.error.meta?.entry, - 0, - 'the refusal names the entry that carried the link', - ); - } - assert.notOk( - existsSync(realmFile('Person/lonely.json')), - 'nothing is written for a refused batch', - ); - }); + test('a named create stages the type and attributes its declaration names', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); - test('a delete of a card that is not there refuses without touching the realm', async function (assert) { - let jobsBefore = await indexJobIds(); - let failure: unknown; - try { - await commit([{ op: 'delete', href: `${testRealmHref}not-a-card` }]); - } catch (err: unknown) { - failure = err; - } - assert.ok(isOperationFailure(failure), 'the delete is refused'); - if (isOperationFailure(failure)) { - assert.strictEqual(failure.error.status, 404); - assert.strictEqual(failure.error.code, 'target-not-found'); - } - assert.deepEqual( - await indexJobIds(), - jobsBefore, - 'no index job is enqueued', - ); - }); + test('a named create resolves the actor and the card it is anchored on', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); - test('two entries changing one card are refused rather than silently ordered', async function (assert) { - let failure: unknown; - try { - await commit([ - { - op: 'update', - href: `${testRealmHref}person-a`, - document: { - data: { - type: 'card', - attributes: { firstName: 'Left' }, - meta: { adoptsFrom: PERSON }, - }, - }, - }, - { - op: 'update', - href: `${testRealmHref}person-a`, - document: { - data: { - type: 'card', - attributes: { hourlyRate: 99 }, - meta: { adoptsFrom: PERSON }, - }, - }, - }, - ]); - } catch (err: unknown) { - failure = err; - } - assert.ok(isOperationFailure(failure), 'the batch is refused'); - if (isOperationFailure(failure)) { - assert.strictEqual(failure.error.code, 'invalid-params'); - assert.strictEqual(failure.error.meta?.conflictsWith, 0); - } - assert.true( - readFileSync(realmFile('person-a.json'), 'utf8').includes('Original'), - 'the card is left as it was', - ); - }); + test('a named create with no target in scope cannot read one', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); - test('a batch cannot change a card to another type', async function (assert) { - let failure: unknown; - try { - await commit([ - { - op: 'update', - href: `${testRealmHref}person-a`, - document: { - data: { - type: 'card', - attributes: { firstName: 'Shapeshifter' }, - meta: { - adoptsFrom: { - module: rri('@cardstack/base/card-api'), - name: 'CardDef', - }, - }, - }, - }, - }, - ]); - } catch (err: unknown) { - failure = err; - } - assert.ok(isOperationFailure(failure), 'the type change is refused'); - if (isOperationFailure(failure)) { - assert.strictEqual(failure.error.status, 400); - } - }); + test('a named create links to a card the same batch mints', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); - test('a base version on anything but an update is refused', async function (assert) { - let failure: unknown; - try { - await commit([ - { op: 'delete', href: `${testRealmHref}person-c`, baseVersion: 'abc' }, - ]); - } catch (err: unknown) { - failure = err; - } - assert.ok(isOperationFailure(failure), 'the base version is refused'); - assert.ok( - existsSync(realmFile('person-c.json')), - 'the card is left in place', - ); + test('a create with nothing to create is refused', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); }); }); diff --git a/packages/realm-server/tests/card-operations-commit-test.ts b/packages/realm-server/tests/card-operations-commit-test.ts new file mode 100644 index 00000000000..bbdc692709e --- /dev/null +++ b/packages/realm-server/tests/card-operations-commit-test.ts @@ -0,0 +1,692 @@ +import QUnit from 'qunit'; +const { module, test } = QUnit; +import { basename, join } from 'path'; +import fsExtra from 'fs-extra'; +const { existsSync, readFileSync } = fsExtra; +import type { Test, SuperTest } from 'supertest'; +import type { DirResult } from 'tmp'; +import type { PgAdapter } from '@cardstack/postgres'; + +import { rri } from '@cardstack/runtime-common'; +import { + commitBatch, + isOperationFailure, + type BatchDocument, + type BatchEntry, +} from '@cardstack/runtime-common/card-operations'; +import type { + DBAdapter, + LooseSingleCardDocument, + Realm, +} from '@cardstack/runtime-common'; +import { APP_BOXEL_REALM_EVENT_TYPE } from '@cardstack/runtime-common/matrix-constants'; +import type { + IncrementalIndexEventContent, + RealmEventContent, +} from '@cardstack/base/matrix-event'; +import type { RealmHttpServer as Server } from '../server.ts'; +import { + setupPermissionedRealmCached, + setupMatrixRoom, + withRealmPath, + type RealmRequest, +} from './helpers/index.ts'; + +const testRealm = new URL('http://127.0.0.1:4445/test/'); +const testRealmHref = testRealm.href; +const PERSON = { module: rri('./person'), name: 'Person' }; + +// ============================================================================ +// The batch coordinator, driven against a real realm. +// +// What is under test is the batch's all-or-nothing property and the things +// that property is observable through: what lands on disk, how many index jobs +// the commit enqueues, how many index events it broadcasts, and what each +// entry's result reports. Those only exist against a real realm — the jobs +// table and the Matrix room are where "one job, one event" is either true or +// not — so the coordinator is called directly with the realm's own batch core +// rather than through a stub. +// ============================================================================ + +function makeFileSystem(): Record { + return { + 'person.gts': ` + import { contains, field, linksTo, CardDef, Component } from "@cardstack/base/card-api"; + import StringField from "@cardstack/base/string"; + import NumberField from "@cardstack/base/number"; + + export class Person extends CardDef { + @field firstName = contains(StringField); + @field hourlyRate = contains(NumberField); + @field friend = linksTo(() => Person, { searchable: true }); + static isolated = class Isolated extends Component { + + } + static embedded = class Embedded extends Component { + + } + static fitted = class Fitted extends Component { + + } + } + `, + 'person-a.json': { + data: { + type: 'card', + attributes: { firstName: 'Original', hourlyRate: 10 }, + meta: { adoptsFrom: PERSON }, + }, + }, + 'person-b.json': { + data: { + type: 'card', + attributes: { firstName: 'Original', hourlyRate: 10 }, + meta: { adoptsFrom: PERSON }, + }, + }, + 'person-c.json': { + data: { + type: 'card', + attributes: { firstName: 'Doomed', hourlyRate: 1 }, + meta: { adoptsFrom: PERSON }, + }, + }, + }; +} + +module(basename(import.meta.filename), function (hooks) { + let realm: Realm; + let testDbAdapter: DBAdapter; + let request: RealmRequest; + let serverRequest: SuperTest; + let testRealmHttpServer: Server; + let dir: DirResult; + + setupPermissionedRealmCached(hooks, { + mode: 'beforeEach', + realmURL: testRealm, + permissions: { + '*': ['read', 'write'], + '@node-test_realm:localhost': ['read', 'write', 'realm-owner'], + }, + subscribeToRealmEvents: true, + fileSystem: makeFileSystem(), + onRealmSetup(args) { + realm = args.testRealm; + testDbAdapter = args.dbAdapter; + request = withRealmPath(args.request, testRealm); + serverRequest = args.request; + testRealmHttpServer = args.testRealmHttpServer; + dir = args.dir; + }, + }); + + let { getMessagesSince } = setupMatrixRoom(hooks, () => ({ + testRealm: realm, + testRealmHttpServer, + request, + serverRequest, + dir, + dbAdapter: testDbAdapter as PgAdapter, + })); + + function realmFile(localPath: string): string { + return join(dir.name, 'realm_server_1', 'test', localPath); + } + + async function indexJobIds(): Promise { + let rows = (await testDbAdapter.execute( + `select id from jobs where job_type = 'incremental-index' + and concurrency_group = $1 order by id`, + { bind: [`indexing:${realm.url}`] }, + )) as { id: number | string }[]; + return rows.map((row) => Number(row.id)); + } + + async function realmEventsSince(since: number) { + let messages = await getMessagesSince(since); + return messages + .filter((message) => message.type === APP_BOXEL_REALM_EVENT_TYPE) + .map((message) => message.content as RealmEventContent); + } + + // The realm events a batch is answerable for: the incremental index event it + // broadcasts, and the file-change event naming a path it touched. Filtering + // by path rather than counting everything keeps the assertion about this + // batch — a realm serving a suite broadcasts for its own reasons too. + function eventsNaming(events: RealmEventContent[], localPath: string) { + let instanceURL = `${testRealmHref}${localPath.replace(/\.json$/, '')}`; + return events.filter((event) => { + if (event.eventName === 'update') { + return [ + ...(event.added ?? []), + ...(event.updated ?? []), + ...(event.removed ?? []), + ].includes(localPath); + } + if (event.eventName === 'index' && event.indexType === 'incremental') { + return event.invalidations.includes(instanceURL); + } + return false; + }); + } + + async function commit(entries: BatchEntry[], clientRequestId?: string) { + return await commitBatch(realm.batchCore, entries, { + clientRequestId: clientRequestId ?? null, + actor: '@tester:localhost', + }); + } + + // The `links.self` a stored relationship holds, resolved against the card + // whose file carries it. Serialization is free to record a link relative to + // the card that holds it, so the absolute identity is what a test compares. + function storedLink(localPath: string, field: string): string | undefined { + let doc = JSON.parse(readFileSync(realmFile(localPath), 'utf8')); + let self = doc.data?.relationships?.[field]?.links?.self; + return self == null + ? undefined + : new URL(self, `${testRealmHref}${localPath}`).href; + } + + test('a batch creates several cards and links them by local id', async function (assert) { + let results = await commit([ + { + op: 'create', + lid: 'author', + document: { + data: { + type: 'card', + attributes: { firstName: 'Mango', hourlyRate: 100 }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + { + op: 'create', + lid: 'sidekick', + document: { + data: { + type: 'card', + attributes: { firstName: 'Van Gogh' }, + relationships: { + friend: { data: { lid: 'author', type: 'card' } }, + }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]); + + assert.strictEqual(results.length, 2, 'one result per entry, in order'); + assert.deepEqual( + results.map((result) => result && 'lid' in result && result.lid), + ['author', 'sidekick'], + 'each create echoes the local id the client named it with', + ); + assert.deepEqual( + results.map((result) => result?.id), + [`${testRealmHref}Person/author`, `${testRealmHref}Person/sidekick`], + 'a local id maps to the URL the realm minted for it', + ); + assert.ok( + existsSync(realmFile('Person/author.json')), + 'the first card is on disk', + ); + assert.ok( + existsSync(realmFile('Person/sidekick.json')), + 'the second card is on disk', + ); + assert.strictEqual( + storedLink('Person/sidekick.json', 'friend'), + `${testRealmHref}Person/author`, + 'the link resolves to the card the other entry in the batch minted', + ); + for (let result of results) { + assert.true( + (result?.meta.version ?? '').length > 0, + 'each result carries the version the file now holds', + ); + } + }); + + test('a failing entry leaves the whole batch unwritten, unindexed and unannounced', async function (assert) { + let jobsBefore = await indexJobIds(); + let since = Date.now(); + + let failure: unknown; + try { + await commit([ + { + op: 'create', + lid: 'never-written', + document: { + data: { + type: 'card', + attributes: { firstName: 'Ghost' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + { + op: 'update', + href: `${testRealmHref}does-not-exist`, + document: { + data: { + type: 'card', + attributes: { firstName: 'Nope' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]); + } catch (err: unknown) { + failure = err; + } + + assert.ok(isOperationFailure(failure), 'the batch is rejected'); + if (isOperationFailure(failure)) { + assert.strictEqual(failure.error.status, 404, "the entry's own status"); + assert.strictEqual(failure.error.code, 'target-not-found'); + assert.strictEqual( + failure.error.meta?.entry, + 1, + 'the refusal names the position of the entry that produced it', + ); + } + assert.notOk( + existsSync(realmFile('Person/never-written.json')), + 'the entry ahead of the failure is not written', + ); + assert.deepEqual( + await indexJobIds(), + jobsBefore, + 'no incremental index job is enqueued', + ); + assert.deepEqual( + eventsNaming(await realmEventsSince(since), 'Person/never-written.json'), + [], + 'nothing about the abandoned batch is announced to the realm', + ); + }); + + test('a batch of one write and one delete commits under one index job and one index event', async function (assert) { + let jobsBefore = await indexJobIds(); + let since = Date.now(); + + let results = await commit( + [ + { + op: 'update', + href: `${testRealmHref}person-a`, + document: { + data: { + type: 'card', + attributes: { firstName: 'Renamed' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + { op: 'delete', href: `${testRealmHref}person-c` }, + ], + 'batch-1', + ); + + assert.strictEqual( + results[1], + null, + 'a delete has no state left to describe', + ); + assert.ok(results[0], 'the write reports its identity'); + assert.notOk( + existsSync(realmFile('person-c.json')), + 'the deleted card is gone from disk', + ); + assert.true( + readFileSync(realmFile('person-a.json'), 'utf8').includes('Renamed'), + 'the written card holds the patched value', + ); + + let newJobs = (await indexJobIds()).filter( + (id) => !jobsBefore.includes(id), + ); + assert.strictEqual( + newJobs.length, + 1, + `the write and the delete share one index job (got ${newJobs.length})`, + ); + + let indexEvents = (await realmEventsSince(since)).filter( + (event): event is IncrementalIndexEventContent => + event.eventName === 'index' && event.indexType === 'incremental', + ); + assert.strictEqual( + indexEvents.length, + 1, + `the batch broadcasts one index event (got ${indexEvents.length})`, + ); + assert.strictEqual( + indexEvents[0].clientRequestId, + 'batch-1', + "the event carries the batch's own client request id", + ); + assert.deepEqual( + [...indexEvents[0].invalidations].sort(), + [`${testRealmHref}person-a`, `${testRealmHref}person-c`].sort(), + 'the one event covers both the write and the removal', + ); + }); + + test('a base version is reported as matched or moved against the version the file held', async function (assert) { + let [first] = await commit([ + { + op: 'update', + href: `${testRealmHref}person-a`, + document: { + data: { + type: 'card', + attributes: { firstName: 'First' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]); + assert.ok(first, 'the first write reports a version'); + let version = first!.meta.version; + assert.strictEqual( + first!.meta.baseMatched, + undefined, + 'an unconditional write reports no base match', + ); + + let [matched] = await commit([ + { + op: 'update', + href: `${testRealmHref}person-a`, + baseVersion: version, + document: { + data: { + type: 'card', + attributes: { firstName: 'Second' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]); + assert.true( + matched!.meta.baseMatched, + 'the base the caller named is the one the file held', + ); + + let [moved] = await commit([ + { + op: 'update', + href: `${testRealmHref}person-a`, + baseVersion: version, + document: { + data: { + type: 'card', + attributes: { firstName: 'Third' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]); + assert.false( + moved!.meta.baseMatched, + 'the file has moved past the base the caller named', + ); + assert.true( + readFileSync(realmFile('person-a.json'), 'utf8').includes('Third'), + 'a moved base is reported, not refused — the write still lands', + ); + }); + + test('an update writes the same bytes a PATCH of the same document writes', async function (assert) { + let patch: BatchDocument = { + data: { + type: 'card', + attributes: { firstName: 'Paparazzi', hourlyRate: 42 }, + meta: { adoptsFrom: PERSON }, + }, + }; + + let response = await request + .patch('/person-a') + .send(patch) + .set('Accept', 'application/vnd.card+json'); + assert.strictEqual(response.status, 200, 'the PATCH is served'); + + await commit([ + { op: 'update', href: `${testRealmHref}person-b`, document: patch }, + ]); + + assert.strictEqual( + readFileSync(realmFile('person-b.json'), 'utf8'), + readFileSync(realmFile('person-a.json'), 'utf8'), + 'the two files are byte-identical', + ); + }); + + test('a patch that changes nothing leaves the file exactly as it is', async function (assert) { + let before = readFileSync(realmFile('person-a.json'), 'utf8'); + let jobsBefore = await indexJobIds(); + + let [result] = await commit([ + { + op: 'update', + href: `${testRealmHref}person-a`, + document: { + data: { + type: 'card', + attributes: { firstName: 'Original' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]); + + assert.strictEqual( + readFileSync(realmFile('person-a.json'), 'utf8'), + before, + 'the stored bytes are untouched', + ); + assert.true( + (result?.meta.version ?? '').length > 0, + 'the result still reports the version the file holds', + ); + assert.deepEqual( + await indexJobIds(), + jobsBefore, + 'nothing is queued for indexing', + ); + }); + + test('a local id claimed twice, and one nothing creates, are both refused', async function (assert) { + let duplicate: unknown; + try { + await commit([ + { + op: 'create', + lid: 'twice', + document: { + data: { + type: 'card', + attributes: { firstName: 'One' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + { + op: 'create', + lid: 'twice', + document: { + data: { + type: 'card', + attributes: { firstName: 'Two' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]); + } catch (err: unknown) { + duplicate = err; + } + assert.ok(isOperationFailure(duplicate), 'a duplicate local id is refused'); + if (isOperationFailure(duplicate)) { + assert.strictEqual(duplicate.error.status, 400); + assert.strictEqual(duplicate.error.code, 'invalid-params'); + } + + let dangling: unknown; + try { + await commit([ + { + op: 'create', + lid: 'lonely', + document: { + data: { + type: 'card', + attributes: { firstName: 'Lonely' }, + relationships: { + friend: { data: { lid: 'nobody', type: 'card' } }, + }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]); + } catch (err: unknown) { + dangling = err; + } + assert.ok( + isOperationFailure(dangling), + 'a link to a local id no entry creates is refused', + ); + if (isOperationFailure(dangling)) { + assert.strictEqual(dangling.error.status, 400); + assert.strictEqual( + dangling.error.meta?.entry, + 0, + 'the refusal names the entry that carried the link', + ); + } + assert.notOk( + existsSync(realmFile('Person/lonely.json')), + 'nothing is written for a refused batch', + ); + }); + + test('a delete of a card that is not there refuses without touching the realm', async function (assert) { + let jobsBefore = await indexJobIds(); + let failure: unknown; + try { + await commit([{ op: 'delete', href: `${testRealmHref}not-a-card` }]); + } catch (err: unknown) { + failure = err; + } + assert.ok(isOperationFailure(failure), 'the delete is refused'); + if (isOperationFailure(failure)) { + assert.strictEqual(failure.error.status, 404); + assert.strictEqual(failure.error.code, 'target-not-found'); + } + assert.deepEqual( + await indexJobIds(), + jobsBefore, + 'no index job is enqueued', + ); + }); + + test('two entries changing one card are refused rather than silently ordered', async function (assert) { + let failure: unknown; + try { + await commit([ + { + op: 'update', + href: `${testRealmHref}person-a`, + document: { + data: { + type: 'card', + attributes: { firstName: 'Left' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + { + op: 'update', + href: `${testRealmHref}person-a`, + document: { + data: { + type: 'card', + attributes: { hourlyRate: 99 }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]); + } catch (err: unknown) { + failure = err; + } + assert.ok(isOperationFailure(failure), 'the batch is refused'); + if (isOperationFailure(failure)) { + assert.strictEqual(failure.error.code, 'invalid-params'); + assert.strictEqual(failure.error.meta?.conflictsWith, 0); + } + assert.true( + readFileSync(realmFile('person-a.json'), 'utf8').includes('Original'), + 'the card is left as it was', + ); + }); + + test('a batch cannot change a card to another type', async function (assert) { + let failure: unknown; + try { + await commit([ + { + op: 'update', + href: `${testRealmHref}person-a`, + document: { + data: { + type: 'card', + attributes: { firstName: 'Shapeshifter' }, + meta: { + adoptsFrom: { + module: rri('@cardstack/base/card-api'), + name: 'CardDef', + }, + }, + }, + }, + }, + ]); + } catch (err: unknown) { + failure = err; + } + assert.ok(isOperationFailure(failure), 'the type change is refused'); + if (isOperationFailure(failure)) { + assert.strictEqual(failure.error.status, 400); + } + }); + + test('a base version on anything but an update is refused', async function (assert) { + let failure: unknown; + try { + await commit([ + { op: 'delete', href: `${testRealmHref}person-c`, baseVersion: 'abc' }, + ]); + } catch (err: unknown) { + failure = err; + } + assert.ok(isOperationFailure(failure), 'the base version is refused'); + assert.ok( + existsSync(realmFile('person-c.json')), + 'the card is left in place', + ); + }); +}); diff --git a/packages/runtime-common/card-operations/executors.ts b/packages/runtime-common/card-operations/executors.ts index a8f77f4eecc..1a27862aebf 100644 --- a/packages/runtime-common/card-operations/executors.ts +++ b/packages/runtime-common/card-operations/executors.ts @@ -197,46 +197,55 @@ export async function stageCreate( `${ctx.realmURL}; a batch commits to one realm`, }); } - let included = entry.document?.included ?? []; - let writes: StagedWrite[] = []; - let primaryIdentity: StagedIdentity | undefined; - // The primary first, then each side-loaded resource. A side-loaded resource - // with no `lid` is not staged: it has no id to be created under and nothing - // in the batch can link to it, so the client sent a resource the realm has - // no way to name. - for (let [index, resource] of [primary, ...included].entries()) { - if (index > 0 && typeof resource.lid !== 'string') { - continue; - } - if (namesForeignRealm(resource, ctx.realmURL)) { + let identity = createIdentity(entry, primary, ctx.paths, ctx.lids); + promoteStagedLinks(primary, ctx); + let writes: StagedWrite[] = [ + { + path: identity.path, + content: await serializeForStorage(primary, identity, ctx), + }, + ]; + for (let resource of entry.document?.included ?? []) { + // A side-loaded resource with no `lid` is not staged: it has no id to be + // created under and nothing in the batch can link to it, so the client + // sent a resource the realm has no way to name. One naming another realm + // is not this batch's to write. + if ( + typeof resource.lid !== 'string' || + namesForeignRealm(resource, ctx.realmURL) + ) { continue; } - let identity = - index === 0 - ? createIdentity(entry, primary, ctx.paths, ctx.lids) - : stagedLid(resource.lid!, ctx); - if (index === 0) { - primaryIdentity = identity; - } else { - // A side-loaded resource's module refs are written relative to the card - // it was sent with, so they are resolved against that card before the - // resource is serialized under its own URL. - visitModuleDeps(resource, (moduleId, setModuleId) => { - setModuleId(ctx.resolveModuleId(moduleId, primaryIdentity!.id)); - }); - } - promoteStagedLinks(resource, ctx); - writes.push({ - path: identity.path, - content: await serializeForStorage(resource, identity, ctx), - }); + writes.push( + await stageSideLoaded(resource, resource.lid, identity.id, ctx), + ); } return { writes, deletes: [], - id: primaryIdentity!.id, + id: identity.id, ...(entry.lid ? { lid: entry.lid } : {}), - primaryPath: primaryIdentity!.path, + primaryPath: identity.path, + }; +} + +// A card side-loaded alongside the one an entry names. Its module refs are +// written relative to the card it was sent with, so they are resolved against +// that card before it is serialized under its own URL. +async function stageSideLoaded( + resource: CardResource, + lid: string, + relativeTo: string, + ctx: StagingContext, +): Promise { + let identity = stagedLid(lid, ctx); + promoteStagedLinks(resource, ctx); + visitModuleDeps(resource, (moduleId, setModuleId) => { + setModuleId(ctx.resolveModuleId(moduleId, relativeTo)); + }); + return { + path: identity.path, + content: await serializeForStorage(resource, identity, ctx), }; } @@ -384,15 +393,7 @@ export async function stageUpdate( ) { continue; } - let identity = stagedLid(resource.lid, ctx); - promoteStagedLinks(resource, ctx); - visitModuleDeps(resource, (moduleId, setModuleId) => { - setModuleId(ctx.resolveModuleId(moduleId, url.href)); - }); - writes.push({ - path: identity.path, - content: await serializeForStorage(resource, identity, ctx), - }); + writes.push(await stageSideLoaded(resource, resource.lid, url.href, ctx)); } } return { writes, deletes: [], id: url.href, primaryPath: sourcePath }; diff --git a/packages/runtime-common/tests/card-operations-batch-test.ts b/packages/runtime-common/tests/card-operations-batch-test.ts new file mode 100644 index 00000000000..c78239db85f --- /dev/null +++ b/packages/runtime-common/tests/card-operations-batch-test.ts @@ -0,0 +1,994 @@ +import { + commitBatch, + isOperationFailure, + type BatchCore, + type BatchEntry, + type OperationDefinition, +} from '../card-operations/index.ts'; +import type { CodeRef } from '../code-ref.ts'; +import type { RealmIdentifier } from '../realm-identifiers.ts'; +import type { Definition } from '../definitions.ts'; +import type { SharedTests } from '../helpers/index.ts'; + +// ============================================================================ +// What the coordinator stages, and what it refuses. +// +// The realm's collaborators are stubbed here, which is the point: what is +// under test is the decision the coordinator makes about a batch — which files +// it would write, which local ids resolve where, and whether it reaches the +// commit at all — not the realm's ability to carry the commit out. The stub +// records what it was handed and never writes anything, so a batch that must +// abandon before committing is checked by the commit simply never having been +// called, which is the property itself rather than a proxy for it. +// +// The commit's own behavior — one index job, one index event, bytes on disk — +// only exists against a real realm, and is covered by the realm-server suite. +// ============================================================================ + +const REALM = 'http://example.com/test/'; +const PERSON = { module: `${REALM}person`, name: 'Person' } as CodeRef; +const PET = { module: `${REALM}pet`, name: 'Pet' } as CodeRef; +const STRING = { module: `${REALM}string`, name: 'default' } as CodeRef; + +interface Commit { + writes: Record; + deletes: string[]; + clientRequestId: string | null | undefined; + waitForIndex: boolean | undefined; +} + +interface Stub { + core: BatchCore; + commits: Commit[]; + lockDepth: () => number; +} + +interface StubOptions { + // The stored source files the realm holds, by local path. + stored?: Record; + // The write-time fingerprint each stored path carries. + hashes?: Record; + // The definition-cache entry a code ref resolves to, keyed by its name. + definitions?: Record; + // What `serializeCard` does to a resource on its way to storage. The default + // is the identity, which keeps a test's expected bytes readable; a test that + // cares about a serializer refusal supplies its own. + serialize?: (doc: any) => any; +} + +function cardFile(attributes: Record, adoptsFrom: unknown) { + return JSON.stringify( + { data: { type: 'card', attributes, meta: { adoptsFrom } } }, + null, + 2, + ); +} + +function stub(opts: StubOptions = {}): Stub { + let { + stored = {}, + hashes = {}, + definitions = {}, + serialize = (doc: any) => doc, + } = opts; + let commits: Commit[] = []; + let held = 0; + let maxHeld = 0; + + let core: BatchCore = { + realmURL: REALM, + async withWriteLock(fn) { + held++; + maxHeld = Math.max(maxHeld, held); + try { + return await fn(); + } finally { + held--; + } + }, + async readSourceFile(localPath) { + let content = stored[localPath]; + return content === undefined + ? undefined + : { content, lastModified: 1000 }; + }, + async contentHashes(localPaths) { + return new Map( + localPaths.map((localPath) => [localPath, hashes[localPath]]), + ); + }, + async commitUnlocked(batch, options) { + let writes = Object.fromEntries( + [...(batch.writes ?? new Map())].map( + ([path, content]) => [path, String(content)], + ), + ); + commits.push({ + writes, + deletes: [...(batch.deletes ?? [])], + clientRequestId: options?.clientRequestId, + waitForIndex: options?.waitForIndex, + }); + return { + // A real commit fingerprints the bytes it wrote; the stub stands in + // with the byte length, which is enough for a test to tell one + // version from another. + writes: Object.entries(writes).map(([path, content]) => ({ + path, + lastModified: 2000, + contentHash: `hash-${content.length}`, + })), + generation: 9, + }; + }, + async serializeCard(doc) { + return serialize(doc); + }, + codeRefKey(codeRef) { + return 'module' in codeRef + ? `${codeRef.module}/${codeRef.name}` + : JSON.stringify(codeRef); + }, + resolveModuleId(moduleId) { + return moduleId; + }, + async lookupDefinition(codeRef) { + return 'name' in codeRef ? definitions[codeRef.name] : undefined; + }, + }; + return { core, commits, lockDepth: () => maxHeld }; +} + +function personDefinition(): Definition { + return { + type: 'card-def', + codeRef: PERSON, + displayName: 'Person', + fields: { friend: 'f0', firstName: 'f1' }, + fieldDefs: { + f0: { + type: 'linksTo', + isPrimitive: false, + isComputed: false, + fieldOrCard: PERSON, + }, + f1: { + type: 'contains', + isPrimitive: true, + isComputed: false, + fieldOrCard: STRING, + }, + }, + } as Definition; +} + +async function refusal( + core: BatchCore, + entries: BatchEntry[], +): Promise<{ status: number; code: string; entry: unknown } | undefined> { + try { + await commitBatch(core, entries, {}); + } catch (err: unknown) { + if (!isOperationFailure(err)) { + throw err; + } + return { + status: err.error.status, + code: err.error.code, + entry: err.error.meta?.entry, + }; + } + return undefined; +} + +const tests: SharedTests> = { + 'a create is staged at the path its local id names': async (assert) => { + let { core, commits } = stub(); + let results = await commitBatch( + core, + [ + { + op: 'create', + lid: 'mango', + document: { + data: { + type: 'card', + attributes: { firstName: 'Mango' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ], + { clientRequestId: 'req-1' }, + ); + + assert.strictEqual(commits.length, 1, 'the batch commits once'); + assert.deepEqual( + Object.keys(commits[0].writes), + ['Person/mango.json'], + 'the file is named after the local id, under the type directory', + ); + assert.deepEqual(commits[0].deletes, [], 'nothing is removed'); + assert.strictEqual( + commits[0].clientRequestId, + 'req-1', + "the commit carries the caller's own request id", + ); + assert.true( + commits[0].waitForIndex, + 'the batch waits for indexing by default', + ); + assert.strictEqual(results[0]?.id, `${REALM}Person/mango`); + assert.strictEqual( + results[0] && 'lid' in results[0] ? results[0].lid : undefined, + 'mango', + 'the result echoes the local id', + ); + assert.strictEqual( + results[0]?.meta.generation, + 9, + "the result carries the commit's index generation", + ); + }, + + 'a later entry links to a card an earlier one mints': async (assert) => { + let { core, commits } = stub(); + await commitBatch( + core, + [ + { + op: 'create', + lid: 'owner', + document: { + data: { + type: 'card', + attributes: { firstName: 'Hassan' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + { + op: 'create', + lid: 'pet', + document: { + data: { + type: 'card', + attributes: { firstName: 'Mango' }, + relationships: { + friend: { data: { type: 'card', lid: 'owner' } }, + }, + meta: { adoptsFrom: PET }, + }, + }, + }, + ], + {}, + ); + + let staged = JSON.parse(commits[0].writes['Pet/pet.json']); + assert.strictEqual( + staged.data.relationships.friend.links.self, + `${REALM}Person/owner`, + 'the link resolves to the URL the other entry will be written at', + ); + assert.deepEqual( + Object.keys(commits[0].writes).sort(), + ['Person/owner.json', 'Pet/pet.json'], + 'both cards are staged in one commit', + ); + }, + + 'a side-loaded resource is created alongside its primary': async (assert) => { + let { core, commits } = stub(); + await commitBatch( + core, + [ + { + op: 'create', + lid: 'primary', + document: { + data: { + type: 'card', + attributes: { firstName: 'Primary' }, + relationships: { + friend: { data: { type: 'card', lid: 'sidecar' } }, + }, + meta: { adoptsFrom: PERSON }, + }, + included: [ + { + type: 'card', + lid: 'sidecar', + attributes: { firstName: 'Sidecar' }, + meta: { adoptsFrom: PET }, + }, + // No local id, so nothing can name it and it is not staged. + { + type: 'card', + attributes: { firstName: 'Anonymous' }, + meta: { adoptsFrom: PET }, + }, + ], + }, + }, + ], + {}, + ); + + assert.deepEqual( + Object.keys(commits[0].writes).sort(), + ['Person/primary.json', 'Pet/sidecar.json'], + 'the side-loaded resource with a local id is staged, the other is not', + ); + let primary = JSON.parse(commits[0].writes['Person/primary.json']); + assert.strictEqual( + primary.data.relationships.friend.links.self, + `${REALM}Pet/sidecar`, + 'the primary links to the side-loaded card', + ); + }, + + 'an entry that cannot be staged abandons the batch before it commits': async ( + assert, + ) => { + let { core, commits } = stub(); + let failed = await refusal(core, [ + { + op: 'create', + lid: 'never-written', + document: { + data: { + type: 'card', + attributes: { firstName: 'Ghost' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + { + op: 'update', + href: `${REALM}absent`, + document: { + data: { type: 'card', attributes: {}, meta: { adoptsFrom: PERSON } }, + }, + }, + ]); + + assert.deepEqual( + failed, + { status: 404, code: 'target-not-found', entry: 1 }, + "the refusal is the entry's own, labelled with its position", + ); + assert.strictEqual( + commits.length, + 0, + 'nothing is committed, so the entry ahead of the failure never lands', + ); + }, + + 'a patch merges over the stored file, replacing arrays': async (assert) => { + let { core, commits } = stub({ + stored: { + 'person-1.json': cardFile( + { firstName: 'Original', nicknames: ['a', 'b'], age: 7 }, + PERSON, + ), + }, + }); + await commitBatch( + core, + [ + { + op: 'update', + href: `${REALM}person-1`, + document: { + data: { + type: 'card', + attributes: { firstName: 'Patched', nicknames: ['c'] }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ], + {}, + ); + + let { data } = JSON.parse(commits[0].writes['person-1.json']); + assert.strictEqual(data.attributes.firstName, 'Patched', 'the patch wins'); + assert.strictEqual( + data.attributes.age, + 7, + 'a field the patch does not name is kept', + ); + assert.deepEqual( + data.attributes.nicknames, + ['c'], + 'a patched array replaces the stored one rather than merging into it', + ); + }, + + 'a patch that changes nothing stages the bytes already on disk': async ( + assert, + ) => { + let content = cardFile({ firstName: 'Original' }, PERSON); + let { core, commits } = stub({ stored: { 'person-1.json': content } }); + let results = await commitBatch( + core, + [ + { + op: 'update', + href: `${REALM}person-1`, + document: { + data: { + type: 'card', + attributes: { firstName: 'Original' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ], + {}, + ); + + assert.strictEqual( + commits[0].writes['person-1.json'], + content, + 'the staged bytes are the stored bytes, so the commit finds nothing to write', + ); + assert.ok(results[0], 'the entry still reports the version the file holds'); + }, + + 'a patch cannot change the type a card adopts': async (assert) => { + let { core, commits } = stub({ + stored: { 'person-1.json': cardFile({ firstName: 'Original' }, PERSON) }, + }); + let failed = await refusal(core, [ + { + op: 'update', + href: `${REALM}person-1`, + document: { + data: { + type: 'card', + attributes: { firstName: 'Shapeshifter' }, + meta: { adoptsFrom: PET }, + }, + }, + }, + ]); + + assert.strictEqual(failed?.status, 400, 'the type change is refused'); + assert.strictEqual(commits.length, 0, 'nothing is committed'); + }, + + 'realm-managed keys in a patch never reach the file': async (assert) => { + let { core, commits } = stub({ + stored: { 'person-1.json': cardFile({ firstName: 'Original' }, PERSON) }, + }); + await commitBatch( + core, + [ + { + op: 'update', + href: `${REALM}person-1`, + document: { + data: { + type: 'card', + attributes: { firstName: 'Patched' }, + meta: { + adoptsFrom: PERSON, + realmInfo: { name: 'Spoofed' }, + realmURL: 'http://elsewhere.example/', + screenshots: { poster: 'http://elsewhere.example/shot.png' }, + }, + } as any, + }, + }, + ], + {}, + ); + + let { data } = JSON.parse(commits[0].writes['person-1.json']); + assert.strictEqual( + data.meta.realmInfo, + undefined, + 'a client cannot persist realm info', + ); + assert.strictEqual( + data.meta.screenshots, + undefined, + 'a client cannot persist a screenshot manifest', + ); + assert.strictEqual( + data.meta.realmURL, + REALM, + 'the realm stamps its own URL rather than taking the one it was sent', + ); + }, + + 'a write and a removal reach the commit together': async (assert) => { + let { core, commits } = stub({ + stored: { + 'person-1.json': cardFile({ firstName: 'Kept' }, PERSON), + 'person-2.json': cardFile({ firstName: 'Doomed' }, PERSON), + }, + }); + let results = await commitBatch( + core, + [ + { + op: 'update', + href: `${REALM}person-1`, + document: { + data: { + type: 'card', + attributes: { firstName: 'Renamed' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + { op: 'delete', href: `${REALM}person-2` }, + ], + {}, + ); + + assert.strictEqual(commits.length, 1, 'one commit covers both'); + assert.deepEqual(Object.keys(commits[0].writes), ['person-1.json']); + assert.deepEqual(commits[0].deletes, ['person-2.json']); + assert.ok(results[0], 'the write reports its identity'); + assert.strictEqual( + results[1], + null, + 'a removal has no state left to describe', + ); + }, + + 'a removal needs something to remove': async (assert) => { + let { core, commits } = stub(); + let failed = await refusal(core, [ + { op: 'delete', href: `${REALM}absent` }, + ]); + assert.deepEqual(failed, { + status: 404, + code: 'target-not-found', + entry: 0, + }); + assert.strictEqual(commits.length, 0, 'nothing is committed'); + }, + + 'a base version is reported against the fingerprint the file carried': async ( + assert, + ) => { + let { core } = stub({ + stored: { 'person-1.json': cardFile({ firstName: 'Original' }, PERSON) }, + hashes: { 'person-1.json': 'v1' }, + }); + let patch = (firstName: string): BatchEntry[] => [ + { + op: 'update', + href: `${REALM}person-1`, + baseVersion: 'v1', + document: { + data: { + type: 'card', + attributes: { firstName }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]; + + let [matched] = await commitBatch(core, patch('Matched'), {}); + assert.true( + matched?.meta.baseMatched, + 'the base the caller named is the one the file carried', + ); + + let { core: moved } = stub({ + stored: { 'person-1.json': cardFile({ firstName: 'Original' }, PERSON) }, + hashes: { 'person-1.json': 'v2' }, + }); + let [result] = await commitBatch(moved, patch('Moved'), {}); + assert.false( + result?.meta.baseMatched, + 'the file has moved past the base the caller named', + ); + }, + + 'an unconditional write reports no base match': async (assert) => { + let { core } = stub({ + stored: { 'person-1.json': cardFile({ firstName: 'Original' }, PERSON) }, + hashes: { 'person-1.json': 'v1' }, + }); + let [result] = await commitBatch( + core, + [ + { + op: 'update', + href: `${REALM}person-1`, + document: { + data: { + type: 'card', + attributes: { firstName: 'Whatever' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ], + {}, + ); + assert.strictEqual(result?.meta.baseMatched, undefined); + }, + + 'a base version belongs only to an update': async (assert) => { + let { core, commits } = stub({ + stored: { 'person-1.json': cardFile({ firstName: 'Original' }, PERSON) }, + }); + let failed = await refusal(core, [ + { op: 'delete', href: `${REALM}person-1`, baseVersion: 'v1' }, + ]); + assert.strictEqual(failed?.status, 400); + assert.strictEqual(commits.length, 0, 'nothing is committed'); + }, + + 'a local id names one card': async (assert) => { + let { core, commits } = stub(); + let entry = (firstName: string): BatchEntry => ({ + op: 'create', + lid: 'twice', + document: { + data: { + type: 'card', + attributes: { firstName }, + meta: { adoptsFrom: PERSON }, + }, + }, + }); + let failed = await refusal(core, [entry('One'), entry('Two')]); + assert.strictEqual(failed?.status, 400, 'a duplicate local id is refused'); + assert.strictEqual(commits.length, 0, 'nothing is committed'); + }, + + 'a link to a local id nothing creates is refused': async (assert) => { + let { core, commits } = stub(); + let failed = await refusal(core, [ + { + op: 'create', + lid: 'lonely', + document: { + data: { + type: 'card', + attributes: { firstName: 'Lonely' }, + relationships: { + friend: { data: { type: 'card', lid: 'nobody' } }, + }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]); + assert.deepEqual(failed, { status: 400, code: 'invalid-params', entry: 0 }); + assert.strictEqual(commits.length, 0, 'nothing is committed'); + }, + + 'two entries changing one card are refused rather than ordered': async ( + assert, + ) => { + let { core, commits } = stub({ + stored: { 'person-1.json': cardFile({ firstName: 'Original' }, PERSON) }, + }); + let entry = (attributes: Record): BatchEntry => ({ + op: 'update', + href: `${REALM}person-1`, + document: { + data: { type: 'card', attributes, meta: { adoptsFrom: PERSON } }, + }, + }); + let failed = await refusal(core, [ + entry({ firstName: 'Left' }), + entry({ age: 9 }), + ]); + assert.strictEqual(failed?.code, 'invalid-params'); + assert.strictEqual( + failed?.entry, + 1, + 'the second claim on the file is the refusal', + ); + assert.strictEqual(commits.length, 0, 'nothing is committed'); + }, + + 'a target outside the realm is not the batch to commit it': async ( + assert, + ) => { + let { core, commits } = stub(); + let failed = await refusal(core, [ + { + op: 'update', + href: 'http://elsewhere.example/other/person-1', + document: { + data: { type: 'card', attributes: {}, meta: { adoptsFrom: PERSON } }, + }, + }, + ]); + assert.deepEqual(failed, { + status: 404, + code: 'target-not-found', + entry: 0, + }); + assert.strictEqual(commits.length, 0, 'nothing is committed'); + }, + + 'a create naming another realm is refused': async (assert) => { + let { core, commits } = stub(); + let failed = await refusal(core, [ + { + op: 'create', + lid: 'foreign', + document: { + data: { + type: 'card', + attributes: { firstName: 'Foreign' }, + meta: { + adoptsFrom: PERSON, + realmURL: 'http://elsewhere.example/' as RealmIdentifier, + }, + }, + }, + }, + ]); + assert.strictEqual(failed?.status, 400); + assert.strictEqual(commits.length, 0, 'nothing is committed'); + }, + + 'a field the type refuses is the payload to fix, not the realm': async ( + assert, + ) => { + let { core, commits } = stub({ + serialize: () => { + throw new Error('field validation error: firstName must be a string'); + }, + }); + let failed = await refusal(core, [ + { + op: 'create', + lid: 'invalid', + document: { + data: { + type: 'card', + attributes: { firstName: 7 }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]); + assert.deepEqual(failed, { status: 400, code: 'invalid-params', entry: 0 }); + assert.strictEqual(commits.length, 0, 'nothing is committed'); + }, + + 'a stored file that is not a card document is the realm to answer for': + async (assert) => { + let { core } = stub({ stored: { 'person-1.json': 'not json {' } }); + let failed = await refusal(core, [ + { + op: 'update', + href: `${REALM}person-1`, + document: { + data: { + type: 'card', + attributes: {}, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]); + assert.deepEqual(failed, { + status: 500, + code: 'internal-error', + entry: 0, + }); + }, + + 'the write lock is taken once for the whole batch': async (assert) => { + let { core, lockDepth } = stub({ + stored: { 'person-1.json': cardFile({ firstName: 'Original' }, PERSON) }, + }); + await commitBatch( + core, + [ + { + op: 'create', + lid: 'one', + document: { + data: { + type: 'card', + attributes: { firstName: 'One' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + { + op: 'update', + href: `${REALM}person-1`, + document: { + data: { + type: 'card', + attributes: { firstName: 'Two' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ], + {}, + ); + assert.strictEqual( + lockDepth(), + 1, + 'the lock is never re-entered, whatever the batch contains', + ); + }, + + 'a named create stages the type and attributes its declaration names': async ( + assert, + ) => { + let definition: OperationDefinition = { + base: 'create', + deterministic: true, + of: PERSON, + params: { + title: { kind: 'field', codeRef: STRING }, + buddy: { kind: 'link', codeRef: PERSON }, + }, + fill: { + firstName: { $ref: 'params', key: 'title' } as any, + friend: { $ref: 'params', key: 'buddy' } as any, + }, + }; + let { core, commits } = stub({ + definitions: { Person: personDefinition() }, + }); + await commitBatch( + core, + [ + { + op: 'create', + lid: 'minted', + definition, + params: { title: 'Declared', buddy: `${REALM}person-1` }, + }, + ], + {}, + ); + + let { data } = JSON.parse(commits[0].writes['Person/minted.json']); + assert.deepEqual( + data.meta.adoptsFrom, + PERSON, + 'the type comes from the declaration', + ); + assert.strictEqual( + data.attributes.firstName, + 'Declared', + 'a field-typed param becomes an attribute', + ); + assert.strictEqual( + data.relationships.friend.links.self, + `${REALM}person-1`, + 'a link-typed param becomes a relationship', + ); + }, + + 'a named create resolves the actor and the card it is anchored on': async ( + assert, + ) => { + let definition: OperationDefinition = { + base: 'create', + deterministic: true, + of: PERSON, + fill: { + firstName: { $ref: 'instance', key: 'firstName' } as any, + friend: { $ref: 'instance', key: 'id' } as any, + author: { $ref: 'actor' } as any, + }, + }; + let { core, commits } = stub({ + stored: { 'person-1.json': cardFile({ firstName: 'Anchor' }, PERSON) }, + definitions: { Person: personDefinition() }, + }); + await commitBatch( + core, + [ + { + op: 'create', + lid: 'derived', + href: `${REALM}person-1`, + definition, + }, + ], + { actor: '@tester:localhost' }, + ); + + let { data } = JSON.parse(commits[0].writes['Person/derived.json']); + assert.strictEqual( + data.attributes.firstName, + 'Anchor', + "a member of the anchoring card's stored document", + ); + assert.strictEqual( + data.relationships.friend.links.self, + `${REALM}person-1`, + 'the anchoring card, in a field the type declares as a link', + ); + assert.strictEqual( + data.attributes.author, + '@tester:localhost', + 'the actor the request was authenticated as', + ); + }, + + 'a named create with no target in scope cannot read one': async (assert) => { + let definition: OperationDefinition = { + base: 'create', + deterministic: true, + of: PERSON, + fill: { firstName: { $ref: 'instance', key: 'firstName' } as any }, + }; + let { core, commits } = stub({ + definitions: { Person: personDefinition() }, + }); + let failed = await refusal(core, [ + { op: 'create', lid: 'orphan', definition }, + ]); + assert.deepEqual(failed, { status: 400, code: 'invalid-params', entry: 0 }); + assert.strictEqual(commits.length, 0, 'nothing is committed'); + }, + + 'a named create links to a card the same batch mints': async (assert) => { + let definition: OperationDefinition = { + base: 'create', + deterministic: true, + of: PERSON, + params: { buddy: { kind: 'link', codeRef: PERSON } }, + fill: { friend: { $ref: 'params', key: 'buddy' } as any }, + }; + let { core, commits } = stub({ + definitions: { Person: personDefinition() }, + }); + await commitBatch( + core, + [ + { + op: 'create', + lid: 'target', + document: { + data: { + type: 'card', + attributes: { firstName: 'Target' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + { + op: 'create', + lid: 'linker', + definition, + params: { buddy: { lid: 'target' } }, + }, + ], + {}, + ); + + let { data } = JSON.parse(commits[0].writes['Person/linker.json']); + assert.strictEqual( + data.relationships.friend.links.self, + `${REALM}Person/target`, + 'a link param given a local id resolves to the URL that card will hold', + ); + }, + + 'a create with nothing to create is refused': async (assert) => { + let { core, commits } = stub(); + let failed = await refusal(core, [{ op: 'create', lid: 'empty' }]); + assert.deepEqual(failed, { status: 400, code: 'invalid-params', entry: 0 }); + assert.strictEqual(commits.length, 0, 'nothing is committed'); + }, +}; + +export default tests; From 5ead5409e9ba9dd7e9818ef29e4320741a991f54 Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Thu, 10 Sep 2026 14:28:18 +0000 Subject: [PATCH 03/14] Name the create test's module absolutely, and wait for the batch's index event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A created card lands under its type's directory, so a root-relative module reference in its `adoptsFrom` resolves against that directory rather than the realm — the test was asking for a module that is not there. Say where module references resolve from, in the one place that decides it, and have the test name the module the way a caller who does not want to reason about the directory would. The realm broadcasts into the Matrix room out of band from the commit, so the one-event assertion waits for the batch's own event to arrive rather than reading the room once. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EBB6JdRo5XCY5WhUkgBckS --- .../tests/card-operations-commit-test.ts | 26 +++++++++++++++---- .../card-operations/coordinator.ts | 6 ++--- .../card-operations/executors.ts | 8 ++++++ 3 files changed, 32 insertions(+), 8 deletions(-) diff --git a/packages/realm-server/tests/card-operations-commit-test.ts b/packages/realm-server/tests/card-operations-commit-test.ts index bbdc692709e..142b5b9471b 100644 --- a/packages/realm-server/tests/card-operations-commit-test.ts +++ b/packages/realm-server/tests/card-operations-commit-test.ts @@ -28,13 +28,17 @@ import type { RealmHttpServer as Server } from '../server.ts'; import { setupPermissionedRealmCached, setupMatrixRoom, + waitUntil, withRealmPath, type RealmRequest, } from './helpers/index.ts'; const testRealm = new URL('http://127.0.0.1:4445/test/'); const testRealmHref = testRealm.href; -const PERSON = { module: rri('./person'), name: 'Person' }; +// The module named absolutely rather than relative to the realm root: a create +// lands under its type's directory, one level deep, so a root-relative +// spelling would resolve against that directory instead of the realm. +const PERSON = { module: rri(`${testRealmHref}person`), name: 'Person' }; // ============================================================================ // The batch coordinator, driven against a real realm. @@ -156,6 +160,15 @@ module(basename(import.meta.filename), function (hooks) { .map((message) => message.content as RealmEventContent); } + async function incrementalIndexEventsSince( + since: number, + ): Promise { + return (await realmEventsSince(since)).filter( + (event): event is IncrementalIndexEventContent => + event.eventName === 'index' && event.indexType === 'incremental', + ); + } + // The realm events a batch is answerable for: the incremental index event it // broadcasts, and the file-change event naming a path it touched. Filtering // by path rather than counting everything keeps the assertion about this @@ -362,10 +375,13 @@ module(basename(import.meta.filename), function (hooks) { `the write and the delete share one index job (got ${newJobs.length})`, ); - let indexEvents = (await realmEventsSince(since)).filter( - (event): event is IncrementalIndexEventContent => - event.eventName === 'index' && event.indexType === 'incremental', - ); + // The realm broadcasts into the Matrix room out of band from the commit, + // so wait for the batch's own event to arrive before counting. + await waitUntil(async () => { + let seen = await incrementalIndexEventsSince(since); + return seen.some((event) => event.clientRequestId === 'batch-1'); + }); + let indexEvents = await incrementalIndexEventsSince(since); assert.strictEqual( indexEvents.length, 1, diff --git a/packages/runtime-common/card-operations/coordinator.ts b/packages/runtime-common/card-operations/coordinator.ts index a2eb82a1d76..26d0b81410c 100644 --- a/packages/runtime-common/card-operations/coordinator.ts +++ b/packages/runtime-common/card-operations/coordinator.ts @@ -385,9 +385,9 @@ async function commitStaged( let written = byPath.get(change.primaryPath); if (!written) { // Every staged write is handed to the commit and every one comes back, - // so a missing result means the two no longer agree about what was - // staged. Reporting an empty version would hand the caller a token it - // could send back as a `baseVersion` that matches nothing. + // so a missing result means the two disagree about what was staged. + // Reporting an empty version would hand the caller a token it could + // send back as a `baseVersion` that matches nothing. throw new OperationFailure({ id: change.id, status: 500, diff --git a/packages/runtime-common/card-operations/executors.ts b/packages/runtime-common/card-operations/executors.ts index 1a27862aebf..78d5e1da129 100644 --- a/packages/runtime-common/card-operations/executors.ts +++ b/packages/runtime-common/card-operations/executors.ts @@ -543,6 +543,14 @@ function promoteStagedLinks(resource: CardResource, ctx: StagingContext): void { // The bytes a card's file holds. The realm stamps its own URL on the resource // before serializing, the same way it does for every card it stores, and the // serializer resolves each field against the type's definition. +// +// The document's own module references resolve against the file it lands in, +// which for a created card is its type's directory rather than the realm root. +// So a caller naming a module relatively has to name it relative to that +// file — a realm-root-relative spelling addresses a module inside the type +// directory, where there is none. An absolute URL or a registered prefix +// resolves the same wherever the card is stored, which is what a caller that +// does not want to reason about the directory sends. async function serializeForStorage( resource: CardResource, identity: StagedIdentity, From 45bf42807dd7cd78f51de7d72d5f91366256fa10 Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Thu, 10 Sep 2026 14:32:00 +0000 Subject: [PATCH 04/14] Keep the realm-driven batch tests to what only a realm can show MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five of the tests were refusals the coordinator makes before it commits — a duplicate local id, a link to one nothing creates, a removal with no target, two entries on one card, a type change. None of them reach the realm, and all of them are already checked against the stub, where "nothing is committed" is the property itself. What is left is what a stub cannot show: bytes on disk, one index job, one index event, and byte-for-byte agreement with the PATCH handler. Each remaining test now works on its own card, so one realm serves the whole file. Rebuilding it per test bought no isolation and cost a Matrix session room each time — enough of them in a burst to trip Synapse's room-creation limit and fail a test on the realm's inability to open a session. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EBB6JdRo5XCY5WhUkgBckS --- .../tests/card-operations-commit-test.ts | 278 ++++-------------- 1 file changed, 51 insertions(+), 227 deletions(-) diff --git a/packages/realm-server/tests/card-operations-commit-test.ts b/packages/realm-server/tests/card-operations-commit-test.ts index 142b5b9471b..2a32c055cf9 100644 --- a/packages/realm-server/tests/card-operations-commit-test.ts +++ b/packages/realm-server/tests/card-operations-commit-test.ts @@ -43,13 +43,15 @@ const PERSON = { module: rri(`${testRealmHref}person`), name: 'Person' }; // ============================================================================ // The batch coordinator, driven against a real realm. // -// What is under test is the batch's all-or-nothing property and the things -// that property is observable through: what lands on disk, how many index jobs -// the commit enqueues, how many index events it broadcasts, and what each -// entry's result reports. Those only exist against a real realm — the jobs -// table and the Matrix room are where "one job, one event" is either true or -// not — so the coordinator is called directly with the realm's own batch core -// rather than through a stub. +// What is under test is what only exists against a real realm: the bytes that +// land on disk, how many index jobs the commit enqueues, how many index events +// it broadcasts, and byte-for-byte agreement with the `PATCH` handler. The +// coordinator is called directly with the realm's own batch core rather than +// through a stub, since a stub is exactly what those properties are not +// observable through. Everything a batch decides before it commits — which +// files it would write, where a local id resolves, whether it commits at all — +// is checked against a stub instead, where "nothing is committed" is the +// property itself rather than a proxy for it. // ============================================================================ function makeFileSystem(): Record { @@ -80,27 +82,27 @@ function makeFileSystem(): Record { } } `, - 'person-a.json': { - data: { - type: 'card', - attributes: { firstName: 'Original', hourlyRate: 10 }, - meta: { adoptsFrom: PERSON }, - }, - }, - 'person-b.json': { - data: { - type: 'card', - attributes: { firstName: 'Original', hourlyRate: 10 }, - meta: { adoptsFrom: PERSON }, - }, - }, - 'person-c.json': { - data: { - type: 'card', - attributes: { firstName: 'Doomed', hourlyRate: 1 }, - meta: { adoptsFrom: PERSON }, - }, - }, + ...Object.fromEntries( + [ + // A card per test that mutates one, so the file's tests do not have to + // be ordered against each other. + 'commit-write', + 'commit-delete', + 'version-target', + 'patch-over-http', + 'patch-over-batch', + 'unchanged', + ].map((name) => [ + `${name}.json`, + { + data: { + type: 'card', + attributes: { firstName: 'Original', hourlyRate: 10 }, + meta: { adoptsFrom: PERSON }, + }, + }, + ]), + ), }; } @@ -113,7 +115,7 @@ module(basename(import.meta.filename), function (hooks) { let dir: DirResult; setupPermissionedRealmCached(hooks, { - mode: 'beforeEach', + mode: 'before', realmURL: testRealm, permissions: { '*': ['read', 'write'], @@ -337,7 +339,7 @@ module(basename(import.meta.filename), function (hooks) { [ { op: 'update', - href: `${testRealmHref}person-a`, + href: `${testRealmHref}commit-write`, document: { data: { type: 'card', @@ -346,7 +348,7 @@ module(basename(import.meta.filename), function (hooks) { }, }, }, - { op: 'delete', href: `${testRealmHref}person-c` }, + { op: 'delete', href: `${testRealmHref}commit-delete` }, ], 'batch-1', ); @@ -358,11 +360,11 @@ module(basename(import.meta.filename), function (hooks) { ); assert.ok(results[0], 'the write reports its identity'); assert.notOk( - existsSync(realmFile('person-c.json')), + existsSync(realmFile('commit-delete.json')), 'the deleted card is gone from disk', ); assert.true( - readFileSync(realmFile('person-a.json'), 'utf8').includes('Renamed'), + readFileSync(realmFile('commit-write.json'), 'utf8').includes('Renamed'), 'the written card holds the patched value', ); @@ -394,7 +396,7 @@ module(basename(import.meta.filename), function (hooks) { ); assert.deepEqual( [...indexEvents[0].invalidations].sort(), - [`${testRealmHref}person-a`, `${testRealmHref}person-c`].sort(), + [`${testRealmHref}commit-write`, `${testRealmHref}commit-delete`].sort(), 'the one event covers both the write and the removal', ); }); @@ -403,7 +405,7 @@ module(basename(import.meta.filename), function (hooks) { let [first] = await commit([ { op: 'update', - href: `${testRealmHref}person-a`, + href: `${testRealmHref}version-target`, document: { data: { type: 'card', @@ -424,7 +426,7 @@ module(basename(import.meta.filename), function (hooks) { let [matched] = await commit([ { op: 'update', - href: `${testRealmHref}person-a`, + href: `${testRealmHref}version-target`, baseVersion: version, document: { data: { @@ -443,7 +445,7 @@ module(basename(import.meta.filename), function (hooks) { let [moved] = await commit([ { op: 'update', - href: `${testRealmHref}person-a`, + href: `${testRealmHref}version-target`, baseVersion: version, document: { data: { @@ -459,7 +461,7 @@ module(basename(import.meta.filename), function (hooks) { 'the file has moved past the base the caller named', ); assert.true( - readFileSync(realmFile('person-a.json'), 'utf8').includes('Third'), + readFileSync(realmFile('version-target.json'), 'utf8').includes('Third'), 'a moved base is reported, not refused — the write still lands', ); }); @@ -474,30 +476,34 @@ module(basename(import.meta.filename), function (hooks) { }; let response = await request - .patch('/person-a') + .patch('/patch-over-http') .send(patch) .set('Accept', 'application/vnd.card+json'); assert.strictEqual(response.status, 200, 'the PATCH is served'); await commit([ - { op: 'update', href: `${testRealmHref}person-b`, document: patch }, + { + op: 'update', + href: `${testRealmHref}patch-over-batch`, + document: patch, + }, ]); assert.strictEqual( - readFileSync(realmFile('person-b.json'), 'utf8'), - readFileSync(realmFile('person-a.json'), 'utf8'), + readFileSync(realmFile('patch-over-batch.json'), 'utf8'), + readFileSync(realmFile('patch-over-http.json'), 'utf8'), 'the two files are byte-identical', ); }); test('a patch that changes nothing leaves the file exactly as it is', async function (assert) { - let before = readFileSync(realmFile('person-a.json'), 'utf8'); + let before = readFileSync(realmFile('unchanged.json'), 'utf8'); let jobsBefore = await indexJobIds(); let [result] = await commit([ { op: 'update', - href: `${testRealmHref}person-a`, + href: `${testRealmHref}unchanged`, document: { data: { type: 'card', @@ -509,7 +515,7 @@ module(basename(import.meta.filename), function (hooks) { ]); assert.strictEqual( - readFileSync(realmFile('person-a.json'), 'utf8'), + readFileSync(realmFile('unchanged.json'), 'utf8'), before, 'the stored bytes are untouched', ); @@ -523,186 +529,4 @@ module(basename(import.meta.filename), function (hooks) { 'nothing is queued for indexing', ); }); - - test('a local id claimed twice, and one nothing creates, are both refused', async function (assert) { - let duplicate: unknown; - try { - await commit([ - { - op: 'create', - lid: 'twice', - document: { - data: { - type: 'card', - attributes: { firstName: 'One' }, - meta: { adoptsFrom: PERSON }, - }, - }, - }, - { - op: 'create', - lid: 'twice', - document: { - data: { - type: 'card', - attributes: { firstName: 'Two' }, - meta: { adoptsFrom: PERSON }, - }, - }, - }, - ]); - } catch (err: unknown) { - duplicate = err; - } - assert.ok(isOperationFailure(duplicate), 'a duplicate local id is refused'); - if (isOperationFailure(duplicate)) { - assert.strictEqual(duplicate.error.status, 400); - assert.strictEqual(duplicate.error.code, 'invalid-params'); - } - - let dangling: unknown; - try { - await commit([ - { - op: 'create', - lid: 'lonely', - document: { - data: { - type: 'card', - attributes: { firstName: 'Lonely' }, - relationships: { - friend: { data: { lid: 'nobody', type: 'card' } }, - }, - meta: { adoptsFrom: PERSON }, - }, - }, - }, - ]); - } catch (err: unknown) { - dangling = err; - } - assert.ok( - isOperationFailure(dangling), - 'a link to a local id no entry creates is refused', - ); - if (isOperationFailure(dangling)) { - assert.strictEqual(dangling.error.status, 400); - assert.strictEqual( - dangling.error.meta?.entry, - 0, - 'the refusal names the entry that carried the link', - ); - } - assert.notOk( - existsSync(realmFile('Person/lonely.json')), - 'nothing is written for a refused batch', - ); - }); - - test('a delete of a card that is not there refuses without touching the realm', async function (assert) { - let jobsBefore = await indexJobIds(); - let failure: unknown; - try { - await commit([{ op: 'delete', href: `${testRealmHref}not-a-card` }]); - } catch (err: unknown) { - failure = err; - } - assert.ok(isOperationFailure(failure), 'the delete is refused'); - if (isOperationFailure(failure)) { - assert.strictEqual(failure.error.status, 404); - assert.strictEqual(failure.error.code, 'target-not-found'); - } - assert.deepEqual( - await indexJobIds(), - jobsBefore, - 'no index job is enqueued', - ); - }); - - test('two entries changing one card are refused rather than silently ordered', async function (assert) { - let failure: unknown; - try { - await commit([ - { - op: 'update', - href: `${testRealmHref}person-a`, - document: { - data: { - type: 'card', - attributes: { firstName: 'Left' }, - meta: { adoptsFrom: PERSON }, - }, - }, - }, - { - op: 'update', - href: `${testRealmHref}person-a`, - document: { - data: { - type: 'card', - attributes: { hourlyRate: 99 }, - meta: { adoptsFrom: PERSON }, - }, - }, - }, - ]); - } catch (err: unknown) { - failure = err; - } - assert.ok(isOperationFailure(failure), 'the batch is refused'); - if (isOperationFailure(failure)) { - assert.strictEqual(failure.error.code, 'invalid-params'); - assert.strictEqual(failure.error.meta?.conflictsWith, 0); - } - assert.true( - readFileSync(realmFile('person-a.json'), 'utf8').includes('Original'), - 'the card is left as it was', - ); - }); - - test('a batch cannot change a card to another type', async function (assert) { - let failure: unknown; - try { - await commit([ - { - op: 'update', - href: `${testRealmHref}person-a`, - document: { - data: { - type: 'card', - attributes: { firstName: 'Shapeshifter' }, - meta: { - adoptsFrom: { - module: rri('@cardstack/base/card-api'), - name: 'CardDef', - }, - }, - }, - }, - }, - ]); - } catch (err: unknown) { - failure = err; - } - assert.ok(isOperationFailure(failure), 'the type change is refused'); - if (isOperationFailure(failure)) { - assert.strictEqual(failure.error.status, 400); - } - }); - - test('a base version on anything but an update is refused', async function (assert) { - let failure: unknown; - try { - await commit([ - { op: 'delete', href: `${testRealmHref}person-c`, baseVersion: 'abc' }, - ]); - } catch (err: unknown) { - failure = err; - } - assert.ok(isOperationFailure(failure), 'the base version is refused'); - assert.ok( - existsSync(realmFile('person-c.json')), - 'the card is left in place', - ); - }); }); From 3073231d3984397eaf5b9820dc82e480f7c7a1af Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Thu, 10 Sep 2026 14:51:30 +0000 Subject: [PATCH 05/14] Hold a created card's id to naming the file it is stored in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `lid` and a directory come from the caller and are spliced into a path, so both are checked before a URL is built from them. A `lid` of `../realm` — or `%2e%2e/realm`, which no character check catches — resolved out of the type's directory and onto the realm's own config file, and a `?` or `#` cut the stored path short so the card's id and its file stopped naming each other. Two checks, because neither covers the other: each has to be a plain path segment, and the path that comes back out of the URL has to be the path that went in. An unchanged file now records the hash it reports as well as returning it. A file written before the realm recorded hashes carries none on its row, so returning a token the row does not hold made the next write quoting it as `baseVersion` report a moved base for a file that had not moved. Refusals from the local-id pre-pass carry the entry's position, like the ones executors raise. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EBB6JdRo5XCY5WhUkgBckS --- .../tests/card-operations-batch-test.ts | 4 ++ .../tests/card-operations-commit-test.ts | 46 ++++++++++++++ .../card-operations/coordinator.ts | 61 +++++++++++-------- .../card-operations/executors.ts | 54 ++++++++++++++-- packages/runtime-common/realm.ts | 16 ++++- .../tests/card-operations-batch-test.ts | 39 ++++++++++++ 6 files changed, 187 insertions(+), 33 deletions(-) diff --git a/packages/realm-server/tests/card-operations-batch-test.ts b/packages/realm-server/tests/card-operations-batch-test.ts index 4f7210cbd8c..66a0688d40f 100644 --- a/packages/realm-server/tests/card-operations-batch-test.ts +++ b/packages/realm-server/tests/card-operations-batch-test.ts @@ -106,6 +106,10 @@ module(basename(import.meta.filename), function () { await runSharedTest(cardOperationsBatchTests, assert, {}); }); + test('a local id cannot name a file outside the type it creates', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); + test('a create with nothing to create is refused', async function (assert) { await runSharedTest(cardOperationsBatchTests, assert, {}); }); diff --git a/packages/realm-server/tests/card-operations-commit-test.ts b/packages/realm-server/tests/card-operations-commit-test.ts index 2a32c055cf9..87d3688f68a 100644 --- a/packages/realm-server/tests/card-operations-commit-test.ts +++ b/packages/realm-server/tests/card-operations-commit-test.ts @@ -92,6 +92,7 @@ function makeFileSystem(): Record { 'patch-over-http', 'patch-over-batch', 'unchanged', + 'legacy-version', ].map((name) => [ `${name}.json`, { @@ -496,6 +497,51 @@ module(basename(import.meta.filename), function (hooks) { ); }); + test('a version reported for an unchanged file is one a later write can name as its base', async function (assert) { + // A file written before the realm recorded content hashes carries none on + // its row. Blanking the row is how that state is reached here. + await testDbAdapter.execute( + `update realm_file_meta set content_hash = null + where realm_url = $1 and file_path = $2`, + { bind: [realm.url, 'legacy-version.json'] }, + ); + + let [unchanged] = await commit([ + { + op: 'update', + href: `${testRealmHref}legacy-version`, + document: { + data: { + type: 'card', + attributes: { firstName: 'Original' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]); + let version = unchanged!.meta.version; + assert.true(version.length > 0, 'the no-op write still reports a version'); + + let [next] = await commit([ + { + op: 'update', + href: `${testRealmHref}legacy-version`, + baseVersion: version, + document: { + data: { + type: 'card', + attributes: { firstName: 'Moved on' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]); + assert.true( + next!.meta.baseMatched, + 'the version the no-op reported is the one the file is recorded at', + ); + }); + test('a patch that changes nothing leaves the file exactly as it is', async function (assert) { let before = readFileSync(realmFile('unchanged.json'), 'utf8'); let jobsBefore = await indexJobIds(); diff --git a/packages/runtime-common/card-operations/coordinator.ts b/packages/runtime-common/card-operations/coordinator.ts index 26d0b81410c..04c026780f9 100644 --- a/packages/runtime-common/card-operations/coordinator.ts +++ b/packages/runtime-common/card-operations/coordinator.ts @@ -230,34 +230,43 @@ function indexLids(entries: BatchEntry[], paths: RealmPaths): LidIndex { if (entry.op === 'delete') { continue; } - let primary = entry.document?.data; - if (entry.op === 'create' && entry.lid) { - claim( - entry.lid, - createIdentity(entry, primary, paths, new Map()), - `entry ${index}`, - ); - } - for (let [offset, resource] of (entry.document?.included ?? []).entries()) { - // A side-loaded resource with no `lid` is not staged and nothing can - // link to it; one naming another realm is not this batch's to write. - // Neither takes an identity here, so neither can be linked to either. - if ( - typeof resource.lid !== 'string' || - namesForeignRealm(resource, paths.url) - ) { - continue; + // Labelled with the entry's position like every other refusal: a caller + // reading one needs to know which of the entries it sent produced it, + // whether it was found here or inside an executor. + try { + let primary = entry.document?.data; + if (entry.op === 'create' && entry.lid) { + claim( + entry.lid, + createIdentity(entry, primary, paths, new Map()), + `entry ${index}`, + ); } - claim( - resource.lid, - stagedIdentity( - resource.meta?.adoptsFrom, + for (let [offset, resource] of ( + entry.document?.included ?? [] + ).entries()) { + // A side-loaded resource with no `lid` is not staged and nothing can + // link to it; one naming another realm is not this batch's to write. + // Neither takes an identity here, so neither can be linked to either. + if ( + typeof resource.lid !== 'string' || + namesForeignRealm(resource, paths.url) + ) { + continue; + } + claim( resource.lid, - entry.op === 'create' ? entry.directory : undefined, - paths, - ), - `entry ${index}, included[${offset}]`, - ); + stagedIdentity( + resource.meta?.adoptsFrom, + resource.lid, + entry.op === 'create' ? entry.directory : undefined, + paths, + ), + `entry ${index}, included[${offset}]`, + ); + } + } catch (err: unknown) { + throw atEntry(err, index); } } return lids; diff --git a/packages/runtime-common/card-operations/executors.ts b/packages/runtime-common/card-operations/executors.ts index 78d5e1da129..534fc4567a6 100644 --- a/packages/runtime-common/card-operations/executors.ts +++ b/packages/runtime-common/card-operations/executors.ts @@ -434,19 +434,63 @@ export function stageDelete( // realm root unless the entry names a directory. Path math only — no read and // no serialization — which is what makes a create's URL knowable before // anything is written. +// +// The id and the directory come from the caller and are spliced into a path, +// so both are held to naming what they appear to name. Two checks, because +// neither covers the other: each has to be a plain path segment, which is what +// refuses a separator; and the path that comes back has to be the path that +// went in, which is what refuses everything that only shows up once a URL is +// resolved — `..` and its percent-encoded spellings walking out of the type's +// directory, and a `?` or `#` cutting the stored path short so the card's id +// and its file stop naming each other. export function stagedIdentity( adoptsFrom: CodeRef | undefined, id: string, directory: string | undefined, paths: RealmPaths, ): StagedIdentity { - let segments = [ - ...(directory ?? '').split('/'), + let directorySegments = (directory ?? '').split('/').filter(Boolean); + for (let segment of directorySegments) { + assertPathSegment(segment, `directory segment "${segment}"`); + } + assertPathSegment(id, `id "${id}"`); + let intended = `${[ + ...directorySegments, getCardDirectoryName(adoptsFrom, paths), id, - ].filter(Boolean); - let url = paths.fileURL(`${segments.join('/')}.json`); - return { id: url.href.replace(/\.json$/, ''), path: paths.local(url) }; + ].join('/')}.json` as LocalPath; + let url = paths.fileURL(intended); + if (paths.local(url) !== intended) { + throw new OperationFailure({ + status: 400, + code: 'invalid-params', + title: 'Invalid id', + detail: + `a card created as "${id}" would be stored at ${paths.local(url)} ` + + `rather than ${intended}, so the id and the file would not name each ` + + `other`, + }); + } + return { id: url.href.replace(/\.json$/, ''), path: intended }; +} + +// One name in a path, and nothing else. A separator would spread one card over +// a path the caller did not ask for, and the relative names address a +// directory rather than a card. +function assertPathSegment(value: string, what: string): void { + if ( + value.length === 0 || + value === '.' || + value === '..' || + /[/\\]/.test(value) + ) { + throw new OperationFailure({ + status: 400, + code: 'invalid-params', + title: 'Invalid id', + detail: `${what} is not a single path segment`, + }); + } } // The identity a create entry's card takes, whether the entry sent a document diff --git a/packages/runtime-common/realm.ts b/packages/runtime-common/realm.ts index 190d903d792..6fdc3b6705d 100644 --- a/packages/runtime-common/realm.ts +++ b/packages/runtime-common/realm.ts @@ -2482,12 +2482,24 @@ export class Realm { // still the file's own — the bytes in hand are the bytes on disk — // so a caller reading a version off this result gets the one the // file already holds rather than nothing. + // + // Recorded on the row as well as returned. A file written before the + // realm began recording hashes has none stored, and returning a + // token the row does not carry would make the next write quoting it + // as `baseVersion` report a moved base for a file that has not + // moved. Writing the hash it already has is a no-op for every file + // that has one. + let unchangedHash = computeContentHash(content); results.push({ path, lastModified: existingFile.lastModified, - contentHash: computeContentHash(content), + contentHash: unchangedHash, + }); + fileMetaRows.push({ + path, + contentHash: unchangedHash, + contentSize: computeContentSize(content), }); - fileMetaRows.push({ path }); continue; } isNewFile = !existingFile; diff --git a/packages/runtime-common/tests/card-operations-batch-test.ts b/packages/runtime-common/tests/card-operations-batch-test.ts index c78239db85f..30f3239293d 100644 --- a/packages/runtime-common/tests/card-operations-batch-test.ts +++ b/packages/runtime-common/tests/card-operations-batch-test.ts @@ -983,6 +983,45 @@ const tests: SharedTests> = { ); }, + 'a local id cannot name a file outside the type it creates': async ( + assert, + ) => { + let { core, commits } = stub(); + let create = (lid: string): BatchEntry[] => [ + { + op: 'create', + lid, + document: { + data: { + type: 'card', + attributes: { firstName: 'Escapee' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]; + // Each of these resolves, through a URL, to a file the create does not + // name: the first two walk out of the type's directory and land on the + // realm's own config, the third spreads one card over a path nobody asked + // for, and the last two are cut short by a query or a fragment so the + // card's id and its file stop naming each other. + for (let lid of ['../realm', '%2e%2e/realm', 'a/b', 'a?b', 'a#b']) { + assert.deepEqual( + await refusal(core, create(lid)), + { status: 400, code: 'invalid-params', entry: 0 }, + `a local id of "${lid}" is refused`, + ); + } + assert.strictEqual(commits.length, 0, 'nothing is committed'); + + await commitBatch(core, create('mango-1'), {}); + assert.deepEqual( + Object.keys(commits[0].writes), + ['Person/mango-1.json'], + 'an ordinary local id still names its own file under the type', + ); + }, + 'a create with nothing to create is refused': async (assert) => { let { core, commits } = stub(); let failed = await refusal(core, [{ op: 'create', lid: 'empty' }]); From c87abb25d9dec415d1ee3508eca77927e95347fb Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Thu, 10 Sep 2026 15:04:02 +0000 Subject: [PATCH 06/14] Say what the batch's all-or-nothing guarantee covers, and what it does not MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The guarantee is over the entries: every way an entry can be wrong is found while the realm is still untouched. It is not over the file system. The realm changes a batch's files one at a time with no rollback, so a mid-commit failure leaves the files handled before it changed — as it does for every multi-file write the realm serves. Reaching past that needs transactional staging in the write primitive, which is not something the coordinator can do above it, so the comments say where the line is rather than implying there isn't one. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EBB6JdRo5XCY5WhUkgBckS --- .../card-operations/coordinator.ts | 28 +++++++++++++------ packages/runtime-common/realm.ts | 6 ++++ 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/packages/runtime-common/card-operations/coordinator.ts b/packages/runtime-common/card-operations/coordinator.ts index 04c026780f9..873640f5e14 100644 --- a/packages/runtime-common/card-operations/coordinator.ts +++ b/packages/runtime-common/card-operations/coordinator.ts @@ -26,14 +26,26 @@ import type { RealmResourceIdentifier } from '../realm-identifiers.ts'; // ============================================================================ // The batch coordinator. // -// A batch is all-or-nothing: several changes to several cards either all land -// or none do, under one index job and one index event. Getting that is a -// matter of ordering. The coordinator takes the realm's write lock, reads -// every file the batch touches, runs every executor in memory, and only then -// commits. Nothing is written until every entry has produced its bytes, so an -// entry that cannot be carried out is found while the realm is still -// untouched — there is no partial write to undo, no index job to cancel, and -// no event a subscriber could have already acted on. +// A batch is all-or-nothing against every way an entry can be wrong: several +// changes to several cards either all land or none do, under one index job and +// one index event. Getting that is a matter of ordering. The coordinator takes +// the realm's write lock, reads every file the batch touches, runs every +// executor in memory, and only then commits. Nothing is written until every +// entry has produced its bytes, so an entry that cannot be carried out — a +// malformed document, a missing target, a rejected field, a local id naming +// nothing or naming two cards — is found while the realm is still untouched: +// no partial write to undo, no index job to cancel, no event a subscriber +// could have already acted on. +// +// What that does not cover is the commit itself failing partway. The realm +// writes a batch's files one at a time and has no rollback, so a file system +// that fails mid-commit — out of space, a path that will not open — leaves the +// files written before it on disk, and this method rejects with the realm in +// that state. Every multi-file write the realm serves behaves this way; a +// batch is not more exposed to it than the atomic endpoint is, and reaching +// past it needs transactional staging in the write primitive rather than +// anything the coordinator can do above it. The guarantee here is over the +// entries, which is what a caller composing a batch controls. // // The lock is taken once, here, and never re-entered. Everything below it // works from the state read inside it, and the commit it hands the staged diff --git a/packages/runtime-common/realm.ts b/packages/runtime-common/realm.ts index 6fdc3b6705d..7780f7ec250 100644 --- a/packages/runtime-common/realm.ts +++ b/packages/runtime-common/realm.ts @@ -2351,6 +2351,12 @@ export class Realm { // Assumes the realm's write lock is held — the caller reads the pre-state it // stages from inside the same critical section. `write`, `writeMany` and // `delete` are the locked public entry points. + // + // Files are changed one at a time and there is no rollback: a file system + // failure partway through leaves the files handled before it changed, and + // this method rejects with the realm in that state. A caller offering its + // own callers an all-or-nothing batch is offering it over what it validates + // before calling here, not over the file system underneath. private async _commitBatchUnlocked( batch: CommitBatch, options?: WriteOptions, From d1159675fefedd03952ddad2776c259ccf98c280 Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Thu, 10 Sep 2026 15:27:54 +0000 Subject: [PATCH 07/14] Refuse a create that would land on a stored card, and close four staging gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A create mints a card; it does not replace one. A caller choosing its own local id can choose one a stored card already answers to, and committing over it destroyed that card with nothing said. Each staged entry now reports the files it brings into existence, and an occupied destination refuses the batch inside the same lock the commit runs in — the answer the atomic endpoint already gives an `add` whose href is taken. Four gaps alongside it: - A named create validates every param its declaration asks for before substituting any of them. A missing value resolved to `undefined` and the field was left off the card, so a caller who forgot one got a card quietly missing it rather than being told. - `included` is held to being a list of card resources. A malformed side-load reached the serializer and came back as an internal failure, where the card endpoints answer the caller with a 400. - A side-loaded resource is rewritten on a copy. Staging is what makes a batch abandonable, and rewriting in place left the caller's own document carrying resolved links after a batch that committed nothing. - A create's local id is read from the resource as well as the entry, the way a POST body carries it. A payload naming its card only on the resource was minted under a generated id, and relationships elsewhere in the batch pointing at that local id resolved to nothing. The local-id pre-pass reads both the local id and the side-loads through the same accessors the executors use, so it claims the identities staging will ask for and refuses a malformed payload in the same terms. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EBB6JdRo5XCY5WhUkgBckS --- .../tests/card-operations-batch-test.ts | 20 ++ .../card-operations/coordinator.ts | 66 ++++++- .../card-operations/executors.ts | 102 +++++++++- packages/runtime-common/realm.ts | 1 + .../tests/card-operations-batch-test.ts | 184 ++++++++++++++++++ 5 files changed, 356 insertions(+), 17 deletions(-) diff --git a/packages/realm-server/tests/card-operations-batch-test.ts b/packages/realm-server/tests/card-operations-batch-test.ts index 66a0688d40f..b677cd831c5 100644 --- a/packages/realm-server/tests/card-operations-batch-test.ts +++ b/packages/realm-server/tests/card-operations-batch-test.ts @@ -110,6 +110,26 @@ module(basename(import.meta.filename), function () { await runSharedTest(cardOperationsBatchTests, assert, {}); }); + test('a create does not commit over a card already stored at its destination', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); + + test('a create names its card by the local id on the resource', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); + + test('a malformed side-load is refused rather than reaching the serializer', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); + + test('staging leaves the document it was handed alone', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); + + test('a named create needs every value its declaration asks for', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); + test('a create with nothing to create is refused', async function (assert) { await runSharedTest(cardOperationsBatchTests, assert, {}); }); diff --git a/packages/runtime-common/card-operations/coordinator.ts b/packages/runtime-common/card-operations/coordinator.ts index 873640f5e14..aed9cf16162 100644 --- a/packages/runtime-common/card-operations/coordinator.ts +++ b/packages/runtime-common/card-operations/coordinator.ts @@ -1,6 +1,8 @@ import { RealmPaths, type LocalPath } from '../paths.ts'; import { createIdentity, + includedResources, + localIdOf, namesForeignRealm, stageCreate, stageDelete, @@ -67,6 +69,10 @@ export interface BatchCore { readSourceFile( localPath: LocalPath, ): Promise<{ content: string; lastModified: number } | undefined>; + // Whether anything is stored at this path. Distinct from reading it: the + // destinations a create mints are checked for being free, and their bytes + // are of no interest. + fileExists(localPath: LocalPath): Promise; // The write-time content fingerprints recorded for these paths, read in one // round-trip. A caller's `baseVersion` names one of these. contentHashes( @@ -146,6 +152,7 @@ export async function commitBatch( }), ); } + await assertDestinationsFree(core, staged); // Past this point the batch is committed. Everything above either produced // bytes for every entry or threw, and a throw leaves the realm as it was. return await commitStaged(core, entries, staged, stored, opts); @@ -247,15 +254,22 @@ function indexLids(entries: BatchEntry[], paths: RealmPaths): LidIndex { // whether it was found here or inside an executor. try { let primary = entry.document?.data; - if (entry.op === 'create' && entry.lid) { - claim( - entry.lid, - createIdentity(entry, primary, paths, new Map()), - `entry ${index}`, - ); + // Both the local id and the side-loads are read through the same + // accessors the executors use, so the pre-pass claims exactly the + // identities staging will ask for and refuses a malformed payload in + // the same terms. + if (entry.op === 'create') { + let primaryLid = localIdOf(entry); + if (primaryLid !== undefined) { + claim( + primaryLid, + createIdentity(entry, primary, paths, new Map()), + `entry ${index}`, + ); + } } - for (let [offset, resource] of ( - entry.document?.included ?? [] + for (let [offset, resource] of includedResources( + entry.document, ).entries()) { // A side-loaded resource with no `lid` is not staged and nothing can // link to it; one naming another realm is not this batch's to write. @@ -351,6 +365,42 @@ function sourcePathOf(href: string, paths: RealmPaths): LocalPath | undefined { // Committing // --------------------------------------------------------------------------- +// A create mints a card; it does not replace one. A caller choosing its own +// local id can choose one a stored card already answers to, and committing +// over it would destroy that card with nothing said — so an occupied +// destination refuses the batch, which is the answer the atomic endpoint +// gives an `add` whose href is taken. +// +// Checked inside the lock, against the same critical section the commit runs +// in, so nothing can take the path between the check and the write. +async function assertDestinationsFree( + core: BatchCore, + staged: StagedChange[], +): Promise { + let occupied = await Promise.all( + staged.map(async (change, index) => { + for (let path of change.mints) { + if (await core.fileExists(path)) { + return { index, path }; + } + } + return undefined; + }), + ); + let taken = occupied.find((entry) => entry !== undefined); + if (taken) { + throw new OperationFailure({ + status: 409, + code: 'invalid-params', + title: 'Resource already exists', + detail: + `a card is already stored at ${taken.path}; a create mints a card ` + + `rather than replacing one`, + meta: { entry: taken.index }, + }); + } +} + async function commitStaged( core: BatchCore, entries: BatchEntry[], diff --git a/packages/runtime-common/card-operations/executors.ts b/packages/runtime-common/card-operations/executors.ts index 534fc4567a6..dcd486f5249 100644 --- a/packages/runtime-common/card-operations/executors.ts +++ b/packages/runtime-common/card-operations/executors.ts @@ -128,6 +128,12 @@ export interface StagedWrite { export interface StagedChange { writes: StagedWrite[]; deletes: LocalPath[]; + // The files this entry brings into existence, as opposed to the ones it + // rewrites. A card already stored at one of these is not this entry's to + // replace, so the coordinator refuses the batch rather than committing over + // it — the same answer the atomic endpoint gives an `add` whose href is + // taken. + mints: LocalPath[]; // The card the entry's result reports. id: string; // Echoed on a create, so a client can match the URL the realm minted back to @@ -205,7 +211,7 @@ export async function stageCreate( content: await serializeForStorage(primary, identity, ctx), }, ]; - for (let resource of entry.document?.included ?? []) { + for (let resource of includedResources(entry.document)) { // A side-loaded resource with no `lid` is not staged: it has no id to be // created under and nothing in the batch can link to it, so the client // sent a resource the realm has no way to name. One naming another realm @@ -220,25 +226,78 @@ export async function stageCreate( await stageSideLoaded(resource, resource.lid, identity.id, ctx), ); } + let lid = localIdOf(entry); return { writes, deletes: [], + mints: writes.map((write) => write.path), id: identity.id, - ...(entry.lid ? { lid: entry.lid } : {}), + ...(lid ? { lid } : {}), primaryPath: identity.path, }; } +// The local id a create entry is named by. A raw JSON:API create carries it on +// the resource, the way a `POST` body does; an entry may also name it +// directly. Read through one function so both spellings name the same card — +// otherwise a payload naming its card only on the resource is minted under a +// generated id, and every relationship elsewhere in the batch pointing at that +// local id resolves to nothing. +export function localIdOf(entry: CreateEntry): string | undefined { + if (typeof entry.lid === 'string') { + return entry.lid; + } + let resourceLid = entry.document?.data?.lid; + return typeof resourceLid === 'string' ? resourceLid : undefined; +} + +// The side-loaded resources a document carries, held to the shape the card +// endpoints require of one: a list, of card resources. A malformed side-load +// is the caller's payload to fix, so it is refused here rather than reaching +// the serializer as an internal failure. +export function includedResources( + document: BatchDocument | undefined, +): CardResource[] { + let included = document?.included; + if (included === undefined) { + return []; + } + if (!Array.isArray(included)) { + throw new OperationFailure({ + status: 400, + code: 'invalid-params', + title: 'Invalid document', + detail: `"included" is not an array`, + }); + } + for (let resource of included) { + if (!isCardResource(resource)) { + throw new OperationFailure({ + status: 400, + code: 'invalid-params', + title: 'Invalid document', + detail: `a side-loaded resource is not a valid card resource`, + }); + } + } + return included; +} + // A card side-loaded alongside the one an entry names. Its module refs are // written relative to the card it was sent with, so they are resolved against // that card before it is serialized under its own URL. async function stageSideLoaded( - resource: CardResource, + sent: CardResource, lid: string, relativeTo: string, ctx: StagingContext, ): Promise { let identity = stagedLid(lid, ctx); + // Rewritten on a copy. Staging is what makes a batch abandonable, and a + // rewrite in place would leave the caller's own document carrying resolved + // links and absolutized modules after a batch that committed nothing — + // state a retry of the same entries would then stage on top of. + let resource = cloneDeep(sent); promoteStagedLinks(resource, ctx); visitModuleDeps(resource, (moduleId, setModuleId) => { setModuleId(ctx.resolveModuleId(moduleId, relativeTo)); @@ -329,7 +388,7 @@ export async function stageUpdate( )}`, }); } - let included = entry.document.included ?? []; + let included = includedResources(entry.document); // Realm-managed keys never come from a patch: `realmInfo` and `realmURL` are // stamped by the realm serving the card, `screenshots` is joined from the // prerendered manifest at serve time, and `type` is fixed by the document @@ -396,7 +455,15 @@ export async function stageUpdate( writes.push(await stageSideLoaded(resource, resource.lid, url.href, ctx)); } } - return { writes, deletes: [], id: url.href, primaryPath: sourcePath }; + return { + writes, + deletes: [], + // The primary rewrites a card that is already there; only the side-loaded + // creates bring a file into existence. + mints: writes.map((write) => write.path).filter((p) => p !== sourcePath), + id: url.href, + primaryPath: sourcePath, + }; } // --------------------------------------------------------------------------- @@ -423,7 +490,7 @@ export function stageDelete( detail: `${url.href} does not exist in realm ${ctx.realmURL}`, }); } - return { writes: [], deletes: [sourcePath], id: url.href }; + return { writes: [], deletes: [sourcePath], mints: [], id: url.href }; } // --------------------------------------------------------------------------- @@ -504,15 +571,16 @@ export function createIdentity( paths: RealmPaths, lids: LidIndex, ): StagedIdentity { - if (entry.lid) { - let staged = lids.get(entry.lid); + let lid = localIdOf(entry); + if (lid !== undefined) { + let staged = lids.get(lid); if (staged) { return staged; } } return stagedIdentity( resource?.meta?.adoptsFrom ?? entry.definition?.of, - entry.lid ?? uuidV4(), + lid ?? uuidV4(), entry.directory, paths, ); @@ -665,6 +733,22 @@ async function resourceFromTemplate( ctx: StagingContext, ): Promise { let of = definition.of!; + // The payload has to satisfy the declaration's own schema before anything is + // substituted from it. A missing value would otherwise resolve to + // `undefined` and the field would simply be left off the card, so a caller + // that forgot a required param would get a card quietly missing it rather + // than being told. This is the check dispatch applies before an executor + // runs, applied here for the callers that stage a batch directly. + for (let key of Object.keys(definition.params ?? {})) { + if (own(entry.params, key) === undefined) { + throw new OperationFailure({ + status: 400, + code: 'invalid-params', + title: 'Invalid params', + detail: `this create requires a value for params("${key}")`, + }); + } + } let anchor = entry.href ? anchorResource(entry, ctx) : undefined; let linkFields = await linkFieldsOf(of, entry, ctx); let resource: CardResource = { type: 'card', meta: { adoptsFrom: of } }; diff --git a/packages/runtime-common/realm.ts b/packages/runtime-common/realm.ts index 7780f7ec250..5bf763ca48e 100644 --- a/packages/runtime-common/realm.ts +++ b/packages/runtime-common/realm.ts @@ -3326,6 +3326,7 @@ export class Realm { this.#batchCore = { realmURL: this.url, withWriteLock: (fn) => this.#dbAdapter.withWriteLock(this.url, fn), + fileExists: (localPath) => this.#adapter.exists(localPath), readSourceFile: async (localPath) => { let file = await this.readFileAsText(localPath); return file diff --git a/packages/runtime-common/tests/card-operations-batch-test.ts b/packages/runtime-common/tests/card-operations-batch-test.ts index 30f3239293d..6347913cff3 100644 --- a/packages/runtime-common/tests/card-operations-batch-test.ts +++ b/packages/runtime-common/tests/card-operations-batch-test.ts @@ -86,6 +86,9 @@ function stub(opts: StubOptions = {}): Stub { held--; } }, + async fileExists(localPath) { + return stored[localPath] !== undefined; + }, async readSourceFile(localPath) { let content = stored[localPath]; return content === undefined @@ -1022,6 +1025,187 @@ const tests: SharedTests> = { ); }, + 'a create does not commit over a card already stored at its destination': + async (assert) => { + let { core, commits } = stub({ + stored: { + 'Person/taken.json': cardFile({ firstName: 'Incumbent' }, PERSON), + }, + }); + let failed = await refusal(core, [ + { + op: 'create', + lid: 'taken', + document: { + data: { + type: 'card', + attributes: { firstName: 'Usurper' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]); + assert.deepEqual(failed, { + status: 409, + code: 'invalid-params', + entry: 0, + }); + assert.strictEqual(commits.length, 0, 'nothing is committed'); + }, + + 'a create names its card by the local id on the resource': async (assert) => { + let { core, commits } = stub(); + await commitBatch( + core, + [ + { + op: 'create', + document: { + data: { + type: 'card', + lid: 'owner', + attributes: { firstName: 'Hassan' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + { + op: 'create', + lid: 'pet', + document: { + data: { + type: 'card', + attributes: { firstName: 'Mango' }, + relationships: { + friend: { data: { type: 'card', lid: 'owner' } }, + }, + meta: { adoptsFrom: PET }, + }, + }, + }, + ], + {}, + ); + assert.deepEqual( + Object.keys(commits[0].writes).sort(), + ['Person/owner.json', 'Pet/pet.json'], + 'a local id carried on the resource names the file, as a POST body does', + ); + let staged = JSON.parse(commits[0].writes['Pet/pet.json']); + assert.strictEqual( + staged.data.relationships.friend.links.self, + `${REALM}Person/owner`, + 'and another entry can link to it', + ); + }, + + 'a malformed side-load is refused rather than reaching the serializer': + async (assert) => { + let { core, commits } = stub({ + stored: { + 'person-1.json': cardFile({ firstName: 'Original' }, PERSON), + }, + }); + let create = (included: unknown): BatchEntry[] => [ + { + op: 'create', + lid: 'primary', + document: { + data: { + type: 'card', + attributes: { firstName: 'Primary' }, + meta: { adoptsFrom: PERSON }, + }, + included, + } as never, + }, + ]; + assert.deepEqual( + await refusal(core, create({ lid: 'not-a-list' })), + { status: 400, code: 'invalid-params', entry: 0 }, + "an `included` that is not a list is the caller's payload to fix", + ); + assert.deepEqual( + await refusal(core, create([{ lid: 'x', attributes: {} }])), + { status: 400, code: 'invalid-params', entry: 0 }, + 'and so is a side-load that is not a card resource', + ); + assert.deepEqual( + await refusal(core, [ + { + op: 'update', + href: `${REALM}person-1`, + document: { + data: { + type: 'card', + attributes: { firstName: 'Patched' }, + meta: { adoptsFrom: PERSON }, + }, + included: { lid: 'not-a-list' }, + } as never, + }, + ]), + { status: 400, code: 'invalid-params', entry: 0 }, + 'an update holds its side-loads to the same shape', + ); + assert.strictEqual(commits.length, 0, 'nothing is committed'); + }, + + 'staging leaves the document it was handed alone': async (assert) => { + let { core } = stub(); + let sideLoaded = { + type: 'card' as const, + lid: 'sidecar', + attributes: { firstName: 'Sidecar' }, + meta: { adoptsFrom: PET }, + }; + let before = JSON.stringify(sideLoaded); + await commitBatch( + core, + [ + { + op: 'create', + lid: 'primary', + document: { + data: { + type: 'card', + attributes: { firstName: 'Primary' }, + meta: { adoptsFrom: PERSON }, + }, + included: [sideLoaded], + }, + }, + ], + {}, + ); + assert.strictEqual( + JSON.stringify(sideLoaded), + before, + 'the side-loaded resource the caller owns is not rewritten in place', + ); + }, + + 'a named create needs every value its declaration asks for': async ( + assert, + ) => { + let definition: OperationDefinition = { + base: 'create', + deterministic: true, + of: PERSON, + params: { title: { kind: 'field', codeRef: STRING } }, + fill: { firstName: { $ref: 'params', key: 'title' } as never }, + }; + let { core, commits } = stub({ + definitions: { Person: personDefinition() }, + }); + assert.deepEqual( + await refusal(core, [{ op: 'create', lid: 'missing', definition }]), + { status: 400, code: 'invalid-params', entry: 0 }, + 'a declared param with no value is refused, not left off the card', + ); + assert.strictEqual(commits.length, 0, 'nothing is committed'); + }, + 'a create with nothing to create is refused': async (assert) => { let { core, commits } = stub(); let failed = await refusal(core, [{ op: 'create', lid: 'empty' }]); From 96e3bca8ce99fd19c8289d531302319b61eefad0 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 16:57:31 +0000 Subject: [PATCH 08/14] Apply the realm's write checks before a batch commits, and close five staging gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A batch's promise is that a refusal costs nothing: the realm untouched, no index job queued, no event out. Several checks the realm applies per file as it writes were only reaching the batch inside the commit, where a refusal already costs the files written ahead of it. - The size ceiling is applied to every staged write up front, so a payload the realm will not store is refused while nothing is written. - Indexing already in flight is drained inside the lock before anything is staged. Staging serializes a card against its type's definition, and a module written moments earlier may still be indexing. - An empty batch returns without taking the lock; broadcasting an empty index event would tell every subscriber that something changed. A `baseVersion` is now compared against a fingerprint of the bytes just read rather than the file's recorded row. A file changed out from under the realm is re-indexed without being rewritten, so the row can name a version the bytes no longer hold — the exact case a base version exists to catch. The staging gaps: - A create's side-loaded resource keeps the module references it was sent with. Resolving them against the primary resolved them against a card one directory deep, which is not where the side-load lands. - An update stages no mints. A side-load addressed by a caller-chosen local id may name a card the caller is deliberately rewriting, which is what the PATCH handler this mirrors does; treating it as a mint answered 409 where the handler succeeds. - An update carrying no `data` is refused rather than dereferenced. - A local id inside a `data` array is refused rather than silently dropped. A collection's edges are stored one key per member, so a local id written there has no key of its own to record the link on. - A template reading `actor` with none in scope is refused, and an `instance(…)` attribute is read as an own property. The id-to-path round trip refuses a path that resolves outside the realm rather than throwing out of `RealmPaths.local` while building its message. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EBB6JdRo5XCY5WhUkgBckS --- .../tests/card-operations-batch-test.ts | 44 +- .../card-operations/coordinator.ts | 86 +++- .../card-operations/executors.ts | 98 +++- packages/runtime-common/realm.ts | 36 +- .../tests/card-operations-batch-test.ts | 473 +++++++++++++++--- 5 files changed, 609 insertions(+), 128 deletions(-) diff --git a/packages/realm-server/tests/card-operations-batch-test.ts b/packages/realm-server/tests/card-operations-batch-test.ts index b677cd831c5..0343994a272 100644 --- a/packages/realm-server/tests/card-operations-batch-test.ts +++ b/packages/realm-server/tests/card-operations-batch-test.ts @@ -46,7 +46,7 @@ module(basename(import.meta.filename), function () { await runSharedTest(cardOperationsBatchTests, assert, {}); }); - test('a base version is reported against the fingerprint the file carried', async function (assert) { + test('a base version is compared to the bytes the merge is computed over', async function (assert) { await runSharedTest(cardOperationsBatchTests, assert, {}); }); @@ -86,7 +86,7 @@ module(basename(import.meta.filename), function () { await runSharedTest(cardOperationsBatchTests, assert, {}); }); - test('the write lock is taken once for the whole batch', async function (assert) { + test('every read and the commit happen inside one holding of the write lock', async function (assert) { await runSharedTest(cardOperationsBatchTests, assert, {}); }); @@ -130,6 +130,46 @@ module(basename(import.meta.filename), function () { await runSharedTest(cardOperationsBatchTests, assert, {}); }); + test('a local id inside a data array is refused rather than dropped', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); + + test('a member written under its own key still links', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); + + test('bytes the realm will not store are refused before anything commits', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); + + test('an empty batch touches nothing at all', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); + + test('a batch drains in-flight indexing before it serializes anything', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); + + test('an update rewrites a side-loaded card that is already stored', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); + + test('a template that reads the actor needs one', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); + + test('an update carries the patch to apply', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); + + test('a patch merges relationship keys rather than replacing the map', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); + + test('a replaced array drops the field metadata of the members it removed', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); + test('a create with nothing to create is refused', async function (assert) { await runSharedTest(cardOperationsBatchTests, assert, {}); }); diff --git a/packages/runtime-common/card-operations/coordinator.ts b/packages/runtime-common/card-operations/coordinator.ts index aed9cf16162..576635cf1f8 100644 --- a/packages/runtime-common/card-operations/coordinator.ts +++ b/packages/runtime-common/card-operations/coordinator.ts @@ -1,3 +1,4 @@ +import { computeContentHash } from '../index.ts'; import { RealmPaths, type LocalPath } from '../paths.ts'; import { createIdentity, @@ -28,16 +29,18 @@ import type { RealmResourceIdentifier } from '../realm-identifiers.ts'; // ============================================================================ // The batch coordinator. // -// A batch is all-or-nothing against every way an entry can be wrong: several +// A batch is all-or-nothing against every way an entry can be wrong — a +// malformed document, a missing target, a rejected field, bytes over the +// realm's size ceiling, a local id naming nothing or naming two cards: several // changes to several cards either all land or none do, under one index job and // one index event. Getting that is a matter of ordering. The coordinator takes // the realm's write lock, reads every file the batch touches, runs every // executor in memory, and only then commits. Nothing is written until every -// entry has produced its bytes, so an entry that cannot be carried out — a -// malformed document, a missing target, a rejected field, a local id naming -// nothing or naming two cards — is found while the realm is still untouched: -// no partial write to undo, no index job to cancel, no event a subscriber -// could have already acted on. +// entry has produced its bytes, and every check the realm would apply per file +// as it writes is applied to the staged bytes first — so an entry that cannot +// be carried out is found while the realm is still untouched: no partial write +// to undo, no index job to cancel, no event a subscriber could have already +// acted on. // // What that does not cover is the commit itself failing partway. The realm // writes a batch's files one at a time and has no rollback, so a file system @@ -73,11 +76,16 @@ export interface BatchCore { // destinations a create mints are checked for being free, and their bytes // are of no interest. fileExists(localPath: LocalPath): Promise; - // The write-time content fingerprints recorded for these paths, read in one - // round-trip. A caller's `baseVersion` names one of these. - contentHashes( - localPaths: LocalPath[], - ): Promise>; + // Refuses bytes the realm will not store at this path — the size ceiling a + // card or a file is held to. Throws the realm's own error, which already + // carries the status a caller sees. + assertWriteSize(localPath: LocalPath, content: string): void; + // Waits for indexing already in flight. A card's serialization resolves the + // definitions its type is built from, and a module written moments earlier + // may still be indexing, so a batch drains before it stages rather than + // failing to resolve a type the realm already holds. + drainIndexing(): Promise; + // The realm's unlocked commit: writes and removals under one index job and // one index event. Assumes the write lock is held, which it is. commitUnlocked( @@ -129,7 +137,18 @@ export async function commitBatch( opts: CommitBatchOptions = {}, ): Promise { let paths = new RealmPaths(new URL(core.realmURL)); + if (entries.length === 0) { + // Nothing to serialize the realm's writers behind, and nothing to + // announce. Taking the lock and broadcasting an empty index event would + // tell every subscriber that something changed. + return []; + } return await core.withWriteLock(async () => { + // Drained inside the lock, before anything is staged. Staging serializes + // each card against its type's definition, and a module written moments + // earlier may still be indexing; without this a batch that follows a + // module upload fails to resolve a type the realm already has on disk. + await core.drainIndexing(); // Every `lid` in the batch resolves to a URL before any executor runs. A // created card's file is named after its `lid`, so its URL is path math // over the type it adopts — no read and no write — which is what lets an @@ -152,9 +171,12 @@ export async function commitBatch( }), ); } + assertWritesFit(core, staged); await assertDestinationsFree(core, staged); - // Past this point the batch is committed. Everything above either produced - // bytes for every entry or threw, and a throw leaves the realm as it was. + // Everything above either produced bytes for every entry or threw, and a + // throw leaves the realm as it was. `commitStaged` still refuses a batch + // whose entries claim one file twice, which it can only see once every + // entry has staged. return await commitStaged(core, entries, staged, stored, opts); }); } @@ -305,9 +327,10 @@ function indexLids(entries: BatchEntry[], paths: RealmPaths): LidIndex { // Every file the batch reads, loaded once inside the write lock: the merge base // for each update, the existence check for each delete, and the anchoring card // a named create reads through `instance(…)`. Loading them up front is what -// makes the executors pure — they resolve nothing and read nothing — and what -// makes the pre-write content hashes, which a `baseVersion` is compared to, a -// snapshot from inside the same critical section as the write. +// makes the executors pure — they resolve nothing and read nothing — and it is +// what makes the bytes a `baseVersion` is compared against the same bytes the +// merge is computed over, read inside the critical section the write happens +// in. async function readStoredFiles( core: BatchCore, entries: BatchEntry[], @@ -325,11 +348,9 @@ async function readStoredFiles( wanted.add(localPath); } } - let localPaths = [...wanted]; - let hashes = await core.contentHashes(localPaths); let stored = new Map(); await Promise.all( - localPaths.map(async (localPath) => { + [...wanted].map(async (localPath) => { let file = await core.readSourceFile(localPath); if (!file) { return; @@ -337,7 +358,14 @@ async function readStoredFiles( stored.set(localPath, { content: file.content, lastModified: file.lastModified, - contentHash: hashes.get(localPath), + // Fingerprinted from the bytes just read, not from the file's + // recorded row. The row is written when the realm writes a file, and + // a file changed out from under the realm is re-indexed without it + // being rewritten — so the row can name a version the bytes no longer + // hold. A `baseVersion` exists to catch exactly that, and comparing + // it to the row would report a match for the state it is meant to + // detect. This is the fingerprint of what the merge is computed over. + contentHash: computeContentHash(file.content), }); }), ); @@ -365,6 +393,24 @@ function sourcePathOf(href: string, paths: RealmPaths): LocalPath | undefined { // Committing // --------------------------------------------------------------------------- +// The realm refuses bytes over its size ceiling, and it refuses them one file +// at a time as it writes. Reaching that inside the commit would leave the +// files written before it on disk and unindexed — the commit rejects before +// it enqueues anything — over a payload the caller could have been told about +// while the realm was still untouched. So the ceiling is applied to every +// staged write here, where a refusal still costs nothing. +function assertWritesFit(core: BatchCore, staged: StagedChange[]): void { + for (let [index, change] of staged.entries()) { + for (let write of change.writes) { + try { + core.assertWriteSize(write.path, write.content); + } catch (err: unknown) { + throw atEntry(err, index); + } + } + } +} + // A create mints a card; it does not replace one. A caller choosing its own // local id can choose one a stored card already answers to, and committing // over it would destroy that card with nothing said — so an occupied diff --git a/packages/runtime-common/card-operations/executors.ts b/packages/runtime-common/card-operations/executors.ts index dcd486f5249..50d7a1c09ec 100644 --- a/packages/runtime-common/card-operations/executors.ts +++ b/packages/runtime-common/card-operations/executors.ts @@ -222,9 +222,11 @@ export async function stageCreate( ) { continue; } - writes.push( - await stageSideLoaded(resource, resource.lid, identity.id, ctx), - ); + // A create's side-load keeps the module references it was sent with. + // Resolving them against the primary would resolve them against a card + // one directory deep, which is not where the side-load lands and not + // what a caller writing them meant. + writes.push(await stageSideLoaded(resource, resource.lid, undefined, ctx)); } let lid = localIdOf(entry); return { @@ -283,13 +285,15 @@ export function includedResources( return included; } -// A card side-loaded alongside the one an entry names. Its module refs are -// written relative to the card it was sent with, so they are resolved against -// that card before it is serialized under its own URL. +// A card side-loaded alongside the one an entry names. `relativeTo` is the +// card its module references were written against, when there is one: an +// update's side-load is sent alongside a stored card and names modules +// relative to it, while a create's is sent alongside a card that does not +// exist yet and names them for itself. async function stageSideLoaded( sent: CardResource, lid: string, - relativeTo: string, + relativeTo: string | undefined, ctx: StagingContext, ): Promise { let identity = stagedLid(lid, ctx); @@ -299,9 +303,11 @@ async function stageSideLoaded( // state a retry of the same entries would then stage on top of. let resource = cloneDeep(sent); promoteStagedLinks(resource, ctx); - visitModuleDeps(resource, (moduleId, setModuleId) => { - setModuleId(ctx.resolveModuleId(moduleId, relativeTo)); - }); + if (relativeTo !== undefined) { + visitModuleDeps(resource, (moduleId, setModuleId) => { + setModuleId(ctx.resolveModuleId(moduleId, relativeTo)); + }); + } return { path: identity.path, content: await serializeForStorage(resource, identity, ctx), @@ -360,6 +366,15 @@ export async function stageUpdate( }); } let original = storedResource(stored.content, url); + if (!entry.document?.data) { + throw new OperationFailure({ + id: url.href, + status: 400, + code: 'invalid-params', + title: 'Invalid document', + detail: `an update carries the patch to apply, and this one carries none`, + }); + } let patch = cloneDeep(entry.document.data); if (!isCardResource(patch)) { throw new OperationFailure({ @@ -458,9 +473,12 @@ export async function stageUpdate( return { writes, deletes: [], - // The primary rewrites a card that is already there; only the side-loaded - // creates bring a file into existence. - mints: writes.map((write) => write.path).filter((p) => p !== sourcePath), + // Nothing here is a mint. The primary rewrites a card that is already + // there, and a side-load addressed by a local id the caller chose may + // name a card it is deliberately rewriting — which is what a `PATCH` + // carrying the same side-load does. Refusing it here would make a batch + // answer 409 where the handler it mirrors succeeds. + mints: [], id: url.href, primaryPath: sourcePath, }; @@ -510,6 +528,11 @@ export function stageDelete( // resolved — `..` and its percent-encoded spellings walking out of the type's // directory, and a `?` or `#` cutting the stored path short so the card's id // and its file stop naming each other. +// +// The type's directory is not one of the two. It comes from the type's own +// name rather than from the request, and a card adopting a type whose name +// carries a separator is stored under the nested path that name spells — the +// same place the card endpoints store it. export function stagedIdentity( adoptsFrom: CodeRef | undefined, id: string, @@ -527,15 +550,24 @@ export function stagedIdentity( id, ].join('/')}.json` as LocalPath; let url = paths.fileURL(intended); - if (paths.local(url) !== intended) { + // `paths.local` throws for a URL that resolved outside the realm, which is + // the loudest of the cases this is here to catch, so the comparison is made + // where that throw becomes the same refusal a merely-wrong path gets. + let resolved: string | undefined; + try { + resolved = paths.local(url); + } catch { + resolved = undefined; + } + if (resolved !== intended) { throw new OperationFailure({ status: 400, code: 'invalid-params', title: 'Invalid id', detail: - `a card created as "${id}" would be stored at ${paths.local(url)} ` + - `rather than ${intended}, so the id and the file would not name each ` + - `other`, + `a card created as "${id}" would be stored at ` + + `${resolved ?? url.href} rather than ${intended}, so the id and the ` + + `file would not name each other`, }); } return { id: url.href.replace(/\.json$/, ''), path: intended }; @@ -635,10 +667,26 @@ function promoteStagedLinks(resource: CardResource, ctx: StagingContext): void { if (!('lid' in item)) { continue; } + // A collection's edges are stored one key per member — `field.0`, + // `field.1` — and that is the only spelling whose links survive + // serialization. A local id written inside a single `data` array has + // no key of its own to carry a link, so it is refused: staging it + // would store the collection with the edge missing and say nothing, + // which is the one outcome worse than a refusal for the mechanism + // the whole batch exists to provide. let indexed = normalized[`${fieldName}.${index}`]; - if (indexed) { - setSelfLink(indexed, item.lid); + if (!indexed) { + throw new OperationFailure({ + status: 400, + code: 'invalid-params', + title: 'Unlinkable local id', + detail: + `"${fieldName}" carries local id "${item.lid}" inside a \`data\` ` + + `array, which has no per-member key to record the link on; ` + + `write each member under its own "${fieldName}.N" key`, + }); } + setSelfLink(indexed, item.lid); } continue; } @@ -928,6 +976,16 @@ function resolveMarker( }; } case 'actor': + if (!ctx.actor) { + throw new OperationFailure({ + status: 400, + code: 'invalid-params', + title: 'No actor in scope', + detail: + `\`${path}\` reads the invoking actor, and this invocation was ` + + `made without one`, + }); + } // The realm knows the actor by identity, which is what a link to them // needs and the only member there is to read. if (marker.key !== undefined && marker.key !== 'id') { @@ -956,7 +1014,7 @@ function resolveMarker( return { value: anchor.id, isLink: false }; } return { - value: anchor.resource.attributes?.[String(marker.key)], + value: own(anchor.resource.attributes, String(marker.key)), isLink: false, }; } diff --git a/packages/runtime-common/realm.ts b/packages/runtime-common/realm.ts index 5bf763ca48e..e36681786f7 100644 --- a/packages/runtime-common/realm.ts +++ b/packages/runtime-common/realm.ts @@ -73,7 +73,6 @@ import { removeFileMeta, getCreatedTime, getContentMeta, - getFileMetaForPaths, } from './file-meta.ts'; import { systemError, @@ -2342,15 +2341,16 @@ export class Realm { // different snapshots of the realm, and two events would let a subscriber // observe the batch half-applied. // - // Writes land before removals. A write's serialization resolves the - // definitions its instance adopts from, which the write leg's own - // module-then-instance flush is what keeps current; a removal invalidates - // rows and touches no definition, so it has nothing to contribute to that - // flush and nothing to gain from running ahead of it. + // Writes land before removals. The write leg carries a mid-loop index flush + // that a module followed by an instance depends on, so it has an ordering + // constraint of its own; a removal invalidates rows, touches no definition, + // and has nothing to contribute to that flush or to gain from running ahead + // of it. // // Assumes the realm's write lock is held — the caller reads the pre-state it - // stages from inside the same critical section. `write`, `writeMany` and - // `delete` are the locked public entry points. + // stages from inside the same critical section. `write` and `writeMany` are + // the locked public entry points that reach this; `delete` and `deleteAll` + // have their own unlocked primitives and do not. // // Files are changed one at a time and there is no rollback: a file system // failure partway through leaves the files handled before it changed, and @@ -3333,18 +3333,14 @@ export class Realm { ? { content: file.content, lastModified: file.lastModified } : undefined; }, - contentHashes: async (localPaths) => { - let meta = await getFileMetaForPaths( - this.#dbAdapter, - this.url, - localPaths, - ); - return new Map( - localPaths.map((localPath) => [ - localPath, - meta.get(localPath)?.contentHash, - ]), - ); + assertWriteSize: (localPath, content) => + this.assertWriteSize( + content, + isCardDocumentString(content) ? 'card' : 'file', + localPath, + ), + drainIndexing: async () => { + await this.incrementalIndexing(); }, commitUnlocked: (batch, options) => this._commitBatchUnlocked(batch, options), diff --git a/packages/runtime-common/tests/card-operations-batch-test.ts b/packages/runtime-common/tests/card-operations-batch-test.ts index 6347913cff3..7c42974f93e 100644 --- a/packages/runtime-common/tests/card-operations-batch-test.ts +++ b/packages/runtime-common/tests/card-operations-batch-test.ts @@ -5,6 +5,7 @@ import { type BatchEntry, type OperationDefinition, } from '../card-operations/index.ts'; +import { computeContentHash } from '../index.ts'; import type { CodeRef } from '../code-ref.ts'; import type { RealmIdentifier } from '../realm-identifiers.ts'; import type { Definition } from '../definitions.ts'; @@ -41,13 +42,15 @@ interface Stub { core: BatchCore; commits: Commit[]; lockDepth: () => number; + drainCount: () => number; + readsOutsideLock: () => number; } interface StubOptions { // The stored source files the realm holds, by local path. stored?: Record; - // The write-time fingerprint each stored path carries. - hashes?: Record; + // The realm's size ceiling, for the paths a batch stages. + sizeLimit?: number; // The definition-cache entry a code ref resolves to, keyed by its name. definitions?: Record; // What `serializeCard` does to a resource on its way to storage. The default @@ -65,15 +68,12 @@ function cardFile(attributes: Record, adoptsFrom: unknown) { } function stub(opts: StubOptions = {}): Stub { - let { - stored = {}, - hashes = {}, - definitions = {}, - serialize = (doc: any) => doc, - } = opts; + let { stored = {}, definitions = {}, serialize = (doc: any) => doc } = opts; let commits: Commit[] = []; let held = 0; let maxHeld = 0; + let drains = 0; + let readsOutsideLock = 0; let core: BatchCore = { realmURL: REALM, @@ -87,20 +87,27 @@ function stub(opts: StubOptions = {}): Stub { } }, async fileExists(localPath) { + readsOutsideLock += held > 0 ? 0 : 1; return stored[localPath] !== undefined; }, async readSourceFile(localPath) { + readsOutsideLock += held > 0 ? 0 : 1; let content = stored[localPath]; return content === undefined ? undefined : { content, lastModified: 1000 }; }, - async contentHashes(localPaths) { - return new Map( - localPaths.map((localPath) => [localPath, hashes[localPath]]), - ); + assertWriteSize(localPath, content) { + let limit = opts.sizeLimit; + if (limit !== undefined && content.length > limit) { + throw new Error(`${localPath} is over the realm's size limit`); + } + }, + async drainIndexing() { + drains++; }, async commitUnlocked(batch, options) { + readsOutsideLock += held > 0 ? 0 : 1; let writes = Object.fromEntries( [...(batch.writes ?? new Map())].map( ([path, content]) => [path, String(content)], @@ -139,7 +146,13 @@ function stub(opts: StubOptions = {}): Stub { return 'name' in codeRef ? definitions[codeRef.name] : undefined; }, }; - return { core, commits, lockDepth: () => maxHeld }; + return { + core, + commits, + lockDepth: () => maxHeld, + drainCount: () => drains, + readsOutsideLock: () => readsOutsideLock, + }; } function personDefinition(): Definition { @@ -500,10 +513,14 @@ const tests: SharedTests> = { undefined, 'a client cannot persist a screenshot manifest', ); - assert.strictEqual( + // `realmURL` is stripped from the patch and then stamped by the realm on + // the way to storage; the serializer drops it again before the bytes + // land, so what this pins is that the client's value never survives the + // merge — not that the realm's does. + assert.notStrictEqual( data.meta.realmURL, - REALM, - 'the realm stamps its own URL rather than taking the one it was sent', + 'http://elsewhere.example/', + 'a client cannot persist a realm URL of its choosing', ); }, @@ -557,18 +574,16 @@ const tests: SharedTests> = { assert.strictEqual(commits.length, 0, 'nothing is committed'); }, - 'a base version is reported against the fingerprint the file carried': async ( + 'a base version is compared to the bytes the merge is computed over': async ( assert, ) => { - let { core } = stub({ - stored: { 'person-1.json': cardFile({ firstName: 'Original' }, PERSON) }, - hashes: { 'person-1.json': 'v1' }, - }); - let patch = (firstName: string): BatchEntry[] => [ + let content = cardFile({ firstName: 'Original' }, PERSON); + let onDisk = computeContentHash(content); + let patch = (firstName: string, baseVersion: string): BatchEntry[] => [ { op: 'update', href: `${REALM}person-1`, - baseVersion: 'v1', + baseVersion, document: { data: { type: 'card', @@ -579,27 +594,24 @@ const tests: SharedTests> = { }, ]; - let [matched] = await commitBatch(core, patch('Matched'), {}); + let { core } = stub({ stored: { 'person-1.json': content } }); + let [matched] = await commitBatch(core, patch('Matched', onDisk), {}); assert.true( matched?.meta.baseMatched, - 'the base the caller named is the one the file carried', + 'the base the caller named fingerprints the bytes on disk', ); - let { core: moved } = stub({ - stored: { 'person-1.json': cardFile({ firstName: 'Original' }, PERSON) }, - hashes: { 'person-1.json': 'v2' }, - }); - let [result] = await commitBatch(moved, patch('Moved'), {}); + let { core: moved } = stub({ stored: { 'person-1.json': content } }); + let [result] = await commitBatch(moved, patch('Moved', 'stale'), {}); assert.false( result?.meta.baseMatched, - 'the file has moved past the base the caller named', + 'the bytes have moved past the base the caller named', ); }, 'an unconditional write reports no base match': async (assert) => { let { core } = stub({ stored: { 'person-1.json': cardFile({ firstName: 'Original' }, PERSON) }, - hashes: { 'person-1.json': 'v1' }, }); let [result] = await commitBatch( core, @@ -789,44 +801,53 @@ const tests: SharedTests> = { }); }, - 'the write lock is taken once for the whole batch': async (assert) => { - let { core, lockDepth } = stub({ - stored: { 'person-1.json': cardFile({ firstName: 'Original' }, PERSON) }, - }); - await commitBatch( - core, - [ - { - op: 'create', - lid: 'one', - document: { - data: { - type: 'card', - attributes: { firstName: 'One' }, - meta: { adoptsFrom: PERSON }, + 'every read and the commit happen inside one holding of the write lock': + async (assert) => { + let { core, lockDepth, readsOutsideLock } = stub({ + stored: { + 'person-1.json': cardFile({ firstName: 'Original' }, PERSON), + }, + }); + await commitBatch( + core, + [ + { + op: 'create', + lid: 'one', + document: { + data: { + type: 'card', + attributes: { firstName: 'One' }, + meta: { adoptsFrom: PERSON }, + }, }, }, - }, - { - op: 'update', - href: `${REALM}person-1`, - document: { - data: { - type: 'card', - attributes: { firstName: 'Two' }, - meta: { adoptsFrom: PERSON }, + { + op: 'update', + href: `${REALM}person-1`, + document: { + data: { + type: 'card', + attributes: { firstName: 'Two' }, + meta: { adoptsFrom: PERSON }, + }, }, }, - }, - ], - {}, - ); - assert.strictEqual( - lockDepth(), - 1, - 'the lock is never re-entered, whatever the batch contains', - ); - }, + ], + {}, + ); + assert.strictEqual( + lockDepth(), + 1, + 'the lock is never re-entered, whatever the batch contains', + ); + assert.strictEqual( + readsOutsideLock(), + 0, + 'nothing the batch acts on is read before the lock is held, and the ' + + 'commit runs while it still is', + ); + }, 'a named create stages the type and attributes its declaration names': async ( assert, @@ -1206,6 +1227,326 @@ const tests: SharedTests> = { assert.strictEqual(commits.length, 0, 'nothing is committed'); }, + 'a local id inside a data array is refused rather than dropped': async ( + assert, + ) => { + let { core, commits } = stub(); + let failed = await refusal(core, [ + { + op: 'create', + lid: 'target', + document: { + data: { + type: 'card', + attributes: { firstName: 'Target' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + { + op: 'create', + lid: 'linker', + document: { + data: { + type: 'card', + attributes: { firstName: 'Linker' }, + // No per-member key to record the link on, so serialization would + // store the collection with the edge missing. + relationships: { + friend: { data: [{ type: 'card', lid: 'target' }] }, + }, + meta: { adoptsFrom: PERSON }, + } as never, + }, + }, + ]); + assert.deepEqual(failed, { status: 400, code: 'invalid-params', entry: 1 }); + assert.strictEqual(commits.length, 0, 'nothing is committed'); + }, + + 'a member written under its own key still links': async (assert) => { + let { core, commits } = stub(); + await commitBatch( + core, + [ + { + op: 'create', + lid: 'target', + document: { + data: { + type: 'card', + attributes: { firstName: 'Target' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + { + op: 'create', + lid: 'linker', + document: { + data: { + type: 'card', + attributes: { firstName: 'Linker' }, + relationships: { + 'friend.0': { data: { type: 'card', lid: 'target' } }, + }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ], + {}, + ); + let { data } = JSON.parse(commits[0].writes['Person/linker.json']); + assert.strictEqual( + data.relationships['friend.0'].links.self, + `${REALM}Person/target`, + ); + }, + + 'bytes the realm will not store are refused before anything commits': async ( + assert, + ) => { + let { core, commits } = stub({ sizeLimit: 1000 }); + let failed = await refusal(core, [ + { + op: 'create', + lid: 'small', + document: { + data: { + type: 'card', + attributes: { firstName: 'Small' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + { + op: 'create', + lid: 'huge', + document: { + data: { + type: 'card', + attributes: { firstName: 'H'.repeat(4000) }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]); + assert.strictEqual(failed?.entry, 1, 'the oversized entry is named'); + assert.strictEqual( + commits.length, + 0, + 'the entry ahead of it is not written either — the size ceiling is ' + + 'reached while the realm is still untouched', + ); + }, + + 'an empty batch touches nothing at all': async (assert) => { + let { core, commits, lockDepth, drainCount } = stub(); + assert.deepEqual(await commitBatch(core, [], {}), []); + assert.strictEqual(commits.length, 0, 'no commit'); + assert.strictEqual(lockDepth(), 0, 'the write lock is never taken'); + assert.strictEqual(drainCount(), 0, 'indexing is not drained'); + }, + + 'a batch drains in-flight indexing before it serializes anything': async ( + assert, + ) => { + let { core, drainCount } = stub(); + await commitBatch( + core, + [ + { + op: 'create', + lid: 'one', + document: { + data: { + type: 'card', + attributes: { firstName: 'One' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ], + {}, + ); + assert.strictEqual( + drainCount(), + 1, + 'a card is serialized against a definition the realm has finished ' + + 'indexing', + ); + }, + + 'an update rewrites a side-loaded card that is already stored': async ( + assert, + ) => { + let { core, commits } = stub({ + stored: { + 'person-1.json': cardFile({ firstName: 'Original' }, PERSON), + 'Pet/side.json': cardFile({ firstName: 'Stored' }, PET), + }, + }); + await commitBatch( + core, + [ + { + op: 'update', + href: `${REALM}person-1`, + document: { + data: { + type: 'card', + attributes: { firstName: 'Patched' }, + meta: { adoptsFrom: PERSON }, + }, + included: [ + { + type: 'card', + lid: 'side', + attributes: { firstName: 'Rewritten' }, + meta: { adoptsFrom: PET }, + }, + ], + }, + }, + ], + {}, + ); + assert.true( + commits[0].writes['Pet/side.json'].includes('Rewritten'), + 'a side-load names a card the caller is rewriting, as it does on a PATCH', + ); + }, + + 'a template that reads the actor needs one': async (assert) => { + let definition: OperationDefinition = { + base: 'create', + deterministic: true, + of: PERSON, + fill: { firstName: { $ref: 'actor' } as never }, + }; + let { core, commits } = stub({ + definitions: { Person: personDefinition() }, + }); + assert.deepEqual( + await refusal(core, [{ op: 'create', lid: 'anon', definition }]), + { status: 400, code: 'invalid-params', entry: 0 }, + 'an actor-less invocation does not write an empty author', + ); + assert.strictEqual(commits.length, 0, 'nothing is committed'); + }, + + 'an update carries the patch to apply': async (assert) => { + let { core, commits } = stub({ + stored: { 'person-1.json': cardFile({ firstName: 'Original' }, PERSON) }, + }); + assert.deepEqual( + await refusal(core, [ + { op: 'update', href: `${REALM}person-1` } as never, + ]), + { status: 400, code: 'invalid-params', entry: 0 }, + 'a document-less update is the payload to fix, not a crash', + ); + assert.strictEqual(commits.length, 0, 'nothing is committed'); + }, + + 'a patch merges relationship keys rather than replacing the map': async ( + assert, + ) => { + let stored = JSON.stringify( + { + data: { + type: 'card', + attributes: { firstName: 'Original' }, + relationships: { + friend: { links: { self: `${REALM}a` } }, + pet: { links: { self: `${REALM}b` } }, + }, + meta: { adoptsFrom: PERSON }, + }, + }, + null, + 2, + ); + let { core, commits } = stub({ stored: { 'person-1.json': stored } }); + await commitBatch( + core, + [ + { + op: 'update', + href: `${REALM}person-1`, + document: { + data: { + type: 'card', + relationships: { pet: { links: { self: `${REALM}c` } } }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ], + {}, + ); + let { data } = JSON.parse(commits[0].writes['person-1.json']); + assert.strictEqual( + data.relationships.friend.links.self, + `${REALM}a`, + 'a relationship the patch does not name is kept', + ); + assert.strictEqual( + data.relationships.pet.links.self, + `${REALM}c`, + 'and one it does name is replaced', + ); + }, + + 'a replaced array drops the field metadata of the members it removed': async ( + assert, + ) => { + let stored = JSON.stringify( + { + data: { + type: 'card', + attributes: { nicknames: ['a', 'b', 'c'] }, + meta: { + adoptsFrom: PERSON, + fields: { + 'nicknames.0': { adoptsFrom: PET }, + 'nicknames.2': { adoptsFrom: PET }, + }, + }, + }, + }, + null, + 2, + ); + let { core, commits } = stub({ stored: { 'person-1.json': stored } }); + await commitBatch( + core, + [ + { + op: 'update', + href: `${REALM}person-1`, + document: { + data: { + type: 'card', + attributes: { nicknames: ['z'] }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ], + {}, + ); + let { data } = JSON.parse(commits[0].writes['person-1.json']); + assert.deepEqual(data.attributes.nicknames, ['z'], 'the array is replaced'); + assert.strictEqual( + data.meta.fields, + undefined, + "the removed members' per-index metadata does not survive to be " + + 're-applied when the array grows again', + ); + }, + 'a create with nothing to create is refused': async (assert) => { let { core, commits } = stub(); let failed = await refusal(core, [{ op: 'create', lid: 'empty' }]); From 2518fe88f930a07c839ba6f78b7be79e93630060 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 16:57:46 +0000 Subject: [PATCH 09/14] Filter the one-index-event assertion to the batch's own paths The realm is shared across this file's tests and index broadcasts are fire-and-forget, so a straggler from an earlier test can land inside the window and fail an assertion counting events over it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EBB6JdRo5XCY5WhUkgBckS --- .../realm-server/tests/card-operations-commit-test.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/realm-server/tests/card-operations-commit-test.ts b/packages/realm-server/tests/card-operations-commit-test.ts index 87d3688f68a..b7df9847df4 100644 --- a/packages/realm-server/tests/card-operations-commit-test.ts +++ b/packages/realm-server/tests/card-operations-commit-test.ts @@ -384,7 +384,14 @@ module(basename(import.meta.filename), function (hooks) { let seen = await incrementalIndexEventsSince(since); return seen.some((event) => event.clientRequestId === 'batch-1'); }); - let indexEvents = await incrementalIndexEventsSince(since); + // Filtered to the paths this batch touched rather than counted over the + // window: the realm is shared across this file's tests and broadcasts are + // fire-and-forget, so a straggler from an earlier test can land inside it. + let indexEvents = (await incrementalIndexEventsSince(since)).filter( + (event) => + event.invalidations.includes(`${testRealmHref}commit-write`) || + event.invalidations.includes(`${testRealmHref}commit-delete`), + ); assert.strictEqual( indexEvents.length, 1, From b2d0c1a0c7b6ef95ccd2dc9539fbe893b99fa6fd Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 19:28:32 +0000 Subject: [PATCH 10/14] Import the content fingerprint from the module that defines it The card-operations barrel is the entry point a consumer imports directly rather than through the package barrel, precisely so it costs less than the package barrel does. Reaching back through `../index.ts` for one function gives that subpath an eager dependency on the whole package. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EBB6JdRo5XCY5WhUkgBckS --- packages/runtime-common/card-operations/coordinator.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/runtime-common/card-operations/coordinator.ts b/packages/runtime-common/card-operations/coordinator.ts index 576635cf1f8..dbafa992700 100644 --- a/packages/runtime-common/card-operations/coordinator.ts +++ b/packages/runtime-common/card-operations/coordinator.ts @@ -1,4 +1,4 @@ -import { computeContentHash } from '../index.ts'; +import { computeContentHash } from '../content-hash.ts'; import { RealmPaths, type LocalPath } from '../paths.ts'; import { createIdentity, From 824d87a3345a004a7dd52be2853213a681cbbdaf Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 20:38:19 +0000 Subject: [PATCH 11/14] Carry the realm's 413 through, and hold a removal to cards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three gaps a convergence review turned up, each a divergence from the handler the batch mirrors: - An oversized payload was reported as `500 internal-error`. `assertWriteSize` throws the realm's own `CardError` carrying 413, and `atEntry` preserved a status only for an `OperationFailure`, so the one thing that tells a caller to send less was replaced by the one thing that tells it to send again. The status is carried across under a new `payload-too-large` code; `POST`, `PATCH` and `/_atomic` all answer 413 for the same bytes. - A removal took any stored `.json` at its href, `realm.json` among them, so a batch could take the realm's own configuration with it. `DELETE` answers 404 there because the index holds no card. The staged bytes answer the same question without a read, which keeps a card written moments ago deletable where consulting the index would not. - A write into `_screenshot/` was staged. The subtree is claimed by capture serving, so a realm file stored there could never be read back; direct writes and `/_atomic` each refuse it, and a batch is the third way in. Removals stay admitted, as they are in both other paths, since they are the recovery route for anything already there. Two tests were green for the wrong reason and are now falsifiable: - The oversized-payload test stubbed `assertWriteSize` with a bare `Error`, which cannot carry a status, so the flattening above was invisible to it. The stub now throws what the realm throws. - The `realmURL` assertion could not fail: the realm stamps its own URL over whatever the patch carried, so comparing against the client's value held whether or not the strip ran. It now pins the stamp, which is the property the stored file actually depends on. The unchanged-file hash is recorded for the file's metadata resource, not for `baseVersion` — that is computed from the bytes read inside the write lock and never consults the row. The comment claiming otherwise is corrected, and its test now asserts the row it was only pretending to exercise. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EBB6JdRo5XCY5WhUkgBckS --- .../tests/card-operations-batch-test.ts | 8 ++ .../tests/card-operations-commit-test.ts | 25 +++++- .../card-operations/coordinator.ts | 48 ++++++++++- .../card-operations/executors.ts | 39 +++++++-- .../runtime-common/card-operations/types.ts | 5 ++ packages/runtime-common/realm.ts | 14 ++-- .../tests/card-operations-batch-test.ts | 79 +++++++++++++++++-- 7 files changed, 192 insertions(+), 26 deletions(-) diff --git a/packages/realm-server/tests/card-operations-batch-test.ts b/packages/realm-server/tests/card-operations-batch-test.ts index 0343994a272..645823c1ad0 100644 --- a/packages/realm-server/tests/card-operations-batch-test.ts +++ b/packages/realm-server/tests/card-operations-batch-test.ts @@ -46,6 +46,14 @@ module(basename(import.meta.filename), function () { await runSharedTest(cardOperationsBatchTests, assert, {}); }); + test('a removal takes a card, not any stored json', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); + + test('a write into the capture subtree is refused', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); + test('a base version is compared to the bytes the merge is computed over', async function (assert) { await runSharedTest(cardOperationsBatchTests, assert, {}); }); diff --git a/packages/realm-server/tests/card-operations-commit-test.ts b/packages/realm-server/tests/card-operations-commit-test.ts index b7df9847df4..3f028e2be77 100644 --- a/packages/realm-server/tests/card-operations-commit-test.ts +++ b/packages/realm-server/tests/card-operations-commit-test.ts @@ -504,9 +504,12 @@ module(basename(import.meta.filename), function (hooks) { ); }); - test('a version reported for an unchanged file is one a later write can name as its base', async function (assert) { + test('a version is read from the file, and an unchanged write records it', async function (assert) { // A file written before the realm recorded content hashes carries none on - // its row. Blanking the row is how that state is reached here. + // its row. Blanking the row reaches that state, and is what makes both + // halves of this test falsifiable: a version is computed from the bytes, + // so it is issued and honored with the row empty, and the no-op write + // fills the row in on its way past. await testDbAdapter.execute( `update realm_file_meta set content_hash = null where realm_url = $1 and file_path = $2`, @@ -529,6 +532,21 @@ module(basename(import.meta.filename), function (hooks) { let version = unchanged!.meta.version; assert.true(version.length > 0, 'the no-op write still reports a version'); + // The row the blanking emptied now carries the version the write + // reported. Nothing about `baseVersion` depends on this — that is read + // from the bytes — but the file's own metadata resource reads the row, + // and a file the realm has never rewritten would otherwise carry none. + let [row] = await testDbAdapter.execute( + `select content_hash from realm_file_meta + where realm_url = $1 and file_path = $2`, + { bind: [realm.url, 'legacy-version.json'] }, + ); + assert.strictEqual( + row?.content_hash, + version, + 'the unchanged write records the version it reported', + ); + let [next] = await commit([ { op: 'update', @@ -545,7 +563,8 @@ module(basename(import.meta.filename), function (hooks) { ]); assert.true( next!.meta.baseMatched, - 'the version the no-op reported is the one the file is recorded at', + 'the version the no-op reported names the bytes the merge is computed ' + + 'over, so a later write can quote it as its base', ); }); diff --git a/packages/runtime-common/card-operations/coordinator.ts b/packages/runtime-common/card-operations/coordinator.ts index dbafa992700..eed4164e5ee 100644 --- a/packages/runtime-common/card-operations/coordinator.ts +++ b/packages/runtime-common/card-operations/coordinator.ts @@ -1,4 +1,5 @@ import { computeContentHash } from '../content-hash.ts'; +import { isCardError } from '../error.ts'; import { RealmPaths, type LocalPath } from '../paths.ts'; import { createIdentity, @@ -77,8 +78,8 @@ export interface BatchCore { // are of no interest. fileExists(localPath: LocalPath): Promise; // Refuses bytes the realm will not store at this path — the size ceiling a - // card or a file is held to. Throws the realm's own error, which already - // carries the status a caller sees. + // card or a file is held to. Throws the realm's own error, whose status the + // coordinator carries through to the caller. assertWriteSize(localPath: LocalPath, content: string): void; // Waits for indexing already in flight. A card's serialization resolves the // definitions its type is built from, and a module written moments earlier @@ -171,6 +172,7 @@ export async function commitBatch( }), ); } + assertWritesAllowed(staged); assertWritesFit(core, staged); await assertDestinationsFree(core, staged); // Everything above either produced bytes for every entry or threw, and a @@ -393,6 +395,31 @@ function sourcePathOf(href: string, paths: RealmPaths): LocalPath | undefined { // Committing // --------------------------------------------------------------------------- +// `_screenshot/` is claimed by capture serving, so a realm file stored there +// could never be read back — it would index and list, and answer every GET as +// an uncaptured miss. Direct writes and `/_atomic` operations each refuse the +// subtree before writing anything, and a batch is the third way in, so it +// refuses it too. Removals are not covered, deliberately: they are the +// recovery path for anything already stored there, which is why the other two +// admit them as well. +function assertWritesAllowed(staged: StagedChange[]): void { + for (let [index, change] of staged.entries()) { + for (let write of change.writes) { + if (write.path.startsWith('_screenshot/')) { + throw atEntry( + new OperationFailure({ + status: 422, + code: 'invalid-params', + title: 'Reserved path', + detail: `cannot write "${write.path}": "_screenshot/" is reserved for serving captures`, + }), + index, + ); + } + } + } +} + // The realm refuses bytes over its size ceiling, and it refuses them one file // at a time as it writes. Reaching that inside the commit would leave the // files written before it on disk and unindexed — the commit rejects before @@ -405,7 +432,22 @@ function assertWritesFit(core: BatchCore, staged: StagedChange[]): void { try { core.assertWriteSize(write.path, write.content); } catch (err: unknown) { - throw atEntry(err, index); + // The realm answers an oversized payload with 413, and that status is + // the whole remedy: it tells the caller to send less rather than to + // send again. Carried across rather than flattened into the generic + // staging failure, which would report the realm as broken and leave + // retrying as the caller's obvious next move. + throw atEntry( + isCardError(err) + ? new OperationFailure({ + status: err.status, + code: 'payload-too-large', + title: err.title ?? 'Payload Too Large', + detail: err.message, + }) + : err, + index, + ); } } } diff --git a/packages/runtime-common/card-operations/executors.ts b/packages/runtime-common/card-operations/executors.ts index 50d7a1c09ec..4ef2468c220 100644 --- a/packages/runtime-common/card-operations/executors.ts +++ b/packages/runtime-common/card-operations/executors.ts @@ -499,7 +499,8 @@ export function stageDelete( // an update merges over it: a card written a moment ago is on disk before it // is in the index, and refusing to delete it until indexing catches up would // make a client unable to remove what it just created. - if (!ctx.stored.has(sourcePath)) { + let stored = ctx.stored.get(sourcePath); + if (!stored) { throw new OperationFailure({ id: url.href, status: 404, @@ -508,6 +509,23 @@ export function stageDelete( detail: `${url.href} does not exist in realm ${ctx.realmURL}`, }); } + // A `.json` file on disk is not by itself a card. `realm.json` is the + // clearest case — it sits at a URL a delete can name, and removing it would + // take the realm's own configuration with it — but any stored JSON that is + // not a card document is one. The bytes already read answer this, so the + // check costs no read and keeps the just-written card above deletable, + // which asking the index would not. This is the answer `DELETE` gives for + // the same URL, where it is the index rather than the bytes that reports no + // card there. + if (!cardResourceIn(stored.content)) { + throw new OperationFailure({ + id: url.href, + status: 404, + code: 'target-not-found', + title: 'Not found', + detail: `${url.href} is not a card in realm ${ctx.realmURL}`, + }); + } return { writes: [], deletes: [sourcePath], mints: [], id: url.href }; } @@ -740,17 +758,26 @@ async function serializeForStorage( return JSON.stringify(serialized, null, 2); } -// The card resource a stored file holds. A file that is not a card document is -// reported as the realm's own fault rather than the caller's: the caller asked -// to patch a card, and what is on disk is not one. -function storedResource(content: string, url: URL): CardResource { +// The card resource a stored file holds, or nothing when the file is not a +// card document. Both callers ask the same question of the same bytes and +// differ only in what they make of a miss, so they read it through here +// rather than each parsing for itself. +function cardResourceIn(content: string): CardResource | undefined { let resource: unknown; try { resource = (JSON.parse(content) as { data?: unknown }).data; } catch (err: unknown) { resource = undefined; } - if (!isCardResource(resource)) { + return isCardResource(resource) ? resource : undefined; +} + +// The card resource a stored file holds. A file that is not a card document is +// reported as the realm's own fault rather than the caller's: the caller asked +// to patch a card, and what is on disk is not one. +function storedResource(content: string, url: URL): CardResource { + let resource = cardResourceIn(content); + if (!resource) { throw new OperationFailure({ id: url.href, status: 500, diff --git a/packages/runtime-common/card-operations/types.ts b/packages/runtime-common/card-operations/types.ts index 2d51bfb2cd0..99a6a413261 100644 --- a/packages/runtime-common/card-operations/types.ts +++ b/packages/runtime-common/card-operations/types.ts @@ -349,6 +349,11 @@ export type OperationErrorCode = // The request named a `baseVersion` the target is no longer at, on an // operation that requires the base to match. | 'version-conflict' + // The bytes an operation would store are over the realm's ceiling for a + // card or a file of that kind. Separate from `invalid-params` because the + // payload is well formed and the remedy is to send less of it, and because + // it carries the realm's own 413. + | 'payload-too-large' // The operation is sound but is not carried out here. A `query` is the case: // it is planned and run on the search engine, so reaching the operation core // with one means the caller used the wrong entry point. diff --git a/packages/runtime-common/realm.ts b/packages/runtime-common/realm.ts index e36681786f7..94ea1acc99b 100644 --- a/packages/runtime-common/realm.ts +++ b/packages/runtime-common/realm.ts @@ -2489,12 +2489,14 @@ export class Realm { // so a caller reading a version off this result gets the one the // file already holds rather than nothing. // - // Recorded on the row as well as returned. A file written before the - // realm began recording hashes has none stored, and returning a - // token the row does not carry would make the next write quoting it - // as `baseVersion` report a moved base for a file that has not - // moved. Writing the hash it already has is a no-op for every file - // that has one. + // Recorded on the row as well as returned. A file written before + // the realm began recording hashes carries none, and the row is + // what the file's metadata resource reports as its content hash — + // so a file the realm has never rewritten would answer without one + // until something changed its bytes. Writing the hash it already + // has is a no-op for every file that has one. This is not what + // makes `baseVersion` work: that is computed from the bytes read + // inside the write lock and never consults the row. let unchangedHash = computeContentHash(content); results.push({ path, diff --git a/packages/runtime-common/tests/card-operations-batch-test.ts b/packages/runtime-common/tests/card-operations-batch-test.ts index 7c42974f93e..f43d6894b74 100644 --- a/packages/runtime-common/tests/card-operations-batch-test.ts +++ b/packages/runtime-common/tests/card-operations-batch-test.ts @@ -6,6 +6,7 @@ import { type OperationDefinition, } from '../card-operations/index.ts'; import { computeContentHash } from '../index.ts'; +import { CardError } from '../error.ts'; import type { CodeRef } from '../code-ref.ts'; import type { RealmIdentifier } from '../realm-identifiers.ts'; import type { Definition } from '../definitions.ts'; @@ -100,7 +101,13 @@ function stub(opts: StubOptions = {}): Stub { assertWriteSize(localPath, content) { let limit = opts.sizeLimit; if (limit !== undefined && content.length > limit) { - throw new Error(`${localPath} is over the realm's size limit`); + // The shape `Realm.assertWriteSize` throws, down to the status: a + // stub that threw a bare `Error` here could not tell whether the + // coordinator carries the realm's 413 through or flattens it. + throw new CardError(`${localPath} is over the realm's size limit`, { + status: 413, + title: 'Payload Too Large', + }); } }, async drainIndexing() { @@ -513,14 +520,15 @@ const tests: SharedTests> = { undefined, 'a client cannot persist a screenshot manifest', ); - // `realmURL` is stripped from the patch and then stamped by the realm on - // the way to storage; the serializer drops it again before the bytes - // land, so what this pins is that the client's value never survives the - // merge — not that the realm's does. - assert.notStrictEqual( + // What keeps a client's `realmURL` out of the file is the realm stamping + // its own over it on the way to storage, not the strip in the merge — + // the strip is defensive, and removing it would change nothing about the + // bytes. So this pins the stamp, which is the property a reader of the + // file depends on, and would fail if the realm stopped applying it. + assert.strictEqual( data.meta.realmURL, - 'http://elsewhere.example/', - 'a client cannot persist a realm URL of its choosing', + REALM, + "the realm's own URL is stored, whatever the client sent", ); }, @@ -574,6 +582,54 @@ const tests: SharedTests> = { assert.strictEqual(commits.length, 0, 'nothing is committed'); }, + 'a removal takes a card, not any stored json': async (assert) => { + // The realm's own configuration is the sharpest case: it is stored as + // `realm.json`, so it answers to a URL a removal can name, and removing + // it would take the realm's settings with it. + let { core, commits } = stub({ + stored: { + 'realm.json': JSON.stringify({ name: 'Test Realm' }, null, 2), + 'notes.json': JSON.stringify({ note: 'not a card' }, null, 2), + }, + }); + for (let href of [`${REALM}realm`, `${REALM}notes`]) { + let failed = await refusal(core, [{ op: 'delete', href }]); + assert.deepEqual( + failed, + { status: 404, code: 'target-not-found', entry: 0 }, + `${href} holds no card, which is what DELETE answers for it too`, + ); + } + assert.strictEqual(commits.length, 0, 'neither file is removed'); + }, + + 'a write into the capture subtree is refused': async (assert) => { + let { core, commits } = stub({ + definitions: { Person: personDefinition() }, + }); + let failed = await refusal(core, [ + { + op: 'create', + lid: 'shot', + directory: '_screenshot', + document: { + data: { + type: 'card', + attributes: { firstName: 'Captured' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]); + assert.strictEqual(failed?.status, 422, 'the answer /_atomic gives'); + assert.strictEqual(failed?.entry, 0); + assert.strictEqual( + commits.length, + 0, + 'a file the realm would never serve back is not written', + ); + }, + 'a base version is compared to the bytes the merge is computed over': async ( assert, ) => { @@ -1333,6 +1389,13 @@ const tests: SharedTests> = { }, ]); assert.strictEqual(failed?.entry, 1, 'the oversized entry is named'); + assert.strictEqual( + failed?.status, + 413, + "the realm's own answer for an oversized payload, so the caller is " + + 'told to send less rather than to send again', + ); + assert.strictEqual(failed?.code, 'payload-too-large'); assert.strictEqual( commits.length, 0, From 1ccbca15246f5a6fbddfa6910bd23043bf40e81a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 21:07:31 +0000 Subject: [PATCH 12/14] Let two entries change one card, composing rather than refusing A batch is a sequence, so two entries naming one card are meant to build on each other. They were refused instead, on the reasoning that both would be computed against the state the batch started from and the second would silently discard the first. That is a consequence of staging every entry against one fixed snapshot, not a fact about batches: feed the later entry the bytes the earlier one staged and they compose, with nothing discarded and the file written once holding the last of them. An entry's staged writes are folded into the state the next entry stages against, and the commit collapses the sequence per file. The refusal is gone; it belongs to parallel siblings, which nothing here can express yet. The corners this opens, each falling out of the sequence rather than needing a rule of its own: - A removal takes its path back out of the staged state, so a change aimed at the same card afterwards finds nothing there and refuses the way it would outside a batch. - A removal after a change drops that change's write entirely rather than writing bytes the same commit then unlinks, and both entries report no state, which is what a removal reports. - A create's bytes are not folded in, so a card the batch mints is still reached by the local id other entries link to rather than by a URL they target. Tying the batch's meaning to path math a caller has to reproduce is the coupling the local id exists to avoid. `baseMatched` now compares against the version the entry actually merged over, captured as it stages. Reading it back from the stored map at commit time was correct only while that map stood still; a second entry on one card would otherwise have been told its base matched when it merged over bytes that base never named. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EBB6JdRo5XCY5WhUkgBckS --- .../tests/card-operations-batch-test.ts | 18 +- .../card-operations/coordinator.ts | 128 ++++++++----- .../tests/card-operations-batch-test.ts | 168 +++++++++++++++++- 3 files changed, 258 insertions(+), 56 deletions(-) diff --git a/packages/realm-server/tests/card-operations-batch-test.ts b/packages/realm-server/tests/card-operations-batch-test.ts index 645823c1ad0..80c95e02cff 100644 --- a/packages/realm-server/tests/card-operations-batch-test.ts +++ b/packages/realm-server/tests/card-operations-batch-test.ts @@ -74,7 +74,23 @@ module(basename(import.meta.filename), function () { await runSharedTest(cardOperationsBatchTests, assert, {}); }); - test('two entries changing one card are refused rather than ordered', async function (assert) { + test('two entries changing one card compose, and the file is written once', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); + + test('a base version names what the entry merged over, not the batch pre-state', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); + + test('a removal after a change takes the card, and the write with it', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); + + test('a change after a removal has nothing to change', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); + + test('a card the batch creates is not a target for a later entry', async function (assert) { await runSharedTest(cardOperationsBatchTests, assert, {}); }); diff --git a/packages/runtime-common/card-operations/coordinator.ts b/packages/runtime-common/card-operations/coordinator.ts index eed4164e5ee..e9e899f9425 100644 --- a/packages/runtime-common/card-operations/coordinator.ts +++ b/packages/runtime-common/card-operations/coordinator.ts @@ -157,32 +157,77 @@ export async function commitBatch( let lids = indexLids(entries, paths); let stored = await readStoredFiles(core, entries, paths); let staged: StagedChange[] = []; + // The version each entry's merge was computed over, captured as it stages + // rather than read back at the end: `stored` moves underneath the batch + // as entries compose, so by the commit it no longer holds what the first + // entry to touch a file merged over. + let baseHashes: (string | undefined)[] = []; for (let [index, entry] of entries.entries()) { - staged.push( - await stageEntry(entry, index, { - realmURL: core.realmURL, - paths, - lids, - stored, - actor: opts.actor ?? '', - serializeCard: core.serializeCard, - codeRefKey: core.codeRefKey, - resolveModuleId: core.resolveModuleId, - lookupDefinition: core.lookupDefinition, - }), + let change = await stageEntry(entry, index, { + realmURL: core.realmURL, + paths, + lids, + stored, + actor: opts.actor ?? '', + serializeCard: core.serializeCard, + codeRefKey: core.codeRefKey, + resolveModuleId: core.resolveModuleId, + lookupDefinition: core.lookupDefinition, + }); + baseHashes.push( + change.primaryPath + ? stored.get(change.primaryPath)?.contentHash + : undefined, ); + compose(stored, entry, change); + staged.push(change); } assertWritesAllowed(staged); assertWritesFit(core, staged); await assertDestinationsFree(core, staged); // Everything above either produced bytes for every entry or threw, and a - // throw leaves the realm as it was. `commitStaged` still refuses a batch - // whose entries claim one file twice, which it can only see once every - // entry has staged. - return await commitStaged(core, entries, staged, stored, opts); + // throw leaves the realm as it was. + return await commitStaged(core, entries, staged, baseHashes, opts); }); } +// Fold what an entry staged into the state the next entry stages against. A +// batch is a sequence, so two entries may name one card and the second is +// meant to build on the first: it merges over the bytes the first staged, not +// over the bytes the batch started from, and the commit writes the file once. +// Composing is what makes that safe — the alternative, letting both merge +// over the pre-batch state, is how the second silently discards the first. +// +// A removal takes its path back out, so an entry that follows one aimed at +// the same card finds nothing there and refuses the way it would outside a +// batch. +// +// A create's bytes are deliberately not folded in. The card it mints is +// reached by the `lid` other entries link to, not by an href they target, and +// letting a later entry address it by URL would tie the batch's meaning to +// path math the caller has to reproduce to predict it. +function compose( + stored: Map, + entry: BatchEntry, + change: StagedChange, +): void { + for (let path of change.deletes) { + stored.delete(path); + } + if (entry.op === 'create') { + return; + } + for (let write of change.writes) { + stored.set(write.path, { + content: write.content, + // The file has not been written yet, so its modification time is still + // the one on disk; the commit reports the real one. + lastModified: stored.get(write.path)?.lastModified ?? 0, + contentHash: computeContentHash(write.content), + }); + } +} + // Run one executor, and label whatever it refuses with the entry's position. // A batch is rejected as a whole, so a caller reading one error needs to know // which of the entries it sent produced it. @@ -493,42 +538,27 @@ async function commitStaged( core: BatchCore, entries: BatchEntry[], staged: StagedChange[], - stored: Map, + baseHashes: (string | undefined)[], opts: CommitBatchOptions, ): Promise { + // One entry per file, in batch order. Two entries may name one card, and + // the second built on the first rather than racing it — it merged over the + // bytes the first staged — so the file is written once holding the last of + // them rather than twice, and a removal that follows drops the write + // entirely instead of writing bytes the same commit then unlinks. let writes = new Map(); - let deletes: LocalPath[] = []; - // Which entry claimed each file. Two entries touching one file would each - // have been computed against the state the batch started from, so the second - // would silently discard the first — the very loss the write lock exists to - // prevent, reintroduced inside one batch. Refused rather than ordered: which - // change the caller meant to keep is not something the realm can infer. - let claimedBy = new Map(); - let claim = (path: LocalPath, index: number) => { - let owner = claimedBy.get(path); - if (owner !== undefined) { - throw new OperationFailure({ - status: 400, - code: 'invalid-params', - title: 'Conflicting entries', - detail: - `entries ${owner} and ${index} both change ${path}; a batch names ` + - `one change per card`, - meta: { entry: index, conflictsWith: owner }, - }); - } - claimedBy.set(path, index); - }; - for (let [index, change] of staged.entries()) { + let deleted = new Set(); + for (let change of staged) { for (let write of change.writes) { - claim(write.path, index); writes.set(write.path, write.content); + deleted.delete(write.path); } for (let path of change.deletes) { - claim(path, index); - deletes.push(path); + deleted.add(path); + writes.delete(path); } } + let deletes = [...deleted]; let committed = await core.commitUnlocked( { writes, deletes }, { @@ -541,6 +571,13 @@ async function commitStaged( if (!change.primaryPath) { return null; } + if (deleted.has(change.primaryPath)) { + // A later entry removed the card this one wrote. There is no state left + // to describe, which is what a removal reports, so this entry reports + // it too rather than a version for a file the commit did not leave + // behind. + return null; + } let written = byPath.get(change.primaryPath); if (!written) { // Every staged write is handed to the commit and every one comes back, @@ -569,10 +606,7 @@ async function commitStaged( // happened, and what a moved base means is the caller's to decide. ...(baseVersion === undefined ? {} - : { - baseMatched: - stored.get(change.primaryPath)?.contentHash === baseVersion, - }), + : { baseMatched: baseHashes[index] === baseVersion }), }, }; }); diff --git a/packages/runtime-common/tests/card-operations-batch-test.ts b/packages/runtime-common/tests/card-operations-batch-test.ts index f43d6894b74..93ba12417b8 100644 --- a/packages/runtime-common/tests/card-operations-batch-test.ts +++ b/packages/runtime-common/tests/card-operations-batch-test.ts @@ -740,7 +740,7 @@ const tests: SharedTests> = { assert.strictEqual(commits.length, 0, 'nothing is committed'); }, - 'two entries changing one card are refused rather than ordered': async ( + 'two entries changing one card compose, and the file is written once': async ( assert, ) => { let { core, commits } = stub({ @@ -753,15 +753,167 @@ const tests: SharedTests> = { data: { type: 'card', attributes, meta: { adoptsFrom: PERSON } }, }, }); + let results = await commitBatch( + core, + [entry({ firstName: 'Left' }), entry({ nickname: 'Lefty' })], + {}, + ); + + assert.strictEqual(commits.length, 1, 'one commit'); + assert.deepEqual( + Object.keys(commits[0].writes), + ['person-1.json'], + 'the card is written once, not twice', + ); + let { data } = JSON.parse(commits[0].writes['person-1.json']); + assert.deepEqual( + data.attributes, + { firstName: 'Left', nickname: 'Lefty' }, + 'the second entry merged over what the first staged, so neither ' + + 'change is discarded', + ); + assert.strictEqual( + results[0]!.meta.version, + results[1]!.meta.version, + 'both entries report the version the file was committed at', + ); + }, + + 'a base version names what the entry merged over, not the batch pre-state': + async (assert) => { + let stored = cardFile({ firstName: 'Original' }, PERSON); + let { core } = stub({ stored: { 'person-1.json': stored } }); + let entry = ( + attributes: Record, + baseVersion: string, + ): BatchEntry => ({ + op: 'update', + href: `${REALM}person-1`, + baseVersion, + document: { + data: { type: 'card', attributes, meta: { adoptsFrom: PERSON } }, + }, + }); + let preBatch = computeContentHash(stored); + let results = await commitBatch( + core, + [ + entry({ firstName: 'Left' }, preBatch), + entry({ nickname: 'Lefty' }, preBatch), + ], + {}, + ); + + assert.true( + results[0]!.meta.baseMatched, + 'the first entry did merge over the bytes the batch started from', + ); + assert.false( + results[1]!.meta.baseMatched, + 'the second merged over what the first staged, so the pre-batch ' + + 'version is not the base it was computed against', + ); + }, + + 'a removal after a change takes the card, and the write with it': async ( + assert, + ) => { + let { core, commits } = stub({ + stored: { 'person-1.json': cardFile({ firstName: 'Original' }, PERSON) }, + }); + let results = await commitBatch( + core, + [ + { + op: 'update', + href: `${REALM}person-1`, + document: { + data: { + type: 'card', + attributes: { firstName: 'Doomed' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + { op: 'delete', href: `${REALM}person-1` }, + ], + {}, + ); + + assert.deepEqual( + Object.keys(commits[0].writes), + [], + 'the superseded write never reaches the commit — writing bytes the ' + + 'same commit then unlinks is work with no observable result', + ); + assert.deepEqual(commits[0].deletes, ['person-1.json']); + assert.deepEqual( + results, + [null, null], + 'neither entry has state left to report', + ); + }, + + 'a change after a removal has nothing to change': async (assert) => { + let { core, commits } = stub({ + stored: { 'person-1.json': cardFile({ firstName: 'Original' }, PERSON) }, + }); let failed = await refusal(core, [ - entry({ firstName: 'Left' }), - entry({ age: 9 }), + { op: 'delete', href: `${REALM}person-1` }, + { + op: 'update', + href: `${REALM}person-1`, + document: { + data: { + type: 'card', + attributes: { firstName: 'Too late' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, ]); - assert.strictEqual(failed?.code, 'invalid-params'); - assert.strictEqual( - failed?.entry, - 1, - 'the second claim on the file is the refusal', + assert.deepEqual( + failed, + { status: 404, code: 'target-not-found', entry: 1 }, + 'the batch already removed it, which is the answer it would get for a ' + + 'card that was never there', + ); + assert.strictEqual(commits.length, 0, 'the removal is abandoned too'); + }, + + 'a card the batch creates is not a target for a later entry': async ( + assert, + ) => { + let { core, commits } = stub(); + let failed = await refusal(core, [ + { + op: 'create', + lid: 'fresh', + document: { + data: { + type: 'card', + attributes: { firstName: 'Fresh' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + { + op: 'update', + href: `${REALM}Person/fresh`, + document: { + data: { + type: 'card', + attributes: { firstName: 'Amended' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + ]); + assert.deepEqual( + failed, + { status: 404, code: 'target-not-found', entry: 1 }, + 'a minted card is reached by the local id other entries link to, not ' + + 'by a URL they target', ); assert.strictEqual(commits.length, 0, 'nothing is committed'); }, From a508d16fdc2f0fe80127273898f4097bed553ada Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 21:49:42 +0000 Subject: [PATCH 13/14] Refuse the conflicts composition opened, and stop claiming realm.json is safe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Composing two changes to one card opened three ways for a batch to lose data quietly, and the removal guard shipped alongside it was documented as protection it does not give. - A write to a path an earlier entry removed cancelled that removal, and a removal reports the same `null` whether it happened or not — so a card could be resurrected while the entry that asked to remove it reported success. Refused: the two entries are asking for opposite things, which is not a sequence to compose. - A side-loaded resource is serialized whole rather than merged, so it cannot compose over an earlier entry's change the way a second patch does. Landing it last dropped that change with nothing said while its entry still reported success and a matching base version. Refused, naming both entries. - A removal took a card under an ignored path. Ignored files are never visited, so they never get an index row, and `DELETE` — which needs one — refuses them forever; reading the bytes off disk is not the same permission. The batch would have destroyed a file no other caller can, and the realm would go on ignoring the absence. `realm.json` is not protected by the card-document check and was never going to be: a realm's config is itself a card document, and `DELETE` removes it too, so refusing it here would put the batch out of step with the endpoint rather than protect anything. The comment saying otherwise is corrected, and the test that appeared to prove it stored a bare settings object no realm writes — it now stores the config the way a realm does and pins the parity. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EBB6JdRo5XCY5WhUkgBckS --- .../tests/card-operations-batch-test.ts | 16 ++ .../card-operations/coordinator.ts | 72 +++++++- .../card-operations/executors.ts | 19 ++- packages/runtime-common/realm.ts | 1 + .../tests/card-operations-batch-test.ts | 159 +++++++++++++++++- 5 files changed, 252 insertions(+), 15 deletions(-) diff --git a/packages/realm-server/tests/card-operations-batch-test.ts b/packages/realm-server/tests/card-operations-batch-test.ts index 80c95e02cff..c878c188f8c 100644 --- a/packages/realm-server/tests/card-operations-batch-test.ts +++ b/packages/realm-server/tests/card-operations-batch-test.ts @@ -50,6 +50,10 @@ module(basename(import.meta.filename), function () { await runSharedTest(cardOperationsBatchTests, assert, {}); }); + test('a realm config is a card, and a removal treats it as one', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); + test('a write into the capture subtree is refused', async function (assert) { await runSharedTest(cardOperationsBatchTests, assert, {}); }); @@ -90,6 +94,18 @@ module(basename(import.meta.filename), function () { await runSharedTest(cardOperationsBatchTests, assert, {}); }); + test('a card the realm ignores has no removal to make', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); + + test('a side-load cannot land on a card another entry is changing', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); + + test('a batch cannot both remove a card and write it', async function (assert) { + await runSharedTest(cardOperationsBatchTests, assert, {}); + }); + test('a card the batch creates is not a target for a later entry', async function (assert) { await runSharedTest(cardOperationsBatchTests, assert, {}); }); diff --git a/packages/runtime-common/card-operations/coordinator.ts b/packages/runtime-common/card-operations/coordinator.ts index e9e899f9425..e26c9386430 100644 --- a/packages/runtime-common/card-operations/coordinator.ts +++ b/packages/runtime-common/card-operations/coordinator.ts @@ -86,6 +86,9 @@ export interface BatchCore { // may still be indexing, so a batch drains before it stages rather than // failing to resolve a type the realm already holds. drainIndexing(): Promise; + // Whether the realm's ignore rules exclude this URL. An ignored file is + // never visited by indexing, so it never gets an index row. + isIgnored(url: URL): Promise; // The realm's unlocked commit: writes and removals under one index job and // one index event. Assumes the write lock is held, which it is. @@ -184,6 +187,7 @@ export async function commitBatch( } assertWritesAllowed(staged); assertWritesFit(core, staged); + await assertRemovalsAllowed(core, paths, staged); await assertDestinationsFree(core, staged); // Everything above either produced bytes for every entry or threw, and a // throw leaves the realm as it was. @@ -465,6 +469,36 @@ function assertWritesAllowed(staged: StagedChange[]): void { } } +// A removal needs the realm to be willing to serve the card back. `DELETE` +// requires an index row, and an ignored path never gets one — it is never +// visited, so no amount of waiting produces it — which makes a card under an +// ignored path permanently un-deletable over HTTP. Reading its bytes off disk +// is not the same permission: a batch that removed one would be destroying a +// file no other caller can, and the realm would go on ignoring the absence. +async function assertRemovalsAllowed( + core: BatchCore, + paths: RealmPaths, + staged: StagedChange[], +): Promise { + for (let [index, change] of staged.entries()) { + for (let path of change.deletes) { + if (await core.isIgnored(paths.fileURL(path))) { + throw atEntry( + new OperationFailure({ + status: 404, + code: 'target-not-found', + title: 'Not found', + detail: + `${paths.fileURL(path).href} is under a path the realm ` + + `ignores, so it holds no card to remove`, + }), + index, + ); + } + } + } +} + // The realm refuses bytes over its size ceiling, and it refuses them one file // at a time as it writes. Reaching that inside the commit would leave the // files written before it on disk and unindexed — the commit rejects before @@ -548,10 +582,44 @@ async function commitStaged( // entirely instead of writing bytes the same commit then unlinks. let writes = new Map(); let deleted = new Set(); - for (let change of staged) { + let writtenBy = new Map(); + for (let [index, change] of staged.entries()) { for (let write of change.writes) { + if (deleted.has(write.path)) { + // The batch already removed this card, so writing it back is not a + // later step in one story — it is two entries asking for opposite + // things. Un-queueing the removal instead would report that entry as + // a completed removal, which is indistinguishable from a real one. + throw new OperationFailure({ + status: 400, + code: 'invalid-params', + title: 'Conflicting entries', + detail: + `entry ${index} writes ${write.path}, which an earlier entry ` + + `removes; a batch cannot both remove a card and write it`, + meta: { entry: index }, + }); + } + let owner = writtenBy.get(write.path); + if (owner !== undefined && write.path !== change.primaryPath) { + // Two entries changing one card compose because the later one merges + // over what the earlier staged. A side-loaded resource is serialized + // whole rather than merged, so it cannot compose over anything — + // letting it land last would drop the earlier entry's change with + // nothing said, while that entry still reported success. + throw new OperationFailure({ + status: 400, + code: 'invalid-params', + title: 'Conflicting entries', + detail: + `entries ${owner} and ${index} both write ${write.path}, and ` + + `entry ${index} carries it as a side-load, which replaces the ` + + `card rather than merging over it`, + meta: { entry: index, conflictsWith: owner }, + }); + } writes.set(write.path, write.content); - deleted.delete(write.path); + writtenBy.set(write.path, index); } for (let path of change.deletes) { deleted.add(path); diff --git a/packages/runtime-common/card-operations/executors.ts b/packages/runtime-common/card-operations/executors.ts index 4ef2468c220..a9d20800b55 100644 --- a/packages/runtime-common/card-operations/executors.ts +++ b/packages/runtime-common/card-operations/executors.ts @@ -509,14 +509,17 @@ export function stageDelete( detail: `${url.href} does not exist in realm ${ctx.realmURL}`, }); } - // A `.json` file on disk is not by itself a card. `realm.json` is the - // clearest case — it sits at a URL a delete can name, and removing it would - // take the realm's own configuration with it — but any stored JSON that is - // not a card document is one. The bytes already read answer this, so the - // check costs no read and keeps the just-written card above deletable, - // which asking the index would not. This is the answer `DELETE` gives for - // the same URL, where it is the index rather than the bytes that reports no - // card there. + // A `.json` file on disk is not by itself a card — a hand-written config, + // a fixture, anything the realm stores but does not serve as one. The bytes + // already read answer this, so the check costs no read and keeps the + // just-written card above deletable, which asking the index would not. This + // is the answer `DELETE` gives for the same URL, where it is the index + // rather than the bytes that reports no card there. + // + // `realm.json` is not covered by this and is not meant to be: a realm's + // config is itself a card document, and `DELETE` removes it too. Refusing + // it here would put the batch out of step with the endpoint rather than + // protecting anything the endpoint protects. if (!cardResourceIn(stored.content)) { throw new OperationFailure({ id: url.href, diff --git a/packages/runtime-common/realm.ts b/packages/runtime-common/realm.ts index 94ea1acc99b..cfbd610e101 100644 --- a/packages/runtime-common/realm.ts +++ b/packages/runtime-common/realm.ts @@ -3344,6 +3344,7 @@ export class Realm { drainIndexing: async () => { await this.incrementalIndexing(); }, + isIgnored: (url) => this.isIgnored(url), commitUnlocked: (batch, options) => this._commitBatchUnlocked(batch, options), serializeCard: (doc, relativeTo) => diff --git a/packages/runtime-common/tests/card-operations-batch-test.ts b/packages/runtime-common/tests/card-operations-batch-test.ts index 93ba12417b8..efa287f0c3a 100644 --- a/packages/runtime-common/tests/card-operations-batch-test.ts +++ b/packages/runtime-common/tests/card-operations-batch-test.ts @@ -52,6 +52,9 @@ interface StubOptions { stored?: Record; // The realm's size ceiling, for the paths a batch stages. sizeLimit?: number; + // Local paths the realm's ignore rules exclude. Such a file is never + // visited by indexing, so it never has an index row to remove. + ignored?: string[]; // The definition-cache entry a code ref resolves to, keyed by its name. definitions?: Record; // What `serializeCard` does to a resource on its way to storage. The default @@ -113,6 +116,9 @@ function stub(opts: StubOptions = {}): Stub { async drainIndexing() { drains++; }, + async isIgnored(url) { + return (opts.ignored ?? []).some((p) => url.href === `${REALM}${p}`); + }, async commitUnlocked(batch, options) { readsOutsideLock += held > 0 ? 0 : 1; let writes = Object.fromEntries( @@ -583,16 +589,13 @@ const tests: SharedTests> = { }, 'a removal takes a card, not any stored json': async (assert) => { - // The realm's own configuration is the sharpest case: it is stored as - // `realm.json`, so it answers to a URL a removal can name, and removing - // it would take the realm's settings with it. let { core, commits } = stub({ stored: { - 'realm.json': JSON.stringify({ name: 'Test Realm' }, null, 2), 'notes.json': JSON.stringify({ note: 'not a card' }, null, 2), + 'empty.json': JSON.stringify({ data: null }, null, 2), }, }); - for (let href of [`${REALM}realm`, `${REALM}notes`]) { + for (let href of [`${REALM}notes`, `${REALM}empty`]) { let failed = await refusal(core, [{ op: 'delete', href }]); assert.deepEqual( failed, @@ -603,6 +606,36 @@ const tests: SharedTests> = { assert.strictEqual(commits.length, 0, 'neither file is removed'); }, + 'a realm config is a card, and a removal treats it as one': async ( + assert, + ) => { + // Stored the way a realm actually stores it, rather than as a bare + // settings object: a realm's config IS a card document. So the guard + // above does not exempt it, and this batch removes it — which is what + // `DELETE` does with the same URL. Pinned so that reading the guard as + // protection for the realm's own configuration fails here rather than in + // a realm. + let { core, commits } = stub({ + stored: { + 'realm.json': cardFile( + { cardInfo: { name: 'Test Realm' } }, + { module: `${REALM}realm-config`, name: 'RealmConfig' }, + ), + }, + }); + let results = await commitBatch( + core, + [{ op: 'delete', href: `${REALM}realm` }], + {}, + ); + assert.deepEqual(results, [null], 'the removal reports as any does'); + assert.deepEqual( + commits[0].deletes, + ['realm.json'], + 'the config is removed, at parity with the endpoint', + ); + }, + 'a write into the capture subtree is refused': async (assert) => { let { core, commits } = stub({ definitions: { Person: personDefinition() }, @@ -881,6 +914,122 @@ const tests: SharedTests> = { assert.strictEqual(commits.length, 0, 'the removal is abandoned too'); }, + 'a card the realm ignores has no removal to make': async (assert) => { + // An ignored file is never visited, so it never gets an index row, and + // `DELETE` — which needs one — refuses it forever. Reading the bytes off + // disk is not the same permission: removing it here would destroy a file + // no other caller can, and the realm would go on ignoring its absence. + let { core, commits } = stub({ + stored: { + 'hidden/Person/secret.json': cardFile({ firstName: 'Secret' }, PERSON), + }, + ignored: ['hidden/Person/secret.json'], + }); + let failed = await refusal(core, [ + { op: 'delete', href: `${REALM}hidden/Person/secret` }, + ]); + assert.deepEqual(failed, { + status: 404, + code: 'target-not-found', + entry: 0, + }); + assert.strictEqual(commits.length, 0, 'the file stays on disk'); + }, + + 'a side-load cannot land on a card another entry is changing': async ( + assert, + ) => { + // A side-loaded resource is serialized whole rather than merged, so it + // cannot compose over an earlier entry's change the way a second patch + // does — landing it last would drop that change silently while its entry + // still reported success. + let { core, commits } = stub({ + stored: { + 'Person/a.json': cardFile( + { firstName: 'Original', keepMe: 'yes' }, + PERSON, + ), + 'b.json': cardFile({ firstName: 'B' }, PERSON), + }, + }); + let failed = await refusal(core, [ + { + op: 'update', + href: `${REALM}Person/a`, + document: { + data: { + type: 'card', + attributes: { firstName: 'Patched' }, + meta: { adoptsFrom: PERSON }, + }, + }, + }, + { + op: 'update', + href: `${REALM}b`, + document: { + data: { + type: 'card', + attributes: { firstName: 'B' }, + meta: { adoptsFrom: PERSON }, + }, + included: [ + { + type: 'card', + lid: 'a', + attributes: { firstName: 'Clobbered' }, + meta: { adoptsFrom: PERSON }, + } as any, + ], + }, + }, + ]); + assert.strictEqual(failed?.code, 'invalid-params'); + assert.strictEqual(failed?.entry, 1, 'the side-load is the refusal'); + assert.strictEqual(commits.length, 0, 'nothing is committed'); + }, + + 'a batch cannot both remove a card and write it': async (assert) => { + // Un-queueing the removal so the write can land would report the removal + // entry as a completed one, which a caller cannot tell from a real + // removal — the two entries are asking for opposite things. + let { core, commits } = stub({ + stored: { + 'Person/a.json': cardFile({ firstName: 'Original' }, PERSON), + 'b.json': cardFile({ firstName: 'B' }, PERSON), + }, + }); + let failed = await refusal(core, [ + { op: 'delete', href: `${REALM}Person/a` }, + { + op: 'update', + href: `${REALM}b`, + document: { + data: { + type: 'card', + attributes: { firstName: 'B' }, + meta: { adoptsFrom: PERSON }, + }, + included: [ + { + type: 'card', + lid: 'a', + attributes: { firstName: 'Resurrected' }, + meta: { adoptsFrom: PERSON }, + } as any, + ], + }, + }, + ]); + assert.strictEqual(failed?.code, 'invalid-params'); + assert.strictEqual(failed?.entry, 1); + assert.strictEqual( + commits.length, + 0, + 'the removal is not quietly cancelled', + ); + }, + 'a card the batch creates is not a target for a later entry': async ( assert, ) => { From 9de3b7aea5eddc4faaa8cb039ee49453994921f1 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 14:31:31 +0000 Subject: [PATCH 14/14] Spy on the enqueue every incremental job actually goes through MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The write-route tagging test shadows the updater to capture each job's `initiatedBy`, and it shadowed `enqueueUpdate` on the reasoning — its own comment's — that the awaited path calls through it, so one spy covered both. That stopped being true when the awaited path became `updateChanges`, which enqueues a change set directly: the spy sat on a method no write reaches any more, recorded nothing, and reported every route as untagged. The tagging itself was never broken. `initiatedBy` travels from the request context through `updateIndexAndCollectInvalidations` to the deferred the drain reads, and the suite's other cases — which exercise that drain rather than the enqueue — passed throughout. Shadowing `enqueueChanges` restores the coverage and widens it: every incremental job is enqueued there, `enqueueUpdate` and both awaited forms delegate to it, so one spy now covers every route whichever form it entered by. Verified to still fail when the tag is dropped at the shared call site, rather than merely to pass. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EBB6JdRo5XCY5WhUkgBckS --- .../realm-server/tests/read-index-drain-test.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/realm-server/tests/read-index-drain-test.ts b/packages/realm-server/tests/read-index-drain-test.ts index 35c84565e52..52c5b90bda7 100644 --- a/packages/realm-server/tests/read-index-drain-test.ts +++ b/packages/realm-server/tests/read-index-drain-test.ts @@ -303,9 +303,10 @@ module(basename(import.meta.filename), function () { // tag for every HTTP write route: a handler that drops its // `initiatingUser` argument silently disables read-your-writes for that // route, which no black-box read test can catch deterministically. The - // spy shadows `enqueueUpdate` (the synchronous `update` path calls - // through it, so one spy covers both), delegates to the prototype, and - // captures each job's `initiatedBy`. + // spy shadows `enqueueChanges`, which is where every incremental job is + // enqueued — `enqueueUpdate` and both awaited forms delegate to it — so + // one spy covers every route regardless of which of them it entered by. + // It delegates to the prototype and captures each job's `initiatedBy`. test('every HTTP write route tags its indexing job with the writer', async function (assert) { let auth = () => `Bearer ${createJWT(testRealm, 'hassan', ['read', 'write'])}`; @@ -313,9 +314,9 @@ module(basename(import.meta.filename), function () { let proto = Object.getPrototypeOf(updater); let tags = new Map(); let currentOp = 'none'; - updater.enqueueUpdate = function (urls: URL[], opts?: any) { + updater.enqueueChanges = function (changes: unknown[], opts?: any) { tags.set(currentOp, opts?.initiatedBy); - return proto.enqueueUpdate.call(this, urls, opts); + return proto.enqueueChanges.call(this, changes, opts); }; let drainJobs = async () => { let pending = testRealm.incrementalIndexing(); @@ -467,7 +468,7 @@ module(basename(import.meta.filename), function () { } } finally { await drainJobs(); - delete updater.enqueueUpdate; + delete updater.enqueueChanges; } }); });