Await durable persistence for existing cards in StoreService.add()

[lukemelia]

Problem

For an existing card, StoreService.add() queued a fire-and-forget autosave and returned immediately, so await add() — and callers like SaveCardCommand — could resolve before serialization and the realm PATCH completed. A caller could report "saved," commit its own transaction, and finish even though no durable revision existed yet. Late persistence errors were stored in autosave state where they could never reach the caller. Only the new-card branch of add() awaited persistAndUpdate().

Fix

In the default (non-optimistic) path, add() now awaits persistAndUpdate() for both new and existing cards, collapsing the two sub-branches. This mirrors what StoreService.patch() already does. As a result:

• A successful result means the realm accepted the serialized mutation.
• Persistence failures return a card error, so SaveCardCommand throws instead of silently reporting success.
• Optimistic behavior remains available but must be requested explicitly via doNotWaitForPersist: true (that branch is unchanged).

Only existing-card add() calls that passed neither doNotPersist nor doNotWaitForPersist change behavior; new-card, render-context, and optimistic callers are untouched.

Tests

Three regression tests in store-test.gts, each written to fail against the old fire-and-forget path:

1. awaits durable persistence — compound scalar + link mutation; reads the backing JSON immediately after add() with no waitUntil.
2. stays pending until persisted — gates persistAndUpdate with a Deferred and asserts add() does not resolve while the gate is closed.
3. surfaces persistence failures — forces the save request to throw and asserts add() returns a card error rather than swallowing it.

Verification

• ember-tsc --noEmit: clean
• eslint / ember-template-lint: clean

The full host QUnit suite was not run locally (needs the realm/matrix server stack).

Fixes CS-12409.

🤖 Generated with Claude Code

[github-actions]

Preview deployments

• Host staging preview

• Host production preview

Host Test Results

    1 files      1 suites   2h 35m 3s ⏱️
3 831 tests 3 817 ✅ 14 💤 0 ❌
3 850 runs  3 836 ✅ 14 💤 0 ❌

Results for commit e26fae9.

Realm Server Test Results

    1 files      1 suites   13m 6s ⏱️
2 063 tests 2 063 ✅ 0 💤 0 ❌
2 142 runs  2 142 ✅ 0 💤 0 ❌

Results for commit e26fae9.

[Copilot]

🟡 Not ready to approve

add() now bypasses the autosave queue for existing cards, which can leave AutoSaveState (e.g., hasUnsavedChanges / lastSaved / lastSaveError) out of sync with the completed save, changing observable UI state compared to the prior save(id) path.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This PR changes StoreService.add() so that, when persistence is enabled and optimistic mode is not requested, add() only resolves after the card mutation has been durably persisted via persistAndUpdate()—including for existing cards. This aligns add()’s resolution semantics with callers that treat await add() as “saved”.

Changes:

• Update StoreService.add() to await persistAndUpdate() for existing cards (non-optimistic path).
• Add integration regression tests ensuring add() (1) waits for persistence, (2) stays pending while persistence is gated, and (3) returns a card error on persistence failure.

File summaries

File	Description
packages/host/app/services/store.ts	Makes add() await durable persistence in the default path (existing + new cards).
packages/host/tests/integration/store-test.gts	Adds regression tests that fail if add() resolves before persistence or swallows persistence errors.

Review details

Suppressed comments (2)

packages/host/tests/integration/store-test.gts:1155

• [Claude Code 🤖] Similar to the persistAndUpdate gate above, storing a bound saveCardDocument and restoring the bound version in finally can subtly change behavior for subsequent tests. Capturing and restoring the original method reference avoids altering the service under test.

    let store = storeService as any;
    let originalSaveCardDocument = store.saveCardDocument.bind(store);
    store.saveCardDocument = async () => {
      throw new Error('intentional persistence failure');
    };

packages/host/tests/integration/store-test.gts:1082

• [Claude Code 🤖] This comment uses temporal phrasing (“reported bug”) and asserts intent (“proves”) rather than stating the contract being tested. It’s clearer and more evergreen to say the test reads the durable backing JSON immediately to ensure add() only resolves after persistence is complete.

    // No waitUntil here: if add() resolved before persistence completed (the
    // reported bug), the durable resource would still hold the pre-mutation
    // state. Reading the backing JSON immediately proves add() waited.

• Files reviewed: 2/2 changed files
• Comments generated: 3
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.