Resolve a code ref's module without a virtual network - #6073
Conversation
`loadCardDef` pulled the network off the loader and refused to run without one, but used it for a single call: `resolveModuleHref`, which wanted only `isRegisteredPrefix` and `resolveURL`. A code ref's module is canonical RRI, so resolving it is path math — `@scope/name/...` and anything carrying a URL scheme are already absolute, and a relative reference joins against `relativeTo`. That is what `resolveRRIReference` does, with no mappings involved, and `codeRefWithAbsoluteIdentifier` already took that path when handed no network. No call site changes. All 67 `loadCardDef` callers pass a loader rather than a network, so this is confined to code-ref.ts. The bare-specifier rejection survives without a registry, because a bare specifier is recognisable by shape: neither URL-like — relative, rooted, or schemed — nor scoped. What it stops rejecting is a scoped reference whose prefix this process has not registered, deliberately: such a reference is absolute and cross-realm by construction, which is the rule the read path already applies, and an unresolvable one now fails at fetch naming the module the caller wrote rather than at a prefix check. `codeRefWithAbsoluteIdentifier` keeps its optional network parameter, now accepted and ignored, so its ~50 call sites stay untouched. Removing it cascades into three more signatures and belongs with the sweep that takes the network off the Loader's remaining consumers.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f4fcf6135
ℹ️ 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 bare-specifier rejection this added was stricter than the contract around it and rejected identifiers the resolver would have handled. `isRelativePath` draws the line: `@scope/name/...` is absolute, anything a URL parser accepts is absolute, and everything else is a relative reference. `resolveRRIReference` draws it identically. So `data:` and `blob:` modules were being rejected before the resolver could preserve them, and a same-realm module written without `./` — `garden-design`, as the checked-in Garden tab refs have it — was rejected too, after which `codeRefWithAbsoluteIdentifier`'s catch returned the ref unresolved and callers went on to look up the wrong identifier. Silent, and in a path that had no guard before this branch. The premise behind that guard was wrong. A bare specifier cannot be recognised by shape: `garden-design` naming a module in this realm and `date-fns` naming one of the ~30 packages the loader shims are indistinguishable, and only the prefix registry ever told them apart. Losing the registry loses that, so the resolution rule is now simply the contract's. Relative wins for a bare name. A code ref names a card definition, so the checked-in refs of that shape are same-realm modules while the shimmed packages are imported by module source rather than referenced as code refs. A scoped specifier that matches no realm prefix — `@cardstack/boxel-host/commands/foo`, the case the removed comment named — still passes through unchanged, which is what the loader's import map needs. What it costs is a bare shimmed specifier used as a code ref, which would resolve into the realm and fail at fetch. Tests follow the contract rather than the guard: a bare name joins against the consumer, and non-http schemes pass through.
Host Test Results 1 files 1 suites 2h 37m 59s ⏱️ Results for commit 0a86a2e. For more details on these errors, see this check. Realm Server Test Results 1 files 209 suites 1h 19m 34s ⏱️ Results for commit 0a86a2e. |
|
[Claude Code 🤖] Parked as a draft. Both review findings are fixed and resolved; what remains is the issue's scope being wrong rather than the code.
The issue's premise — that That is exactly what CS-12758 predicts for its real-URL-keying endpoint, quoting it: "Changes the output spelling of So this waits on the endpoint decision. Under narrow injected functions it becomes small, but needs a port Kept from the attempt, and recorded on the issue: the bare-specifier rejection cannot be rebuilt without the registry. |
loadCardDefpulled the virtual network off the loader and refused to run without one:It needed far less than it asked for. The only thing it did with the network was hand it to
resolveModuleHref, which used two capabilities:isRegisteredPrefixandresolveURL.Resolving a code ref is path math
A code ref's module is canonical RRI.
@scope/name/...and anything carrying a URL scheme are already absolute; a relative reference joins againstrelativeTo. That is exactlyresolveRRIReference, which consults no mappings — andcodeRefWithAbsoluteIdentifieralready took that path when handed no network, so the direction was half-built.No call site changes. All 67
loadCardDefcallers pass a loader rather than a network, so the diff is confined tocode-ref.ts.The bare-specifier error survives, without a registry
Dropping
isRegisteredPrefixrisked losing the clear failure for a bare specifier, which would otherwise join against the consumer and fetch a URL nobody wrote. It does not need a registry: a bare specifier is recognisable by shape — neither URL-like (relative, rooted, or schemed) nor scoped.What this deliberately stops rejecting is a scoped reference whose prefix this process has not registered. Such a reference is absolute and cross-realm by construction — the rule the read path already applies via
isScopedReference— and an unresolvable one now fails at fetch, naming the module the caller actually wrote rather than failing a prefix check.Scope
codeRefWithAbsoluteIdentifierkeeps its optional network parameter, now accepted and ignored, so its ~50 call sites stay untouched. Removing it was measured: 16 call sites drop the argument cleanly, but that leaves the parameter unused in three further functions and cascades into their signatures too. That belongs with the sweep that takes the network off the Loader's remaining consumers, not here.Tests
resolve-module-href-test.tscovers what the resolution rules now are: a scoped reference passed through, a scoped reference with no registered prefix also passed through (the deliberate change), a relative reference joined both down and up a level, an absolute URL untouched, a bare specifier rejected, and an absolute reference resolving with norelativeToat all.