Add RealmService.contains for cross-form membership checks#4628
Add RealmService.contains for cross-form membership checks#4628
Conversation
Substring-prefix checks like `card.id.startsWith(realm)` only work when both sides are in the same form (URL vs registered prefix). After RRI migration, IDs may be prefix-form while realm URLs are URL-form (or vice versa), silently breaking those checks. `contains` resolves both sides to URL form via `cardIdToURL` before delegating to `RealmPaths.inRealm`, so cross-form comparisons work as long as the prefix is registered. Sweeps the two known callers in `search.ts` and `workspace.gts`.
Preview deploymentsHost Test Results 1 files ±0 1 suites ±0 1h 57m 22s ⏱️ - 2m 52s Results for commit 50bf89b. ± Comparison against earlier commit 558a060. Realm Server Test Results 1 files ± 0 1 suites ±0 16m 57s ⏱️ -19s Results for commit 50bf89b. ± Comparison against earlier commit 558a060. |
There was a problem hiding this comment.
Pull request overview
This PR introduces a reliable realm-membership helper on the Host-side RealmService to avoid incorrect startsWith() realm checks when identifiers may be in mixed forms (URL-form vs registered-prefix form) after the RRI migration.
Changes:
- Added
RealmService.contains(realm, resource)that normalizes both inputs to URL-form viacardIdToURL()and then usesRealmPaths.inRealm(). - Updated
SearchResource.instancesByRealmto userealm.contains()instead ofcard.id.startsWith(realm). - Updated workspace deletion “active workspace” detection to use
realm.contains()for open card IDs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/host/app/services/realm.ts | Adds RealmService.contains() for cross-form realm membership checks (URL/prefix). |
| packages/host/app/resources/search.ts | Switches realm grouping logic to use realm.contains() instead of string prefix checks. |
| packages/host/app/components/operator-mode/workspace-chooser/workspace.gts | Uses realm.contains() when checking if any open card belongs to the workspace realm. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…-membership-cs-10982
| // (and vice versa) as long as the prefix is registered in `prefixMappings`. | ||
| // Returns `false` if either side fails to resolve, including when a | ||
| // non-URL-like string does not match a registered prefix exactly. | ||
| contains(realm: string | URL, resource: string | URL): boolean { |
There was a problem hiding this comment.
Is there a named type we should use here?
| } catch { | ||
| return false; | ||
| } | ||
| return new RealmPaths(new URL(realmHref)).inRealm(rri(resourceHref)); |
There was a problem hiding this comment.
Isn't there a util that can do this with with RRIs?
…-membership-cs-10982
Per review: use the existing branded types in the helper signature so intent is documented through the type system. Callers cast their unbranded strings via `ri()` / `rri()` for now; tightening the source types of `availableRealmURLs`, `Workspace.args.realmURL`, and `CardDef.id` is tracked separately.
This was split off #4539, although it’s so small it could have been included!
Substring-prefix checks like
card.id.startsWith(realm)only work when both sides are in the same form (URL vs registered prefix). After RRI migration, IDs may be prefix-form while realm URLs are URL-form (or vice versa), silently breaking those checks.containsresolves both sides to URL form viacardIdToURLbefore delegating toRealmPaths.inRealm, so cross-form comparisons work as long as the prefix is registered.Sweeps the two known callers in
search.tsandworkspace.gts.