Skip to content

Fix TypeError: Invalid URL when serving cards from prefix-mapped realms#4241

Merged
habdelra merged 8 commits intomainfrom
cs-10498-system-card-is-not-working
Mar 24, 2026
Merged

Fix TypeError: Invalid URL when serving cards from prefix-mapped realms#4241
habdelra merged 8 commits intomainfrom
cs-10498-system-card-is-not-working

Conversation

@habdelra
Copy link
Contributor

@habdelra habdelra commented Mar 24, 2026

Summary

  • Fixes TypeError: Invalid URL that broke all cards in prefix-mapped realms, causing the system card to fail on staging

  • After the import maps change (55a0f23, "Change to use import maps for @cardstack/catalog"), unresolveResourceInstanceURLs converts card IDs in the index from their full HTTP URLs to registered prefix form. These prefix mappings are configured in the realm server startup scripts (e.g. packages/realm-server/scripts/start-staging.sh):

    --fromUrl='@cardstack/catalog/' \
    --toUrl="${CATALOG_REALM_URL}" \    # https://realms-staging.stack.cards/catalog/
    ...
    --fromUrl='@cardstack/openrouter/' \
    --toUrl='https://realms-staging.stack.cards/openrouter/' \

    So a card with URL https://realms-staging.stack.cards/openrouter/OpenRouterModel/anthropic-claude-sonnet-4-6 gets its ID stored in the index as @cardstack/openrouter/OpenRouterModel/anthropic-claude-sonnet-4-6.

  • The relativizeResource function used resource.id directly as a URL base for resolveCardReference. When resource.id is a prefix string like @cardstack/openrouter/..., new URL('../openrouter-model', '@cardstack/openrouter/...') throws TypeError: Invalid URL because prefix strings are not valid URL bases.

  • The fix uses cardIdToURL() to resolve the prefix to a real URL first, consistent with all other call sites that were already updated in the import maps commit.

Test plan

  • Added regression test that calls relativizeDocument directly with a prefix-form resource ID — fails without the fix (TypeError: Invalid URL), passes with it
  • Verify the system card loads successfully on staging after deploy
  • Verify OpenRouterModel cards can be fetched directly (e.g., curl -H "Accept: application/vnd.card+json" https://realms-staging.stack.cards/openrouter/OpenRouterModel/anthropic-claude-sonnet-4-6)

Fixes CS-10498

🤖 Generated with Claude Code

After the import maps change (55a0f23), card IDs in the index are
stored in registered prefix form (e.g. @cardstack/openrouter/...) via
unresolveResourceInstanceURLs. The relativizeResource function used
resource.id directly as a URL base for resolveCardReference, but prefix
strings are not valid URL bases for new URL(). This caused all cards in
prefix-mapped realms (like the openrouter realm) to return 500 errors.

Use cardIdToURL() to resolve the prefix to a real URL first, consistent
with all other call sites updated in the import maps commit.

Fixes CS-10498

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@chatgpt-codex-connector
Copy link

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

habdelra and others added 2 commits March 24, 2026 11:55
Tests that resolving a relative URL against a prefix-form card ID
(e.g. @cardstack/openrouter/...) requires cardIdToURL() to convert the
prefix to a real URL first. Without this, new URL() throws TypeError:
Invalid URL because prefix strings are not valid URL bases.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Export relativizeDocument and test it with a prefix-form resource ID.
The test now fails without the fix (TypeError: Invalid URL) and passes
with it, confirming the regression is properly guarded.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes TypeError: Invalid URL when serving cards from prefix-mapped realms by ensuring prefix-form resource.id values are resolved to real URLs before being used as the base for relative URL/module resolution.

Changes:

  • Resolve resource.id via cardIdToURL() inside relativizeResource() to avoid passing non-URL prefix strings into new URL(...).
  • Export relativizeDocument() for regression testing.
  • Add shared runtime-common regression tests and wire them into the realm-server test suite.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
packages/runtime-common/realm-index-query-engine.ts Fixes URL base handling for prefix-form IDs during document/resource relativization.
packages/runtime-common/tests/card-reference-resolver-test.ts Adds regression coverage for prefix-form data.id not causing Invalid URL.
packages/realm-server/tests/card-reference-resolver-test.ts Runs the shared runtime-common regression tests under realm-server’s QUnit suite.
packages/realm-server/tests/index.ts Registers the new realm-server test module in the test index.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@habdelra habdelra requested a review from a team March 24, 2026 16:17
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The test only runs in realm-server, so there's no need for the
SharedTests indirection through runtime-common.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link

github-actions bot commented Mar 24, 2026

Host Test Results

    1 files  ±0      1 suites  ±0   2h 6m 54s ⏱️ -55s
2 030 tests ±0  2 015 ✅ ±0  15 💤 ±0  0 ❌ ±0 
2 045 runs  ±0  2 030 ✅ ±0  15 💤 ±0  0 ❌ ±0 

Results for commit 3e20731. ± Comparison against base commit b77095c.

♻️ This comment has been updated with latest results.

habdelra and others added 2 commits March 24, 2026 13:30
…eMessage

The test was missing await settled() after simulateRemoteMessage(), causing a
race where waitFor ran before Ember processed the new message event. Also
tightened the waitFor selector to wait for the actual command-apply button
rather than just the message container.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link

Preview deployments

@habdelra habdelra merged commit 7ce5f7b into main Mar 24, 2026
117 of 118 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants