gabriel / musehub public
marketplace-domain-link-backfill-plan.md markdown
445 lines 24.0 KB
Raw
sha256:ab1b983db7f325870da455064eed5dd8a6972ad6f914b7ec3ad4068fbfb00932 docs: record musehub#120 MDL_16 staging backfill applied an… Sonnet 5 12 days ago

Repos are never linked to their marketplace domain — fix + backfill

Background

The proximate trigger

While verifying muse#74 (auto-derived supported_commands) against local musehub, https://localhost:1337/domains showed @gabriel/code with exactly 1 linked repo, despite https://localhost:1337/explore listing roughly 180 code repos. Not a display bug — confirmed directly against Postgres:

SELECT domain_id, marketplace_domain_id, count(*)
FROM musehub_repos GROUP BY domain_id, marketplace_domain_id;

 domain_id | marketplace_domain_id | count
-----------+------------------------+-------
 code      | (null)                 |   179
 identity  | (null)                 |     4
 mist      | (null)                 |     2
 code      | sha256:b5d4...         |     1   ← only a throwaway verification repo

Root cause — verified against source, not assumed

MusehubRepo has two distinct domain columns (see musehub#117 Phase 3 / the list_repos_for_domain fix that already shipped for the read side):

  • domain_id — a plain VCS-plugin category string ("code", "mist", "identity"), set from the CLI's --domain flag at repo creation.
  • marketplace_domain_id — an FK-shaped link to a specific MusehubDomain.domain_id (a sha256 genesis ID) — the column list_repos_for_domain actually queries.

create_repo() (musehub/services/musehub_repository.py:146) sets domain_id from the caller's domain argument but never touches marketplace_domain_id. The only code path that ever sets it is update_repo_settings()'s explicit --capabilities-style patch (musehub_repository.py:2342) — a repo only gets linked if someone later calls PATCH /api/repos/{id}/settings with an explicit marketplace_domain_id. Nothing in the create flow, the CLI, or any wizard does that today.

This is not just a one-time historical gap: every repo created from now on will keep landing with marketplace_domain_id = NULL unless this is fixed at the source. A backfill alone would only paper over the existing 185 repos and the bug would silently reopen on the next commit.

The ambiguity this issue must resolve — the reason it isn't a one-line fix

domain at repo-creation time is a bare category string ("code"), not a scoped domain ID ("@gabriel/code") — there is no author context to resolve against. Today exactly one non-deprecated MusehubDomain row exists per category (all owned by gabriel), so category → domain is unambiguous. That will not always be true: nothing stops a second author from publishing their own @alice/code tomorrow. Auto-linking must never guess when more than one candidate exists — silently picking one author's domain over another's would be a data-integrity bug, not a fix for one.

Resolution rule (this issue's core design decision): auto-link only when exactly one non-deprecated MusehubDomain matches the category by slug. Zero matches or 2+ matches → leave marketplace_domain_id NULL, unchanged from today's (broken but honest) behavior. An explicit, caller-supplied marketplace_domain_id always overrides auto-resolution and is validated to exist, exactly like update_repo_settings already does — this is the future-proof way for a caller to disambiguate once multiple domains of the same category exist (e.g. eventually a CLI flag --marketplace-domain @alice/code).

Design — the shape to build toward

A single resolution helper, consulted by both the create path (new) and the backfill script (one-time), with the same never-guess contract both places.

musehub/services/musehub_domains.py:

async def resolve_unambiguous_domain_id_by_category(
    session: AsyncSession, category: str
) -> str | None:
    """Return the sole non-deprecated MusehubDomain.domain_id whose slug
    matches *category*, or None if zero or multiple candidates exist.

    Never guesses: ambiguity (2+ live domains sharing a category slug)
    and absence (0 domains) are both honest None, not a fallback.
    """

create_repo() gains an optional marketplace_domain_id: str | None = None parameter:

  • Explicit value provided → validated via get_domain_by_id (same ValueError("marketplace_domain_not_found") contract update_repo_settings already uses), then stored as-is.
  • Omitted → auto-resolve via resolve_unambiguous_domain_id_by_category(session, domain_id) using the same _domain_id the row is about to store. Result (a real ID or None) is stored — never guessed.
  • Whenever the repo ends up with a non-None marketplace_domain_id (explicit or auto-resolved) at creation time, call record_domain_install(session, owner_user_id, marketplace_domain_id) — mirrors what update_repo_settings already does on a later link, so install_count is correct from the moment of creation instead of only reflecting repos that were explicitly re-linked after the fact.

scripts/backfill_marketplace_domain_links.py (new, one-off, following the existing scripts/ convention — see stale_branches.py for style):

  • --dry-run (default): reports counts — linked, skipped-ambiguous, skipped-no-match — without writing anything.
  • --apply: performs the writes reported by dry-run, calling record_domain_install for each repo it links (real install-count correctness, not just backfilling the FK).
  • Idempotent: repos with marketplace_domain_id IS NOT NULL are always skipped, dry-run or apply, re-run or first-run.
  • Every skipped-ambiguous and skipped-no-match repo is printed by repo_id — no silent under-reporting of what wasn't fixed.

Goal — definition of done

  1. create_repo() never leaves an unambiguously-resolvable repo unlinked — every new repo in a single-domain-per-category world (today's reality) is linked at creation time.
  2. Ambiguous or unmatched categories are left NULL, not guessed — verified by a test that deliberately creates two competing domains for one category and confirms no link is made.
  3. install_count reflects real installs from creation forward, not only from later explicit re-links.
  4. All currently-orphaned repos get backfilled correctly, with a full dry-run report reviewed before any write. (Note: the original "185 local repos" count above is now stale — 181 of those were confirmed ephemeral bench/QA test repos and were deleted outright in a separate cleanup pass, unrelated to this issue's mechanism fix. Current real counts as of Phase 3: 5 unlinked on localmuse, musehub, identity, and 2 mists — and 18 unlinked on stagingmuse, musehub, muse-zsh, identity, and 14 mists. All of these are real, not test junk.)
  5. /domains on both local and staging shows accurate repo counts per domain after the backfill runs.
  6. Every deliverable is TDD'd: red test first, then green.

Phases

Ordered by load-bearing dependency — the resolution helper must exist and be correct before either consumer (create path, backfill script) can use it.

Phase 1 — The resolution helper, in isolation

  • [x] MDL_01resolve_unambiguous_domain_id_by_category returns the matching domain_id when exactly one non-deprecated MusehubDomain has that slug. Done.
  • [x] MDL_02 — Returns None when zero domains match the category. Done.
  • [x] MDL_03 — Returns None when 2+ non-deprecated domains share the same category slug — the core ambiguity guard this issue exists to add. Red test: seed two live domains both slugged "code" (different authors), assert resolution is None, not either one. Done.
  • [x] MDL_04 — A deprecated domain sharing a slug with a live one does not count as a second candidate — resolves unambiguously to the live domain. Proves deprecation status is part of the candidate filter, not just a display-layer concern. Done.

Exit gate — met. tests/test_domains.py::TestIntegrationResolveUnambiguousDomainByCategory: 4/4 new tests pass, confirmed genuinely red first (ImportError before the function existed). Full file regression check: all 107 tests in test_domains.py still pass (was 103 before this phase). mypy: identical error count on musehub_domains.py before and after (3 — an import-untyped note pair on muse.core.types and one pre-existing no-any-return, both unrelated to this change and both present before my edit; verified by diffing against HEAD content). Implementation: resolve_unambiguous_domain_id_by_category(session, category) in musehub/services/musehub_domains.py, querying MusehubDomain.domain_id where slug == category and is_deprecated.is_(False), returning the single match or None for zero/multiple. Committed on feat/marketplace-domain-link-backfill, not yet merged to dev.

Exit gate: all four resolution-contract cases pass in isolation, with no dependency yet on create_repo or the backfill script.

Phase 2 — Wire into create_repo (stops the bug from recurring)

  • [x] MDL_05 — Creating a repo with domain="code" and no explicit marketplace_domain_id, in a fixture with exactly one live "code" domain, auto-links to it. Done.
  • [x] MDL_06 — Creating a repo with a category that matches zero marketplace domains leaves marketplace_domain_id NULL — today's honest behavior, unchanged for the no-match case. Done — was already passing before the fix (accidental correctness); kept as a pinned regression guard.
  • [x] MDL_07 — An explicit marketplace_domain_id argument to create_repo overrides auto-resolution entirely (even if it disagrees with what auto-resolution would have picked). Done.
  • [x] MDL_08 — An explicit marketplace_domain_id that doesn't exist raises ValueError("marketplace_domain_not_found") — same contract update_repo_settings already has, pinned here too so the two call sites can't silently diverge. Done.
  • [x] MDL_09 — When a repo ends up linked at creation (explicit or auto-resolved), record_domain_install is called exactly once for owner_user_idinstall_count on the target MusehubDomain increments immediately, not only after a later explicit patch. Done.
  • [x] MDL_10 — Creating a repo whose category has 2+ live marketplace candidates leaves marketplace_domain_id NULL — the create-path-level regression guard mirroring MDL_03. Done — also already accidentally correct before the fix; pinned now.

Exit gate — met. New class tests/test_repository_service.py::TestMarketplaceDomainLinkAtCreation: 6/6 tests pass. Confirmed genuinely red first by temporarily reverting musehub_repository.py to its pre-Phase-2 HEAD content and re-running — 4/6 failed (MDL_05, MDL_07, MDL_08, MDL_09); MDL_06/MDL_10 passed even against the old code since NULL-on-no-match/ambiguity was already the (accidental) behavior of a create_repo that never touched marketplace_domain_id at all — kept as explicit regression guards now that the field is actively populated. Zero regressions: full test_repository_service.py (78), test_musehub_repos.py (82), and test_domains.py (107) all still pass — 267 total, all green. mypy: identical error count on musehub_repository.py before and after (15, all pre-existing; verified by diffing against HEAD content). Implementation: create_repo gained an optional marketplace_domain_id: str | None = None parameter; resolution happens before the MusehubRepo row is constructed (explicit value validated via get_domain_by_id, else auto-resolved via resolve_unambiguous_domain_id_by_category), and record_domain_install is called once after the row is flushed/refreshed whenever the resolved value is non-None. Committed on feat/marketplace-domain-link-backfill, not yet merged to dev.

Exit gate: every new repo created after this phase lands correctly linked whenever its category is unambiguous; ambiguous/unmatched cases stay honestly NULL; install bookkeeping is correct from creation.

Phase 3 — Backfill existing repos

  • [x] MDL_11scripts/backfill_marketplace_domain_links.py (dry-run is the default; no --apply flag) reports accurate counts (linked / skipped-ambiguous / skipped-no-match) against a fixture DB, writes nothing. Done.
  • [x] MDL_12--apply performs exactly the writes the matching dry-run reported, and calls record_domain_install once per linked repo. Done.
  • [x] MDL_13 — Repos that already have a non-NULL marketplace_domain_id are left untouched by both dry-run and --apply — idempotent, safe to re-run after Phase 2 ships (new repos created in between are correctly skipped, not re-linked). Done — pinned by two tests: a repo linked before the script ever runs, and a repo linked by a first --apply pass then confirmed untouched (and not double-counted in install_count) by a second --apply pass.
  • [x] MDL_14 — Every skipped-ambiguous and skipped-no-match repo is printed by repo_id in both modes — confirms the script never silently under-reports what it left unfixed. Done.

Exit gate — met. New file tests/test_backfill_marketplace_domain_links.py: 6/6 tests pass, covering dry-run (no mutation), apply (writes + install bookkeeping), idempotency (pre-linked repos untouched; safe re-run after apply without double-counting install_count), and full reporting of both skip categories. This is genuinely new code (the script did not exist before this phase — no prior "buggy" behavior to diff against for a red state, unlike Phases 1-2), so red was confirmed the ordinary way: every test failed with ImportError until the module and function existed. Zero regressions: test_domains.py + test_repository_service.py together (185 tests) still pass. mypy: scripts/backfill_marketplace_domain_links.py is clean (0 errors) — one real issue caught and fixed during implementation: MusehubRepo.domain_id is nullable at the DB level (Mapped[str | None]) even though the app always sets it in practice; the backfill now explicitly buckets a None category as skipped-no-match rather than passing None to the resolver. Smoke-tested end-to-end against local musehub's real container (docker exec musehub python3 scripts/backfill_marketplace_domain_links.py, dry-run, no --apply): reported exactly 5 linked / 0 ambiguous / 0 no-match, matching the corrected Phase-3-era count from the Goal section above. Committed on feat/marketplace-domain-link-backfill, not yet merged to dev.

Exit gate: dry-run report reviewed and matches expectations against a full copy of local musehub's real data before --apply is ever run for real (Phase 4).

Phase 4 — Live re-verification

  • [x] MDL_15 — Run --dry-run against local musehub's real DB, review the report (expect 5 linked as of Phase 3 — muse, musehub, identity, 2 mists — given today's one-domain-per-category reality), then run --apply. Confirm via GET /api/domains/@gabriel/code (and identity/mist) that install_count/repo listings now reflect the real numbers, and confirm https://localhost:1337/domains in-browser. Done.
  • [x] MDL_16 — Repeat against staging once local is confirmed correct. Done — 22 linked (more than the original 18-owned-by-gabriel estimate; the backfill correctly scans every repo server-wide, not just gabriel's own — staging has repos owned by other/test identities muse hub repo list doesn't surface, since that command is scoped to the caller's own + collaborated repos).

Exit gate — met, both local and staging. Local: ran docker exec musehub python3 scripts/backfill_marketplace_domain_links.py (dry-run then --apply) against local musehub's real container/DB: 5 linked / 0 ambiguous / 0 no-match. Verified directly against Postgres — all 5 repos now correctly marketplace_domain_id-linked. Verified live: GET /api/domains/@gabriel/code/repostotal: 2; .../mist/repostotal: 2. Full regression check post-write: test_domains.py + test_repository_service.py + test_backfill_marketplace_domain_links.py — 191/191 pass.

Staging: since scripts/ is never copied into the Docker image (only musehub/, alembic/, docs/, deploy/ — confirmed by reading the Dockerfile's COPY list), the backfill script had to be transferred onto the running musehub-blue container manually — base64-encoded via SSM, written to the host, docker cp'd in as root (the container's default musehub user can't mkdir under /app), then chown'd back to the musehub user before exec. Dry-run reported 22 linked / 0 ambiguous / 0 no-match; --apply produced the identical 22. Verified live: GET /domains?format=json@gabriel/code repo_count: 6 (includes real collaborator repos like knowtation, gabriel-muse, scooling-lab, not just gabriel's own muse/musehub), @gabriel/mist repo_count: 13, @gabriel/identity repo_count: 0 (private) — confirmed identically in the rendered HTML of both https://staging.musehub.ai/domains and https://staging.musehub.ai/domains/@gabriel/code (stat card now reads 6, matching the live repo list below it). This ad hoc script-transfer process is a one-off for this backfill, not a new standing deploy convention — copying scripts/ into the image permanently, if ever needed again, would be a separate, deliberate Dockerfile change.

Exit gate: Live, in-browser/curl confirmation against both local and staging — not just the test suite. /domains shows real repo counts; no repo is silently mis-linked to the wrong author's domain.

Addendum — a second, more serious bug found during MDL_15 live verification

After applying the local backfill, gabriel reported the domain detail page (/domains/@gabriel/code) still showing "1 repo" despite muse and musehub both being listed underneath. Investigation surfaced two distinct, previously-undetected problems — one about this issue's own verification process, one an unrelated display bug:

1. list_repos_for_domain was never actually fixed on dev. The Background section above claimed the wrong-column bug (MusehubRepo.domain_id vs marketplace_domain_id) "already shipped" on a separate branch, fix/list-repos-for-domain-wrong-column. That was false — the fix was committed but never merged to dev, carried forward incorrectly from an earlier session summary that conflated "committed on a branch" with "merged." Worse: the local musehub container had been running since before this session's work began, with list_repos_for_domain loaded into memory from whatever branch happened to be checked out on disk at container-start time — which, by coincidence, was the fixed branch. Every "live verification" during Phases 1-4 (including MDL_15's initial pass) was silently validated against stale in-memory bytecode from a different, unmerged branch, not the code that would actually ship. A container restart exposed the regression immediately (GET /api/domains/@gabriel/code/repostotal: 0).

Fixed by merging fix/list-repos-for-domain-wrong-column into dev (clean fast-forward, 0 conflicts, 104/104 tests pass), pushing dev to local, deleting the now-merged branch, then merging dev into feat/marketplace-domain-link-backfill (clean 3-way merge, 0 conflicts, 192/192 tests pass). Both containers restarted and confirmed booting clean before re-verifying. Re-verified live post-restart: @gabriel/code/repostotal: 2 (muse, musehub); @gabriel/mist/repostotal: 2 (both mists); @gabriel/identity/repostotal: 0 (correct — that repo is private, correctly excluded by the visibility == "public" filter, not a bug).

Process lesson: this workspace's own CLAUDE.md already documents "ALWAYS restart both containers after code changes" as a hard-won rule from a prior incident — I did not follow it during Phases 1-3 of this issue, and it directly caused a false-positive verification. Restarting before any live check is now non-negotiable for the rest of this issue (and should be treated as such for all future musehub work).

2. Separate, unrelated display bug — now fully fixed on both pages. The "Repositories" stat card on the domain detail page was bound to domain.install_count (a count of distinct users who installed the domain, keyed on (user_id, domain_id)) under a label that reads "Repositories" — a pre-existing mislabel, not caused by this issue's mechanism work. Fixed by swapping the bound value to repos_total (the real, already-computed live repo count) in domain_detail.html.

The /domains listing page had the identical mislabel (domain.install_count rendered as "N repo(s)" per card), but with no equivalent value already computed to swap to — the listing route never queried per-domain repo counts at all. Fixed by adding count_public_repos_by_domain(session, domain_ids) to musehub_domains.py — one grouped-by query batching the count for every domain on the current page (never an N+1 call to list_repos_for_domain per row), matched on marketplace_domain_id exactly like list_repos_for_domain. Wired into domains_page as a new repo_count field alongside (not replacing) install_count — they are genuinely distinct metrics, both worth keeping. 5 new unit tests (TestIntegrationCountPublicReposByDomain) plus one E2E regression test against GET /domains?format=json confirming repo_count and install_count diverge correctly (2 repos, 1 install). Confirmed genuinely red first by reverting both the query wiring and the template binding and re-running the E2E test (KeyError: 'repo_count'), then restored and re-ran green. Zero regressions: full test_domains.py — 114/114 pass (was 107 before this fix, then 113 after the count function, then 114 after the regression test). mypy: identical error count on both musehub_domains.py (2, pre-existing) and ui_domains.py (5, pre-existing) before and after — verified by diffing against HEAD content.

Both containers restarted and confirmed clean boot before live verification. Live confirmation via GET /domains?format=json: @gabriel/coderepo_count: 2, @gabriel/identityrepo_count: 0, @gabriel/mistrepo_count: 2install_count on all three independently still 1 (each domain installed once by gabriel, a correct and distinct number). Confirmed in the rendered HTML too: curl https://localhost:1337/domains shows "2 repos" / "0 repos" / "2 repos" for the three domain cards, matching exactly.

Acceptance criteria (whole-issue gate)

  • create_repo never leaves an unambiguous-category repo unlinked.
  • Ambiguous or unmatched categories are always NULL, never guessed — proven by tests that construct real ambiguity, not just the happy path.
  • install_count is correct for repos linked at creation time, not only for repos explicitly re-linked later.
  • All existing orphaned repos backfilled where unambiguously resolvable; every unresolved repo explicitly reported, not silently skipped.
  • Full TDD coverage: every phase's deliverables have a red-then-green test.
  • Verified live on both local and staging, not just unit-tested.

Out of scope (explicit, for future issues)

  • Any UI or CLI affordance for a human to manually resolve an ambiguous or no-match repo (e.g. "pick which @author/code this repo belongs to"). This issue makes today's single-domain-per-category reality correct and safe; multi-domain-per-category UX is real future work once a second author actually publishes a competing domain.
  • Changing update_repo_settings's existing explicit-link behavior — already correct, already tested; this issue only adds an equivalent auto-resolve path at creation time.
  • A CLI flag to pass --marketplace-domain explicitly at muse push / repo-creation time — the service-layer parameter is added in this issue so a future CLI change can wire into it, but exposing it through the CLI itself is a separate, smaller follow-up once the service contract is proven.
File History 1 commit
sha256:ab1b983db7f325870da455064eed5dd8a6972ad6f914b7ec3ad4068fbfb00932 docs: record musehub#120 MDL_16 staging backfill applied an… Sonnet 5 12 days ago