Skip to content

Fix remixed cards keeping their module reference pointed at the catalog realm#652

Closed
richardhjtan wants to merge 4 commits into
mainfrom
fix/listing-install-adopts-from-module
Closed

Fix remixed cards keeping their module reference pointed at the catalog realm#652
richardhjtan wants to merge 4 commits into
mainfrom
fix/listing-install-adopts-from-module

Conversation

@richardhjtan

@richardhjtan richardhjtan commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • buildInstanceOperation computed the correct target module reference (copyInstanceMeta.targetCodeRef) for a copied instance but never wrote it onto the copied card's meta.adoptsFrom, so remixing/installing a listing left the installed card's module pointing back at the catalog realm instead of the destination realm.
  • Unskips the listing install live suite and adds an assertion that specifically catches this (installed card's module must resolve into the destination realm, not the catalog realm).
  • Unskips the listing remix live suite: remix rides the same install pipeline and catalog-RRI resolution, so it goes green with the same companion fix. (Its other prerequisite — the catalog-listing skill pointing at @cardstack/catalog/commands/* — already landed in boxel-skills via 29df01a.)
  • Fixes a stray relationships.categories.0 placement bug in the skill listing test fixture, found while getting the suite green.

Companion fix in boxel addresses a separate, pre-existing bug in planInstanceInstall's base-realm detection that this change uncovered (it was invisible until adoptsFrom actually started getting rewritten). Both are needed for a catalog remix/install to correctly resolve module references end to end.

Merge order: boxel #5478 first, then re-run CI here — this repo's live-test job checks out boxel main, so the suites stay red until the companion fix lands (a run pinned to the companion branch was green: see 251ab33).

Test plan

  • Full live test suite against a real catalog realm with the companion fix: install 5/5, remix 4/4, 0 fail
  • CI run pinned to the companion branch passed end to end

…og realm

buildInstanceOperation computed the correct target module reference for a
copied instance but never wrote it onto the copied card's adoptsFrom, so
remixing a listing left the installed card's module pointing back at the
catalog realm instead of the destination realm.

Also unskips the listing install live test and adds an assertion that
catches this specifically, and fixes a stray relationships.categories.0
placement bug in the skill listing test fixture uncovered along the way.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 13, 2026 10:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes module reference rewriting during catalog listing installs/remixes by ensuring copied instance cards persist the computed destination adoptsFrom code reference, and it strengthens live coverage to prevent regressions.

Changes:

  • Write copyInstanceMeta.targetCodeRef onto the copied card resource (meta.adoptsFrom) during instance install operations.
  • Unskip the catalog listing install live test and add assertions that the installed card’s module resolves within the destination realm (not the catalog realm).
  • Fix an incorrect relationships['categories.0'] placement in the skill listing test fixture.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
tests/live/catalog-app/listing-install.test.gts Unskips the live install test and adds assertions verifying installed module resolution points to the destination realm.
tests/helpers/test-fixtures.ts Fixes categories.0 relationship placement in the mock catalog fixture for skill listings.
commands/listing-install.ts Ensures copied instance cards have meta.adoptsFrom rewritten to the destination targetCodeRef.

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

Comment on lines +139 to +142
let store = getService('store');
let installedCard = (await store.get(
result.exampleCardId as string,
)) as CardDef;
Comment on lines +143 to +146
let installedRef = identifyCard(installedCard.constructor);
if (!installedRef || !isResolvedCodeRef(installedRef)) {
throw new Error('expected a resolved code ref');
}
richardhjtan and others added 2 commits July 13, 2026 19:35
… merge

Points the boxel monorepo checkout at fix/catalog-remix-adopts-from-base-realm
so this PR's own Live Tests job can prove the fix works end to end before
boxel#5478 merges. Must be reverted to the default branch before this PR
is merged.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Remix rides the same install pipeline and catalog-RRI resolution as
listing-install, so it goes green with the same companion host fix; the
suite also needs the catalog-listing skill's command refs to point at
@cardstack/catalog/commands/* (fixed in boxel-skills #29df01a).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@richardhjtan

Copy link
Copy Markdown
Collaborator Author

[Claude Code 🤖] Closing alongside boxel#5478 — the team decided to fix the root cause (the RRI-native install planner, CS-12078) rather than ship the interim planner fix.

This PR can't proceed without that companion: the unskipped install/remix suites are red against boxel main (the card-listing install test hangs without the planner change), and the adoptsFrom rewrite here is only correct when the planner correctly exempts base-realm modules — without that, the rewrite would point instances at wrongly-copied base modules.

When CS-12078 lands, the pieces here are worth salvaging from this branch: the buildInstanceOperation targetCodeRef fix, the relationships.categories.0 fixture fix, the install-destination assertion, and the two suite unskips.

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.

2 participants