Skip to content

fix(editor): restore profile URL resolution#584

Open
ion-lion wants to merge 1 commit into
seed-hypermedia:mainfrom
ion-lion:ion-fix-profile-url-paste-582
Open

fix(editor): restore profile URL resolution#584
ion-lion wants to merge 1 commit into
seed-hypermedia:mainfrom
ion-lion:ion-fix-profile-url-paste-582

Conversation

@ion-lion
Copy link
Copy Markdown
Collaborator

@ion-lion ion-lion commented May 11, 2026

Summary

  • fixes the intended domain-store resolver path for pasted https://.../hm/... URLs instead of special-casing public hyper.media
  • persists/exposes the existing /hm/api/config isGateway value through the backend domain store, daemon DomainInfo, generated clients, and shared domain resolver
  • only treats /hm/* as an embedded canonical HM id when the cached domain info says the domain is a gateway; non-gateway custom domains keep /hm/... as normal site paths
  • keeps the earlier editor/comment-editor plumbing so pasted HTTPS Hypermedia links use the normal resolveHypermediaUrl(..., {domainResolver}) workflow and are not short-circuited by temporary linkify marks
  • fixes web page/OPTIONS metadata for /:profile/<accountUid> URLs so metadata resolves through the profile view-term path consistently instead of treating :profile as a document path

Fixes #582.
Fixes #587.

Before-fix reproduction

  • Reproduced Pasted hyper.media profile URLs are not converted into hm:// links #582 against current origin/main with a focused resolveHypermediaUrl regression test:
    • URL: https://hyper.media/hm/z6Mktarget/:profile
    • cached resolver result: gateway account UID
    • actual before fix: hm://z6Mkgateway/hm/z6Mktarget/:profile
    • expected: hm://z6Mktarget/:profile
  • Reproduced Profile metadata is resolved incorrectly from web #587 against the live/current web behavior for https://seedteamtalks.hyper.media/:profile/z6MkwRWdU6HY11Qzi9SQZrwSx3rxFw94ipYyiN7Dtbrcg2yU:
    • GET response/meta before fix reported the site document id hm://z6MkuBbsB1HbSNXLvJCRCrPhimY6g7tzhr4qvcYKPuSZzhno
    • OPTIONS did not expose the addressed profile metadata for that URL
  • The Profile metadata is resolved incorrectly from web #587 code-path inspection showed normal page metadata used the loaded site document id, while profile routes carry the addressed accountUid separately for rendering.

Fix summary

  • /hm/api/config already emits isGateway; this PR now decodes, stores, and returns it from the backend domain store.
  • DomainInfo now includes is_gateway, and frontend domain info mapping exposes isGateway.
  • createDomainResolver() returns {registeredAccountUid, isGateway} so resolveHypermediaUrl() can distinguish gateway domains from ordinary custom domains.
  • resolveHypermediaUrl() preserves compatibility with old resolvers returning string | null, but only canonicalizes /hm/* as embedded HM URLs when isGateway === true.
  • Web metadata now computes the profile metadata id once in the loader and passes it to Remix meta, avoiding duplicate helper callers.
  • OPTIONS request metadata now uses extractViewTermFromUrl()/viewTerm handling from entity-id-url.ts, so :profile is interpreted as a view term rather than a document path.
  • Removed the as ExtendedSitePayload[...] cast from routes/$.tsx by validating inspect tabs before passing them through.

After-fix verification

  • corepack pnpm --filter @seed-hypermedia/client exec vitest --run src/hm-resolver.test.ts
  • corepack pnpm --filter @seed-hypermedia/client run typecheck
  • corepack pnpm --filter @shm/shared run typecheck
  • corepack pnpm --filter @shm/web exec vitest --run app/hypermedia-id.test.ts
  • corepack pnpm --filter @shm/web run typecheck
  • corepack pnpm --filter @shm/editor run typecheck
  • corepack pnpm --filter @shm/desktop run typecheck
  • corepack pnpm --filter @shm/editor test -- src/tiptap-extension-link/helpers/pasteHandler.test.ts
  • corepack pnpm --filter @shm/desktop run test:unit (43 files / 414 tests passed)
  • git diff --check

Blocked locally:

  • go test ./backend/blob ./backend/api/daemon/v1alpha could not run because this environment does not have go installed (go: command not found). I still added a backend regression test for the domain store preserving isGateway from /hm/api/config.

Self-review notes

  • The Pasted hyper.media profile URLs are not converted into hm:// links #582 patch remains in the intended layer Eric pointed to: the domain store and normal HTTPS-to-HM resolution workflow.
  • The Profile metadata is resolved incorrectly from web #587 patch follows the existing web metadata/header path instead of adding a separate resolver; it only changes which HM id is passed into the existing metadata builders for profile URLs.
  • No public-gateway hard-code remains; /hm/* canonicalization depends on cached domain metadata.
  • Non-gateway custom-domain /hm/... paths are covered by a regression test and remain relative to that domain account.
  • The resolver type remains backward-compatible with older simple resolver functions returning only string | null.
  • Review passes removed the duplicate getSiteProfileMetadataId helper, avoid treating :profile as path, remove the TypeScript as cast from the reviewed route code, and clarify why profile URLs need both a daemon load id (loadResourceId) and a public metadata id (publicMetadataId).

@ericvicenti
Copy link
Copy Markdown
Collaborator

this code is totally the wrong approach. we should be using the domain store and the normal https to hm url resolution workflow.

We used to have this working, so we have suffered a regression. This PR attempts to write new code instead of fixing the broken code path

@ion-lion ion-lion force-pushed the ion-fix-profile-url-paste-582 branch from aa11b91 to bbf4874 Compare May 11, 2026 12:37
@ion-lion
Copy link
Copy Markdown
Collaborator Author

You’re right — my first pass was the wrong layer. I backed out the ad-hoc public hyper.media handling and force-pushed a new version that uses the existing resolver path instead.

What changed now:

  • resolveHypermediaUrl(..., {domainResolver}) handles /hm/... gateway paths as canonical embedded HM IDs before custom-domain path mapping, so a domain-store hit cannot turn https://hyper.media/hm/<target>/:profile into a path under the gateway/domain account.
  • Desktop comment editors now receive the same domain resolver plumbing as document editors.
  • The paste handler no longer lets linkify’s temporary web mark short-circuit HM canonicalization, and it has a fallback for comment editors without grpcClient.

Validation is updated in the PR body.

@ericvicenti
Copy link
Copy Markdown
Collaborator

I think the domain store should keep track of domains who are gateways. So the is_gateway sort of flag must be externalized from the web side of things in the /hm/api/config endpoint, and then saved in the domain store. Then if the cached domain store knows of this domain as a gateway then it can rapidly convert all the /hm/* URLs to hm://

@ion-lion ion-lion force-pushed the ion-fix-profile-url-paste-582 branch from bbf4874 to 1b8935e Compare May 11, 2026 14:18
@ion-lion ion-lion changed the title fix(editor): canonicalize pasted profile URLs fix(editor): restore pasted profile URL resolution May 11, 2026
@ion-lion
Copy link
Copy Markdown
Collaborator Author

Thanks — I traced that path and updated the PR to carry the existing /hm/api/config isGateway value through the domain store instead of inferring gateway behavior in the resolver.

What changed in the new push:

  • backend domain config now decodes/stores isGateway
  • daemon DomainInfo exposes is_gateway
  • shared domain resolver returns {registeredAccountUid, isGateway} from cached/checkDomain results
  • resolveHypermediaUrl() only treats /hm/* as an embedded canonical HM id when cached domain metadata says the domain is a gateway
  • non-gateway custom domains keep /hm/... as a normal site path

I also updated the PR body with the before-fix reproduction, verification, and self-review notes. Frontend/editor/desktop gates pass; Go tests are still blocked locally because this node doesn’t have go installed, but I added a backend regression test for preserving isGateway from /hm/api/config.

@ion-lion ion-lion force-pushed the ion-fix-profile-url-paste-582 branch from 1b8935e to 9cdc00a Compare May 11, 2026 14:26
@ion-lion ion-lion changed the title fix(editor): restore pasted profile URL resolution fix(editor): restore profile URL resolution May 11, 2026
Copy link
Copy Markdown
Collaborator

@ericvicenti ericvicenti left a comment

Choose a reason for hiding this comment

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

can you please combine the getSiteProfileMetadataId callers into one place (ideally the loader will pass this information down into the component).

Then once the two callers are combined, get rid of that function entirely. It's simpler to avoid a new confusingly-named function in simple cases like that

Comment thread frontend/apps/web/app/hypermedia-id.ts Outdated
): UnpackedHypermediaId {
if (viewTerm === 'profile') {
return hmId(accountUid || siteId.uid, {
path: [':profile'],
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

you should be using viewTerm instead of path for this. See how its used in entity-id-url.ts

When there is a : prefix, its not a path. its a viewTerm

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in the latest push. I switched the OPTIONS/profile metadata path to use extractViewTermFromUrl() from entity-id-url.ts, so :profile is handled as a viewTerm instead of being parsed as a document path. I also removed the duplicate getSiteProfileMetadataId helper and compute the page metadata id once in the loader.

Comment thread frontend/apps/web/app/routes/$.tsx Outdated
}
const metadataId = getSiteProfileMetadataId(
siteDocument.id,
(siteDocument as ExtendedSitePayload).viewTerm,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

you should not need to use ts as keyword.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed — removed this cast. inspectTab now goes through a small validator before it is passed into loader data, and the meta path now consumes metadataId from the loader instead of casting the payload shape here.

@ion-lion ion-lion force-pushed the ion-fix-profile-url-paste-582 branch from 9cdc00a to a95f7ca Compare May 11, 2026 14:44
@ion-lion
Copy link
Copy Markdown
Collaborator Author

Updated for this review pass and force-pushed a95f7ca68.

Changes:

  • removed getSiteProfileMetadataId
  • compute metadataId once in the loader and pass it through to Remix meta
  • OPTIONS metadata now uses extractViewTermFromUrl() so :profile is handled as a view term, not as a document path
  • removed the reviewed TS as cast by validating inspectTab before storing it in loader data

Re-ran:

  • corepack pnpm --filter @shm/web exec vitest --run app/hypermedia-id.test.ts
  • corepack pnpm --filter @shm/web run typecheck
  • corepack pnpm --filter @seed-hypermedia/client exec vitest --run src/hm-resolver.test.ts
  • corepack pnpm --filter @seed-hypermedia/client run typecheck
  • corepack pnpm --filter @shm/shared run typecheck
  • git diff --check

I also replied on the two inline threads and updated the PR body with the review-pass notes.

import type {UnpackedHypermediaId} from '@seed-hypermedia/client/hm-types'
import {extractViewTermFromUrl, hmId, viewTermToRouteKey} from '@shm/shared'

export type RequestResourceIds = {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

what is the purpose of having both resourceId and metadataId here?

I'm trying to understand if this complexity is justified. Either it should be simplified or the comments+naming conventions should make it obvious why this exists

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good call — I kept the split because the two IDs are genuinely different for profile URLs, but renamed/commented it so the reason is explicit.

For profile URLs:

  • loadResourceId is the daemon-loadable account root (hm://<account>), because :profile is a view term, not a document path.
  • publicMetadataId is what HTML meta / OPTIONS should advertise (hm://<account>/:profile).

For normal document URLs they are identical. I force-pushed 4d19a9f00 with those names and comments, and reran the focused web test, web typecheck, and git diff --check.

@ion-lion ion-lion force-pushed the ion-fix-profile-url-paste-582 branch from a95f7ca to 4d19a9f Compare May 11, 2026 15:24
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.

Profile metadata is resolved incorrectly from web Pasted hyper.media profile URLs are not converted into hm:// links

2 participants