Change the host store to allow prefix RRI keys#5470
Open
backspace wants to merge 16 commits into
Open
Conversation
asURL's string branch normalized remote ids to URL form while its document branch returned data.id verbatim — which is canonical (prefix form for mapped realms) as served — so the two branches could key the same mapped-realm card under different spellings. Fold the string branch to the canonical spelling too: parse through the VN's URL machinery for normalization, then back to prefix form where a realm mapping exists. The render store's key normalization gets the same treatment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Preview deploymentsHost Test Results 1 files ±0 1 suites ±0 2h 50m 7s ⏱️ - 7m 31s Results for commit 061552d. ± Comparison against earlier commit d95b506. Realm Server Test Results 1 files ±0 1 suites ±0 17m 50s ⏱️ +40s Results for commit c553362. ± Comparison against earlier commit e26b2c8. |
The render store's cache-key normalization and its document-load normalization shared one helper, so folding keys to canonical spelling also changed the URL that loadCardDocument stamps onto the returned document's id. Split them: cache keys fold to the canonical spelling; load targets resolve to the real URL at the fetch boundary. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
backspace
marked this pull request as ready for review
July 13, 2026 17:24
![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: d95b506a08
ℹ️ 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 store keys mapped-realm instances in prefix form, and addReference passes those ids to subscribeToRealm -> realm.realmOf. realmOf scanned its URL-keyed realms with a VirtualNetwork-less RealmPaths, whose inRealm only matches same-form inputs — so prefix-form ids never matched, no Matrix subscription was installed, and cards in mapped realms missed invalidation events. Pass the VirtualNetwork into RealmPaths so its cross-form matching applies. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates host-side identifier normalization so that, when a realm mapping exists in the VirtualNetwork, the prefix-form RRI becomes the canonical key (instead of URL-form), and related subsystems (render-service cache keying, realm membership checks) align with that.
Changes:
- Canonicalize remote ids to prefix form (when mapped) in the host store’s
asURL()normalization. - Align
CardStoreWithErrorscache key normalization with the store’s canonical (prefix-preferred) keying. - Update
RealmService#realmOf()realm membership checks to allow prefix-form ids against URL-keyed realms, and add an integration test covering subscription installation via a prefix-form reference.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/host/tests/integration/store-test.gts | Adds an integration test asserting prefix-form references install realm subscriptions. |
| packages/host/app/services/store.ts | Changes asURL() normalization to prefer prefix-form keys for mapped realms. |
| packages/host/app/services/render-service.ts | Adjusts render-service in-memory cache key normalization to match the store’s canonical spelling. |
| packages/host/app/services/realm.ts | Passes VirtualNetwork into RealmPaths so inRealm() can match prefix-form ids against URL-form realms. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
unsubscribeFromInstance tested realm membership with a bare id.startsWith(realmURL), but the store now keys mapped-realm instances and reference counts in prefix form while subscription keys stay realm-root URLs (realm events route by URL). The prefix-form id never matched the URL-form realm key, so a mapped realm's Matrix subscription leaked and its reference scan mis-counted. Resolve each id to URL space via toURLHref before the comparison, mirroring the cross-form fix on the subscribe side in realmOf. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
asURL runs on nearly every store operation and resolved ids via vn.toURL(id).href, allocating a URL per call. toURLHref is the memoized equivalent — same resolution and throw behavior — so repeat ids become a Map lookup. Also apply it to the two prefix-resolution read sites that used the same pattern. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
normalizeKey runs on every render-store cache get/set and normalizeURL on the load path; both resolved via vn.toURL(id).href, allocating a URL per call. Swap to the memoized toURLHref, which has identical resolution and throw behavior, so repeat ids become a Map lookup. normalizeURL keeps resolving the original id (not the .json-stripped key) so the loaded document's id is stamped from the fetch-form URL. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The realm-subscription lifecycle test installed the subscription and dropped the reference but never asserted the teardown, so the unsubscribe realm-membership mismatch went uncaught. Assert the subscription is gone after the last drop; this fails on a URL-space startsWith against a prefix-form id and passes with the URL-space comparison. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
habdelra
approved these changes
Jul 14, 2026
Enabling teardown of mapped-realm subscriptions (which the store now keys in prefix form) destabilized acceptance tests: a subscription torn down on a reference drop stops delivering the realm invalidations a test awaits, so the test hangs to timeout. The pre-existing behavior leaks the subscription but keeps invalidations flowing, so restore it here. A safe teardown needs the subscription lifecycle stabilized separately, not the key flip plus eager unsubscribe in one step. The toURLHref performance changes are unaffected and stay. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Realm subscriptions are keyed by realm-root URL (invalidation events route by URL) while the store keys instances and reference counts in canonical form (prefix form for mapped realms). Reconciling the two forms on every reference drop to decide when to unsubscribe races invalidation delivery: tearing a subscription down as the last reference drops loses updates a caller is still awaiting, which hung acceptance tests once the prefix-form keying made the drop path actually fire. Realms are few and their subscriptions cheap, so stop reference-counting them: install one on first use and release them all together when the store resets or is destroyed. unsubscribeFromInstance now only stops per-instance field-change tracking; a new unsubscribeFromAllRealms runs from resetState and the destructor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
resetState runs at mid-session boundaries (realm/workspace switch, route deactivate), not just teardown. Unsubscribing there stops realm invalidations reaching cards that outlive the reset, hanging any test that loads a card, crosses a resetState, then waits for an update. Keep resetState's original behavior (drop the map, leave the subscriptions to be re-established on next use) and release subscriptions only from the destructor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Realm subscriptions are no longer torn down when a reference count hits zero, so this test's post-drop assertion is inverted: the subscription now persists past the last reference (released only on store destroy). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The session-lifetime subscription change removed the deterministic hang but regressed invalidation-heavy tests ~2.7x (28s -> 78s): keeping every realm subscription up means the store processes invalidations it previously dropped during reference-count churn. Restore the pre-existing per-reference teardown, which leaves mapped-realm (prefix-keyed) subscriptions installed for the session as a benign, documented leak. §3 core canonical keying and the toURLHref perf tweaks are unaffected. The subscription-lifecycle rework (safe teardown + cheaper invalidation handling) is deferred; see the realm-subscription-lifecycle follow-up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Restores the store and render-service identifier keying to exactly the form that was green (the §3-core state before any PR-feedback commits). The toURLHref change was a perf suggestion layered on afterward; with the unsubscribe fix and subscription-lifetime rework already reverted, this returns the branch to pure §3-core canonical keying on current main. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
re the automated PR feedback, I originally replied with SHAs of fixes, but they were causing CI problems. They’re now recorded for followup in issues 12114 and 12143. |
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
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.
If a mapping exists, the prefix form is the preferred key.