From a3298f3af0a6ac65f7de791c98152e1e0a8dae5a Mon Sep 17 00:00:00 2001 From: Richard Tan Date: Thu, 2 Jul 2026 13:00:37 +0800 Subject: [PATCH 1/2] fix: close race window in StoreService.patch() between snapshot and write-back patch() snapshotted the instance's full document before resolving linked relationships (a real network gap), then reapplied that snapshot wholesale afterward. Any field written on the same live instance by other code during that gap was silently reverted when the stale snapshot landed. Resolving linked relationships first, then snapshotting immediately before the write-back, closes the window. Co-Authored-By: Claude Sonnet 5 --- packages/host/app/services/store.ts | 39 ++++++++------- .../host/tests/integration/store-test.gts | 50 +++++++++++++++++++ 2 files changed, 72 insertions(+), 17 deletions(-) diff --git a/packages/host/app/services/store.ts b/packages/host/app/services/store.ts index 20357df552..50519821ee 100644 --- a/packages/host/app/services/store.ts +++ b/packages/host/app/services/store.ts @@ -965,6 +965,28 @@ export default class StoreService extends Service implements StoreInterface { if (opts?.doNotPersist) { await this.stopAutoSaving(instance); } + // Resolve any linked-card relationships first. This can require a + // network fetch, and a sibling task elsewhere may mutate other fields on + // this same live instance while we wait. Snapshotting the instance below + // (for the merge + write-back further down) only after this resolves + // keeps that snapshot from going stale and reverting the sibling's write. + let linkedCards = await this.loadPatchedInstances(patch, instance.id); + for (let [field, value] of Object.entries(linkedCards)) { + if (field.includes('.')) { + let parts = field.split('.'); + let leaf = parts.pop(); + if (!leaf) { + throw new Error(`bug: error in field name "${field}"`); + } + let inner = instance; + for (let part of parts) { + inner = (inner as any)[part]; + } + (inner as any)[leaf.match(/^\d+$/) ? Number(leaf) : leaf] = value; + } else { + (instance as any)[field] = value; + } + } let doc = await this.cardService.serializeCard(instance, { omitQueryFields: true, }); @@ -988,23 +1010,6 @@ export default class StoreService extends Service implements StoreInterface { if (patch.meta) { doc.data.meta = merge(doc.data.meta, patch.meta); } - let linkedCards = await this.loadPatchedInstances(patch, instance.id); - for (let [field, value] of Object.entries(linkedCards)) { - if (field.includes('.')) { - let parts = field.split('.'); - let leaf = parts.pop(); - if (!leaf) { - throw new Error(`bug: error in field name "${field}"`); - } - let inner = instance; - for (let part of parts) { - inner = (inner as any)[part]; - } - (inner as any)[leaf.match(/^\d+$/) ? Number(leaf) : leaf] = value; - } else { - (instance as any)[field] = value; - } - } let api = await this.cardService.getAPI(); await api.updateFromSerialized(instance, doc, this.store); let shouldPersist = !opts?.doNotPersist; diff --git a/packages/host/tests/integration/store-test.gts b/packages/host/tests/integration/store-test.gts index e5fd238da1..cd0d5700c3 100644 --- a/packages/host/tests/integration/store-test.gts +++ b/packages/host/tests/integration/store-test.gts @@ -1593,6 +1593,56 @@ module('Integration | Store', function (hooks) { ); }); + test('a concurrent field write during store.patch is not clobbered by the patch’s stale snapshot', async function (assert) { + let targetId = `${testRealmURL}Person/hassan`; + let instance = (await storeService.get(targetId)) as any; + + let gate = new Deferred(); + let originalLoadPatchedInstances = ( + storeService as any + ).loadPatchedInstances.bind(storeService); + (storeService as any).loadPatchedInstances = async (...args: any[]) => { + let result = await originalLoadPatchedInstances(...args); + // simulate a slow relationship load (e.g. fetching a not-yet-cached + // linked card) so a concurrent field write on the same live instance + // can land in the window between the patch's initial snapshot and its + // eventual write-back + await gate.promise; + return result; + }; + + try { + let patchPromise = storeService.patch(targetId, { + relationships: { + bestFriend: { + links: { self: `${testRealmURL}Person/jade` }, + }, + }, + }); + + // a sibling background task (e.g. an unrelated auto-linking step, as in + // the catalog listing-create flow) mutates a different field on the + // same live instance while the patch above is still in flight + instance.name = 'Race Winner'; + + gate.fulfill(); + await patchPromise; + } finally { + (storeService as any).loadPatchedInstances = originalLoadPatchedInstances; + } + + assert.strictEqual( + instance.name, + 'Race Winner', + "the concurrent field write is not clobbered by the patch's stale snapshot", + ); + assert.strictEqual( + instance.bestFriend?.id, + `${testRealmURL}Person/jade`, + 'the patch itself was still applied', + ); + }); + test('loads FileDef links from included resources', async function (assert) { await testRealm.writeMany( new Map([ From 96f3e98e243b4c4851bcaac0dc12b9c5a49ecf1e Mon Sep 17 00:00:00 2001 From: Richard Tan Date: Thu, 2 Jul 2026 14:44:44 +0800 Subject: [PATCH 2/2] fix: gate the concurrent write on entering the stub, not on calling patch() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The write ran synchronously right after invoking patch(), before the function had actually reached the relationship-load step it awaits on — patch() yields at its own initial instance lookup first. That let the write land before either the old or new snapshot point, so the test could pass without the fix under test. Wait for a signal fired from inside the stub once relationship loading is genuinely paused, then write. Also restore the literal original loadPatchedInstances reference in the finally block instead of a rebound copy of it. Co-Authored-By: Claude Sonnet 5 --- .../host/tests/integration/store-test.gts | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/packages/host/tests/integration/store-test.gts b/packages/host/tests/integration/store-test.gts index cd0d5700c3..a28e29282f 100644 --- a/packages/host/tests/integration/store-test.gts +++ b/packages/host/tests/integration/store-test.gts @@ -1598,15 +1598,19 @@ module('Integration | Store', function (hooks) { let instance = (await storeService.get(targetId)) as any; let gate = new Deferred(); - let originalLoadPatchedInstances = ( - storeService as any - ).loadPatchedInstances.bind(storeService); - (storeService as any).loadPatchedInstances = async (...args: any[]) => { - let result = await originalLoadPatchedInstances(...args); + let enteredGate = new Deferred(); + let originalLoadPatchedInstances = (storeService as any) + .loadPatchedInstances; + (storeService as any).loadPatchedInstances = async function ( + this: unknown, + ...args: any[] + ) { + let result = await originalLoadPatchedInstances.apply(this, args); // simulate a slow relationship load (e.g. fetching a not-yet-cached // linked card) so a concurrent field write on the same live instance - // can land in the window between the patch's initial snapshot and its + // can land in the window between the patch's snapshot and its // eventual write-back + enteredGate.fulfill(); await gate.promise; return result; }; @@ -1620,6 +1624,11 @@ module('Integration | Store', function (hooks) { }, }); + // wait until the patch has actually reached the gated relationship + // load before mutating a sibling field, so the write lands squarely in + // the window the patch's snapshot needs to still be sensitive to + await enteredGate.promise; + // a sibling background task (e.g. an unrelated auto-linking step, as in // the catalog listing-create flow) mutates a different field on the // same live instance while the patch above is still in flight