Skip the link-closure readback on card writes

[lukemelia]

Problem

The card POST/PATCH handlers read the just-written card back out of the index with cardDocument({ loadLinks: true }). loadLinks walks the card's full transitive link closure into included[] and expands its query-backed fields (which issue further searches of their own) — the same closure walk that dominates read-handler time.

The write response is then consumed only for the primary card's assigned id and realm-info. The host client deletes data.attributes and data.relationships and never touches included[] (store.ts persistAndUpdate), and no other writer reads the closure off a write response. So the entire assembly is thrown away — and on the two PATCH readbacks it is assembled inside the realm-wide write lock, lengthening the critical section every other writer on the realm is serialized behind.

Fix

Drop loadLinks from the three write-path readbacks (POST create, PATCH no-op short-circuit, PATCH write). The primary card's own indexed row — id, attributes, relationships, realm-info, screenshots, lastModified — is returned exactly as before; only the transitive included[] closure and query-field expansion are skipped. The read (GET) path is untouched and still assembles the full closure.

skipQueryBackedExpansion is dropped along with loadLinks because it only takes effect inside the loadLinks pass. (The ticket's suggestion of omitIncluded would have been inert here — that flag is honoured by the search handler, not by cardDocument, whose only closure lever is loadLinks.)

Safety

• Client contract — needsServerStateMerge compares only data.id and data.meta.realmInfo; updateFromSerialized runs against a server doc with attributes/relationships deleted; included[] is never read.
• Side-loaded children round-trip — a create that co-creates unsaved children (lid-bearing included[] in the request) still works: each child's remote id is derived from the lid the client chose ({realm}/{cardDir}/{lid}), not learned from the response. The existing round-trip test verifies children via the filesystem and a follow-up GET, never the write response's included[].
• No non-host writer reads included[]/data.relationships off a write response (checked boxel-cli and the batch executors — they read the inbound request or search responses only).
• ETags — the PATCH echoes no longer emit a validator at all. The echo now omits the closure a GET assembles, so sharing the GET's ETag (which is unchanged: indexedAt + realm-info hash + screenshots fingerprint) would let a client cache the echo body and be 304'd onto that closure-less representation on a later conditional GET. The client discards the echo body anyway, so there is no validator to lose. The GET path's ETag and its 304 behaviour are untouched.

Test

Adds a test asserting the create response carries id + realm-info + lastModified but not included[], while a GET of the same card still inlines the linked card into included[] — pinning the read-vs-write contract.

Verification status

• Typecheck (runtime-common + realm-server): clean
• ESLint: clean
• Static impact analysis: every existing included / links.search assertion in the write-endpoint tests traces to a GET, not a write response — nothing else needed updating
• Realm-server suite not yet executed locally (the node tests need the dev stack rebuilt against this checkout); will run before marking ready.

Follow-up

The ticket also asks for write-path stage timing (a realm:search-timing equivalent for cardDocument on writes) so the win is measurable post-deploy. That's a separate, additive change — proposed as a stacked follow-up rather than bundled here.

🤖 Generated with Claude Code

[lukemelia]

[Claude Code 🤖] Reviewed for what dropping loadLinks from the three write readbacks could break: any consumer that reads included[] or data.relationships off a write response (host persistAndUpdate → needsServerStateMerge / updateFromSerialized / onSaveSubscriber, the batch executors, boxel-cli), whether the existing write-endpoint tests assert included on a write response rather than on a follow-up GET, and whether the write echo's ETag still describes its body once that body diverges from a GET's.

No blocking issues — dropping the closure is safe. The host merges only data.id + data.meta.realmInfo and strips attributes/relationships before updateFromSerialized, so included[] is unreachable there; the batch executors and boxel-cli read the inbound request / search responses, never the write echo; and every included assertion in the write-endpoint tests anchors on a follow-up GET, not the POST/PATCH response. The new test genuinely pins the contract — pre-fix the create response inlines the linked Friend into included[], so strictEqual(json.included, undefined) would not hold.

1. (non-blocking) The write echo now carries the full-closure GET ETag on a body that no longer has the closure. Both PATCH branches still build the validator with buildCardJsonEtag(entry.indexedAt, realmInfoHash, screenshotsFingerprint) (short-circuit branch and the main write readback in realm.ts) — resolveLinksOnly defaults to false, so the echo emits the exact validator a full GET of this card produces at the same indexed_at, while the echo body no longer has included[]. The -links-only variant exists precisely to keep same-generation / different-shape representations under distinct validators; the echo is now a third shape with no variant of its own, which cuts against the "an etagBase must be a TOTAL identity of the body" note in this file. I could not find a reachable failure — PATCH/POST responses aren't stored by HTTP caches, the response document cache is populated only on the GET path, and the host discards the write body without ever revalidating a later GET against its ETag — so this looks moot in practice. If that is the whole of the safety, a one-line note (or simply not emitting the ETag on the echo, which the client discards anyway) would pin it; if there is a revalidation path I've missed, it is a stale-shape 304.

Relatedly, the short-circuit branch's comment — "The PATCH echo is the same served representation as a GET … and the same validator components" — is now false in the included dimension: this PR deliberately makes the echo not the same representation as a GET. Worth narrowing so a future reader doesn't rebuild the closure coupling on that premise.

CI: all checks are pending on the fresh push; nothing red. The realm-server node suite is the one to watch — the PR notes it has not been run locally.

[lukemelia]

[Claude Code 🤖] Addressed recommendation 1 (write-echo ETag) in 5471bcff27: both PATCH echoes now emit no validator, and the realm-info-hash refresh + foreign-deps suppression signal that only fed it are gone. The GET path's ETag and 304 behaviour are untouched.

Worth recording that this wasn't only latent — the old PATCH response carries an ETag … test explicitly sent the PATCH's ETag as If-None-Match on a follow-up GET and asserted a 304, so a client caching the echo body would have been 304'd onto the closure-less representation. That test (and the no-op-PATCH ETag test) now assert the echo carries no validator, while still pinning that a write rotates the validator a GET reports (stale → 200, advanced → 304).

Not yet run against the realm-server node suite locally (same dev-stack constraint noted in the description); CI will exercise it.

[habdelra]

This ticket is assigned to me--I have a PR for it #6166, why did you pick it up?

[github-actions]

Host Test Results

    1 files      1 suites   2h 40m 39s ⏱️
4 611 tests 4 597 ✅ 12 💤 1 ❌ 1 🔥
4 626 runs  4 611 ✅ 12 💤 2 ❌ 1 🔥

Results for commit 7a7a0bc.

For more details on these errors, see this check.

Realm Server Test Results

    1 files    235 suites   1h 17m 30s ⏱️
3 377 tests 3 376 ✅ 0 💤 1 ❌
3 424 runs  3 423 ✅ 0 💤 1 ❌

Results for commit 7a7a0bc.

For more details on these errors, see this check.

[richardhjtan]

[Claude Code 🤖] Lens: the three readbacks, every consumer of a write response (host store, the _onSave subscriber, boxel-cli, the card-operations executors), and the CI failures. I did not run the realm-server suite locally.

The core thesis holds — no production reader of a write response is broken by this. What blocks it is three CI failures and the validator question.

On the duplication asked about above: the change you linked makes the same edit at the same three readbacks, and differs in one substantive way — it gives the echo its own write-echo ETag variant rather than dropping the validator, and it carries the two host-test updates this branch's CI is failing on. The validator question is worth settling once across both rather than twice.

1. The new create test never reaches its assertions — 500 on its first POST. Thread on card-endpoints-test.ts.
2. The echo drops relationship linkage, not just included[], and two host tests assert the old shape. Thread on realm.ts.
3. Dropping the validator outright: the hazard the second commit names — a client caching the echo body and being 304'd onto it — needs a cache that stores a PATCH response keyed by its ETag. PATCH responses aren't cacheable, and nothing in the repo stores one. Against that, the in-flight If-Match-on-card-writes work makes an echo validator useful: without one, a client doing back-to-back conditional writes has to GET in between. Worth either justifying the removal or taking the distinct-variant route.

Red checks: Realm Server Tests shard 6 and Host Tests shards 8 and 13 are all this change (1 and 2) — no flake among them.

[richardhjtan]

[Claude Code 🤖] This 500s on the first POST in CI, so the test never reaches the included assertions it exists to make — the contract it is here to pin is unpinned.

rri('./friend.gts') resolves against the created card's own URL, and a POST to / lands the card at <realm>/Friend/<uuid>.json, so the ref resolves to <realm>/Friend/friend.gts: fileSerialization throws FilterRefersToNonexistentTypeError (Module entry not found for URL) and the handler turns it into a 500. The other POST-to-/ tests in this file spell the ref as a prefixed module (rri('@cardstack/base/card-api')).

While you're in here: both PATCH echoes change too, and neither the write branch nor the no-op short-circuit has a test asserting the closure is absent.

Regression, blocking.

[richardhjtan]

[Claude Code 🤖] "the client discards its attributes, relationships and included[]" understates the change: loadLinks also writes relationships.<field>.data onto the primary resource (entry.relationship.data = …), so the echo now carries relationships with links.self and no resource linkage. That is a different document for the written card itself, not only a missing included[] — and to a JSON:API client a relationship without data reads as "linkage unknown" rather than "no target".

That is what both host failures are. card-copy-test.gts deep-equals data.relationships.pet including its data member and then dereferences json.included[0]; the interact submode … new linked card is created in a different realm acceptance test deep-equals relationships['friends.1'] with its data member. The copy one surfaces as unable to save copied card instance rather than a failed assertion because the TypeError on included[0] is thrown inside the onSave subscriber, which runs inside persistAndUpdate's try — store.create returns a CardErrorJSONAPI and copy-card.ts throws on it.

Ask: update both tests to the new shape, and reword this comment, its twin on the PATCH branch, and the description's "relationships … returned exactly as before" to say the relationships lose their linkage too.

Regression, blocking.

[habdelra]

This is a dupe Pr. let's prefer #6166

[backspace]

I’m closing this in favour of this, reopen if I misunderstood!