Wait on the held save itself for optimistic persistence

[habdelra]

Integration | Command | patch-fields > Optimistic persistence behavior: patches do not await persistence fails intermittently in host CI with Error: waitUntil timed out.

What the test was waiting on

The wait was waitUntil(() => saves > 0) at @ember/test-helpers' 1000ms default. store.patch is called with doNotWaitForPersist, so the persist runs in the background: a request through the virtual network to the in-browser realm, the realm's write, its index update, and the store folding the result back in. None of that is bounded by anything the test controls, and on a loaded runner it routinely exceeds a second — so the poll expired while the save was still legitimately in flight. On the shard that failed, the runner sat at 90–100% CPU across 4 cores.

The optimistic window the test checks was riding on luck too. withSlowSave restores the original persistAndUpdate at its callback's first await — in an async function try { return cb() } finally {} runs the finally before the returned promise settles — and store.patch reaches the persist well after that. The artificial 100ms delay never covered the save, so assert.strictEqual(saves, 0, 'no persistence yet') was really asserting that the save is slower than a microtask.

The change

withHeldSave holds every save its callback triggers until the callback returns, then releases them and waits for them to land. The window where a mutation is applied locally and no save can have reached the realm is guaranteed rather than raced, and the persisted result is ready the moment the helper resolves — so the test has no wall clock in it at all. The store's save state rides along in the assertion message, so a save that fails rather than merely running long names its own cause.

withSlowSave is left as it is. Its remaining callers observe the document a save serializes at the moment that save is initiated; holding entry into persistAndUpdate changes what they see. Running them with the hold made real turned Integration | Store: an instance can debounce auto saves red — the leading-edge save then writes the final value instead of the intermediate one. That is a question about what those tests should assert, separate from this failure.

Two tests, because one of them cannot report its own regression

withHeldSave holds the save a non-optimistic patch would await, so if the patch ever stops handing the save to the background the two wait on each other and the suite reports Test took longer than 60000ms; test timed out — true, but naming nothing about persistence. Bounding the hold with a timer would put back the wall clock that made this flaky in the first place, so the handoff is asserted separately, against a save that runs normally.

Verified by building with doNotWaitForPersist removed from the tool and running each against it:

Test	Duration	Message
patches do not await persistence	63338 ms	Test took longer than 60000ms; test timed out
the patch hands the save to the background	6422 ms	store.patch receives doNotWaitForPersist option

Both go red, so neither passes for free; the second one says why.

Verification

The timing pair on CI is the direct evidence that the deadline, not the save, was the discriminator:

Run	Duration	Outcome
Reported failure, 1000ms waitUntil	9565 ms	waitUntil timed out
This branch, no deadline	8838 ms	pass

Near-identical slowness, opposite outcomes. Raising the timeout would have bought room until the next slow shard; removing it makes the test indifferent to runner load.

• Shard 11 on this PR — where the reported failure occurred — is green, with all 44 patch-fields tests present and the shard at 290 tests / 0 fail.
• Locally, the patch-fields module passes 44/44 across two full runs.
• Reproduction control, one build carrying both shapes with an identical 1.5s delay injected into the store's persist:
  • withHeldSave — ok
  • the shape on main (withSlowSave + bare waitUntil) — not ok, Error: waitUntil timed out, the same signature as the reported job.
• The other five withSlowSave call sites (store-test ×2, operator-mode, code-submode editor, patch-instance) pass unchanged.
• eslint and ember-tsc --noEmit clean for packages/host.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-15T22:51:25.411205Z	d8175ca	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

Preview deployments

• Host staging preview

• Host production preview

Host Test Results

    1 files      1 suites   2h 16m 30s ⏱️
4 835 tests 4 821 ✅ 14 💤 0 ❌
4 850 runs  4 836 ✅ 14 💤 0 ❌

Results for commit ddda1d8.

Realm Server Test Results

    1 files    226 suites   1h 11m 41s ⏱️
3 109 tests 3 109 ✅ 0 💤 0 ❌
3 155 runs  3 155 ✅ 0 💤 0 ❌

Results for commit ddda1d8.