Remove VirtualNetwork from (most of) card API#5349
Conversation
Preview deploymentsHost Test Results 1 files ±0 1 suites ±0 2h 30m 28s ⏱️ - 5m 23s Results for commit 89367ca. ± Comparison against earlier commit 0d67174. Realm Server Test Results 1 files ±0 1 suites ±0 10m 33s ⏱️ -4s Results for commit 89367ca. ± Comparison against earlier commit 0d67174. |
…and serialize paths Identifiers are canonical RRI inside the runtime, so the form-bridging that was threaded through these interior paths is no longer needed: - resolveRef resolves relative references with pure RRI path math (new resolveRRIReference in url.ts); no VirtualNetwork. - isLocalId is a pure syntactic test (not a URL, not an @-prefix). - SerializeOpts no longer carries a VirtualNetwork; the serialize path (card-serialization.ts, serializers/code-ref.ts) preserves prefix-form refs and resolves URL-form refs with plain URL math. The host Store's asURL keeps resolving keys to normalized URL form (via the VN) so it stays consistent with gc-card-store, which keys instances by their URL-form data.id. Collapsing the store's canonical key to an opaque RRI token is deferred to CS-11730 — it needs gc-card-store keyed the same way and must preserve the URL normalization toURL provides. VirtualNetwork stays where it resolves an RRI to a real URL at the network boundary (document loading), at render-time pill resolution, and for the Store's URL-form keying. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
6516d9e to
674b39c
Compare
…reading-card-api-store # Conflicts: # packages/base/card-api.gts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc4d2a31ed
ℹ️ 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".
The VirtualNetwork-free codeRefAdjustments/serialize only built a base for
http(s) ids, so a relative CodeRef module (`./person`) with a prefix-form RRI
base (`@cardstack/catalog/specs/foo`) left the base undefined: serialize
failed to absolutize it and deserializeAbsolute reached
`new URL('./person', undefined)`. Resolve in RRI space via resolveRRIReference
(handles both URL- and prefix-form bases) and preserve a prefix-form base
through serialize.
Addresses PR review feedback on #5349.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
resolveRRIReference treated a `$REALM/`-rooted reference as an ordinary relative ref and path-joined it (e.g. `$REALM/string` from `@cardstack/base/fields/number` produced `@cardstack/base/fields/$REALM/string`), where VirtualNetwork.resolveRRI roots it at the realm. Resolve `$REALM/` against the realm root — the `@scope/name` namespace of a prefix-form base — so realm- root card/relationship references keep resolving after the VN-free switch. A URL-form base implies an unmapped realm, where resolveRRI likewise has no realm root to resolve against without mappings. Addresses PR review feedback on #5349. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR continues the shift to canonical RRIs by removing VirtualNetwork threading from most of the card serialization/deserialization API, replacing it with mapping-free, form-preserving RRI/path resolution. It introduces a new resolveRRIReference helper and updates host/base code to rely on it, while leaving pill/query-field virtual-network work for follow-up PRs.
Changes:
- Added
resolveRRIReference()for mapping-free resolution of relative references in RRI space (supports URL-form bases, prefix-form bases, and$REALM/). - Refactored CodeRef serialization/deserialization and
resolveRef()call sites to avoidVirtualNetworkand preserve prefix-form RRIs. - Updated host services/test helpers/tests to stop passing
virtualNetworkthroughSerializeOpts, and simplifiedisLocalId()to be purely syntactic.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/runtime-common/url.ts | Adds resolveRRIReference() to resolve references in RRI space without realm mappings. |
| packages/runtime-common/serializers/code-ref.ts | Removes VN dependency; resolves relative modules via resolveRRIReference. |
| packages/runtime-common/index.ts | Simplifies isLocalId() to no longer require a VN. |
| packages/host/tests/unit/url-test.ts | Adds unit coverage for resolveRRIReference() behavior. |
| packages/host/tests/unit/code-ref-test.ts | Updates tests for VN removal and adds prefix-form base resolution coverage. |
| packages/host/tests/helpers/indexer.ts | Stops requiring a VN for serializeCard() test helper. |
| packages/host/tests/helpers/index.gts | Removes VN requirement from helper serialization. |
| packages/host/tests/helpers/base-realm.ts | Removes VN requirement from base-realm serialization helpers. |
| packages/host/tests/helpers/adapter.ts | Removes VN requirement when serializing cards in the test adapter. |
| packages/host/tests/acceptance/interact-submode-creation-and-permissions-test.gts | Updates isLocalId() usage to the new signature. |
| packages/host/scripts/check-memory-baseline.mjs | Formatting-only adjustments (no functional change). |
| packages/host/app/services/store.ts | Updates isLocalId() usage; removes VN from serializeFileDef opts. |
| packages/host/app/services/spec-panel-service.ts | Updates isLocalId() usage. |
| packages/host/app/services/render-service.ts | Updates isLocalId() usage while retaining URL-key normalization via VN. |
| packages/host/app/services/recent-cards-service.ts | Updates isLocalId() usage. |
| packages/host/app/services/playground-panel-service.ts | Updates isLocalId() usage for local-id tracking logic. |
| packages/host/app/services/operator-mode-state-service.ts | Updates isLocalId() usage in persisted operator-mode state handling. |
| packages/host/app/services/card-service.ts | Stops injecting virtualNetwork into serialize opts. |
| packages/host/app/routes/render/meta.ts | Removes virtualNetwork from serialize opts (still uses VN for relativization helper). |
| packages/host/app/lib/gc-card-store.ts | Updates isLocalId() usage for local/remote id splitting. |
| packages/host/app/components/operator-mode/code-submode/spec-preview.gts | Updates isLocalId() usage. |
| packages/host/app/commands/copy-and-edit.ts | Removes VN requirement when serializing cards for relationship discovery. |
| packages/base/searchable.ts | Updates resolveRef() usage to the new signature. |
| packages/base/card-serialization.ts | Removes VN from SerializeOpts and uses resolveRRIReference() for absolutization prior to relativization. |
| packages/base/card-api.gts | Updates resolveRef() signature and all call sites; removes VN-based resolution path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Per PR review (Copilot): resolveRRIReference treated only http://https:// (and @-prefix) as absolute, so a reference with another scheme (data:, blob:, mailto:, …) fell through into the prefix-base resolution path and was incorrectly rewritten into @scope/name… form. Match any RFC-3986 `<scheme>:` prefix and return such references unchanged. Adds url-test coverage. Addresses PR review feedback on #5349. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Now that the runtime works with RRIs, virtual networks shouldn’t be needed in the card API. This removes most of them, but excludes pills and query fields, which await #5356, will be addressed in #5366 (maybe not fully).