Fix card embeds in Markdown - #5710
Merged
Merged
Conversation
`Contains.serialize`'s composite branch dropped the `opts` argument, so the nested resource was built without `includeComputeds`. `getFields` then treated the missing flag as false and filtered out every computed declared inside the FieldDef. The other three field serialize sites — `ContainsMany` primitive and composite, `Contains` primitive — all forward it, so the same FieldDef kept its computeds through `containsMany` and lost them through `contains`. This broke rich-markdown `:card[…]` / `:file[…]` references end to end on the server side. `RichMarkdownField.cardReferenceUrls` is a computed on the field, so it never reached the indexed document; the query-backed `linkedCards` field interpolates `$this.cardReferenceUrls` from the serialized resource, found nothing, and aborted its query. The relationship then indexed as an ordinary unresolved link — no `links.search`, no results, and no error to surface it. Forward `opts`, matching the sibling call sites. `visited` stays a fresh set there, as it is at all three of them.
Forwarding `opts` through a composite `contains` moves three things in
exact-document assertions, all of them bringing `contains` into line with what
`containsMany` already did:
- `includeUnrenderedFields` now reaches nested composites, so a composite's
never-authored links serialize as `{ links: { self: null } }`. Every CardDef
contains a `cardInfo`, so every such assertion gains `cardInfo.cardThumbnail`
and `cardInfo.theme` — hoisted into a shared `cardInfoLinks` helper rather
than repeated inline.
- Relationship links on a nested `linksTo` now relativize against the primary
resource's base. Those relationships live on the primary, so that is the
correct base, and it matches top-level `linksTo`.
- `adoptsFrom` on an `included[]` resource reached through a nested composite
relativizes the same way.
The indexer serializes with `includeComputeds: true`, so a computed declared
inside a nested FieldDef now lands in the indexed document — the fix itself,
visible in the cyclic-relationship indexing baseline.
Assertions that serialize without `includeUnrenderedFields` are unchanged,
confirming the option, not the propagation, is what governs the new keys.
`serializeCard` built one reference-relativizer bound to the primary model's base and threaded it through the whole document, so a link target serialized into `included[]` described its `adoptsFrom.module` and its own relationship links relative to the primary rather than to itself. Deserialization reads them the other way: `cardClassFromResource` resolves an included resource's module against that resource's id. A pet at `Pet/mango` carrying `module: './test-cards'` therefore resolved to `Pet/test-cards`. Extract the relativizer as `relativeReferenceFor(model)` and re-bind it at the boundary where a link target becomes its own resource. A target with no id has no base of its own and keeps the parent's — the same rule the index engine's `relativizeResource` applies (`resource.id ? toURL(resource.id) : primaryURL`), so client and server now encode documents identically. Included-resource references in the baselines move from the primary's base to their own (`../test-cards` rather than an absolute or primary-relative form). Primary-resource references are unchanged.
Contributor
Preview deploymentsHost Test Results 1 files ±0 1 suites ±0 2h 27m 18s ⏱️ - 43m 25s Results for commit 96e12ed. ± Comparison against earlier commit 69038f6. Realm Server Test Results 1 files ± 0 1 suites ±0 14m 44s ⏱️ +27s Results for commit 96e12ed. ± Comparison against earlier commit 69038f6. |
`meta.adoptsFrom` is a `CodeRef` union, and its `ancestorOf` variant has no
`module`, so reading `.module` off it doesn't type-check. Compare the whole
`adoptsFrom` instead — the shape is exactly `{ module, name }`, `module` needs
its `RealmResourceIdentifier` brand, and the assertion now also pins the class
name. Matches how the RRI form audit test reads the same field.
A query field resolved its realm to exactly one: `resolveRealm` defaulted to the realm holding the instance and threw `only supports a single realm` on more. A field filtering on absolute references is cross-realm by nature, so scoping it to the containing realm meant a reference into another realm could never match — the query simply returned nothing, with no error to explain it. Everything downstream was already built for a set. `Query` has carried `realms: string[]` beside `realm`, `getSearchResource` takes an array of realm hrefs, and the engine's remote-fetch path reads `query.realms` and aggregates results. The normalizer was the one place that collapsed it. So resolve `realms` instead of `realm`, and fan out across the result: each realm is queried and the results merged, deduped by id. A realm that fails — most often because the caller can't read it — contributes its error and no results, leaving the realms that did answer intact rather than failing the field. An entry in `realms` may name a realm or a resource inside one, the latter resolving to its holder. That is what lets `RichMarkdownField` say "search wherever these references live" by interpolating the reference ids themselves. Doing so needs the realm mappings, which a card definition deliberately cannot reach, so the caller supplies a resolver: `VirtualNetwork.realmForReference` on the indexing path, and a new `CardStore.realmForId` on the client — the same boundary `resolveURL` and `canonicalizeId` already sit on, handing back a value rather than the network. A realm root is whatever was registered, at whatever depth, so it cannot be recovered from the path: `/user/alice/` is a realm and `/user/` is not. A reference no realm holds is dropped rather than folded into the containing realm, and a query left with no realm at all resolves to nothing — searching the wrong realm would be worse than returning empty. `links.search` now names every realm covered, and the client reads the whole set back off that seed instead of keeping only the realm the URL was addressed to.
`realmForReference` answered with the prefix mapping's target, which is where a realm is reachable rather than the URL it calls itself. Those differ for the base realm: it is served as `https://cardstack.com/base/` and merely mapped onto a real host, and its index rows are stored under that virtual URL. A cross-realm query naming the mapped target therefore searched a realm the index had never heard of and came back empty — the request succeeded, so nothing surfaced as an error. Fold the answer back through the virtual mapping when one exists. Every other realm maps to itself, so only base changes.
backspace
changed the title
backspace
changed the title
Resolving `realms` through a reference resolver made an explicitly named peer realm disappear: the resolver only knows realms this process holds mappings for, so a field targeting another server's realm resolved to nothing and the query never ran. That is a behavior a field could rely on before — a named realm was used as written. Fall back to the value when the resolver can't place it, but only when it is addressed as a realm root. A realm ends in a slash and a resource identifier does not, so an explicit cross-realm target still works while a reference into an unknown realm is still dropped rather than mistaken for a realm to search.
backspace
marked this pull request as ready for review
August 10, 2026 13:13
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 691b4e789a
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Falling back only for values ending in a slash dropped a realm named directly as `https://peer.example`: the resolver can't place a realm this process holds no mapping for, and the shape test then classified it as a resource. A realm href may legitimately omit the trailing slash — `buildQuerySearchURL` and `parseRealmsParam` both normalize that — so spelling says nothing about whether a string names a realm or a resource. Record instead whether the target was authored as an interpolation, before interpolation substitutes values and the distinction is lost. Realms written into the query are honored as written; values drawn from the instance's data are dropped when unplaceable, so a reference is still never mistaken for a realm to search.
`realmForReference` matched only the realm-prefix mappings, which covers fewer realms than it appears to. A realm registered as `@cardstack/<name>/` or a scoped prefix lands there, but every other realm — experiments, submissions, a user's own realm — is registered solely as a self URL mapping and so could not be placed at all. The base realm could be placed by prefix but not by the `https://cardstack.com/base/` space it is actually addressed through, which is the spelling a markdown reference is most likely to carry. Match against both maps: a prefix mapping offers its prefix and its target, a URL mapping offers the virtual space and the real one. Longest match still decides, so a realm nested under another is attributed to the nested one.
habdelra
reviewed
Aug 10, 2026
habdelra
left a comment
habdelra
left a comment
Contributor
There was a problem hiding this comment.
[Claude Code 🤖] Review focus: the two mechanisms this change rests on — reference relativization across the client/server boundary, and cross-realm realm resolution for query-backed fields — and whether the serialization fix lands without collateral.
Bottom line: no blocking issues. The change is well-factored and the invariants it depends on hold on both sides. CI is green (host and realm suites), the two automated-review findings are resolved, and the new integration + unit tests pin the actual failure chains rather than just output shape. The inline comments are mostly confirmations that document the load-bearing contracts so the next editor doesn't break them, plus one non-blocking follow-up and one adjacent note.
What lands right (mechanistically, not courtesy):
- The root cause is pinned exactly: composite
containswas the single field-serialize site droppingopts, which filtered computeds out of nested resources and broke thecardReferenceUrls→linkedCardsinterpolation chain end to end. The fix is the minimal one that restores parity with the other three sites. - Relativizing an included resource against its own id (
relativeReferenceFor+rebaseReferencesFor) makes client serialization and the server'srelativizeResourceencode documents identically — a real correctness fix, not just the embed repair. - The realm resolution is careful about both directions that matter:
realmForReferenceplaces a reference by any spelling its realm is addressable under and folds base back to its virtual URL (where its index rows actually live), and the normalizer separates "realm authored into the query" (honored verbatim) from "realm drawn from instance data" (dropped when unplaceable) by provenance rather than trailing-slash spelling — the right axis, and the fix that resolved the earlier automated finding. - Per-realm error isolation in the index-engine fan-out: a realm the caller can't read contributes its error and no results instead of sinking the whole field.
Notes (detail in the inline threads):
rebaseReferencesFor— confirmation that thevalue.id-present-vs-absent rule is the exact twin ofrelativizeResource'sresource.id ? own : primary; a guard against silent client/server drift. (card-serialization.ts)- Composite
containsoptsforwarding — confirmation the fix is safe because it matches the long-standingcontainsManycomposite behavior; theincludeUnrenderedFieldsfallout is the same shape that path already produced. (card-api.gts) - Provenance is tracked per whole
realmsvalue, so a literal realm in a mixed literal+interpolation list inherits interpolation provenance and can be dropped; no caller writes that shape today. Follow-up. (query-field-utils.ts)
Adjacent, out of scope: the index-engine fan-out issues one _search request per targeted realm (fetchRemoteQueryResults is called once per realm in the loop). For the markdown case references usually live in one or two realms, so it's immaterial; but if a single field ever spans many remote realms on the same server, the /_search endpoint already accepts a multi-realm realms param and could batch them. Not asked of this change.
Generated by Claude Code
`relativizeResource` and `rebaseReferencesFor` apply the same own-id-or-primary rule so a document encodes included references the same whether the client serialized it or the server served it. The serializer already points here; this is the pointer back, since changing one side alone produces documents that deserialize to different ids with nothing failing loudly.
habdelra
approved these changes
Aug 11, 2026
Provenance was computed once over the whole `realms` value, so one interpolation anywhere in a list marked every entry as interpolated. A literal realm written alongside it was then dropped when the resolver couldn't place it, though being written into the query is exactly what should have kept it. Expand the authored value into one entry per realm, each carrying whether it came from an interpolation, instead of substituting through `interpolateNode` and losing that. Expanding also flattens an interpolation that yields a list, which previously became a nested array and then failed the string check — so a list mixing literal realms with interpolations now works at all, and each entry behaves as it was authored.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Various types of card embeds are currently broken:
This fixes them: