Conversation
A shim registered for a realm-mapped identifier such as `@cardstack/base/date/day` is keyed by the realm URL that identifier resolves to, because the realm mapping also feeds the network's import map. A loader import of the identifier resolves to the same realm URL, but the network's fetch pipeline only answers shims on the fake `https://packages/` origin, so such a shim was never served and the module was fetched from the realm server instead. The Loader now asks the network for a shim from its module-fetch path, which is the one place that knows the request is for a module rather than a card instance that may live at the same realm URL. The lookup folds every spelling of the identifier onto the real URL, so the prefix, virtual-alias, and url-mapped forms all land on one module. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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. |
![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: 5137798487
ℹ️ 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".
Preview deploymentsHost Test Results 1 files ±0 1 suites ±0 2h 48m 59s ⏱️ + 13m 55s Results for commit 08ca772. ± Comparison against earlier commit 65fc58d. Realm Server Test Results 1 files 248 suites 1h 32m 8s ⏱️ Results for commit 08ca772. |
A shim is keyed by the URL its identifier resolved to when it was registered, and a lookup resolves through whatever mapping is current. Re-pointing a realm prefix after the shim is installed strands it under the old URL: the import resolves to the new one, misses, and goes to the network. Re-registering a prefix to a different target is supported and tested, so the gap is reachable rather than theoretical. Each shim is now registered under both spellings — the URL its identifier resolved to and the identifier itself — and a lookup tries the request unresolved against the current mapping as well as resolved. The resolved key goes stale on a remap; the identifier does not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
habdelra
left a comment
habdelra
left a comment
There was a problem hiding this comment.
[Claude Code 🤖] Review went after the shim key algebra: whether every spelling of an identifier really converges on one module across all three registration shapes (shimModule, shimAsyncModule by id, shimAsyncModule by prefix), whether the mapping configuration the app actually registers puts the registration key and the lookup key on the same side of the virtual/real split, and whether the new lookup can be reached for anything other than a module. Not covered: the follow-on @cardstack/base bundling this enables, and behaviour under a prefix shim that claims a whole realm.
No blocking issues. One ask: decide how far the remap fix is meant to reach, since it currently covers one of the three registration shapes while the comments read as unconditional.
- Extend the dual-key registration to
shimModuleand theprefixbranch — or narrow the comments to the async-idform and say which shape realm-mapped shims will use. Detail in the thread on the registration loop inpackage-shim-handler.ts.
Adjacent, out of scope: once a prefix shim is registered against a realm-mapped prefix, lookupModule's getModuleByPrefix leg answers for every module URL under that realm, including paths the resolver has nothing for — worth deciding what that resolver does with an unknown rest before the base-realm work lands on it, rather than in this PR.
Generated by Claude Code
The dual-key registration covered only `shimAsyncModule` by `id`, while the comments around it read as a property of shimming generally. `shimModule` — the API every registration in `externals.ts` uses — still stored one resolved key, as did the `prefix` branch, so re-pointing a prefix stranded either exactly as the `id` form had been. The prefix form missed twice over: `lookupModule` passed the unresolved identifier to `getModule` but never to `getModuleByPrefix`. Those are the shapes that matter. Serving a whole realm from one shim is most naturally the `prefix` descriptor, which is where the base bundling is headed, and it was the shape with neither remap survival nor a test. Both spellings now come from one `registrationKeys` helper that all three paths call, and `lookupModule` tries the unresolved spelling against prefixes as well. The remap test is parameterized over the three shapes rather than pinned to one. Against the parent commit all six lookups miss; with this, all six are served. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Registering a shim under every spelling it can be asked for put one module into `resolvedExports` several times, so the missing-export message listed it once per spelling: "exported from `mod-a`, `mod-a`, `mod-b`, and `mod-b`". Caught by `PackageShimHandler error message lists every shim that owns the symbol when more than one matches`. The message is about which module to import from, so it collects by module rather than by key. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2 participants
Split out of #6087 so it can be reviewed on its own.
A module shimmed on the virtual network is keyed by the URL its identifier resolves to. For a realm-mapped prefix such as
@cardstack/base/, that is the realm URL — because the realm mapping also feeds the network's import map. But the network's fetch pipeline only answers shims on the fakehttps://packages/origin, so a shim registered under a realm URL is never served: the loader fetches the module over the network instead and the shim sits inert.The loader's module-fetch path is the one place that knows a request is for a module rather than a card instance that may live at the same realm URL, so the lookup belongs there, ahead of any fetch.
VirtualNetwork.getShimmedModulefolds every spelling of an identifier — realm-prefix form, virtual alias, url-mapped alias — onto the real URL, so all of them land on one module.This is enabling, not a live bug fix. Every shim registered today is a bare package specifier (
@cardstack/runtime-common,@ember/*,@cardstack/boxel-ui/*), which resolves to the fake packages origin and is served fine. Nothing in the current app registers a realm-mapped shim, so nothing is broken on main. It is a prerequisite for serving@cardstack/basefrom the host bundle.Proof
Unit | loader: a module shimmed on the virtual network is served to a loader whose fetch throws, under the prefix, resolved-URL and alias spellings. Without this change the fetch is reached and the test fails.🤖 Generated with Claude Code